fix: state mutations recurse when Current Position has dollar amounts (#3463)

* fix(state): prevent  backreference expansion in state mutations

* chore(changeset): set pr field for #3463

* test(state): use structured STATE parser in bug-3454 regression
This commit is contained in:
Tom Boucher
2026-05-13 13:54:30 -04:00
committed by GitHub
parent d0f916728b
commit c5d4cf35e6
5 changed files with 171 additions and 7 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3463
---
**State mutations now preserve literal dollar amounts** — begin-phase, advance-plan, and complete-phase no longer recurse Current Position content when values contain `$N` patterns.

View File

@@ -264,7 +264,7 @@ function updateCurrentPositionFields(content, fields) {
posBody = posBody.replace(/^Plan:.*$/m, `Plan: ${fields.plan}`);
}
return content.replace(posPattern, `${posMatch[1]}${posBody}`);
return content.replace(posPattern, () => `${posMatch[1]}${posBody}`);
}
function cmdStateAdvancePlan(cwd, raw) {
@@ -1171,7 +1171,7 @@ function cmdStateBeginPhase(cwd, phaseNumber, phaseName, planCount, raw) {
posBody = posBody.replace(/^Last activity:.*$/im, newActivity);
}
content = content.replace(positionPattern, `${header}${posBody}`);
content = content.replace(positionPattern, () => `${header}${posBody}`);
updated.push('Current Position');
}
} else {
@@ -1185,7 +1185,7 @@ function cmdStateBeginPhase(cwd, phaseNumber, phaseName, planCount, raw) {
const resumeActivity = `Last activity: ${today} -- Phase ${phaseNumber} execution resumed (wave continue)`;
if (/^Last activity:/im.test(posBody)) {
posBody = posBody.replace(/^Last activity:.*$/im, resumeActivity);
content = content.replace(positionPattern, `${header}${posBody}`);
content = content.replace(positionPattern, () => `${header}${posBody}`);
updated.push('Last activity (resume)');
}
}
@@ -1278,7 +1278,7 @@ function updatePerformanceMetricsSection(content, cwd, phaseNum, planCount, summ
tableBody = tableBody ? tableBody + '\n' + newRow : newRow;
}
content = content.replace(byPhaseTablePattern, `$1${tableBody}\n`);
content = content.replace(byPhaseTablePattern, (_match, tableHeader) => `${tableHeader}${tableBody}\n`);
}
return content;
@@ -1852,7 +1852,7 @@ function cmdStateCompletePhase(cwd, raw, overridePhase) {
posBody = posBody.replace(/^Last activity:.*$/im, newActivity);
}
content = content.replace(positionPattern, `${header}${posBody}`);
content = content.replace(positionPattern, () => `${header}${posBody}`);
updated.push('Current Position');
}

View File

@@ -533,6 +533,21 @@ describe('stateBeginPhase', () => {
const content = await readFile(join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
expect(content).toContain('Plan: 1 of 3');
});
it('preserves literal dollar amounts in Current Position body', async () => {
const { stateBeginPhase } = await import('./state-mutation.js');
const withBudget = MINIMAL_STATE.replace(
'Last activity: 2026-04-08 -- Phase 10 execution started',
'Last activity: 2026-04-08 -- Phase 10 execution started\nBudget: $2,500 max test',
);
await setupTestProject(tmpDir, withBudget);
await stateBeginPhase(['11', 'State Mutations', '3'], tmpDir);
const content = await readFile(join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
expect(content).toContain('Budget: $2,500 max test');
expect((content.match(/^Budget:/gm) || []).length).toBe(1);
});
});
// ─── stateAdvancePlan ───────────────────────────────────────────────────────
@@ -557,6 +572,23 @@ describe('stateAdvancePlan', () => {
expect(data.advanced).toBe(true);
expect(data.current_plan).toBe(3);
});
it('keeps literal dollar amounts stable after multiple updates', async () => {
const { stateAdvancePlan } = await import('./state-mutation.js');
const withBudget = MINIMAL_STATE.replace(
'Last activity: 2026-04-08 -- Phase 10 execution started',
'Last activity: 2026-04-08 -- Phase 10 execution started\nBudget: $2,500 max test',
).replace('Plan: 2 of 3', 'Plan: 1 of 20');
await setupTestProject(tmpDir, withBudget);
for (let i = 0; i < 8; i += 1) {
await stateAdvancePlan([], tmpDir);
}
const content = await readFile(join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
expect(content).toContain('Budget: $2,500 max test');
expect((content.match(/^Budget:/gm) || []).length).toBe(1);
});
});
// ─── stateAddDecision ───────────────────────────────────────────────────────

View File

