Fix STATE.md decision corruption and dollar handling (#701)

2026-02-23 10:55:54 -05:00
parent 0176a3ec5b
commit 77d434709d
4 changed files with 177 additions and 19 deletions
--- a/get-shit-done/bin/gsd-tools.cjs
+++ b/get-shit-done/bin/gsd-tools.cjs
@@ -102,7 +102,9 @@
 *   state update-progress              Recalculate progress bar
 *   state add-decision --summary "..."  Add decision to STATE.md
 *     [--phase N] [--rationale "..."]
+ *     [--summary-file path] [--rationale-file path]
 *   state add-blocker --text "..."     Add blocker
+ *     [--text-file path]
 *   state resolve-blocker --text "..." Remove blocker
 *   state record-session               Update session continuity
 *     --stopped-at "..."
@@ -209,15 +211,23 @@ async function main() {
      } else if (subcommand === 'add-decision') {
        const phaseIdx = args.indexOf('--phase');
        const summaryIdx = args.indexOf('--summary');
+        const summaryFileIdx = args.indexOf('--summary-file');
        const rationaleIdx = args.indexOf('--rationale');
+        const rationaleFileIdx = args.indexOf('--rationale-file');
        state.cmdStateAddDecision(cwd, {
          phase: phaseIdx !== -1 ? args[phaseIdx + 1] : null,
          summary: summaryIdx !== -1 ? args[summaryIdx + 1] : null,
+          summary_file: summaryFileIdx !== -1 ? args[summaryFileIdx + 1] : null,
          rationale: rationaleIdx !== -1 ? args[rationaleIdx + 1] : '',
+          rationale_file: rationaleFileIdx !== -1 ? args[rationaleFileIdx + 1] : null,
        }, raw);
      } else if (subcommand === 'add-blocker') {
        const textIdx = args.indexOf('--text');
-        state.cmdStateAddBlocker(cwd, textIdx !== -1 ? args[textIdx + 1] : null, raw);
+        const textFileIdx = args.indexOf('--text-file');
+        state.cmdStateAddBlocker(cwd, {
+          text: textIdx !== -1 ? args[textIdx + 1] : null,
+          text_file: textFileIdx !== -1 ? args[textFileIdx + 1] : null,
+        }, raw);
      } else if (subcommand === 'resolve-blocker') {
        const textIdx = args.indexOf('--text');
        state.cmdStateResolveBlocker(cwd, textIdx !== -1 ? args[textIdx + 1] : null, raw);
--- a/get-shit-done/bin/lib/state.cjs
+++ b/get-shit-done/bin/lib/state.cjs
@@ -86,6 +86,17 @@ function cmdStateGet(cwd, section, raw) {
  }
 }

+function readTextArgOrFile(cwd, value, filePath, label) {
+  if (!filePath) return value;
+
+  const resolvedPath = path.isAbsolute(filePath) ? filePath : path.join(cwd, filePath);
+  try {
+    return fs.readFileSync(resolvedPath, 'utf-8').trimEnd();
+  } catch {
+    throw new Error(`${label} file not found: ${filePath}`);
+  }
+}
+
 function cmdStatePatch(cwd, patches, raw) {
  const statePath = path.join(cwd, '.planning', 'STATE.md');
  try {
@@ -97,7 +108,7 @@ function cmdStatePatch(cwd, patches, raw) {
      const pattern = new RegExp(`(\\*\\*${fieldEscaped}:\\*\\*\\s*)(.*)`, 'i');

      if (pattern.test(content)) {
-        content = content.replace(pattern, `$1${value}`);
+        content = content.replace(pattern, (_match, prefix) => `${prefix}${value}`);
        results.updated.push(field);
      } else {
        results.failed.push(field);
@@ -125,7 +136,7 @@ function cmdStateUpdate(cwd, field, value) {
    const fieldEscaped = field.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
    const pattern = new RegExp(`(\\*\\*${fieldEscaped}:\\*\\*\\s*)(.*)`, 'i');
    if (pattern.test(content)) {
-      content = content.replace(pattern, `$1${value}`);
+      content = content.replace(pattern, (_match, prefix) => `${prefix}${value}`);
      fs.writeFileSync(statePath, content, 'utf-8');
      output({ updated: true });
    } else {
@@ -148,7 +159,7 @@ function stateReplaceField(content, fieldName, newValue) {
  const escaped = fieldName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
  const pattern = new RegExp(`(\\*\\*${escaped}:\\*\\*\\s*)(.*)`, 'i');
  if (pattern.test(content)) {
-    return content.replace(pattern, `$1${newValue}`);
+    return content.replace(pattern, (_match, prefix) => `${prefix}${newValue}`);
  }
  return null;
 }
@@ -199,7 +210,6 @@ function cmdStateRecordMetric(cwd, options, raw) {
  const metricsMatch = content.match(metricsPattern);

  if (metricsMatch) {
-    const tableHeader = metricsMatch[1];
    let tableBody = metricsMatch[2].trimEnd();
    const newRow = `| Phase ${phase} P${plan} | ${duration} | ${tasks || '-'} tasks | ${files || '-'} files |`;

@@ -209,7 +219,7 @@ function cmdStateRecordMetric(cwd, options, raw) {
      tableBody = tableBody + '\n' + newRow;
    }

-    content = content.replace(metricsPattern, `${tableHeader}${tableBody}\n`);
+    content = content.replace(metricsPattern, (_match, header) => `${header}${tableBody}\n`);
    fs.writeFileSync(statePath, content, 'utf-8');
    output({ recorded: true, phase, plan, duration }, raw, 'true');
  } else {
@@ -246,7 +256,7 @@ function cmdStateUpdateProgress(cwd, raw) {

  const progressPattern = /(\*\*Progress:\*\*\s*).*/i;
  if (progressPattern.test(content)) {
-    content = content.replace(progressPattern, `$1${progressStr}`);
+    content = content.replace(progressPattern, (_match, prefix) => `${prefix}${progressStr}`);
    fs.writeFileSync(statePath, content, 'utf-8');
    output({ updated: true, percent, completed: totalSummaries, total: totalPlans, bar: progressStr }, raw, progressStr);
  } else {
@@ -258,11 +268,22 @@ function cmdStateAddDecision(cwd, options, raw) {
  const statePath = path.join(cwd, '.planning', 'STATE.md');
  if (!fs.existsSync(statePath)) { output({ error: 'STATE.md not found' }, raw); return; }

-  const { phase, summary, rationale } = options;
-  if (!summary) { output({ error: 'summary required' }, raw); return; }
+  const { phase, summary, summary_file, rationale, rationale_file } = options;
+  let summaryText = null;
+  let rationaleText = '';
+
+  try {
+    summaryText = readTextArgOrFile(cwd, summary, summary_file, 'summary');
+    rationaleText = readTextArgOrFile(cwd, rationale || '', rationale_file, 'rationale');
+  } catch (err) {
+    output({ added: false, reason: err.message }, raw, 'false');
+    return;
+  }
+
+  if (!summaryText) { output({ error: 'summary required' }, raw); return; }

  let content = fs.readFileSync(statePath, 'utf-8');
-  const entry = `- [Phase ${phase || '?'}]: ${summary}${rationale ? ` — ${rationale}` : ''}`;
+  const entry = `- [Phase ${phase || '?'}]: ${summaryText}${rationaleText ? ` — ${rationaleText}` : ''}`;

  // Find Decisions section (various heading patterns)
  const sectionPattern = /(###?\s*(?:Decisions|Decisions Made|Accumulated.*Decisions)\s*\n)([\s\S]*?)(?=\n###?|\n##[^#]|$)/i;
@@ -273,7 +294,7 @@ function cmdStateAddDecision(cwd, options, raw) {
    // Remove placeholders
    sectionBody = sectionBody.replace(/None yet\.?\s*\n?/gi, '').replace(/No decisions yet\.?\s*\n?/gi, '');
    sectionBody = sectionBody.trimEnd() + '\n' + entry + '\n';
-    content = content.replace(sectionPattern, `${match[1]}${sectionBody}`);
+    content = content.replace(sectionPattern, (_match, header) => `${header}${sectionBody}`);
    fs.writeFileSync(statePath, content, 'utf-8');
    output({ added: true, decision: entry }, raw, 'true');
  } else {
@@ -284,10 +305,20 @@ function cmdStateAddDecision(cwd, options, raw) {
 function cmdStateAddBlocker(cwd, text, raw) {
  const statePath = path.join(cwd, '.planning', 'STATE.md');
  if (!fs.existsSync(statePath)) { output({ error: 'STATE.md not found' }, raw); return; }
-  if (!text) { output({ error: 'text required' }, raw); return; }
+  const blockerOptions = typeof text === 'object' && text !== null ? text : { text };
+  let blockerText = null;
+
+  try {
+    blockerText = readTextArgOrFile(cwd, blockerOptions.text, blockerOptions.text_file, 'blocker');
+  } catch (err) {
+    output({ added: false, reason: err.message }, raw, 'false');
+    return;
+  }
+
+  if (!blockerText) { output({ error: 'text required' }, raw); return; }

  let content = fs.readFileSync(statePath, 'utf-8');
-  const entry = `- ${text}`;
+  const entry = `- ${blockerText}`;

  const sectionPattern = /(###?\s*(?:Blockers|Blockers\/Concerns|Concerns)\s*\n)([\s\S]*?)(?=\n###?|\n##[^#]|$)/i;
  const match = content.match(sectionPattern);
@@ -296,9 +327,9 @@ function cmdStateAddBlocker(cwd, text, raw) {
    let sectionBody = match[2];
    sectionBody = sectionBody.replace(/None\.?\s*\n?/gi, '').replace(/None yet\.?\s*\n?/gi, '');
    sectionBody = sectionBody.trimEnd() + '\n' + entry + '\n';
-    content = content.replace(sectionPattern, `${match[1]}${sectionBody}`);
+    content = content.replace(sectionPattern, (_match, header) => `${header}${sectionBody}`);
    fs.writeFileSync(statePath, content, 'utf-8');
-    output({ added: true, blocker: text }, raw, 'true');
+    output({ added: true, blocker: blockerText }, raw, 'true');
  } else {
    output({ added: false, reason: 'Blockers section not found in STATE.md' }, raw, 'false');
  }
@@ -328,7 +359,7 @@ function cmdStateResolveBlocker(cwd, text, raw) {
      newBody = 'None\n';
    }

-    content = content.replace(sectionPattern, `${match[1]}${newBody}`);
+    content = content.replace(sectionPattern, (_match, header) => `${header}${newBody}`);
    fs.writeFileSync(statePath, content, 'utf-8');
    output({ resolved: true, blocker: text }, raw, 'true');
  } else {
--- a/get-shit-done/workflows/execute-plan.md
+++ b/get-shit-done/workflows/execute-plan.md
@@ -349,11 +349,12 @@ From SUMMARY: Extract decisions and add to STATE.md:

 ```bash
 # Add each decision from SUMMARY key-decisions
+# Prefer file inputs for shell-safe text (preserves `$`, `*`, etc. exactly)
 node ~/.claude/get-shit-done/bin/gsd-tools.cjs state add-decision \
-  --phase "${PHASE}" --summary "${DECISION_TEXT}" --rationale "${RATIONALE}"
+  --phase "${PHASE}" --summary-file "${DECISION_TEXT_FILE}" --rationale-file "${RATIONALE_FILE}"

 # Add blockers if any found
-node ~/.claude/get-shit-done/bin/gsd-tools.cjs state add-blocker "Blocker description"
+node ~/.claude/get-shit-done/bin/gsd-tools.cjs state add-blocker --text-file "${BLOCKER_TEXT_FILE}"
 ```
 </step>

--- a/tests/state.test.cjs
+++ b/tests/state.test.cjs
@@ -180,7 +180,123 @@ describe('state-snapshot command', () => {
  });
 });

+describe('state mutation commands', () => {
+  let tmpDir;
+
+  beforeEach(() => {
+    tmpDir = createTempProject();
+  });
+
+  afterEach(() => {
+    cleanup(tmpDir);
+  });
+
+  test('add-decision preserves dollar amounts without corrupting Decisions section', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      `# Project State
+
+## Decisions
+No decisions yet.
+
+## Blockers
+None
+`
+    );
+
+    const result = runGsdTools(
+      "state add-decision --phase 11-01 --summary 'Benchmark prices moved from $0.50 to $2.00 to $5.00' --rationale 'track cost growth'",
+      tmpDir
+    );
+    assert.ok(result.success, `Command failed: ${result.error}`);
+
+    const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
+    assert.match(
+      state,
+      /- \[Phase 11-01\]: Benchmark prices moved from \$0\.50 to \$2\.00 to \$5\.00 — track cost growth/,
+      'decision entry should preserve literal dollar values'
+    );
+    assert.strictEqual((state.match(/^## Decisions$/gm) || []).length, 1, 'Decisions heading should not be duplicated');
+    assert.ok(!state.includes('No decisions yet.'), 'placeholder should be removed');
+  });
+
+  test('add-blocker preserves dollar strings without corrupting Blockers section', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      `# Project State
+
+## Decisions
+None
+
+## Blockers
+None
+`
+    );
+
+    const result = runGsdTools("state add-blocker --text 'Waiting on vendor quote $1.00 before approval'", tmpDir);
+    assert.ok(result.success, `Command failed: ${result.error}`);
+
+    const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
+    assert.match(state, /- Waiting on vendor quote \$1\.00 before approval/, 'blocker entry should preserve literal dollar values');
+    assert.strictEqual((state.match(/^## Blockers$/gm) || []).length, 1, 'Blockers heading should not be duplicated');
+  });
+
+  test('add-decision supports file inputs to preserve shell-sensitive dollar text', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      `# Project State
+
+## Decisions
+No decisions yet.
+
+## Blockers
+None
+`
+    );
+
+    const summaryPath = path.join(tmpDir, 'decision-summary.txt');
+    const rationalePath = path.join(tmpDir, 'decision-rationale.txt');
+    fs.writeFileSync(summaryPath, 'Price tiers: $0.50, $2.00, else $5.00\n');
+    fs.writeFileSync(rationalePath, 'Keep exact currency literals for budgeting\n');
+
+    const result = runGsdTools(
+      `state add-decision --phase 11-02 --summary-file "${summaryPath}" --rationale-file "${rationalePath}"`,
+      tmpDir
+    );
+    assert.ok(result.success, `Command failed: ${result.error}`);
+
+    const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
+    assert.match(
+      state,
+      /- \[Phase 11-02\]: Price tiers: \$0\.50, \$2\.00, else \$5\.00 — Keep exact currency literals for budgeting/,
+      'file-based decision input should preserve literal dollar values'
+    );
+  });
+
+  test('add-blocker supports --text-file for shell-sensitive text', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      `# Project State
+
+## Decisions
+None
+
+## Blockers
+None
+`
+    );
+
+    const blockerPath = path.join(tmpDir, 'blocker.txt');
+    fs.writeFileSync(blockerPath, 'Vendor quote updated from $1.00 to $2.00 pending approval\n');
+
+    const result = runGsdTools(`state add-blocker --text-file "${blockerPath}"`, tmpDir);
+    assert.ok(result.success, `Command failed: ${result.error}`);
+
+    const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
+    assert.match(state, /- Vendor quote updated from \$1\.00 to \$2\.00 pending approval/);
+  });
+});
+
 // ─────────────────────────────────────────────────────────────────────────────
 // summary-extract command
 // ─────────────────────────────────────────────────────────────────────────────
-