* fix(#3374): phase.complete stops harvesting stale body stopped_at Variant A: cmdPhaseComplete's adapter calls syncStateFrontmatter directly (deliberately - STATE.md commits atomically with ROADMAP/REQUIREMENTS), which also bypassed the #948/#1230 preservation pass every RMW write gets. A stale body 'Stopped at:' line then silently clobbered a fresher frontmatter stopped_at on every phase completion, with warnings: []. Three layers close it without reversing #3517's refresh expectation: - completePhaseCore now refreshes the body continuity line it implies ('Phase N complete, ready to plan Phase N+1'; ADR-2207 phrasing on the last phase), session-scoped via the new stateReplaceFieldInSession seam so a decoy bold Stopped-at line in an unrelated section cannot absorb the refresh. Replace-only - a layout with no session line keeps its shape and its frontmatter value survives via the preservation delta. - the RMW post-sync preservation chunk (snapshots + table-driven applyStatePreservation + #2736 re-assert, full bodyDeltas wired) is extracted into the shared applyPostSyncPreservation helper; the phase.complete adapter and writeStateMd (milestone complete / state sync - the gap the closed PR #3442 review flagged) now run it too. - cmdStateRecordSession pushed 'Stopped At' onto updated[] on any label MATCH, including a value already on disk - reporting a write that never changed a byte. It now reports only on real change, and the match is tracked separately so an identical value does not arm the #944 DWIM section rewrite (which would reset an executor-authored resume file to None). * docs(#3374): backfill changeset pr field to 3491 * fix(#3374): drop the writeStateMd preservation pass - state sync's #905 contract is body-wins CI on this PR caught what the closed PR #3442 review's MAJOR remediation option (a) would have broken: state sync's #905 contract ('body annotation beats existing frontmatter when both are present') is the opposite by design - sync exists to re-derive frontmatter from the body. A blanket applyStatePreservation pass on writeStateMd re-locked stale frontmatter (current_phase 3 over the body's 5) on every sync. Take the review's sanctioned option (b) instead: the scope claim is accurate (phase.complete only) and the milestone complete / state sync exposure is tracked as follow-up issue #3492. --------- Co-authored-by: sim <sim@local>
This commit is contained in:
@@ -2207,6 +2207,86 @@ describe('cmdStateRecordSession (state record-session)', () => {
|
||||
assert.strictEqual(output.recorded, false, 'recorded should be false when no session fields found');
|
||||
assert.ok(output.reason !== undefined, 'should have a reason');
|
||||
});
|
||||
|
||||
// #3374 Variant B: stateReplaceField returns the replaced string on any label
|
||||
// MATCH, including when the value is already the target — so `updated` used
|
||||
// to report 'Stopped At' for a write that never changed a byte (and that the
|
||||
// #948 no-op guard then discarded entirely), leaving a stale frontmatter
|
||||
// stopped_at undetectable to the caller. The report must reflect real change,
|
||||
// and a matched-but-identical value must NOT arm the #944 DWIM insertion
|
||||
// branch (which wholesale-rewrites the session section and would reset an
|
||||
// executor-authored resume file to 'None').
|
||||
test('#3374: --stopped-at with the value already in the body is not reported updated and writes nothing', () => {
|
||||
const PINNED_MS = Date.parse('2020-09-01T09:00:00.000Z');
|
||||
const PINNED_ISO = '2020-09-01T09:00:00.000Z';
|
||||
const executorResume = '.planning/phases/02/02-01-PLAN.md';
|
||||
const fixture = [
|
||||
'# Project State',
|
||||
'',
|
||||
'## Session Continuity',
|
||||
'',
|
||||
`**Last session:** ${PINNED_ISO}`,
|
||||
'**Stopped at:** Phase 2, Plan 1',
|
||||
`**Resume file:** ${executorResume}`,
|
||||
].join('\n') + '\n';
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
fs.writeFileSync(statePath, fixture);
|
||||
|
||||
const result = runGsdTools('state record-session --stopped-at "Phase 2, Plan 1"', tmpDir, {
|
||||
GSD_TEST_MODE: '1',
|
||||
GSD_NOW_MS: String(PINNED_MS),
|
||||
});
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.ok(
|
||||
!((output.updated || []).includes('Stopped At')),
|
||||
`updated must not report a Stopped At write that changed nothing; got ${JSON.stringify(output.updated)} (#3374 Variant B)`,
|
||||
);
|
||||
|
||||
// The pinned clock makes the Last-session replacement an identity too, so
|
||||
// the whole transform is a no-op and the #948 no-op guard must skip the
|
||||
// write entirely — the file must be byte-identical.
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
assert.strictEqual(
|
||||
after,
|
||||
fixture,
|
||||
'no field changed, so no write may occur (#3374 Variant B)',
|
||||
);
|
||||
assert.strictEqual(
|
||||
stateDocument.stateExtractField(after, 'Resume file'),
|
||||
executorResume,
|
||||
'an identical --stopped-at must not arm the #944 DWIM section rewrite (executor-authored resume file reset to None)',
|
||||
);
|
||||
});
|
||||
|
||||
test('#3374: --stopped-at with a new value still reports Stopped At and syncs the frontmatter', () => {
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), sessionFixture);
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
const newValue = 'Phase 3 complete, ready to plan Phase 4';
|
||||
|
||||
const result = runGsdTools(['state', 'record-session', '--stopped-at', newValue], tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.ok(
|
||||
(output.updated || []).includes('Stopped At'),
|
||||
`a real change must keep reporting Stopped At; got ${JSON.stringify(output.updated)}`,
|
||||
);
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
assert.strictEqual(
|
||||
stateDocument.stateExtractField(after, 'Stopped at'),
|
||||
newValue,
|
||||
'the body Stopped at line should carry the new value',
|
||||
);
|
||||
const fm = frontmatterLib.extractFrontmatter(after);
|
||||
assert.strictEqual(
|
||||
fm.stopped_at,
|
||||
newValue,
|
||||
`the RMW sync must reflect the new value in frontmatter; got ${JSON.stringify(fm.stopped_at)}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user