From 77d434709de943904a6645dc90ab4d5c3ce10cc0 Mon Sep 17 00:00:00 2001 From: Colin Johnson Date: Mon, 23 Feb 2026 10:55:54 -0500 Subject: [PATCH] Fix STATE.md decision corruption and dollar handling (#701) --- get-shit-done/bin/gsd-tools.cjs | 12 ++- get-shit-done/bin/lib/state.cjs | 61 +++++++++--- get-shit-done/workflows/execute-plan.md | 5 +- tests/state.test.cjs | 118 +++++++++++++++++++++++- 4 files changed, 177 insertions(+), 19 deletions(-) diff --git a/get-shit-done/bin/gsd-tools.cjs b/get-shit-done/bin/gsd-tools.cjs index 8c721c39d..60ff131b3 100755 --- 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); diff --git a/get-shit-done/bin/lib/state.cjs b/get-shit-done/bin/lib/state.cjs index 456f6517a..78c2ed410 100644 --- 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 { diff --git a/get-shit-done/workflows/execute-plan.md b/get-shit-done/workflows/execute-plan.md index 0c2edb802..fc6c61c37 100644 --- 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}" ``` diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 95c853633..9e6562e05 100644 --- 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 // ───────────────────────────────────────────────────────────────────────────── -