@@ -77,7 +77,7 @@ function updateCurrentPositionFields(content: string, fields: Record<string, str
posBody = posBody.replace(/^Plan:.*$/m, `Plan: ${fields.plan}`);
}
return content.replace(posPattern, `${posMatch[1]}${posBody}`);
return content.replace(posPattern, () => `${posMatch[1]}${posBody}`);
}
/** Port of `readTextArgOrFile` from `state.cjs` — inline text or file path under project root. */
@@ -508,7 +508,7 @@ export const stateBeginPhase: QueryHandler = async (args, projectDir, workstream
posBody = posBody.replace(/^Last activity:.*$/im, newActivity);
}
content = content.replace(positionPattern, `${header}${posBody}`);
content = content.replace(positionPattern, () => `${header}${posBody}`);
updated.push('Current Position');
}

View File

@@ -0,0 +1,127 @@
'use strict';
const { describe, test, beforeEach, afterEach } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const { createTempProject, cleanup, runGsdTools } = require('./helpers.cjs');
function seedState(tmpDir, planLine = '1 of 2') {
const state = `# Project State
**Status:** executing
**Current Phase:** 1
## Current Position
Phase: 1 of 1
Plan: ${planLine}
Status: Ready
Last activity: 2026-01-01
Budget: $2,500 max test
## Session Continuity
Last session: 2026-01-01
`;
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), state, 'utf8');
}
function parseStateFile(tmpDir) {
const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf8');
const sections = {};
const keyCountsBySection = {};
let currentSection = '__root__';
sections[currentSection] = {};
keyCountsBySection[currentSection] = {};
for (const rawLine of content.split(/\r?\n/u)) {
const headingMatch = /^##\s+(.+)$/u.exec(rawLine);
if (headingMatch) {
currentSection = headingMatch[1].trim();
sections[currentSection] = sections[currentSection] || {};
keyCountsBySection[currentSection] = keyCountsBySection[currentSection] || {};
continue;
}
const trimmed = rawLine.trim();
if (!trimmed) continue;
const boldFieldMatch = /^\*\*([^*]+)\*\*:\s*(.*)$/u.exec(trimmed);
if (boldFieldMatch) {
const key = boldFieldMatch[1].trim();
const value = boldFieldMatch[2].trim();
sections[currentSection][key] = value;
keyCountsBySection[currentSection][key] = (keyCountsBySection[currentSection][key] || 0) + 1;
continue;
}
const colonIndex = trimmed.indexOf(':');
if (colonIndex <= 0) continue;
const key = trimmed.slice(0, colonIndex).trim();
const value = trimmed.slice(colonIndex + 1).trim();
sections[currentSection][key] = value;
keyCountsBySection[currentSection][key] = (keyCountsBySection[currentSection][key] || 0) + 1;
}
return { content, sections, keyCountsBySection };
}
describe('bug #3454: state mutation must preserve literal $N amounts', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject('bug-3454-');
});
afterEach(() => {
cleanup(tmpDir);
});
test('state advance-plan keeps Current Position dollar amount literal', () => {
seedState(tmpDir, '1 of 20');
const result = runGsdTools(['state', 'advance-plan'], tmpDir);
assert.equal(result.success, true, `state advance-plan failed: ${result.error || result.output}`);
const parsed = parseStateFile(tmpDir);
const currentPosition = parsed.sections['Current Position'] || {};
assert.equal(currentPosition.Budget, '$2,500 max test');
assert.equal((parsed.keyCountsBySection['Current Position'] || {}).Budget, 1);
});
test('state begin-phase keeps Current Position dollar amount literal', () => {
seedState(tmpDir);
const result = runGsdTools(['state', 'begin-phase', '--phase', '1', '--name', 'setup', '--plans', '2'], tmpDir);
assert.equal(result.success, true, `state begin-phase failed: ${result.error || result.output}`);
const parsed = parseStateFile(tmpDir);
const currentPosition = parsed.sections['Current Position'] || {};
assert.equal(currentPosition.Budget, '$2,500 max test');
assert.equal((parsed.keyCountsBySection['Current Position'] || {}).Budget, 1);
});
test('state complete-phase keeps Current Position dollar amount literal', () => {
seedState(tmpDir);
const result = runGsdTools(['state', 'complete-phase', '--phase', '1'], tmpDir);
assert.equal(result.success, true, `state complete-phase failed: ${result.error || result.output}`);
const parsed = parseStateFile(tmpDir);
const currentPosition = parsed.sections['Current Position'] || {};
assert.equal(currentPosition.Budget, '$2,500 max test');
assert.equal((parsed.keyCountsBySection['Current Position'] || {}).Budget, 1);
});
test('repeated state advance-plan stays size-bounded with dollar amounts', () => {
seedState(tmpDir, '1 of 20');
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
let stabilizedSize = null;
for (let i = 0; i < 8; i += 1) {
const result = runGsdTools(['state', 'advance-plan'], tmpDir);
assert.equal(result.success, true, `iteration ${i + 1} failed: ${result.error || result.output}`);
if (i === 0) stabilizedSize = fs.statSync(statePath).size;
}
const endSize = fs.statSync(statePath).size;
const growth = endSize / stabilizedSize;
assert.ok(growth <= 1.5, `expected <=1.5x growth after first write, got ${growth.toFixed(2)}x (${stabilizedSize} -> ${endSize})`);
});
});