fix(sdk): make state.complete-phase idempotent (#3489)

Re-invoking `state complete-phase --phase <N>` on a phase that was
already marked complete in STATE.md silently rolled STATE.md back to
that phase's moment-of-completion — clobbering Status, Last Activity,
Last Activity Description, and the ## Current Position body. The bug
fired whenever a follow-up phase had been inserted (or the next phase
had begun) and a downstream workflow re-ran complete-phase on the
already-closed phase. Damage was silent: handler reported
{"updated":["Status","Last Activity","Current Position"]} and no error.

Root cause: cmdStateCompletePhase wrote unconditionally — it never
consulted STATE.md to detect that the requested phase had been
superseded. The handler is a legacy-bridge fallback (no native SDK
registration), so the SDK CLI fell through to gsd-tools.cjs.

Fix: add an idempotency guard at the top of cmdStateCompletePhase.
If STATE.md's canonical Current Phase field already names a phase
distinct from the one we are being asked to mark complete, return a
no-op payload ({updated:[], phase:"<N>", idempotent:true, note:"phase
already superseded; no-op"}) without writing to STATE.md.

The guard is conservative — it only fires when Current Phase is set
and differs from the resolved target. First-time completion (Current
Phase == target, or Current Phase absent) is unaffected, so the four
existing complete-phase test cases (#2761, #3063) continue to pass.

Regression test: tests/bug-3489-complete-phase-idempotent.test.cjs
- re-running complete-phase --phase 02.2 with Current Phase=02.2.1
  in STATE.md leaves the file byte-identical and reports idempotent:true
- normal first-time completion is NOT flagged idempotent

Scope: handler-level idempotency only. Does not address the related
stopped_at filename-sort ordering issue called out in the bug report
(filed under suggested fix #2) or porting to the native registry.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-14 07:47:08 -04:00
parent 49e7c48007
commit beca07cf3a
3 changed files with 150 additions and 0 deletions

View File

@@ -1805,6 +1805,27 @@ function cmdStateCompletePhase(cwd, raw, overridePhase) {
return;
}
// Idempotency guard (#3489). If STATE.md's canonical `Current Phase` field
// already names a phase distinct from the one we are being asked to mark
// complete, the project has advanced past the requested phase (e.g. a
// follow-up phase was inserted, or the next phase began). Re-running
// `state complete-phase --phase <N>` in that situation previously rolled
// STATE.md back to <N>'s moment-of-completion — silently clobbering Status,
// Last Activity, Last Activity Description, and the Current Position body.
// The handler is now a no-op in that case so re-invocation from downstream
// workflows cannot regress the project state.
const existingCurrentPhaseRaw = stateExtractField(content, 'Current Phase') || '';
const existingCurrentPhaseMatch = String(existingCurrentPhaseRaw).match(/(\d+[A-Z]?(?:\.\d+)*)/i);
const existingCurrentPhase = existingCurrentPhaseMatch ? existingCurrentPhaseMatch[1] : null;
if (existingCurrentPhase && existingCurrentPhase !== resolvedPhase) {
output(
{ updated: [], phase: resolvedPhase, idempotent: true, note: 'phase already superseded; no-op' },
raw,
'false',
);
return;
}
const today = new Date().toISOString().split('T')[0];
const updated = [];