* fix(#4186): anchored status vocabulary, record-session arg guard, recount pin Three defects from #4186: 1. normalizeStateStatus ran a first-match-wins SUBSTRING chain over the free-prose body Status field, so prose merely mentioning a status word was silently rewritten to a credible wrong token (a .planning/ path in Italian prose -> status: planning; verifica -> verifying; completezza -> completed). Recognition is now an ANCHORED whole-field match against a declared vocabulary (STATUS_EXACT_TOKENS + STATUS_ANCHORED_PATTERNS, state-document.cts) — case/whitespace-tolerant, branch-order artifacts preserved (Planning complete -> planning; Phase complete — ready for verification -> verifying). The recorded lenient fallback (#3873 row 26) stands: unrecognized prose passes through verbatim. Read-side consumers (W011, statusline) ride the same function. 2. The progress recount skew (stray *-SUMMARY.md inflating completed_plans) is already dead on next via #1988/PR #2016 (countMatchedSummaries pairs summaries to plans) — verified live and pinned with regression rows composed against the #4129/#4359 ratchet. 3. state record-session with no args executed and wrote STATE.md; it now errors like state update (stopped-at or resume-file required), handler- side so SDK callers are covered too. Four tests pinning the bare-call write are updated to the new contract. * fix(#4186): update status pins to the anchored vocabulary contract Bench round 1 follow-ups: - Legacy bare 'Milestone complete' kept as reader-side vocabulary (ADR-2207 removed the writers, not recognition of legacy files). - state.test pins updated: 'Paused at Plan 3' and round-trip 'Executing Plan 5' were pins of the substring guessing itself — the round-trip now uses the real handler form 'Executing Phase 5'. - record-session no-op/no-fields tests repurposed to the usage-error contract (CLI + SDK-level ExitError), byte-unchanged assertions kept. - statusline tests repinned: vocabulary values collapse to keywords; narratives render the documented first-word fallback instead of a guessed token. Hook doc comment updated to match. - docs-guard exempt baseline: state.test.cjs now cites docs/CLI-TOOLS.md. - docs/CLI-TOOLS.md: record-session signature notes the required flag. * fix(#4186): repair a dangling sentence in the schema docstring * test(#4186): bound the completed_plans scan regex (#2128 class) * chore(#4186): backfill changeset PR number --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
@@ -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<Record<string, string>> = 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
|
||||
// `<version> 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<readonly [RegExp, string]> = 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;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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` →
|
||||
* `<version> 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` → `<version> 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',
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user