diff --git a/.changeset/zesty-tunas-bark.md b/.changeset/zesty-tunas-bark.md new file mode 100644 index 000000000..3920b9795 --- /dev/null +++ b/.changeset/zesty-tunas-bark.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4381 +--- +**`state` no longer guesses the STATE.md `status` token from substrings of the status prose** — a status line mentioning `.planning/` (or Italian `verifica`, `completezza`, `fasi complete`) no longer silently becomes `status: planning`/`verifying`/`completed`; recognized vocabulary values keep normalizing and unrecognized prose stays visible, `state record-session` without arguments now errors instead of writing, and stray `*-SUMMARY.md` files without a plan twin stay excluded from `progress.completed_plans` recounts. (#4186) diff --git a/CONTEXT.md b/CONTEXT.md index 1f2e6a438..294ae108c 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -91,7 +91,7 @@ Module owning STATE.md lifecycle/maintenance transitions as intent-based methods The one declaration (ADR-3473 §8.8, #3873) for "which STATE.md keys exist and what they carry", replacing three hand-maintained tables that were already observed to disagree: `FIELD_CLASSIFICATION` and `FRONTMATTER_BODY_SOURCE` (STATE.md Transition Module) and `FRONTMATTER_KEY_TO_BODY_LABEL` (STATE.md Document Module's `bodyLabelFor`, `state.cts`). One frozen, null-prototype `STATE_FIELD_SCHEMA` row per key carries `type`, `cardinality`, `source`, `preservation`, `guard`/`mergeStrategy` (ADR-3408's closed vocabularies, whose type declarations moved here), `bodySource`/`bodyLabel`, `acceptedShapes` (declared value shapes a hand-written parser accepts — e.g. `current_plan`'s `N` / `N of M`, #3784 — never a predicate; Greenspun's Tenth Rule still applies), and `emitted` (mirrors `buildStateFrontmatter`'s null-guards). The three original tables are now PROJECTIONS derived from this schema at module load, byte-identical in shape/key-order/frozen-and-null-prototype-ness to what they were before #3873 — every existing consumer (the preservation dispatch loop, `getFieldClassification`, `getPreserveWhenUnchangedFields`, `bodyLabelFor`, #3872's `declaredLeavesOf`) is unaffected. **The `last_activity` disagreement is resolved by declaration, not by picking the table that "looks right":** it carries a `bodySource` (it IS body-derived) but deliberately no `bodyLabel`, matching what ships today — its `preservation` is `derive`, so it can never reach `bodyLabelFor`'s `STATE_BODY_LABEL_UNWIRED_ROW` throw, pinned by `tests/state.test.cjs`'s `lastActivityLabelResolutionMatchesShippedBehavior`. Leaf module: imports from neither `state-transition.cts` nor `state.cts` (both import it), avoiding the CJS require-cycle `src/health-diagnostic-types.cts` was split out to break. `scripts/lint-state-field-drift.cjs` is UNCHANGED and retained — it guards the #3187 STATE.md field-extraction fallback-chain re-derivation, an orthogonal concern this schema does not make unrepresentable (§8.8's own claim that the guard is deleted here was verified false). Source of truth: `gsd-core/bin/lib/state-md-schema.cjs` (generated from `src/state-md-schema.cts`). ### STATE.md Status Lifecycle (ADR-2207) -The `Status` field in STATE.md follows a strict lifecycle: `Ready to plan` → `All phases complete` (all phases done, milestone awaiting formal close) → ` milestone complete` (terminal, written only by the milestone-close verb `milestoneCompleteCore`) → `Awaiting next milestone` (archived). Phase-completion verbs write `All phases complete` on the last phase — never `Milestone complete` (the overloaded bare value was removed in #2204 per ADR-2207 to decouple phase-level writes from milestone termination). `normalizeStateStatus` maps any status containing "complete" → `completed`, so consumers using the normalized projection (workstream inventory's `status` field, statusline) recognize `All phases complete` without code changes. Note: `isCompletedInventory` (workstream-inventory-builder.cts) intentionally checks only for the terminal `\bmilestone\s+complete\b` / `\barchived\b` — `All phases complete` returns `false` (intermediate, not terminal). +The `Status` field in STATE.md follows a strict lifecycle: `Ready to plan` → `All phases complete` (all phases done, milestone awaiting formal close) → ` milestone complete` (terminal, written only by the milestone-close verb `milestoneCompleteCore`) → `Awaiting next milestone` (archived). Phase-completion verbs write `All phases complete` on the last phase — never `Milestone complete` (the overloaded bare value was removed in #2204 per ADR-2207 to decouple phase-level writes from milestone termination). `normalizeStateStatus` recognizes the whole-field status vocabulary (exact values plus anchored `Executing Phase N` / `Phase N complete` / ` milestone complete` forms — `STATUS_EXACT_TOKENS` / `STATUS_ANCHORED_PATTERNS`, src/state-document.cts; #4186 ended the pre-#4186 substring scanning, so prose merely CONTAINING a status word passes through verbatim), so consumers using the normalized projection (workstream inventory's `status` field, statusline) recognize `All phases complete` and the other lifecycle values without code changes. Note: `isCompletedInventory` (workstream-inventory-builder.cts) intentionally checks only for the terminal `\bmilestone\s+complete\b` / `\barchived\b` — `All phases complete` returns `false` (intermediate, not terminal). ### Query Execution Policy Module Module owning query transport routing policy projection (`preferNative`, fallback policy, workstream subprocess forcing) at execution seam. diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index 960fa56f5..ff0925d1a 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -113,7 +113,7 @@ node gsd-tools.cjs state add-decision --summary-file path [--rationale-file path node gsd-tools.cjs state add-blocker --text "..." node gsd-tools.cjs state resolve-blocker --text "..." -# Record session continuity +# Record session continuity (at least one of --stopped-at / --resume-file is required) node gsd-tools.cjs state record-session --stopped-at "..." [--resume-file path] # Phase start — update STATE.md Status/Last activity for a new phase diff --git a/docs/adr/2207-status-field-lifecycle-ownership.md b/docs/adr/2207-status-field-lifecycle-ownership.md index b936815d8..a1fd13722 100644 --- a/docs/adr/2207-status-field-lifecycle-ownership.md +++ b/docs/adr/2207-status-field-lifecycle-ownership.md @@ -30,4 +30,4 @@ STATE.md's `Status` field is written by two transitions with an **overloaded** v **Positive:** the overload is removed; the intermediate and terminal "complete" states are distinct; a phase-level verb no longer writes the terminal milestone state; the wrong-phase flip becomes a parse-correctness concern already owned upstream. -**Cost / follow-through (implemented in #2204):** consumers that key on the `Milestone complete` string must recognize `All phases complete` — `workflows/progress.md` (Route D), `verify.cts`, and `workstream-inventory-builder.cts`. `normalizeStateStatus` already maps any status containing "complete" → `completed`, so it needs no change. A `CONTEXT.md` glossary entry enumerating the `Status` lifecycle lands with the #2204 implementation. +**Cost / follow-through (implemented in #2204):** consumers that key on the `Milestone complete` string must recognize `All phases complete` — `workflows/progress.md` (Route D), `verify.cts`, and `workstream-inventory-builder.cts`. `normalizeStateStatus` already maps any status containing "complete" → `completed`, so it needs no change. *(Superseded mechanism, #4186: recognition is now an ANCHORED whole-field vocabulary match — `All phases complete` and ` milestone complete` still map to `completed`; prose merely containing "complete" passes through verbatim. The consumer-recognition consequence above is unchanged.)* A `CONTEXT.md` glossary entry enumerating the `Status` lifecycle lands with the #2204 implementation. diff --git a/hooks/gsd-statusline.js b/hooks/gsd-statusline.js index c73d56401..37e110e9e 100755 --- a/hooks/gsd-statusline.js +++ b/hooks/gsd-statusline.js @@ -449,13 +449,17 @@ function contextTokenSuffix(currentUsage) { // --- Compact state format (opt-in) --------------------------------------------- /** - * Collapse GSD's free-text status (often a multi-sentence narrative) to a - * single keyword, built on the canonical normalizer (#2162 approval - * condition): normalizeStateStatus() in state-document.cjs owns the status - * vocabulary (discussing / planning / executing / verifying / completed / - * paused) so the two can't drift. "paused" — the canonical stuck state — is - * uppercased to PAUSED, the one state worth shouting about. Statuses the - * normalizer passes through unrecognized fall back to their first word, + * Collapse GSD's status value to a single keyword, built on the canonical + * normalizer (#2162 approval condition): normalizeStateStatus() in + * state-document.cjs owns the status vocabulary (discussing / planning / + * executing / verifying / completed / paused) so the two can't drift. + * #4186: the normalizer recognizes the DECLARED vocabulary by anchored + * whole-field match — vocabulary values (the state writer persists tokens) + * collapse to their keyword; free-text narratives are no longer + * keyword-guessed from substrings (a `.planning/` mention in non-English + * prose used to render `planning`), and pass through unrecognized to the + * first-word fallback below. "paused" — the canonical stuck state — is + * uppercased to PAUSED, the one state worth shouting about. The fallback is * capped at 16 chars so a rogue STATE.md can't blow up the line. * Returns null for empty input. */ diff --git a/scripts/lint-docs-guard-registration.exempt-baseline.cjs b/scripts/lint-docs-guard-registration.exempt-baseline.cjs index fb641a3c2..157747b9a 100644 --- a/scripts/lint-docs-guard-registration.exempt-baseline.cjs +++ b/scripts/lint-docs-guard-registration.exempt-baseline.cjs @@ -191,7 +191,9 @@ const DOCS_GUARD_EXEMPT_DOCS_PATHS = { 'runtime-name-policy.test.cjs': ['docs/customize/skills'], 'security-prompt-injection.security.test.cjs': ['docs/notes.md'], 'shipped-reference-cites.test.cjs': [], - 'state.test.cjs': ['docs/CONFIGURATION.md', 'docs/reference/state-md.md'], + // #4186: the record-session usage-contract test cites the documented + // signature in docs/CLI-TOOLS.md in its explanatory comment. + 'state.test.cjs': ['docs/CLI-TOOLS.md', 'docs/CONFIGURATION.md', 'docs/reference/state-md.md'], 'worktree-safety.test.cjs': ['docs/SUMMARY.md'], }; diff --git a/src/state-document.cts b/src/state-document.cts index 5921b31d2..a8fc16bb8 100644 --- a/src/state-document.cts +++ b/src/state-document.cts @@ -611,31 +611,115 @@ export function stateReplaceFieldInSession(content: string, primary: string, fal return withSection(content, target, (sectionBody) => stateReplaceFieldWithFallback(sectionBody, primary, fallback, value)); } +/** + * #4186: the DECLARED raw-status vocabulary `normalizeStateStatus` recognizes. + * Keys are whole-field values, compared against the caller's input after + * lowercasing, trimming, and collapsing internal whitespace runs to single + * spaces — so case and whitespace variants the vocabulary documents + * (`EXECUTING PHASE 5`, ` Paused `, `In progress`) keep normalizing. + * Values are members of `STATUS_LIFECYCLE_ENUM` (`src/state-md-schema.cts`) + * — the set the normalizer maps recognized input ONTO. + * + * The mapping preserves the PRE-#4186 branch ORDER's observable artifacts for + * every value the old substring chain recognized: `Planning complete` → + * `planning` (the `planning` branch outranked `complete`) and + * `Phase complete — ready for verification` → `verifying` (`verif` outranked + * `complete`; pinned by tests/state.test.cjs's advance-plan case-5 comment). + * + * Everything else — prose that merely CONTAINS a status word — falls through + * to the caller's raw value (the recorded lenient fallback, #3873 phase-3 + * row 26). A token guessed from a substring inside a sentence is worse than + * a visible paragraph: the paragraph is visibly prose, the wrong token is + * not (a `.planning/` path in Italian prose silently produced + * `status: planning`; `verificata` produced `verifying`; `completezza` + * produced `completed`). + */ +export const STATUS_EXACT_TOKENS: Readonly> = Object.freeze({ + paused: 'paused', + stopped: 'paused', + executing: 'executing', + 'in progress': 'executing', + 'ready to execute': 'executing', + planning: 'planning', + 'ready to plan': 'planning', + 'planning complete': 'planning', + discussing: 'discussing', + verifying: 'verifying', + completed: 'completed', + done: 'completed', + complete: 'completed', + 'phase complete': 'completed', + // advance-plan's phase-complete write (state-transition.cts:1812) — maps + // to `verifying`, preserving the pre-#4186 branch order where `verif` + // outranked `complete`. + 'phase complete — ready for verification': 'verifying', + 'all phases complete': 'completed', + // Legacy bare terminal form. ADR-2207/#2204 removed it from every WRITER + // (phase verbs write `All phases complete`; milestone close writes + // ` milestone complete`) — kept here as READER recognition so a + // legacy STATE.md still normalizes, exactly the way KNOWN_TEMPLATE_DEFAULTS + // keeps the other legacy Status strings. + 'milestone complete': 'completed', + unknown: 'unknown', +} as const); + +/** + * #4186: ANCHORED patterns for handler-written raw statuses whose text + * carries a variable component (a phase number, a milestone version, a + * #1070 completion glyph). Each pattern is matched against the same + * normalized key as `STATUS_EXACT_TOKENS` (lowercased, trimmed, + * whitespace-collapsed) and must match the WHOLE value — never a substring — + * mirroring how `KNOWN_STATUS_PATTERNS` anchors its template-default checks. + * `Executing Phase 5 — final stretch` (executor-appended prose) matches + * NOTHING and passes through verbatim, the same discipline #1070 applies to + * "Complete but needs manual QA". + */ +export const STATUS_ANCHORED_PATTERNS: ReadonlyArray = Object.freeze([ + // begin-phase / planned transitions write `Executing Phase ${N}`. + [/^executing phase\s+\S+$/, 'executing'], + [/^planning phase\s+\S+$/, 'planning'], + [/^verifying phase\s+\S+$/, 'verifying'], + // phase-complete verbs write `Phase ${N} complete` (state.cts) — the exact + // shape the #3578 demote guard then re-checks against the disk counters. + [/^phase\s+\S+\s+complete$/, 'completed'], + // milestoneCompleteCore writes `${version} milestone complete` (terminal, + // ADR-2207); the version label is a single token (milestone.cts's charset + // validation admits letters, digits, '.', '-', '_'). + [/^\S+\s+milestone complete$/, 'completed'], + // #1070: LLM executors may write "Complete ✓" or bare "Complete" when + // finishing a phase. + [/^complete\s*[✓✔✅☑]?$/, 'completed'], +] as const); + +/** + * Normalize a raw `Status` body-field value to the canonical status token. + * + * #4186: recognition is ANCHORED — the whole field value (lowercased, + * trimmed, whitespace-collapsed) must be a member of the declared vocabulary + * (`STATUS_EXACT_TOKENS` / `STATUS_ANCHORED_PATTERNS` above). The pre-#4186 + * implementation ran a first-match-wins chain of SUBSTRING tests over the + * free-prose field, so any prose merely CONTAINING a trigger word was + * silently rewritten to a credible wrong token: a `.planning/...` path + * mentioned in a non-English status line landed on `planning` (the trigger + * word lives in the directory name and outranked the `verif`/`complete` + * branches), Italian `verifica*` landed on `verifying`, `completezza` and + * `fasi complete` landed on `completed`. The lenient FALLBACK is unchanged + * and recorded (#3873 phase-3 row 26): an unrecognized value passes through + * verbatim — visible prose, never a guessed token. + * + * `pausedAt` keeps its documented force (issue #4186: intended behavior): a + * truthy value yields `paused` regardless of the prose. + */ export function normalizeStateStatus(status: string | null | undefined, pausedAt: unknown): string { - let normalizedStatus = status || 'unknown'; - const statusLower = (status || '').toLowerCase(); - if (statusLower.includes('paused') || statusLower.includes('stopped') || pausedAt) { - normalizedStatus = 'paused'; + if (pausedAt) return 'paused'; + if (!status) return 'unknown'; + const key = status.trim().toLowerCase().replace(/\s+/g, ' '); + const exact = STATUS_EXACT_TOKENS[key]; + if (exact) return exact; + for (const [pattern, token] of STATUS_ANCHORED_PATTERNS) { + if (pattern.test(key)) return token; } - else if (statusLower.includes('executing') || statusLower.includes('in progress')) { - normalizedStatus = 'executing'; - } - else if (statusLower.includes('planning') || statusLower.includes('ready to plan')) { - normalizedStatus = 'planning'; - } - else if (statusLower.includes('discussing')) { - normalizedStatus = 'discussing'; - } - else if (statusLower.includes('verif')) { - normalizedStatus = 'verifying'; - } - else if (statusLower.includes('complete') || statusLower.includes('done')) { - normalizedStatus = 'completed'; - } - else if (statusLower.includes('ready to execute')) { - normalizedStatus = 'executing'; - } - return normalizedStatus; + return status; } /** diff --git a/src/state-md-schema.cts b/src/state-md-schema.cts index 3ea1306a0..ea31d61a7 100644 --- a/src/state-md-schema.cts +++ b/src/state-md-schema.cts @@ -67,24 +67,31 @@ export type FieldMergeStrategy = 'progress-ratchet'; /** * The seven CANONICAL values `normalizeStateStatus` (`src/state-document.cts`) * maps recognized raw status prose TO — the function's default fallback plus - * each branch's literal output, in the order the function tests them. This is - * NOT the raw body prose vocabulary `CONTEXT.md`'s "STATE.md Status Lifecycle - * (ADR-2207)" entry documents (`Ready to plan` → `All phases complete` → - * ` milestone complete` → `Awaiting next milestone`, plus the - * handler-authored strings in `KNOWN_TEMPLATE_DEFAULTS['Status']`) — that is - * free-form prose `normalizeStateStatus` READS. + * each vocabulary entry's literal output. This is NOT the raw body prose + * vocabulary `CONTEXT.md`'s "STATE.md Status Lifecycle (ADR-2207)" entry + * documents (`Ready to plan` → `All phases complete` → ` milestone + * complete` → `Awaiting next milestone`, plus the handler-authored strings + * in `KNOWN_TEMPLATE_DEFAULTS['Status']`) — that is free-form prose + * `normalizeStateStatus` READS. * * CORRECTED (#3873 phase-3 test-matrix row 26 — verified by executing * `normalizeStateStatus`, not by reading this docstring's prior claim): * this is NOT a closed set the `status` frontmatter key is restricted to at - * runtime. `normalizeStateStatus` is deliberately LENIENT: its fallback is - * `normalizedStatus = status || 'unknown'`, and when none of its - * substring-match branches recognize the raw input, that fallback — the - * caller's raw, UNRECOGNIZED prose — is returned unchanged. A status value - * outside this seven-member set is not rejected, coerced, or normalized; it - * passes straight through into the frontmatter. `STATUS_LIFECYCLE_ENUM` is - * therefore the set of values the normalizer maps recognized input ONTO, not - * a runtime-enforced closed vocabulary for the field. + * runtime. `normalizeStateStatus` is deliberately LENIENT: its fallback + * returns the caller's raw, UNRECOGNIZED prose unchanged — when none of its + * vocabulary entries recognize the whole-field input, that raw value is what + * the function returns. A status value outside this seven-member set is not + * rejected, coerced, or normalized; it passes straight through into the + * frontmatter. `STATUS_LIFECYCLE_ENUM` is therefore the set of values the + * normalizer maps recognized input ONTO, not a runtime-enforced closed + * vocabulary for the field. + * + * #4186: recognition is ANCHORED (whole-field match against the declared + * `STATUS_EXACT_TOKENS` / `STATUS_ANCHORED_PATTERNS` tables in + * `src/state-document.cts`), never a substring scan of the prose — prose + * merely CONTAINING a status word (a `.planning/` path, `verifica*`, + * `completezza`) passes through verbatim instead of being rewritten to a + * credible wrong token. */ export const STATUS_LIFECYCLE_ENUM = Object.freeze([ 'unknown', diff --git a/src/state.cts b/src/state.cts index e55f8105a..9f91423f6 100644 --- a/src/state.cts +++ b/src/state.cts @@ -1921,6 +1921,18 @@ function cmdStateResolveBlocker(cwd: string, text: string, raw: boolean): void { } function cmdStateRecordSession(cwd: string, options: StateRecordSessionOptions, raw: boolean): void { + // #4186: a bare invocation is a usage error, not a heartbeat write. The + // pre-#4186 handler accepted zero arguments and still refreshed + // `Last session` / `Last Date` / `last_updated` — a caller probing the + // command's signature (the way other subcommands encourage) silently + // mutated STATE.md. Mirrors `state update`'s required-arg guard + // (cmdStateUpdate: `error('field and value required for state update')`), + // including its ordering: validation precedes the STATE.md existence + // check. Either flag suffices — `--resume-file` alone carries an explicit + // value the handler must persist. + if (!options.stopped_at && (options.resume_file === undefined || options.resume_file === null)) { + error('stopped-at or resume-file required for state record-session'); + } const statePath = planningPaths(cwd).state; if (!fs.existsSync(statePath)) { output({ error: 'STATE.md not found' }, raw, undefined); return; } @@ -3147,19 +3159,19 @@ function buildStateFrontmatter( } let normalizedStatus = normalizeStateStatus(status, pausedAt); - // #3578: normalizeStateStatus matches 'complete' as a case-insensitive - // SUBSTRING, so the phase-completion prose cmdStateCompletePhase writes to - // the body (`Phase ${N} complete`) collapses to the milestone-level - // 'completed' status even when other phases remain open. Phase-level - // prose must never decide milestone-level status — completedPhases / - // totalPhases / diskScope, already derived above from a disk scan, are - // the authority on whether the MILESTONE is actually done. Only override - // when: (a) normalizeStateStatus actually landed on 'completed'; (b) the - // raw prose is UNAMBIGUOUSLY phase-completion prose — the anchored - // pattern below deliberately excludes "All phases complete" (no `\S+` - // phase token) and milestone-close prose like "v1.0 milestone complete" - // (no leading "phase"); and (c) the counters are trustworthy (a COMPLETE - // disk scope, both counts are finite numbers, and a positive + // #3578: the declared status vocabulary (#4186) recognizes + // `Phase ${N} complete` (state.cts's own phase-completion write) and maps + // it to `completed`, so the phase-completion prose still collapses to the + // milestone-level status even when other phases remain open — this guard + // demotes it back. Phase-level prose must never decide milestone-level + // status — completedPhases / totalPhases / diskScope, already derived above + // from a disk scan, are the authority on whether the MILESTONE is actually + // done. Only override when: (a) normalizeStateStatus actually landed on + // 'completed'; (b) the raw prose is UNAMBIGUOUSLY phase-completion prose — + // the anchored pattern below deliberately excludes "All phases complete" + // (no `\S+` phase token) and milestone-close prose like "v1.0 milestone + // complete" (no leading "phase"); and (c) the counters are trustworthy (a + // COMPLETE disk scope, both counts are finite numbers, and a positive // denominator) and affirmatively disagree with 'completed'. In every // other case normalizedStatus is left exactly as normalizeStateStatus // returned it. diff --git a/tests/gsd-statusline.test.cjs b/tests/gsd-statusline.test.cjs index 782f5c879..91c5011a9 100644 --- a/tests/gsd-statusline.test.cjs +++ b/tests/gsd-statusline.test.cjs @@ -1741,16 +1741,30 @@ test('config-set statusline.show_context_tokens yes → rejected', () => { assert.equal(shortGsdStatus(''), null); assert.equal(shortGsdStatus(undefined), null); }); - test('paused — the canonical stuck state — wins and renders uppercase (#2162 condition)', () => { - assert.equal(shortGsdStatus('paused — waiting on credentials'), 'PAUSED'); - assert.equal(shortGsdStatus('stopped by user'), 'PAUSED'); + test('paused — the canonical stuck state — renders uppercase (#2162 condition)', () => { + // The #2162 shout applies to the canonical token, which is what the + // state writer persists (#4186 anchored vocabulary). + assert.equal(shortGsdStatus('paused'), 'PAUSED'); + assert.equal(shortGsdStatus('Paused'), 'PAUSED'); + // #4186: narrative prose is no longer keyword-guessed — a paused-led + // narrative renders its first word (visible, never a silent wrong + // token), so the shout is reserved for the recognized token itself. + assert.equal(shortGsdStatus('paused — waiting on credentials'), 'paused'); + assert.equal(shortGsdStatus('stopped by user'), 'stopped'); }); - test('collapses lifecycle narratives to canonical keywords via normalizeStateStatus', () => { - assert.equal(shortGsdStatus('Executing phase 7 of the parser milestone'), 'executing'); - assert.equal(shortGsdStatus('Ready to plan next phase'), 'planning'); - assert.equal(shortGsdStatus('Discussing scope with user'), 'discussing'); - assert.equal(shortGsdStatus('Verifying UAT criteria'), 'verifying'); - assert.equal(shortGsdStatus('Work complete'), 'completed'); + test('collapses vocabulary values to canonical keywords via normalizeStateStatus (#4186 anchored)', () => { + // #4186: normalizeStateStatus recognizes the DECLARED vocabulary + // (whole-field match), so exactly those values collapse to keywords. + assert.equal(shortGsdStatus('Executing Phase 7'), 'executing'); + assert.equal(shortGsdStatus('ready to plan'), 'planning'); + assert.equal(shortGsdStatus('Discussing'), 'discussing'); + assert.equal(shortGsdStatus('Verifying Phase 2'), 'verifying'); + assert.equal(shortGsdStatus('Work complete'), 'Work'); + // Narrative prose — vocabulary words embedded in longer sentences — is + // no longer guessed at (a `.planning/` mention in non-English prose + // used to render `planning`); it falls back to the first word. + assert.equal(shortGsdStatus('Executing phase 7 of the parser milestone'), 'Executing'); + assert.equal(shortGsdStatus('Ready to plan next phase'), 'Ready'); }); test('matches the canonical vocabulary exactly — no drift from normalizeStateStatus', () => { const { normalizeStateStatus } = require('../gsd-core/bin/lib/state-document.cjs'); @@ -1777,9 +1791,15 @@ test('config-set statusline.show_context_tokens yes → rejected', () => { test('renders version · phase/total · status', () => { const out = formatGsdStateCompact({ milestone: 'v1.12', phaseNum: '7', phaseTotal: '12', - status: 'Executing phase 7 — building the parser', + status: 'Executing Phase 7', }); assert.equal(out, 'v1.12 · P7/12 · executing'); + // #4186: narrative status is not keyword-guessed — first word renders. + const narrative = formatGsdStateCompact({ + milestone: 'v1.12', phaseNum: '7', phaseTotal: '12', + status: 'Executing phase 7 — building the parser', + }); + assert.equal(narrative, 'v1.12 · P7/12 · Executing'); }); test('prefers lifecycle active_phase over body phase number', () => { const out = formatGsdStateCompact({ @@ -1789,7 +1809,7 @@ test('config-set statusline.show_context_tokens yes → rejected', () => { }); test('paused state renders uppercase in the compact line', () => { const out = formatGsdStateCompact({ - milestone: 'v2.0', activePhase: '4.5', status: 'paused — waiting on review', + milestone: 'v2.0', activePhase: '4.5', status: 'paused', }); assert.equal(out, 'v2.0 · P4.5 · PAUSED'); }); diff --git a/tests/state-document.test.cjs b/tests/state-document.test.cjs index 01569eefd..4a4ada130 100644 --- a/tests/state-document.test.cjs +++ b/tests/state-document.test.cjs @@ -2236,3 +2236,147 @@ describe('#3642 — single-section leak controls and seam pins', () => { assert.strictEqual(roadmapParser.hasMilestoneSectioning(two), true, '>=2 predicate unchanged: two headings is sectioning'); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// #4186 — normalizeStateStatus must derive the `status` token by ANCHORED +// matching against the declared status vocabulary (whole-field value, +// case-insensitive, whitespace-collapsed), never by substring-scanning the +// free-prose body Status field. Prose that merely MENTIONS a status word (a +// `.planning/` path, Italian `verifica*`, `completezza`, `fasi complete`) +// must pass through verbatim — the visible paragraph beats a valid, credible, +// wrong token. The lenient fallback itself is a recorded contract +// (state-md-schema.cts #3873 phase-3 row 26) and is preserved. +// ───────────────────────────────────────────────────────────────────────────── + +describe('#4186: normalizeStateStatus anchored status vocabulary — substring traps', () => { + const { normalizeStateStatus } = require('../gsd-core/bin/lib/state-document.cjs'); + + // Row 1 — the failing-first regression: a `.planning/` path inside Italian + // prose must NOT land on `planning` (the trigger word lives in the + // DIRECTORY NAME; the reporter measured this exact counter-pair). + test('row 1: a .planning/ path inside prose never yields `planning`', () => { + assert.strictEqual( + normalizeStateStatus('Lavoro sospeso, vedi .planning/STATE.md', null), + 'Lavoro sospeso, vedi .planning/STATE.md', + ); + }); + + test('rows 2-5: Italian prose mentioning paths/verification/completeness passes through verbatim', () => { + const prose = [ + 'Esecuzione in corso su .planning/', + 'Fase 34 COMPLETA, VERIFICATA 8/8 e FUSA IN PRODUZIONE', + 'Aggiornato .planning/STATE.md dopo la verifica di fase', + 'Referto in .planning/phases/34, completezza ok', + ]; + for (const p of prose) { + assert.strictEqual(normalizeStateStatus(p, null), p, `prose must pass through verbatim: ${p}`); + } + }); + + test('row 6: Italian verifica-family words do not match the `verif` trigger', () => { + for (const p of ['verifica', 'verificata', 'verifiche', 'ri-verifica']) { + assert.strictEqual(normalizeStateStatus(p, null), p); + } + }); + + test('rows 7-8: `completezza` and `fasi complete` do not yield `completed`', () => { + assert.strictEqual(normalizeStateStatus('riportata per completezza', null), 'riportata per completezza'); + assert.strictEqual(normalizeStateStatus('fasi complete', null), 'fasi complete'); + }); + + test('rows 9-10: control — prose with no trigger words was already verbatim and stays so', () => { + const control = [ + 'completata / completato / completo / completa / incompleta / completamente', + 'Lavoro sospeso in attesa del CEO', + ]; + for (const p of control) { + assert.strictEqual(normalizeStateStatus(p, null), p); + } + }); + + test('row 11: English status words embedded in prose sentences are not rewrites either', () => { + assert.strictEqual(normalizeStateStatus('Waiting on the planning department', null), 'Waiting on the planning department'); + assert.strictEqual(normalizeStateStatus('Notes done, see log', null), 'Notes done, see log'); + assert.strictEqual( + normalizeStateStatus('Discussed the verif steps with QA, paused decision', null), + 'Discussed the verif steps with QA, paused decision', + ); + }); + + test('row 12 (existing #3873 row-26 contract): unrecognized text passes through unchanged', () => { + assert.strictEqual( + normalizeStateStatus('totally-unrecognized-status-text', null), + 'totally-unrecognized-status-text', + ); + }); +}); + +describe('#4186: normalizeStateStatus anchored status vocabulary — documented vocabulary still normalizes', () => { + const { normalizeStateStatus } = require('../gsd-core/bin/lib/state-document.cjs'); + + test('rows 13-15: variable handler-written phase statuses (case/whitespace variants)', () => { + assert.strictEqual(normalizeStateStatus('Executing Phase 5', null), 'executing'); + assert.strictEqual(normalizeStateStatus('EXECUTING PHASE 5', null), 'executing'); + assert.strictEqual(normalizeStateStatus('executing phase 46', null), 'executing'); + assert.strictEqual(normalizeStateStatus('Planning Phase 3', null), 'planning'); + assert.strictEqual(normalizeStateStatus('Verifying Phase 2', null), 'verifying'); + }); + + test('row 16: `Phase N complete` still lands on `completed` (composes with the #3578 demote guard)', () => { + assert.strictEqual(normalizeStateStatus('Phase 12 complete', null), 'completed'); + assert.strictEqual(normalizeStateStatus('Phase 3A complete', null), 'completed'); + }); + + test('rows 17-18: ADR-2207 lifecycle terminal statuses (CONTEXT.md:94 recorded contract)', () => { + assert.strictEqual(normalizeStateStatus('All phases complete', null), 'completed'); + assert.strictEqual(normalizeStateStatus('ALL PHASES COMPLETE', null), 'completed'); + assert.strictEqual(normalizeStateStatus('v1.0 milestone complete', null), 'completed'); + assert.strictEqual(normalizeStateStatus('1.0 milestone complete', null), 'completed'); + }); + + test('rows 19-25: fixed-form handler defaults, including case and whitespace variants', () => { + assert.strictEqual(normalizeStateStatus('Ready to plan', null), 'planning'); + assert.strictEqual(normalizeStateStatus('Ready to execute', null), 'executing'); + assert.strictEqual(normalizeStateStatus('In progress', null), 'executing'); + assert.strictEqual(normalizeStateStatus('In progress', null), 'executing'); + assert.strictEqual(normalizeStateStatus(' Paused ', null), 'paused'); + assert.strictEqual(normalizeStateStatus('Paused', null), 'paused'); + assert.strictEqual(normalizeStateStatus('Stopped', null), 'paused'); + assert.strictEqual(normalizeStateStatus('stopped', null), 'paused'); + assert.strictEqual(normalizeStateStatus('Discussing', null), 'discussing'); + assert.strictEqual(normalizeStateStatus('Verifying', null), 'verifying'); + assert.strictEqual(normalizeStateStatus('Completed', null), 'completed'); + assert.strictEqual(normalizeStateStatus('Done', null), 'completed'); + assert.strictEqual(normalizeStateStatus('done', null), 'completed'); + assert.strictEqual(normalizeStateStatus('Complete', null), 'completed'); + assert.strictEqual(normalizeStateStatus('Complete ✓', null), 'completed'); + assert.strictEqual(normalizeStateStatus('Complete✔', null), 'completed'); + }); + + test('rows 26-27: branch-order artifacts are preserved byte-for-behaviour', () => { + // `verif` outranks `complete` (pinned by tests/state.test.cjs's + // advance-plan case-5 comment); `planning` outranks `complete`. + assert.strictEqual(normalizeStateStatus('Phase complete — ready for verification', null), 'verifying'); + assert.strictEqual(normalizeStateStatus('Planning complete', null), 'planning'); + }); + + test('rows 29-30: unknown fallback and the pausedAt force are unchanged', () => { + assert.strictEqual(normalizeStateStatus(null, null), 'unknown'); + assert.strictEqual(normalizeStateStatus('', null), 'unknown'); + assert.strictEqual(normalizeStateStatus('Executing Phase 5', '2026-09-01'), 'paused'); + }); + + test('row 31: statuses with no branch today stay verbatim', () => { + assert.strictEqual(normalizeStateStatus('Awaiting next milestone', null), 'Awaiting next milestone'); + assert.strictEqual(normalizeStateStatus('Defining requirements', null), 'Defining requirements'); + assert.strictEqual(normalizeStateStatus('Active', null), 'Active'); + }); + + test('row 32: trailing prose after a vocabulary form is executor-authored and passes through', () => { + // Same discipline as #1070's KNOWN_STATUS_PATTERNS: "Complete but needs + // manual QA" is NOT a template default. The anchored vocabulary only + // recognizes the whole-field value. + assert.strictEqual(normalizeStateStatus('Executing Phase 5 — final stretch', null), 'Executing Phase 5 — final stretch'); + assert.strictEqual(normalizeStateStatus('Complete but needs manual QA', null), 'Complete but needs manual QA'); + }); +}); diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 64e7b2c3c..0e7daf716 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -788,10 +788,16 @@ stopped_at: Plan 2 of Phase 3 }); test('normalizes various status values', () => { + // #4186: recognition is an ANCHORED whole-field vocabulary match. The + // vocabulary's own values normalize to their token; prefix/suffix NARRATIVE + // prose ('Paused at Plan 3') passes through verbatim — a visible paragraph + // beats a guessed token — and the legacy bare `Milestone complete` stays + // recognized (reader-side legacy vocabulary, ADR-2207 removed the writers). const statusTests = [ { input: 'In progress', expected: 'executing' }, { input: 'Ready to execute', expected: 'executing' }, - { input: 'Paused at Plan 3', expected: 'paused' }, + { input: 'Paused', expected: 'paused' }, + { input: 'Paused at Plan 3', expected: 'Paused at Plan 3' }, { input: 'Ready to plan', expected: 'planning' }, { input: 'Phase complete — ready for verification', expected: 'verifying' }, { input: 'Milestone complete', expected: 'completed' }, @@ -1140,7 +1146,9 @@ current_phase: 3 ` ); - runGsdTools('state update Status "Executing Plan 5"', tmpDir); + // #4186: use the handler vocabulary's real form (`Executing Phase N`, + // state-transition.cts:1216) — narrative variants are no longer guessed. + runGsdTools('state update Status "Executing Phase 5"', tmpDir); const result = runGsdTools('state json', tmpDir); assert.ok(result.success, `state json failed: ${result.error}`); @@ -3262,22 +3270,39 @@ describe('cmdStateRecordSession (state record-session)', () => { assert.ok(updated.includes(PINNED_ISO), `Last session should be the pinned ISO timestamp ${PINNED_ISO}`); }); - test('updates Last session timestamp even with no other options', () => { + // #4186: `state record-session` with NO arguments used to execute and write + // `Last session` / `last_updated` to STATE.md (this test previously pinned + // that bare-invocation write). It now follows the house pattern every other + // required-arg verb uses (`state update`: `error('field and value required + // for state update')`) — a bare invocation is a usage error, not a + // heartbeat write. Documented signature (docs/CLI-TOOLS.md): + // `state record-session --stopped-at "..." [--resume-file path]`. + test('#4186: no args errors instead of writing (STATE.md byte-unchanged)', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), sessionFixture); + const before = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + + const result = runGsdTools('state record-session', tmpDir); + + assert.ok(!result.success, 'bare record-session must exit non-zero'); + assert.match( + result.error, + /stopped-at or resume-file required for state record-session/, + 'stderr must name the required flags', + ); + const after = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.strictEqual(after, before, 'STATE.md must be byte-unchanged by a rejected bare call'); + }); + + // #4186: validation precedes the STATE.md existence check (same ordering as + // cmdStateUpdate), and a lone --resume-file still carries an explicit value. + test('#4186: --resume-file alone is a valid explicit value and persists', () => { fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), sessionFixture); - const PINNED_MS = Date.parse('2020-08-01T08:30:00.000Z'); - const PINNED_ISO = '2020-08-01T08:30:00.000Z'; - const result = runGsdTools('state record-session', tmpDir, { - GSD_TEST_MODE: '1', - GSD_NOW_MS: String(PINNED_MS), - }); + const result = runGsdTools('state record-session --resume-file ".continue-here.md"', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); - const output = JSON.parse(result.output); - assert.strictEqual(output.recorded, true, 'recorded should be true'); - const updated = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); - assert.ok(updated.includes(PINNED_ISO), `Last session should contain the pinned ISO timestamp ${PINNED_ISO}`); + assert.ok(updated.includes('.continue-here.md'), 'Resume file should be updated'); }); test('sets Resume file to None when not specified', () => { @@ -3295,7 +3320,9 @@ describe('cmdStateRecordSession (state record-session)', () => { }); test('returns error when STATE.md missing', () => { - const result = runGsdTools('state record-session', tmpDir); + // #4186: supply a value so this test keeps exercising the STATE.md-missing + // decline rather than the (now earlier) no-args usage error. + const result = runGsdTools('state record-session --stopped-at "somewhere"', tmpDir); assert.ok(result.success, `Command should exit 0: ${result.error}`); const output = JSON.parse(result.output); @@ -3303,18 +3330,20 @@ describe('cmdStateRecordSession (state record-session)', () => { assert.ok(output.error.includes('STATE.md'), 'error should mention STATE.md'); }); - test('returns recorded false when no session fields found', () => { + // #4186: a STATE.md with no session labels is no longer reachable bare + // (the no-args usage error fires first, before the existence check even) — + // the bare-call contract this test used to pin (recorded:false decline) is + // now the error contract pinned above; this variant proves the validation + // fires regardless of the body's shape. + test('#4186: no args errors even against a STATE.md with no session fields', () => { fs.writeFileSync( path.join(tmpDir, '.planning', 'STATE.md'), '# Project State\n\n**Status:** Active\n**Phase:** 03\n' ); const result = runGsdTools('state record-session', tmpDir); - assert.ok(result.success, `Command should exit 0: ${result.error}`); - - const output = JSON.parse(result.output); - assert.strictEqual(output.recorded, false, 'recorded should be false when no session fields found'); - assert.ok(output.reason !== undefined, 'should have a reason'); + assert.ok(!result.success, 'bare record-session must exit non-zero'); + assert.match(result.error, /stopped-at or resume-file required for state record-session/); }); // #3374 Variant B: stateReplaceField returns the replaced string on any label @@ -3398,6 +3427,175 @@ describe('cmdStateRecordSession (state record-session)', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// #4186 — the write path must not derive the frontmatter `status` token from +// SUBSTRINGS of the free-prose body Status field. Every STATE.md write funnels +// through readModifyWriteStateMd → syncStateFrontmatter → +// buildStateFrontmatter → normalizeStateStatus, so exercising `state update` +// here covers the shared seam for all write paths. +// ───────────────────────────────────────────────────────────────────────────── + +describe('#4186: state update does not rewrite status from prose substrings', () => { + let tmpDir; + + function writeStateMd(statusValue) { + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), [ + '---', + "gsd_state_version: '1.0'", + 'status: unknown', + '---', + '', + '# Project State', + '', + '## Current Position', + '', + 'Phase: 1 of 2 (Fondamenta)', + 'Status: ' + statusValue, + 'Last activity: 2026-09-02 — aggiornato lo stato', + '', + ].join('\n') + '\n'); + } + + function frontmatterStatus() { + const text = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + const fm = text.split('---')[1] || ''; + const m = fm.match(/^status:(.*)$/m); + assert.ok(m, 'frontmatter must still carry a status key'); + return m[1].trim().replace(/^['"]|['"]$/g, ''); + } + + beforeEach(() => { + tmpDir = createFixture(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('row 33: Italian prose mentioning .planning/ keeps the visible prose token', () => { + writeStateMd('Lavoro sospeso, vedi .planning/STATE.md'); + const r = runGsdTools(['state', 'update', 'Last activity', '2026-09-03 — verifica del defect'], tmpDir); + assert.ok(r.success, `state update failed: ${r.error}`); + assert.strictEqual( + frontmatterStatus(), + 'Lavoro sospeso, vedi .planning/STATE.md', + 'prose containing a .planning/ path must pass through verbatim, not become `planning`', + ); + }); + + test('row 34: a recognized handler status still lands its canonical token', () => { + writeStateMd('Executing Phase 5'); + const r = runGsdTools(['state', 'update', 'Last activity', '2026-09-03 — executing'], tmpDir); + assert.ok(r.success, `state update failed: ${r.error}`); + assert.strictEqual(frontmatterStatus(), 'executing'); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// #4186 — progress recount pin (#1988, PR #2016): a *-SUMMARY.md without a +// plan twin must not inflate progress.completed_plans on any recount trigger +// (the issue measured `34-TRIAGE-SUMMARY.md` → completed_plans 62 → 63 on +// GSD 1.5.0; the pairing fix landed after). These rows pin the correct +// behavior so it cannot regress, composed with the #4129/#4359 ratchet. +// ───────────────────────────────────────────────────────────────────────────── + +describe('#4186: progress recount ignores stray summaries (pin of #1988)', () => { + let tmpDir; + + function seedStrayFixture() { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '34-triage'); + fs.mkdirSync(phaseDir, { recursive: true }); + for (const n of ['01', '02', '03']) fs.writeFileSync(path.join(phaseDir, `34-${n}-PLAN.md`), '# plan\n'); + // Two PAIRED summaries + one stray with no plan twin. + fs.writeFileSync(path.join(phaseDir, '34-01-SUMMARY.md'), '# summary\n'); + fs.writeFileSync(path.join(phaseDir, '34-02-SUMMARY.md'), '# summary\n'); + fs.writeFileSync(path.join(phaseDir, '34-TRIAGE-SUMMARY.md'), '# stray\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), [ + '---', + "gsd_state_version: '1.0'", + 'status: executing', + 'progress:', + ' total_phases: 1', + ' completed_phases: 0', + ' total_plans: 3', + ' completed_plans: 0', + ' percent: 0', + '---', + '', + '# Project State', + '', + '## Current Position', + '', + 'Phase: 34 of 34 (Triage)', + 'Plan: 3 of 3 in current phase', + 'Status: Executing Phase 34', + 'Last activity: 2026-09-02 — executing', + '', + 'Progress: [..........] 0%', + 'Total Plans in Phase: 3', + '', + ].join('\n') + '\n'); + } + + function completedPlans() { + const text = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + // Bounded scan (#2128 class): the progress block sits within a few hundred + // bytes of its opening key in every fixture this suite writes. + const m = text.match(/^progress:[\s\S]{0,400}?completed_plans:\s*(\d+)/m); + assert.ok(m, 'progress.completed_plans must be present after a recount write'); + return Number(m[1]); + } + + beforeEach(() => { + tmpDir = createFixture(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('row 36: update on the Progress field recounts paired summaries only', () => { + seedStrayFixture(); + const r = runGsdTools(['state', 'update', 'Progress', '[..........] 0%'], tmpDir); + assert.ok(r.success, `state update failed: ${r.error}`); + assert.strictEqual(completedPlans(), 2, + 'the stray 34-TRIAGE-SUMMARY.md (no plan twin) must not inflate completed_plans'); + }); + + test('row 37: record-session (the issue trigger) recounts paired summaries only', () => { + seedStrayFixture(); + const r = runGsdTools(['state', 'record-session', '--stopped-at', 'after plan 34-02'], tmpDir); + assert.ok(r.success, `record-session failed: ${r.error}`); + assert.strictEqual(completedPlans(), 2, + 'record-session must derive completed_plans from plan-paired summaries, not raw *-SUMMARY.md counts'); + }); + + test('row 38: scanPhasePlans keeps listing the stray file but does not count it', () => { + seedStrayFixture(); + const scanPhasePlans = require('../gsd-core/bin/lib/plan-scan.cjs'); + const scan = scanPhasePlans(path.join(tmpDir, '.planning', 'phases', '34-triage')); + assert.strictEqual(scan.planCount, 3); + assert.strictEqual(scan.summaryCount, 2, 'paired summaries only'); + assert.ok(scan.summaryFiles.includes('34-TRIAGE-SUMMARY.md'), + 'the stray file stays visible in summaryFiles for callers that list summaries'); + }); + + test('row 39: ratchet still preserves an existing higher completed_plans (#4129/#4359 semantics)', () => { + seedStrayFixture(); + // Poison the frontmatter with a value ABOVE the derived paired count (2). + // `state update Progress` EXPLICITLY names a progress field, which by + // design (#3242 / ADR-3473 §8.6 early-out) makes the fresh derivation + // authoritative — so the ratchet row must use an INCIDENTAL resync write + // (record-session) instead: the curated higher counter survives there. + const statePath = path.join(tmpDir, '.planning', 'STATE.md'); + fs.writeFileSync(statePath, fs.readFileSync(statePath, 'utf-8').replace('completed_plans: 0', 'completed_plans: 9')); + const r = runGsdTools(['state', 'record-session', '--stopped-at', 'after plan 34-02'], tmpDir); + assert.ok(r.success, `record-session failed: ${r.error}`); + assert.strictEqual(completedPlans(), 9, + 'the progress ratchet (monotonic completed counts on incidental resyncs) is unchanged by #4186'); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // Milestone-scoped phase counting in frontmatter // ───────────────────────────────────────────────────────────────────────────── @@ -10052,17 +10250,19 @@ describe('T6 section-splice characterization — record-session', () => { '', ].join('\n'); - test('record-session no-op: no session fields → recorded:false, STATE.md byte-unchanged', () => { + test('record-session no-op: bare call rejected, STATE.md byte-unchanged (#4186)', () => { + // #4186: the bare call that used to reach the recorded:false decline is + // now a usage error (stopped-at or resume-file required). The byte- + // unchanged assertion below keeps guarding the #952 no-op posture — a + // rejected call must not trample anything, milestone_name included. const d = createTempProject(); try { fs.writeFileSync(path.join(d, '.planning', 'STATE.md'), STATE_NO_SESSION_LABELS); const result = runGsdTools(['state', 'record-session'], d); - assert.ok(result.success, `Command failed: ${result.error}`); - const output = JSON.parse(result.output); - assert.strictEqual(output.recorded, false, 'recorded must be false when no session fields exist'); - // milestone_name must NOT be trampled (#952 no-op guard) + assert.ok(!result.success, 'bare record-session must exit non-zero'); + assert.match(result.error, /stopped-at or resume-file required for state record-session/); const after = fs.readFileSync(path.join(d, '.planning', 'STATE.md'), 'utf-8'); - assert.strictEqual(after, STATE_NO_SESSION_LABELS, 'STATE.md must be byte-unchanged on no-op'); + assert.strictEqual(after, STATE_NO_SESSION_LABELS, 'STATE.md must be byte-unchanged by the rejected call'); } finally { cleanup(d); } @@ -13964,19 +14164,21 @@ describe('#944: record-session persists values even when body lacks session labe '--resume-file value must be present in STATE.md'); }); - test('record-session with no args against a body-less file returns recorded:false (no regression)', () => { - // When NO values are supplied and no session fields can be found/updated, - // recorded:false is the correct behaviour — we only changed the contract - // when the caller supplies values. + test('record-session with no args against a body-less file errors (#4186 usage contract)', () => { + // #4186 superseded the bare-call contract this test used to pin + // (recorded:false decline): a no-values invocation is now a usage error + // for every STATE.md shape, handler-side, before any read. The + // value-supplied decline paths keep their own dedicated tests above + // (#3374 Variant B, #3957). const statePath = path.join(tmpDir, '.planning', 'STATE.md'); fs.writeFileSync(statePath, buildStateMdWithoutSessionSection()); + const before = fs.readFileSync(statePath, 'utf-8'); const result = runGsdTools('state record-session', tmpDir); - assert.ok(result.success, `should exit 0: ${result.error}`); - - const output = JSON.parse(result.output); - assert.strictEqual(output.recorded, false, - 'recorded should still be false when no session fields exist AND no values were supplied'); + assert.ok(!result.success, 'should exit non-zero: usage error'); + assert.match(result.error, /stopped-at or resume-file required for state record-session/); + assert.strictEqual(fs.readFileSync(statePath, 'utf-8'), before, + 'a rejected bare call must not touch the file'); }); test('canonical session section still updates in place (no regression)', () => { @@ -20203,21 +20405,26 @@ describe('#3957 (epic #3473 B9): no-op decline reports the real condition', () = if (tmpDir) cleanup(tmpDir); }); - // Row 8 - test('record-session: nothing to update reports no-fields-found', () => { + // Row 8 — #4186 repurposed: the empty-options call that used to reach + // the no-fields-found decline is now a usage error (handler-side guard, + // same contract as cmdStateUpdate). The decline itself remains covered + // for value-supplied calls by the matched-but-unchanged row below; what + // this row now pins is that the SDK-level contract matches the CLI's and + // that a rejected call never touches the file. + test('record-session: no values supplied is a usage error, STATE.md unchanged (#4186)', () => { tmpDir = createFixture(); const statePath = writeState(tmpDir, '# Project State\n\n## Decisions\n\n- none yet\n'); const before = fs.readFileSync(statePath, 'utf-8'); - const { stdout, stderr } = captureCliIO(() => { - stateLib.cmdStateRecordSession(tmpDir, {}, false); - }); - - const out = JSON.parse(stdout); - assert.strictEqual(out.recorded, false); - assert.strictEqual(out.reason, 'no session fields found in STATE.md to update'); + // error() writes the user-facing message to stderr and throws a bare + // ExitError (v1: message 'process exit ' — the message itself is + // NOT on the exception; asserted via the CLI-level tests above). + assert.throws( + () => stateLib.cmdStateRecordSession(tmpDir, {}, false), + (err) => err && err.name === 'ExitError' && err.code === 1, + 'empty options must be rejected before any read or write', + ); assert.strictEqual(fs.readFileSync(statePath, 'utf-8'), before, 'STATE.md must be unchanged'); - assert.match(stderr, /^\[gsd-tools\] WARNING: state record-session skipped — no session fields found in STATE\.md to update\./); }); // Row 9 (hardest — signature B, collapsed reconciliation). A frozen clock