* fix(#3351): reconcile state.patch report with persisted state.md * chore(#3351): add changeset fragment * chore(#3351): backfill pr number in changeset fragment --------- Co-authored-by: sim <sim@local>
This commit is contained in:
5
.changeset/calm-wasps-dart.md
Normal file
5
.changeset/calm-wasps-dart.md
Normal file
@@ -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
|
||||
@@ -513,6 +513,34 @@ function cmdStatePatch(cwd: string, patches: Record<string, string>, 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<string, unknown>;
|
||||
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');
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user