diff --git a/.changeset/calm-wasps-dart.md b/.changeset/calm-wasps-dart.md new file mode 100644 index 000000000..64386d99c --- /dev/null +++ b/.changeset/calm-wasps-dart.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3487 +--- +state.patch now reports a field as updated only when its post-write on-disk value matches the requested value; fields the write pipeline re-derives away (e.g. current_phase, current_phase_name) are reported as failed instead of phantom updated diff --git a/src/state.cts b/src/state.cts index 516ca2bd3..5a1b8bb96 100644 --- a/src/state.cts +++ b/src/state.cts @@ -513,6 +513,34 @@ function cmdStatePatch(cwd: string, patches: Record, raw: boolea return result.content; }, cwd, { resync: shouldResync }); + // #3351: reconcile the report against the bytes actually persisted. + // patchCore's bookkeeping says whether the stateReplaceField text-replace + // MATCHED — but its plain-line pattern (`m` flag over the full document) + // can match the YAML frontmatter line for a lower-cased key, and the write + // pipeline (syncStateFrontmatter re-derivation + the FIELD_CLASSIFICATION + // preservation rows) then discards or restores that text before the file is + // saved. A field is only reported `updated` when its post-write on-disk + // value equals the requested value: the frontmatter key when present, + // else the body field (the legitimate working case for state.patch is + // display-cased BODY fields — Status, Current Plan, Phase — which are + // never frontmatter keys). + const persisted = platformReadSync(statePath) || ''; + const postFm = extractFrontmatter(persisted, statePath) as Record; + const postBody = stripFrontmatter(persisted); + const updated: string[] = []; + const failed: string[] = []; + for (const [field, value] of Object.entries(patches)) { + const persistedValue = Object.prototype.hasOwnProperty.call(postFm, field) + ? String(postFm[field]) + : stateExtractField(postBody, field); + if (persistedValue !== null && persistedValue.trim() === String(value).trim()) { + updated.push(field); + } else { + failed.push(field); + } + } + results = { updated, failed }; + output(results, raw, results.updated.length > 0 ? 'true' : 'false'); } catch { error('STATE.md not found'); diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 67b9e2233..33b9e7aac 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -1365,6 +1365,87 @@ describe('cmdStatePatch and cmdStateUpdate (state patch, state update)', () => { assert.ok(output.failed.includes('Missing'), 'Missing should be in failed list'); }); + // #3351: state.patch's `updated`/`failed` report must reflect what actually + // persisted to STATE.md after the write completes — not whether the internal + // text-replace matched frontmatter text that the write pipeline then + // re-derives away (syncStateFrontmatter re-derives current_phase / + // current_phase_name from the body `Phase:` line on every write, and the + // FIELD_CLASSIFICATION preservation rows restore the pre-write values when + // the body source did not change). + describe('#3351: state.patch report reconciled against persisted STATE.md', () => { + const phaseStateMd = [ + '---', + 'gsd_state_version: 1.0', + 'current_phase: 1', + 'current_phase_name: alpha', + 'risk_level: low', + 'status: executing', + '---', + '', + '# Project State', + '', + '## Current Position', + '', + 'Phase: 1 (alpha)', + '', + ].join('\n'); + + function readFm(dir) { + return frontmatterLib.extractFrontmatter( + fs.readFileSync(path.join(dir, '.planning', 'STATE.md'), 'utf-8'), + ); + } + + test('body-derived/curated frontmatter fields re-derived by the write are reported failed, not updated', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), phaseStateMd); + + const result = runGsdTools([ + 'query', + 'state.patch', + JSON.stringify({ current_phase: '7', current_phase_name: 'omega' }), + ], tmpDir); + assert.ok(result.success, `state patch failed: ${result.error}`); + + const report = JSON.parse(result.output); + assert.deepEqual(report.updated, [], `phantom updates must not be reported: ${result.output}`); + assert.deepEqual(report.failed.sort(), ['current_phase', 'current_phase_name'].sort()); + + // On-disk truth: neither requested value persisted. + const fm = readFm(tmpDir); + assert.notEqual(String(fm.current_phase), '7', 'current_phase was re-derived away by the write pipeline'); + assert.notEqual(String(fm.current_phase_name), 'omega', 'current_phase_name was restored by the curated preservation row'); + }); + + test('mixed patch reports an accurate updated/failed split (one lands, one is re-derived away)', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), phaseStateMd); + + const result = runGsdTools([ + 'query', + 'state.patch', + JSON.stringify({ risk_level: 'high', current_phase: '7' }), + ], tmpDir); + assert.ok(result.success, `state patch failed: ${result.error}`); + + const report = JSON.parse(result.output); + assert.deepEqual(report.updated, ['risk_level']); + assert.deepEqual(report.failed, ['current_phase']); + + const fm = readFm(tmpDir); + assert.equal(String(fm.risk_level), 'high', 'the custom frontmatter key must still land'); + assert.notEqual(String(fm.current_phase), '7'); + }); + + test('empty patch still reports both arrays empty', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), phaseStateMd); + + const result = runGsdTools(['query', 'state.patch', '{}'], tmpDir); + assert.ok(result.success, `state patch failed: ${result.error}`); + + const report = JSON.parse(result.output); + assert.deepEqual(report, { updated: [], failed: [] }); + }); + }); + test('state update changes a single field', () => { fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), stateMd);