From 744bb7aaee9dd4ad6cfb763b245caf7e966b36bc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 27 Jun 2026 13:41:03 -0400 Subject: [PATCH] =?UTF-8?q?refactor(#1771):=20ADR-1769=20Phase=201=20?= =?UTF-8?q?=E2=80=94=20STATE.md=20Transition=20Module=20substrate=20+=20be?= =?UTF-8?q?ginPhase=20(#1775)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(#1771): ADR-1769 Phase 1 — STATE.md Transition Module substrate + beginPhase Lands the Phase 1 substrate per ADR-1769: - src/state-transition.cts (new Module): - Field-classification table (FieldClass enum + FIELD_CLASSIFICATION rows) - STATE_MD_SECTIONS constants block - Pure transitionCore(content, intent, deps) dispatch - beginPhase intent implementation (first-time + #3127 resume paths) - src/state.cts:cmdStateBeginPhase — collapses ~190 lines to a thin dispatch onto transitionCore via readModifyWriteStateMd. The lock, no-op write guard, and #1230 post-sync delta heuristic stay in the RMW seam; the body-mutation policy moves to transitionCore. - tests/state-transition.test.cjs (24 tests): - Substrate invariants (table enum, section constants) - Characterization: 6 first-time body field updates - Characterization: 5 #3127 idempotency-guard resume behaviors - Characterization: 3 Current Position section mutations - Characterization: Current focus body text line (#1104) - Property (RULESET.TESTS.property-based-testing): beginPhase status propagation + FIELD_CLASSIFICATION own-property contract - Resume Current Position mutation (preserves Plan/Phase/Status) No external behavior change. Full state.test.cjs regression (177 tests) plus bug-3127/#3242/#905/#948 pass. Property tests surfaced two pre-existing quirks (state-document.cjs greedy \s* on whitespace-only field values; Object.prototype method leakage on FIELD_CLASSIFICATION lookups for strings like 'toString') — documented in test comments; fix-out-of-scope for Phase 1. Closes #1771 * refactor(#1771): ADR-1769 Phase 1 codex review corrections Addresses 3 blocking findings from codex gpt-5.5/high review: 1. FIELD_CLASSIFICATION shape (state-transition.cts): - Was flat FieldClass enum (collapsed source + preservation) - Now two-column {source, preservation} rows per ADR-1769 §4 - Added missing fields verified via Memtrace against buildStateFrontmatter (state.cts:1633-1653): gsd_state_version, last_updated, last_activity_desc, progress.{total_phases, completed_phases, total_plans, completed_plans, percent} - Field 2-7 preservation dispatch can now consult the table 2. Prototype-pollution hardening (state-transition.cts): - Table is now Object.freeze(Object.assign(Object.create(null), {...})) - getFieldClassification() helper uses Object.hasOwn; returns null for inherited prototype methods (toString/valueOf/__proto__) - Old code: FIELD_CLASSIFICATION['toString'] returned the function 3. STATE_MD_SECTIONS aligned to canonical template: - Verified against gsd-core/templates/state.md via Memtrace - Was: 8 entries including non-template sections (## Session, ## Decisions, ## Operator Next Steps, ## Session Log, ## Roadmap Evolution) - Now: 6 canonical top-level sections (## Project Reference, ## Current Position, ## Performance Metrics, ## Accumulated Context, ## Deferred Items, ## Session Continuity) Also: beginPhase now consults getFieldClassification() per touched field (codex finding: 'table not consulted by transitionCore'). Unknown fields raise immediately — adding a field without a table row is caught at runtime. Repo-hygiene catches from gsd-test (not node --test, which missed these): - gsd-core/bin/lib/state-transition.cjs added to eslint.config.mjs ignore list (ADR-457 tsc-generated) - docs/INVENTORY-MANIFEST.json regenerated via node scripts/gen-inventory-manifest.cjs --write Property test for Object.prototype leakage tightened to verify getFieldClassification() returns null for toString/valueOf/__proto__. Ref #1771 * fix(#1771): cast Object.create(null) to satisfy @typescript-eslint/no-unsafe-assignment ESLint CI failed on src/state-transition.cts:72:14 — Object.create(null) returns `any`, which leaked through Object.assign to the typed `FIELD_CLASSIFICATION` declaration. Adding an explicit cast to `Record` eliminates the unsafe-assignment while preserving the null-prototype protection codex review recommended. gsd-test: 21888/21888 PASS. * fix(#1771): add 'see #1771' to allow-test-rule exemption per ADR-456 CI lint-allow-test-rule-refs failed: 'New allow-test-rule exemption without an issue ref — add `see #NNN` per ADR-456'. Updated comment on tests/state-transition.test.cjs to reference the Phase 1 issue. * fix(#1771): remove unnecessary allow-test-rule exemption The exemption was added speculatively. The test file does not use readFileSync + .includes()/.match()/.startsWith() on source content — it calls transitionCore() with string literals and verifies results via stateExtractField() and array .includes() on the updated[] array. No exemption needed. --- docs/INVENTORY-MANIFEST.json | 1 + eslint.config.mjs | 1 + gsd-core/bin/lib/state-transition.cjs | 339 ++++++++++++++++++++ src/state-transition.cts | 436 ++++++++++++++++++++++++++ src/state.cts | 201 ++---------- tests/state-transition.test.cjs | 417 ++++++++++++++++++++++++ 6 files changed, 1219 insertions(+), 176 deletions(-) create mode 100644 gsd-core/bin/lib/state-transition.cjs create mode 100644 src/state-transition.cts create mode 100644 tests/state-transition.test.cjs diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 68c20baf9..923fed8cb 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -394,6 +394,7 @@ "stale-bake-guard.cjs", "state-command-router.cjs", "state-document.cjs", + "state-transition.cjs", "state.cjs", "surface.cjs", "task-command-router.cjs", diff --git a/eslint.config.mjs b/eslint.config.mjs index 0beede7f5..53334498c 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -76,6 +76,7 @@ export default tseslint.config( 'gsd-core/bin/lib/context-utilization.cjs', 'gsd-core/bin/lib/artifacts.cjs', 'gsd-core/bin/lib/assumption-delta.cjs', + 'gsd-core/bin/lib/state-transition.cjs', 'gsd-core/bin/lib/command-arg-projection.cjs', 'gsd-core/bin/lib/clock.cjs', 'gsd-core/bin/lib/ui-safety-gate.cjs', diff --git a/gsd-core/bin/lib/state-transition.cjs b/gsd-core/bin/lib/state-transition.cjs new file mode 100644 index 000000000..ca8572b2d --- /dev/null +++ b/gsd-core/bin/lib/state-transition.cjs @@ -0,0 +1,339 @@ +"use strict"; +/** + * STATE.md Transition Module — ADR-1769. + * + * Phase 1 substrate: field-classification table, section constants, the pure + * `transitionCore` dispatch, and the `beginPhase` intent (migrating + * `cmdStateBeginPhase` in state.cts onto this seam). + * + * Sibling/super-module of the STATE.md Document Module (state-document.cjs): + * consumes its `stateExtractField` / `stateReplaceField` primitives. Body + * section headings live as constants here (single writer after migration). + * + * Pure core + injected I/O (ADR-1769 §3): the exported `transitionCore` is a + * pure function `(content, intent, deps) → result`; adapters that own locks, + * file I/O, and the disk-scan wrap it. + */ +Object.defineProperty(exports, "__esModule", { value: true }); +exports.STATE_MD_SECTIONS = exports.FIELD_CLASSIFICATION = void 0; +exports.getFieldClassification = getFieldClassification; +exports.transitionCore = transitionCore; +// eslint-disable-next-line @typescript-eslint/no-require-imports +const frontmatter = require("./frontmatter.cjs"); +const state_document_cjs_1 = require("./state-document.cjs"); +const markdown_sectionizer_cjs_1 = require("./markdown-sectionizer.cjs"); +// eslint-disable-next-line @typescript-eslint/no-require-imports +const phaseIdMod = require("./phase-id.cjs"); +const { extractFrontmatter, reconstructFrontmatter } = frontmatter; +const { escapeRegex } = phaseIdMod; +// Stop predicate for section-body slicing: a level-2+ heading ends the section. +const STOP_H2_PLUS = (lv) => lv >= 2; +/** + * Single source of truth for "which fields win when frontmatter and body + * disagree". Transitions declare which body fields they touch; the core + * consults the table to apply the preservation policy uniformly. + * + * Adding a new STATE.md field = one row here, not 9 transition edits. + * + * Field set verified against `buildStateFrontmatter` (state.cts:1474) — every + * frontmatter key emitted there has a row here. + * + * Frozen null-prototype object: prevents prototype-pollution lookups + * (`FIELD_CLASSIFICATION['toString']` returns undefined, not the inherited + * function). Use `getFieldClassification()` for lookups. + */ +exports.FIELD_CLASSIFICATION = Object.freeze(Object.assign(Object.create(null), { + // Schema + gsd_state_version: { source: 'free', preservation: 'derive' }, + // Milestone (external — from ROADMAP.md) + milestone: { source: 'external', preservation: 'preserve-if-placeholder' }, + milestone_name: { source: 'external', preservation: 'preserve-if-placeholder' }, + // Phase / plan position (body-derived) + current_phase: { source: 'body', preservation: 'preserve-when-unchanged' }, + current_phase_name: { source: 'curated', preservation: 'preserve-always' }, // #1743, #1695 + current_plan: { source: 'body', preservation: 'preserve-when-unchanged' }, + // Status / lifecycle (body-derived; #1230 delta heuristic applies) + status: { source: 'body', preservation: 'preserve-when-unchanged' }, + stopped_at: { source: 'body', preservation: 'preserve-when-unchanged' }, + paused_at: { source: 'body', preservation: 'preserve-when-unchanged' }, + // Activity log + last_updated: { source: 'free', preservation: 'derive' }, // realClock.nowIso() + last_activity: { source: 'body', preservation: 'derive' }, // always refresh on transition + last_activity_desc: { source: 'body', preservation: 'preserve-when-unchanged' }, + // Progress block (disk-derived, except the curated progress ratchet) + progress: { source: 'curated', preservation: 'preserve-always' }, // #3242, #1446 + 'progress.total_phases': { source: 'disk', preservation: 'derive' }, + 'progress.completed_phases': { source: 'disk', preservation: 'derive' }, + 'progress.total_plans': { source: 'disk', preservation: 'derive' }, + 'progress.completed_plans': { source: 'disk', preservation: 'derive' }, + 'progress.percent': { source: 'disk', preservation: 'derive' }, +})); +/** + * Own-property classification lookup. Returns `null` for unknown fields + * (including inherited prototype methods like `toString`/`valueOf`). + */ +function getFieldClassification(field) { + if (!Object.prototype.hasOwnProperty.call(exports.FIELD_CLASSIFICATION, field)) + return null; + return exports.FIELD_CLASSIFICATION[field]; +} +// ---------------------------------------------------------------------------- +// Body section constants (ADR-1769 §6 — single writer after migration) +// ---------------------------------------------------------------------------- +/** + * Top-level STATE.md section headings (H2). Aligned byte-for-byte with the + * canonical template at `gsd-core/templates/state.md`. Sub-headings (H3) like + * `### Decisions` / `### Pending Todos` / `### Blockers/Concerns` live under + * `## Accumulated Context` and are not mutated by any Phase 1–7 transition; + * they will be added here if a future transition needs them. + * + * Verified against `gsd-core/templates/state.md` (codex Phase 1 review). + */ +exports.STATE_MD_SECTIONS = { + projectReference: '## Project Reference', + currentPosition: '## Current Position', + performanceMetrics: '## Performance Metrics', + accumulatedContext: '## Accumulated Context', + deferredItems: '## Deferred Items', + sessionContinuity: '## Session Continuity', +}; +// ---------------------------------------------------------------------------- +// transitionCore — pure dispatch (ADR-1769 §3) +// ---------------------------------------------------------------------------- +/** + * Pure transition core. `(content, intent, deps) → result`. + * + * Discriminated-union dispatch via plain `switch` (ADR-1769 §2.7 Kernighan's + * Law: debuggability over conciseness; the substrate sets the pattern). + * + * Phases 2–7 add cases for the remaining 9 intent kinds. A missing case is + * a compile-time error (the function would not return on that path). + */ +function transitionCore(content, intent, deps) { + switch (intent.kind) { + case 'beginPhase': + return beginPhaseCore(content, intent, deps); + } +} +// ---------------------------------------------------------------------------- +// beginPhase — intent implementation (Phase 1) +// ---------------------------------------------------------------------------- +/** + * Apply a `beginPhase` transition to STATE.md content. + * + * Phase 1 scope (this file): the Status field update only. Subsequent + * behaviors land via RED-GREEN cycles per the ADR-1769 migration plan: + * - Current Phase, Current Phase Name, Current Plan, Total Plans + * - Current Position section mutation + * - Idempotency guard (#3127) + * - Resume vs first-time branching + * - #1255 / #1257 format-detection parity + * + * Adapters that acquire the STATE.md lock and call this core live in + * state.cts and consume the existing `readModifyWriteStateMd` post-sync + * machinery (preserves the #1230 delta heuristic without re-implementing it). + */ +function beginPhaseCore(content, intent, deps) { + const updated = []; + // #1255: body-field replacements operate on body only (frontmatter stripped), + // not on the full content. The YAML `status:` key matches `^Status:\s*` + // before the body pipe-table row if full content is passed. + const existingFm = extractFrontmatter(content); + const hasFrontmatter = Object.keys(existingFm).length > 0; + let body = stripFrontmatter(content); + const reassemble = (b) => hasFrontmatter + ? `---\n${reconstructFrontmatter(existingFm)}\n---\n\n${b}` + : b; + const today = deps.clock.today(); + // Consult the field-classification table for the frontmatter keys this + // transition touches (codex Phase 1 review: "table not consulted by + // transitionCore"). The table tracks FRONTMATTER keys (lowercase: `status`, + // `current_phase`, `last_activity`); body field names like `Status` / + // `Current Phase` are aliases and aren't enforced here — they're driven by + // the first-time/resume branching below, which encodes the same rules. + // Phase 2+ will dispatch preservation based on this lookup. + for (const fmKey of ['status', 'current_phase', 'current_plan', 'last_activity']) { + const cls = getFieldClassification(fmKey); + if (cls === null) { + throw new Error(`transitionCore beginPhase: frontmatter key ${JSON.stringify(fmKey)} is not in FIELD_CLASSIFICATION; ` + + `add a row per ADR-1769 §4 before touching it.`); + } + } + // Helper: try to replace a body field; push to `updated` on success. + // Body field names (Title Case: 'Status', 'Current Phase') are not in the + // table — they're body-side aliases of classified frontmatter keys. + const tryField = (name, value) => { + const replaced = (0, state_document_cjs_1.stateReplaceField)(body, name, value); + if (replaced !== null) { + body = replaced; + updated.push(name); + } + }; + // #3127 idempotency guard: if Status already contains "Executing Phase N" for + // the current phase number, this is a resume (e.g. --wave N continue). Skip + // the first-time-only fields so mid-flight state (Current Plan, Total Plans, + // Current Phase Name, Last Activity Description) is preserved. + // Extract from body (not full content) so the YAML `status:` key cannot + // shadow the body Status field (#1255). + const currentStatus = (0, state_document_cjs_1.stateExtractField)(body, 'Status') || ''; + const isAlreadyExecuting = new RegExp(`Executing Phase\\s+${escapeRegex(String(intent.phaseNumber))}\\b`, 'i').test(currentStatus); + // Status update (applies on both first-time and resume — Status is always refreshed). + tryField('Status', `Executing Phase ${intent.phaseNumber}`); + // Last Activity date — safe to refresh on resume (tracks when execute-phase ran). + tryField('Last Activity', today); + if (!isAlreadyExecuting) { + // First-time execution: set all progress fields. + tryField('Last Activity Description', `Phase ${intent.phaseNumber} execution started`); + tryField('Current Phase', String(intent.phaseNumber)); + if (intent.phaseName) { + tryField('Current Phase Name', intent.phaseName); + } + tryField('Current Plan', '1'); + if (intent.planCount) { + tryField('Total Plans in Phase', String(intent.planCount)); + } + // **Current focus:** body text line (#1104). + const focusLabel = intent.phaseName + ? `Phase ${intent.phaseNumber} — ${intent.phaseName}` + : `Phase ${intent.phaseNumber}`; + const focusPattern = /(\*\*Current focus:\*\*\s*).*/i; + if (focusPattern.test(body)) { + body = body.replace(focusPattern, (_match, prefix) => `${prefix}${focusLabel}`); + updated.push('Current focus'); + } + // ## Current Position section mutation (#1104, #1365). + // ADR-1372 T6: tokenizeHeadings + offset splicing (replaceSection adoption + // deferred to a later phase). Mirrors state.cts:2261-2324 byte-for-behaviour. + body = mutateCurrentPositionFirstTime(body, intent, today, updated); + } + else { + // Resume path: only update Last activity timestamp in Current Position + // (do not touch Plan:, Phase:, Status:, stopped_at, progress.percent). + body = mutateCurrentPositionResume(body, intent, today, updated); + } + return { content: reassemble(body), updated }; +} +/** + * Find the `## Current Position` section, return its `{start, end}` byte + * offsets in `body` (end is exclusive — first byte of the next section or + * body.length). Returns `null` when the section is absent. + * + * ADR-1372 T6: tokenizeHeadings-based locator (fence-aware). + */ +function locateCurrentPosition(body) { + const hs = (0, markdown_sectionizer_cjs_1.tokenizeHeadings)(body); + const idx = hs.findIndex(h => h.level === 2 && /^current\s+position$/i.test(h.text)); + if (idx === -1) + return null; + const h = hs[idx]; + const lines = body.split('\n'); + const hl = lines[h.line - 1]; + const start = h.offset + hl.length + 1; + let end = body.length; + for (let j = idx + 1; j < hs.length; j++) { + if (STOP_H2_PLUS(hs[j].level)) { + end = hs[j].offset - 1; + break; + } + } + return { start, end }; +} +/** + * First-time ## Current Position mutation: update Phase / Plan / Status / + * Last activity lines. Mirrors state.cts:2261-2324 byte-for-behaviour + * (inline regex first, pipe-table fallback via stateReplaceField — #1257). + */ +function mutateCurrentPositionFirstTime(body, intent, today, updated) { + const span = locateCurrentPosition(body); + if (span === null) + return body; + let sectionBody = body.slice(span.start, span.end); + // Phase line — inline first, then pipe-table fallback (#1257). + const phaseLabel = `${intent.phaseNumber}${intent.phaseName ? ` (${intent.phaseName})` : ''} — EXECUTING`; + if (/^Phase:/m.test(sectionBody)) { + sectionBody = sectionBody.replace(/^Phase:.*$/m, `Phase: ${phaseLabel}`); + } + else { + const replaced = (0, state_document_cjs_1.stateReplaceField)(sectionBody, 'Phase', phaseLabel); + if (replaced !== null) + sectionBody = replaced; + } + // Plan line. + const planValue = `1 of ${intent.planCount || '?'}`; + if (/^Plan:/m.test(sectionBody)) { + sectionBody = sectionBody.replace(/^Plan:.*$/m, `Plan: ${planValue}`); + } + else { + const replaced = (0, state_document_cjs_1.stateReplaceField)(sectionBody, 'Plan', planValue); + if (replaced !== null) + sectionBody = replaced; + } + // Status line. + const statusValue = `Executing Phase ${intent.phaseNumber}`; + if (/^Status:/m.test(sectionBody)) { + sectionBody = sectionBody.replace(/^Status:.*$/m, `Status: ${statusValue}`); + } + else { + const replaced = (0, state_document_cjs_1.stateReplaceField)(sectionBody, 'Status', statusValue); + if (replaced !== null) + sectionBody = replaced; + } + // Last activity line. The inline value carries date + narrative. + const activityValue = `${today} — Phase ${intent.phaseNumber} execution started`; + if (/^Last activity:/im.test(sectionBody)) { + sectionBody = sectionBody.replace(/^Last activity:.*$/im, `Last activity: ${activityValue}`); + } + else { + const replaced = (0, state_document_cjs_1.stateReplaceField)(sectionBody, 'Last Activity', activityValue) ?? + (0, state_document_cjs_1.stateReplaceField)(sectionBody, 'Last activity', activityValue); + if (replaced !== null) + sectionBody = replaced; + } + updated.push('Current Position'); + return body.slice(0, span.start) + sectionBody + body.slice(span.end); +} +/** + * Resume ## Current Position mutation: only update Last activity line + * (preserves Plan/Phase/Status — #3127). Mirrors state.cts:2329-2363 + * byte-for-behaviour. + */ +function mutateCurrentPositionResume(body, intent, today, updated) { + const span = locateCurrentPosition(body); + if (span === null) + return body; + let sectionBody = body.slice(span.start, span.end); + const resumeActivity = `Last activity: ${today} — Phase ${intent.phaseNumber} execution resumed (wave continue)`; + if (/^Last activity:/im.test(sectionBody)) { + sectionBody = sectionBody.replace(/^Last activity:.*$/im, resumeActivity); + updated.push('Last activity (resume)'); + } + else { + // Pipe-table format fallback (#1255). + const replaced = (0, state_document_cjs_1.stateReplaceField)(sectionBody, 'Last Activity', resumeActivity) ?? + (0, state_document_cjs_1.stateReplaceField)(sectionBody, 'Last activity', resumeActivity); + if (replaced !== null) { + sectionBody = replaced; + updated.push('Last activity (resume)'); + } + } + return body.slice(0, span.start) + sectionBody + body.slice(span.end); +} +/** + * Strip ALL frontmatter blocks from the start of `content`. + * + * TODO (ADR-1769 follow-up): move to `frontmatter.cjs` or `state-document.cjs` + * so it's a shared primitive. Inlined here in Phase 1 to avoid touching + * `state.cjs` (which is the migration target itself) and to keep the Phase 1 + * diff contained. Body is byte-identical to `state.cts:1653 stripFrontmatter` + * (same CRLF + stacked-block handling). + */ +function stripFrontmatter(content) { + let result = content; + while (true) { + const stripped = result.replace(/^\s*---\r?\n[\s\S]*?\r?\n---\s*/, ''); + if (stripped === result) + break; + result = stripped; + } + return result; +} diff --git a/src/state-transition.cts b/src/state-transition.cts new file mode 100644 index 000000000..3c968d23f --- /dev/null +++ b/src/state-transition.cts @@ -0,0 +1,436 @@ +/** + * STATE.md Transition Module — ADR-1769. + * + * Phase 1 substrate: field-classification table, section constants, the pure + * `transitionCore` dispatch, and the `beginPhase` intent (migrating + * `cmdStateBeginPhase` in state.cts onto this seam). + * + * Sibling/super-module of the STATE.md Document Module (state-document.cjs): + * consumes its `stateExtractField` / `stateReplaceField` primitives. Body + * section headings live as constants here (single writer after migration). + * + * Pure core + injected I/O (ADR-1769 §3): the exported `transitionCore` is a + * pure function `(content, intent, deps) → result`; adapters that own locks, + * file I/O, and the disk-scan wrap it. + */ + +// eslint-disable-next-line @typescript-eslint/no-require-imports +import frontmatter = require('./frontmatter.cjs'); +import { stateReplaceField, stateExtractField } from './state-document.cjs'; +import { tokenizeHeadings } from './markdown-sectionizer.cjs'; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import phaseIdMod = require('./phase-id.cjs'); + +const { extractFrontmatter, reconstructFrontmatter } = frontmatter; +const { escapeRegex } = phaseIdMod; + +// Stop predicate for section-body slicing: a level-2+ heading ends the section. +const STOP_H2_PLUS = (lv: number): boolean => lv >= 2; + +// ---------------------------------------------------------------------------- +// Field-classification table (ADR-1769 §4) +// ---------------------------------------------------------------------------- +// +// Two-column shape per ADR: `{ source, preservation }`. Source alone is +// insufficient because two fields can share a source but need different +// preservation rules (e.g. `current_phase` and `last_activity` are both +// `derived-from-body`, but `current_phase` preserves-when-unchanged per #1230 +// while `last_activity` always re-derives). Codex review of Phase 1 caught +// the collapsed-enum shape as a substrate defect that wouldn't survive +// Phases 2–7. + +export type FieldSource = + | 'body' // value is derived from a body field (Phase:, Status:, etc.) + | 'disk' // value is derived from a disk scan (.planning/phases/* counts) + | 'external' // value is derived from an external file (ROADMAP.md milestone) + | 'curated' // value is set by humans/tools; preserve unless explicitly overwritten + | 'free'; // caller's word is law (no preservation) + +export type FieldPreservation = + | 'derive' // always re-derive from source + | 'preserve-when-unchanged' // #1230 delta heuristic: keep existing if body source field unchanged + | 'preserve-always' // never overwrite unless the caller explicitly names this field + | 'preserve-if-placeholder' // overwrite only when derived value is a known placeholder (#948) + | 'clear'; // remove the field entirely + +export type FieldClassification = { source: FieldSource; preservation: FieldPreservation }; + +/** + * Single source of truth for "which fields win when frontmatter and body + * disagree". Transitions declare which body fields they touch; the core + * consults the table to apply the preservation policy uniformly. + * + * Adding a new STATE.md field = one row here, not 9 transition edits. + * + * Field set verified against `buildStateFrontmatter` (state.cts:1474) — every + * frontmatter key emitted there has a row here. + * + * Frozen null-prototype object: prevents prototype-pollution lookups + * (`FIELD_CLASSIFICATION['toString']` returns undefined, not the inherited + * function). Use `getFieldClassification()` for lookups. + */ +export const FIELD_CLASSIFICATION: Readonly> = Object.freeze( + Object.assign( + Object.create(null) as Record, + { + // Schema + gsd_state_version: { source: 'free', preservation: 'derive' } as FieldClassification, + + // Milestone (external — from ROADMAP.md) + milestone: { source: 'external', preservation: 'preserve-if-placeholder' } as FieldClassification, + milestone_name: { source: 'external', preservation: 'preserve-if-placeholder' } as FieldClassification, + + // Phase / plan position (body-derived) + current_phase: { source: 'body', preservation: 'preserve-when-unchanged' } as FieldClassification, + current_phase_name: { source: 'curated', preservation: 'preserve-always' } as FieldClassification, // #1743, #1695 + current_plan: { source: 'body', preservation: 'preserve-when-unchanged' } as FieldClassification, + + // Status / lifecycle (body-derived; #1230 delta heuristic applies) + status: { source: 'body', preservation: 'preserve-when-unchanged' } as FieldClassification, + stopped_at: { source: 'body', preservation: 'preserve-when-unchanged' } as FieldClassification, + paused_at: { source: 'body', preservation: 'preserve-when-unchanged' } as FieldClassification, + + // Activity log + last_updated: { source: 'free', preservation: 'derive' } as FieldClassification, // realClock.nowIso() + last_activity: { source: 'body', preservation: 'derive' } as FieldClassification, // always refresh on transition + last_activity_desc: { source: 'body', preservation: 'preserve-when-unchanged' } as FieldClassification, + + // Progress block (disk-derived, except the curated progress ratchet) + progress: { source: 'curated', preservation: 'preserve-always' } as FieldClassification, // #3242, #1446 + 'progress.total_phases': { source: 'disk', preservation: 'derive' } as FieldClassification, + 'progress.completed_phases': { source: 'disk', preservation: 'derive' } as FieldClassification, + 'progress.total_plans': { source: 'disk', preservation: 'derive' } as FieldClassification, + 'progress.completed_plans': { source: 'disk', preservation: 'derive' } as FieldClassification, + 'progress.percent': { source: 'disk', preservation: 'derive' } as FieldClassification, + } satisfies Record, + ), +); + +/** + * Own-property classification lookup. Returns `null` for unknown fields + * (including inherited prototype methods like `toString`/`valueOf`). + */ +export function getFieldClassification(field: string): FieldClassification | null { + if (!Object.prototype.hasOwnProperty.call(FIELD_CLASSIFICATION, field)) return null; + return FIELD_CLASSIFICATION[field]; +} + +// ---------------------------------------------------------------------------- +// Body section constants (ADR-1769 §6 — single writer after migration) +// ---------------------------------------------------------------------------- + +/** + * Top-level STATE.md section headings (H2). Aligned byte-for-byte with the + * canonical template at `gsd-core/templates/state.md`. Sub-headings (H3) like + * `### Decisions` / `### Pending Todos` / `### Blockers/Concerns` live under + * `## Accumulated Context` and are not mutated by any Phase 1–7 transition; + * they will be added here if a future transition needs them. + * + * Verified against `gsd-core/templates/state.md` (codex Phase 1 review). + */ +export const STATE_MD_SECTIONS = { + projectReference: '## Project Reference', + currentPosition: '## Current Position', + performanceMetrics: '## Performance Metrics', + accumulatedContext: '## Accumulated Context', + deferredItems: '## Deferred Items', + sessionContinuity: '## Session Continuity', +} as const; + +// ---------------------------------------------------------------------------- +// Intent + deps + result types (ADR-1769 §3) +// ---------------------------------------------------------------------------- + +export type ProgressRecord = Record; + +export type StateTransitionDeps = { + progressProvider: () => ProgressRecord | null; + clock: { today: () => string; nowIso: () => string }; +}; + +export type StateTransitionIntent = + | { kind: 'beginPhase'; phaseNumber: string | number; phaseName: string | null; planCount: number | null }; +// Phases 2–7 add the remaining 9 intent kinds to this discriminated union. + +export type StateTransitionResult = { content: string; updated: string[] }; + +// ---------------------------------------------------------------------------- +// transitionCore — pure dispatch (ADR-1769 §3) +// ---------------------------------------------------------------------------- + +/** + * Pure transition core. `(content, intent, deps) → result`. + * + * Discriminated-union dispatch via plain `switch` (ADR-1769 §2.7 Kernighan's + * Law: debuggability over conciseness; the substrate sets the pattern). + * + * Phases 2–7 add cases for the remaining 9 intent kinds. A missing case is + * a compile-time error (the function would not return on that path). + */ +export function transitionCore( + content: string, + intent: StateTransitionIntent, + deps: StateTransitionDeps, +): StateTransitionResult { + switch (intent.kind) { + case 'beginPhase': + return beginPhaseCore(content, intent, deps); + } +} + +// ---------------------------------------------------------------------------- +// beginPhase — intent implementation (Phase 1) +// ---------------------------------------------------------------------------- + +/** + * Apply a `beginPhase` transition to STATE.md content. + * + * Phase 1 scope (this file): the Status field update only. Subsequent + * behaviors land via RED-GREEN cycles per the ADR-1769 migration plan: + * - Current Phase, Current Phase Name, Current Plan, Total Plans + * - Current Position section mutation + * - Idempotency guard (#3127) + * - Resume vs first-time branching + * - #1255 / #1257 format-detection parity + * + * Adapters that acquire the STATE.md lock and call this core live in + * state.cts and consume the existing `readModifyWriteStateMd` post-sync + * machinery (preserves the #1230 delta heuristic without re-implementing it). + */ +function beginPhaseCore( + content: string, + intent: { kind: 'beginPhase'; phaseNumber: string | number; phaseName: string | null; planCount: number | null }, + deps: StateTransitionDeps, +): StateTransitionResult { + const updated: string[] = []; + + // #1255: body-field replacements operate on body only (frontmatter stripped), + // not on the full content. The YAML `status:` key matches `^Status:\s*` + // before the body pipe-table row if full content is passed. + const existingFm = extractFrontmatter(content) as Record; + const hasFrontmatter = Object.keys(existingFm).length > 0; + let body = stripFrontmatter(content); + + const reassemble = (b: string): string => + hasFrontmatter + ? `---\n${reconstructFrontmatter(existingFm as unknown as Frontmatter)}\n---\n\n${b}` + : b; + + const today = deps.clock.today(); + + // Consult the field-classification table for the frontmatter keys this + // transition touches (codex Phase 1 review: "table not consulted by + // transitionCore"). The table tracks FRONTMATTER keys (lowercase: `status`, + // `current_phase`, `last_activity`); body field names like `Status` / + // `Current Phase` are aliases and aren't enforced here — they're driven by + // the first-time/resume branching below, which encodes the same rules. + // Phase 2+ will dispatch preservation based on this lookup. + for (const fmKey of ['status', 'current_phase', 'current_plan', 'last_activity']) { + const cls = getFieldClassification(fmKey); + if (cls === null) { + throw new Error( + `transitionCore beginPhase: frontmatter key ${JSON.stringify(fmKey)} is not in FIELD_CLASSIFICATION; ` + + `add a row per ADR-1769 §4 before touching it.`, + ); + } + } + + // Helper: try to replace a body field; push to `updated` on success. + // Body field names (Title Case: 'Status', 'Current Phase') are not in the + // table — they're body-side aliases of classified frontmatter keys. + const tryField = (name: string, value: string): void => { + const replaced = stateReplaceField(body, name, value); + if (replaced !== null) { + body = replaced; + updated.push(name); + } + }; + + // #3127 idempotency guard: if Status already contains "Executing Phase N" for + // the current phase number, this is a resume (e.g. --wave N continue). Skip + // the first-time-only fields so mid-flight state (Current Plan, Total Plans, + // Current Phase Name, Last Activity Description) is preserved. + // Extract from body (not full content) so the YAML `status:` key cannot + // shadow the body Status field (#1255). + const currentStatus = stateExtractField(body, 'Status') || ''; + const isAlreadyExecuting = new RegExp( + `Executing Phase\\s+${escapeRegex(String(intent.phaseNumber))}\\b`, + 'i', + ).test(currentStatus); + + // Status update (applies on both first-time and resume — Status is always refreshed). + tryField('Status', `Executing Phase ${intent.phaseNumber}`); + + // Last Activity date — safe to refresh on resume (tracks when execute-phase ran). + tryField('Last Activity', today); + + if (!isAlreadyExecuting) { + // First-time execution: set all progress fields. + tryField('Last Activity Description', `Phase ${intent.phaseNumber} execution started`); + tryField('Current Phase', String(intent.phaseNumber)); + if (intent.phaseName) { + tryField('Current Phase Name', intent.phaseName); + } + tryField('Current Plan', '1'); + if (intent.planCount) { + tryField('Total Plans in Phase', String(intent.planCount)); + } + + // **Current focus:** body text line (#1104). + const focusLabel = intent.phaseName + ? `Phase ${intent.phaseNumber} — ${intent.phaseName}` + : `Phase ${intent.phaseNumber}`; + const focusPattern = /(\*\*Current focus:\*\*\s*).*/i; + if (focusPattern.test(body)) { + body = body.replace(focusPattern, (_match, prefix: string) => `${prefix}${focusLabel}`); + updated.push('Current focus'); + } + + // ## Current Position section mutation (#1104, #1365). + // ADR-1372 T6: tokenizeHeadings + offset splicing (replaceSection adoption + // deferred to a later phase). Mirrors state.cts:2261-2324 byte-for-behaviour. + body = mutateCurrentPositionFirstTime(body, intent, today, updated); + } else { + // Resume path: only update Last activity timestamp in Current Position + // (do not touch Plan:, Phase:, Status:, stopped_at, progress.percent). + body = mutateCurrentPositionResume(body, intent, today, updated); + } + + return { content: reassemble(body), updated }; +} + +// Local frontmatter type aliases matching frontmatter.cts so we can call +// reconstructFrontmatter without cross-module type re-exports. +type FrontmatterValue = string | string[] | Record; +type Frontmatter = Record; + +/** + * Find the `## Current Position` section, return its `{start, end}` byte + * offsets in `body` (end is exclusive — first byte of the next section or + * body.length). Returns `null` when the section is absent. + * + * ADR-1372 T6: tokenizeHeadings-based locator (fence-aware). + */ +function locateCurrentPosition(body: string): { start: number; end: number } | null { + const hs = tokenizeHeadings(body); + const idx = hs.findIndex(h => h.level === 2 && /^current\s+position$/i.test(h.text)); + if (idx === -1) return null; + const h = hs[idx]; + const lines = body.split('\n'); + const hl = lines[h.line - 1]; + const start = h.offset + hl.length + 1; + let end = body.length; + for (let j = idx + 1; j < hs.length; j++) { + if (STOP_H2_PLUS(hs[j].level)) { end = hs[j].offset - 1; break; } + } + return { start, end }; +} + +/** + * First-time ## Current Position mutation: update Phase / Plan / Status / + * Last activity lines. Mirrors state.cts:2261-2324 byte-for-behaviour + * (inline regex first, pipe-table fallback via stateReplaceField — #1257). + */ +function mutateCurrentPositionFirstTime( + body: string, + intent: { phaseNumber: string | number; phaseName: string | null; planCount: number | null }, + today: string, + updated: string[], +): string { + const span = locateCurrentPosition(body); + if (span === null) return body; + let sectionBody = body.slice(span.start, span.end); + + // Phase line — inline first, then pipe-table fallback (#1257). + const phaseLabel = `${intent.phaseNumber}${intent.phaseName ? ` (${intent.phaseName})` : ''} — EXECUTING`; + if (/^Phase:/m.test(sectionBody)) { + sectionBody = sectionBody.replace(/^Phase:.*$/m, `Phase: ${phaseLabel}`); + } else { + const replaced = stateReplaceField(sectionBody, 'Phase', phaseLabel); + if (replaced !== null) sectionBody = replaced; + } + + // Plan line. + const planValue = `1 of ${intent.planCount || '?'}`; + if (/^Plan:/m.test(sectionBody)) { + sectionBody = sectionBody.replace(/^Plan:.*$/m, `Plan: ${planValue}`); + } else { + const replaced = stateReplaceField(sectionBody, 'Plan', planValue); + if (replaced !== null) sectionBody = replaced; + } + + // Status line. + const statusValue = `Executing Phase ${intent.phaseNumber}`; + if (/^Status:/m.test(sectionBody)) { + sectionBody = sectionBody.replace(/^Status:.*$/m, `Status: ${statusValue}`); + } else { + const replaced = stateReplaceField(sectionBody, 'Status', statusValue); + if (replaced !== null) sectionBody = replaced; + } + + // Last activity line. The inline value carries date + narrative. + const activityValue = `${today} — Phase ${intent.phaseNumber} execution started`; + if (/^Last activity:/im.test(sectionBody)) { + sectionBody = sectionBody.replace(/^Last activity:.*$/im, `Last activity: ${activityValue}`); + } else { + const replaced = + stateReplaceField(sectionBody, 'Last Activity', activityValue) ?? + stateReplaceField(sectionBody, 'Last activity', activityValue); + if (replaced !== null) sectionBody = replaced; + } + + updated.push('Current Position'); + return body.slice(0, span.start) + sectionBody + body.slice(span.end); +} + +/** + * Resume ## Current Position mutation: only update Last activity line + * (preserves Plan/Phase/Status — #3127). Mirrors state.cts:2329-2363 + * byte-for-behaviour. + */ +function mutateCurrentPositionResume( + body: string, + intent: { phaseNumber: string | number }, + today: string, + updated: string[], +): string { + const span = locateCurrentPosition(body); + if (span === null) return body; + let sectionBody = body.slice(span.start, span.end); + + const resumeActivity = `Last activity: ${today} — Phase ${intent.phaseNumber} execution resumed (wave continue)`; + if (/^Last activity:/im.test(sectionBody)) { + sectionBody = sectionBody.replace(/^Last activity:.*$/im, resumeActivity); + updated.push('Last activity (resume)'); + } else { + // Pipe-table format fallback (#1255). + const replaced = + stateReplaceField(sectionBody, 'Last Activity', resumeActivity) ?? + stateReplaceField(sectionBody, 'Last activity', resumeActivity); + if (replaced !== null) { + sectionBody = replaced; + updated.push('Last activity (resume)'); + } + } + + return body.slice(0, span.start) + sectionBody + body.slice(span.end); +} + +/** + * Strip ALL frontmatter blocks from the start of `content`. + * + * TODO (ADR-1769 follow-up): move to `frontmatter.cjs` or `state-document.cjs` + * so it's a shared primitive. Inlined here in Phase 1 to avoid touching + * `state.cjs` (which is the migration target itself) and to keep the Phase 1 + * diff contained. Body is byte-identical to `state.cts:1653 stripFrontmatter` + * (same CRLF + stacked-block handling). + */ +function stripFrontmatter(content: string): string { + let result = content; + while (true) { + const stripped = result.replace(/^\s*---\r?\n[\s\S]*?\r?\n---\s*/, ''); + if (stripped === result) break; + result = stripped; + } + return result; +} diff --git a/src/state.cts b/src/state.cts index ed072f0a1..d909939e9 100644 --- a/src/state.cts +++ b/src/state.cts @@ -30,6 +30,11 @@ import frontmatter = require('./frontmatter.cjs'); const { extractFrontmatter, reconstructFrontmatter } = frontmatter; // eslint-disable-next-line @typescript-eslint/no-require-imports import scanPhasePlans = require('./plan-scan.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +import stateTransitionMod = require('./state-transition.cjs'); +const { transitionCore } = stateTransitionMod; +type StateTransitionIntent = stateTransitionMod.StateTransitionIntent; +type StateTransitionDeps = stateTransitionMod.StateTransitionDeps; import { computeProgressPercent, normalizeProgressNumbers, @@ -2179,184 +2184,28 @@ function cmdStateBeginPhase(cwd: string, phaseNumber: string | number, phaseName return; } - const today = realClock.today(); - const updated: string[] = []; + // ADR-1769 Phase 1: dispatches to the STATE.md Transition Module. The 175-line + // RMW callback that used to live here (format detection + preservation policy + // + section mutation + idempotency guard + resume branching) is now the pure + // `transitionCore` function in src/state-transition.cts, backed by the + // field-classification table. readModifyWriteStateMd still owns the lock, + // #1230 post-sync preservation, and the no-op write guard. + const intent: StateTransitionIntent = { + kind: 'beginPhase', + phaseNumber, + phaseName: phaseName ?? null, + planCount: planCount ?? null, + }; + const deps: StateTransitionDeps = { + clock: realClock, + progressProvider: () => null, // beginPhase doesn't consult disk progress; syncStateFrontmatter's scan is authoritative + }; + let updated: string[] = []; readModifyWriteStateMd(statePath, (content) => { - // Bug #1255: all body-field replacements must operate on the body only - // (frontmatter stripped), not on the full content. When the full content is - // passed to stateReplaceField the YAML `status: planning` key matches the - // plain-text pattern (`^Status:\s*`) before the body pipe-table row, so the - // pipe-table `| Status | Planning |` is never updated and syncStateFrontmatter - // re-derives 'planning' from the unchanged body — the status never advances. - const existingFm = extractFrontmatter(content) as Record; - const hasFrontmatter = Object.keys(existingFm).length > 0; - let body = stripFrontmatter(content); - - // Helper to reassemble content for field-replacement checks; callers that - // only need to test/replace body fields use `body` directly, and the final - // return reassembles the frontmatter block with the updated body. - const reassemble = (b: string) => - hasFrontmatter ? `---\n${reconstructFrontmatter(existingFm as unknown as Frontmatter)}\n---\n\n${b}` : b; - - // Idempotency guard (#3127): if the phase is already mid-flight, do NOT - // overwrite execution-progress fields (Current Plan, plan body line, - // Last Activity Description). Only update fields that are safe to - // refresh on resume (Last Activity date, Status if inconsistent). - // A phase is considered mid-flight when Status contains 'Executing Phase N' - // for the current phase number. - // #1255: extract from body (not full content) so the YAML `status:` key - // cannot shadow the body Status field. - const currentStatus = stateExtractField(body, 'Status') || ''; - const isAlreadyExecuting = new RegExp(`Executing Phase\\s+${escapeRegex(String(phaseNumber))}\\b`, 'i').test(currentStatus); - - // Update Status field (body only — #1255) - const statusValue = `Executing Phase ${phaseNumber}`; - let result = stateReplaceField(body, 'Status', statusValue); - if (result) { body = result; updated.push('Status'); } - - // Update Last Activity (safe to update on resume — tracks when execute-phase ran) - result = stateReplaceField(body, 'Last Activity', today); - if (result) { body = result; updated.push('Last Activity'); } - - if (!isAlreadyExecuting) { - // First-time execution: set all progress fields - - // Update Last Activity Description - const activityDesc = `Phase ${phaseNumber} execution started`; - result = stateReplaceField(body, 'Last Activity Description', activityDesc); - if (result) { body = result; updated.push('Last Activity Description'); } - - // Update Current Phase - result = stateReplaceField(body, 'Current Phase', String(phaseNumber)); - if (result) { body = result; updated.push('Current Phase'); } - - // Update Current Phase Name - if (phaseName) { - result = stateReplaceField(body, 'Current Phase Name', phaseName); - if (result) { body = result; updated.push('Current Phase Name'); } - } - - // Update Current Plan to 1 (starting from the first plan) - result = stateReplaceField(body, 'Current Plan', '1'); - if (result) { body = result; updated.push('Current Plan'); } - - // Update Total Plans in Phase - if (planCount) { - result = stateReplaceField(body, 'Total Plans in Phase', String(planCount)); - if (result) { body = result; updated.push('Total Plans in Phase'); } - } - - // Update **Current focus:** body text line (#1104) - const focusLabel = phaseName ? `Phase ${phaseNumber} — ${phaseName}` : `Phase ${phaseNumber}`; - const focusPattern = /(\*\*Current focus:\*\*\s*).*/i; - if (focusPattern.test(body)) { - body = body.replace(focusPattern, (_match, prefix: string) => `${prefix}${focusLabel}`); - updated.push('Current focus'); - } - - // Update ## Current Position section (#1104, #1365) - // ADR-1372 T6: positionPattern → tokenizeHeadings + spliceStateSection. - // Mirrors /(##\s*Current Position\s*\n)([\s\S]*?)(?=\n##|$)/i; stop at level ≥ 2. - const posHs = tokenizeHeadings(body); - const posIdx = posHs.findIndex(h => h.level === 2 && /^current\s+position$/i.test(h.text)); - if (posIdx !== -1) { - const posH = posHs[posIdx]; - const bodyLines = body.split('\n'); - const posHL = bodyLines[posH.line - 1]; - const posBodyStart = posH.offset + posHL.length + 1; - let posBodyEnd = body.length; - for (let j = posIdx + 1; j < posHs.length; j++) { - if (STOP_H2_PLUS(posHs[j].level)) { posBodyEnd = posHs[j].offset - 1; break; } - } - let posBody = body.slice(posBodyStart, posBodyEnd); - - // Update or insert Phase line - const newPhase = `Phase: ${phaseNumber}${phaseName ? ` (${phaseName})` : ''} — EXECUTING`; - if (/^Phase:/m.test(posBody)) { - posBody = posBody.replace(/^Phase:.*$/m, newPhase); - } else { - // Pipe-table format in Current Position (#1257): update the | Phase | … | - // cell rather than prepending a spurious inline `Phase:` line (which left - // the table cell stale). Mirrors the Status/Last-activity table branches. - const phaseValue = `${phaseNumber}${phaseName ? ` (${phaseName})` : ''} — EXECUTING`; - const replaced = stateReplaceField(posBody, 'Phase', phaseValue); - if (replaced !== null) posBody = replaced; - } - - // Update or insert Plan line - const newPlan = `Plan: 1 of ${planCount || '?'}`; - if (/^Plan:/m.test(posBody)) { - posBody = posBody.replace(/^Plan:.*$/m, newPlan); - } else { - // Pipe-table format in Current Position (#1257): update the | Plan | … | - // cell rather than appending after a prepended inline line. - const planValue = `1 of ${planCount || '?'}`; - const replaced = stateReplaceField(posBody, 'Plan', planValue); - if (replaced !== null) posBody = replaced; - } - - // Update Status line if present - const newStatus = `Status: Executing Phase ${phaseNumber}`; - if (/^Status:/m.test(posBody)) { - posBody = posBody.replace(/^Status:.*$/m, newStatus); - } else { - // Pipe-table format in Current Position (#1255) - const replaced = stateReplaceField(posBody, 'Status', `Executing Phase ${phaseNumber}`); - if (replaced !== null) posBody = replaced; - } - - // Update Last activity line if present - const newActivity = `Last activity: ${today} — Phase ${phaseNumber} execution started`; - if (/^Last activity:/im.test(posBody)) { - posBody = posBody.replace(/^Last activity:.*$/im, newActivity); - } else { - // Pipe-table format in Current Position (#1255) - // Value must match the inline branch (date + narrative), not bare date. - const activityValue = `${today} — Phase ${phaseNumber} execution started`; - const replaced = stateReplaceField(posBody, 'Last Activity', activityValue) - ?? stateReplaceField(posBody, 'Last activity', activityValue); - if (replaced !== null) posBody = replaced; - } - - body = body.slice(0, posBodyStart) + posBody + body.slice(posBodyEnd); - updated.push('Current Position'); - } - } else { - // Resume path: only update Last activity timestamp in Current Position - // (do not touch Plan:, stopped_at, progress.percent, or plan counter) - // ADR-1372 T6: positionPattern → tokenizeHeadings; stop at level ≥ 2. - const posHsR = tokenizeHeadings(body); - const posIdxR = posHsR.findIndex(h => h.level === 2 && /^current\s+position$/i.test(h.text)); - if (posIdxR !== -1) { - const posHR = posHsR[posIdxR]; - const bodyLinesR = body.split('\n'); - const posHLR = bodyLinesR[posHR.line - 1]; - const posBodyStartR = posHR.offset + posHLR.length + 1; - let posBodyEndR = body.length; - for (let j = posIdxR + 1; j < posHsR.length; j++) { - if (STOP_H2_PLUS(posHsR[j].level)) { posBodyEndR = posHsR[j].offset - 1; break; } - } - let posBody = body.slice(posBodyStartR, posBodyEndR); - const resumeActivity = `Last activity: ${today} — Phase ${phaseNumber} execution resumed (wave continue)`; - if (/^Last activity:/im.test(posBody)) { - posBody = posBody.replace(/^Last activity:.*$/im, resumeActivity); - body = body.slice(0, posBodyStartR) + posBody + body.slice(posBodyEndR); - updated.push('Last activity (resume)'); - } else { - // Pipe-table format in Current Position (#1255) - const replaced = stateReplaceField(posBody, 'Last Activity', resumeActivity) - ?? stateReplaceField(posBody, 'Last activity', resumeActivity); - if (replaced !== null) { - posBody = replaced; - body = body.slice(0, posBodyStartR) + posBody + body.slice(posBodyEndR); - updated.push('Last activity (resume)'); - } - } - } - } - - return reassemble(body); + const result = transitionCore(content, intent, deps); + updated = result.updated; + return result.content; }, cwd); output({ updated, phase: phaseNumber, phase_name: phaseName || null, plan_count: planCount || null }, raw, updated.length > 0 ? 'true' : 'false'); diff --git a/tests/state-transition.test.cjs b/tests/state-transition.test.cjs new file mode 100644 index 000000000..2bb548674 --- /dev/null +++ b/tests/state-transition.test.cjs @@ -0,0 +1,417 @@ +'use strict'; + +// Phase 1 tests for the STATE.md Transition Module (ADR-1769). +// These are characterization tests: they pin the behavior the new +// `transitionCore` / `beginPhase` API must preserve as the old +// `cmdStateBeginPhase` callback in state.cts is migrated onto it. +// +// Discipline: TDD vertical slices. One behavior → one test → minimal code → repeat. + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fc = require('fast-check'); + +const { + transitionCore, + FIELD_CLASSIFICATION, + getFieldClassification, + STATE_MD_SECTIONS, +} = require('../gsd-core/bin/lib/state-transition.cjs'); +const { stateExtractField } = require('../gsd-core/bin/lib/state-document.cjs'); + +const fixedClock = Object.freeze({ + today: () => '2026-06-27', + nowIso: () => '2026-06-27T12:00:00.000Z', +}); + +const noProgress = () => null; + +describe('ADR-1769 substrate: field-classification table', () => { + const allowedSources = new Set(['body', 'disk', 'external', 'curated', 'free']); + const allowedPreservation = new Set([ + 'derive', + 'preserve-when-unchanged', + 'preserve-always', + 'preserve-if-placeholder', + 'clear', + ]); + + test('every classified field has a { source, preservation } row with known enum values', () => { + for (const [field, cls] of Object.entries(FIELD_CLASSIFICATION)) { + assert.ok( + allowedSources.has(cls.source), + `field ${JSON.stringify(field)} has unknown source ${JSON.stringify(cls.source)}`, + ); + assert.ok( + allowedPreservation.has(cls.preservation), + `field ${JSON.stringify(field)} has unknown preservation ${JSON.stringify(cls.preservation)}`, + ); + } + }); + + test('current_phase_name is curated / preserve-always (ADR-1769 §4 — kills #1743/#1695 by construction)', () => { + const cls = getFieldClassification('current_phase_name'); + assert.strictEqual(cls && cls.source, 'curated'); + assert.strictEqual(cls && cls.preservation, 'preserve-always'); + }); + + test('progress is curated / preserve-always (ADR-1769 §4 — curated-progress ratchet)', () => { + const cls = getFieldClassification('progress'); + assert.strictEqual(cls && cls.source, 'curated'); + assert.strictEqual(cls && cls.preservation, 'preserve-always'); + }); + + test('table covers every frontmatter key emitted by buildStateFrontmatter (codex Phase 1 review)', () => { + // Verified against src/state.cts:1633-1653 (buildStateFrontmatter emit block). + const requiredFields = [ + 'gsd_state_version', + 'milestone', + 'milestone_name', + 'current_phase', + 'current_phase_name', + 'current_plan', + 'status', + 'stopped_at', + 'paused_at', + 'last_updated', + 'last_activity', + 'last_activity_desc', + 'progress', + 'progress.total_phases', + 'progress.completed_phases', + 'progress.total_plans', + 'progress.completed_plans', + 'progress.percent', + ]; + for (const f of requiredFields) { + assert.ok(getFieldClassification(f) !== null, + `frontmatter key ${JSON.stringify(f)} must have a classification row`); + } + }); + + test('getFieldClassification returns null for unknown fields AND inherited prototype methods', () => { + // Classic prototype-pollution guard: queries for 'toString' / 'valueOf' / '__proto__' + // must return null, not inherited Object.prototype functions. + assert.strictEqual(getFieldClassification('toString'), null); + assert.strictEqual(getFieldClassification('valueOf'), null); + assert.strictEqual(getFieldClassification('hasOwnProperty'), null); + assert.strictEqual(getFieldClassification('__proto__'), null); + assert.strictEqual(getFieldClassification('not-a-real-field'), null); + }); +}); + +describe('ADR-1769 substrate: STATE_MD_SECTIONS constants (aligned to gsd-core/templates/state.md)', () => { + test('every section heading starts with "## "', () => { + for (const [name, heading] of Object.entries(STATE_MD_SECTIONS)) { + assert.ok( + heading.startsWith('## '), + `section ${name} heading ${JSON.stringify(heading)} must start with "## "`, + ); + } + }); + + test('matches the six canonical top-level sections of the STATE.md template', () => { + assert.strictEqual(STATE_MD_SECTIONS.projectReference, '## Project Reference'); + assert.strictEqual(STATE_MD_SECTIONS.currentPosition, '## Current Position'); + assert.strictEqual(STATE_MD_SECTIONS.performanceMetrics, '## Performance Metrics'); + assert.strictEqual(STATE_MD_SECTIONS.accumulatedContext, '## Accumulated Context'); + assert.strictEqual(STATE_MD_SECTIONS.deferredItems, '## Deferred Items'); + assert.strictEqual(STATE_MD_SECTIONS.sessionContinuity, '## Session Continuity'); + }); +}); + +describe('ADR-1769 Phase 1: beginPhase transition — tracer bullet', () => { + test('updates body Status field to "Executing Phase N" on first-time begin', () => { + const input = [ + '# Project State', + '', + '**Status:** Planning', + '', + '## Current Position', + '', + 'Phase: 2 — DONE', + 'Plan: —', + 'Status: Planning', + '', + ].join('\n'); + + const result = transitionCore( + input, + { kind: 'beginPhase', phaseNumber: 3, phaseName: 'Test Phase', planCount: 5 }, + { clock: fixedClock, progressProvider: noProgress }, + ); + + assert.ok(result.updated.includes('Status'), `updated should include Status; got ${JSON.stringify(result.updated)}`); + // The transition must produce a body Status field carrying "Executing Phase 3". + // Use the same primitive the production code uses, not a source-grep. + const bodyStatus = stateExtractField(result.content, 'Status'); + assert.ok( + /Executing Phase\s+3\b/.test(bodyStatus || ''), + `body Status should match /Executing Phase 3/; got ${JSON.stringify(bodyStatus)}`, + ); + }); +}); + +// Shared fixture for first-time begin: a clean STATE.md body where no +// "Executing Phase N" status is present yet. +function firstTimeBody() { + return [ + '# Project State', + '', + '**Status:** Planning', + '**Current Phase:** 02', + '**Current Phase Name:** Previous Phase', + '**Current Plan:** 02', + '**Total Plans in Phase:** 3', + '**Last Activity:** 2026-06-20', + '**Last Activity Description:** previous work', + '**Current focus:** Phase 2 — Previous Phase', + '', + '## Current Position', + '', + 'Phase: 2 (Previous Phase)', + 'Plan: 2 of 3', + 'Status: Planning', + 'Last activity: 2026-06-20 — context gathered', + '', + ].join('\n'); +} + +describe('ADR-1769 Phase 1: beginPhase first-time body field updates', () => { + const intent = { kind: 'beginPhase', phaseNumber: 3, phaseName: 'Test Phase', planCount: 5 }; + const deps = { clock: fixedClock, progressProvider: noProgress }; + + test('updates Current Phase to N', () => { + const result = transitionCore(firstTimeBody(), intent, deps); + assert.strictEqual(stateExtractField(result.content, 'Current Phase'), '3'); + assert.ok(result.updated.includes('Current Phase')); + }); + + test('updates Current Phase Name when phaseName is provided', () => { + const result = transitionCore(firstTimeBody(), intent, deps); + assert.strictEqual(stateExtractField(result.content, 'Current Phase Name'), 'Test Phase'); + assert.ok(result.updated.includes('Current Phase Name')); + }); + + test('sets Current Plan to 1 on first-time begin', () => { + const result = transitionCore(firstTimeBody(), intent, deps); + assert.strictEqual(stateExtractField(result.content, 'Current Plan'), '1'); + assert.ok(result.updated.includes('Current Plan')); + }); + + test('updates Total Plans in Phase to planCount when provided', () => { + const result = transitionCore(firstTimeBody(), intent, deps); + assert.strictEqual(stateExtractField(result.content, 'Total Plans in Phase'), '5'); + assert.ok(result.updated.includes('Total Plans in Phase')); + }); + + test('updates Last Activity to clock.today()', () => { + const result = transitionCore(firstTimeBody(), intent, deps); + assert.strictEqual(stateExtractField(result.content, 'Last Activity'), '2026-06-27'); + assert.ok(result.updated.includes('Last Activity')); + }); + + test('updates Last Activity Description to "Phase N execution started"', () => { + const result = transitionCore(firstTimeBody(), intent, deps); + assert.strictEqual( + stateExtractField(result.content, 'Last Activity Description'), + 'Phase 3 execution started', + ); + assert.ok(result.updated.includes('Last Activity Description')); + }); + + test('updates **Current focus:** body text line (#1104)', () => { + const result = transitionCore(firstTimeBody(), intent, deps); + // The **Current focus:** line should now carry the new phase label. + const focusMatch = result.content.match(/\*\*Current focus:\*\*\s*(.*)/i); + assert.ok(focusMatch, '**Current focus:** line must still be present'); + assert.strictEqual(focusMatch[1].trim(), 'Phase 3 — Test Phase'); + assert.ok(result.updated.includes('Current focus'), + `updated should include 'Current focus'; got ${JSON.stringify(result.updated)}`); + }); +}); + +// Fixture for resume: a STATE.md body where Status already contains +// "Executing Phase 3" — the #3127 idempotency guard must detect this and +// skip the first-time-only field writes. +function resumeBody() { + return [ + '# Project State', + '', + '**Status:** Executing Phase 3', + '**Current Phase:** 03', + '**Current Phase Name:** Test Phase', + '**Current Plan:** 02', + '**Total Plans in Phase:** 5', + '**Last Activity:** 2026-06-26', + '**Last Activity Description:** mid-flight context from plan 3-02', + '', + '## Current Position', + '', + 'Phase: 3 (Test Phase) — EXECUTING', + 'Plan: 2 of 5', + 'Status: Executing Phase 3', + 'Last activity: 2026-06-26 — mid-flight context', + '', + ].join('\n'); +} + +describe('ADR-1769 Phase 1: #3127 idempotency guard — resume path', () => { + const intent = { kind: 'beginPhase', phaseNumber: 3, phaseName: 'Test Phase', planCount: 5 }; + const deps = { clock: fixedClock, progressProvider: noProgress }; + + test('Status is still refreshed on resume (Last Activity Date tracks execute-phase runs)', () => { + const result = transitionCore(resumeBody(), intent, deps); + assert.strictEqual(stateExtractField(result.content, 'Last Activity'), '2026-06-27'); + assert.ok(result.updated.includes('Last Activity')); + }); + + test('Current Plan is NOT overwritten on resume (#3127 — preserves mid-flight plan number)', () => { + const result = transitionCore(resumeBody(), intent, deps); + assert.strictEqual(stateExtractField(result.content, 'Current Plan'), '02'); + assert.ok(!result.updated.includes('Current Plan'), + `Current Plan must not be in updated on resume; got ${JSON.stringify(result.updated)}`); + }); + + test('Total Plans in Phase is NOT overwritten on resume', () => { + const result = transitionCore(resumeBody(), intent, deps); + assert.strictEqual(stateExtractField(result.content, 'Total Plans in Phase'), '5'); + assert.ok(!result.updated.includes('Total Plans in Phase')); + }); + + test('Last Activity Description is NOT overwritten on resume (#3127 — preserves mid-flight context)', () => { + const result = transitionCore(resumeBody(), intent, deps); + assert.strictEqual( + stateExtractField(result.content, 'Last Activity Description'), + 'mid-flight context from plan 3-02', + ); + assert.ok(!result.updated.includes('Last Activity Description')); + }); + + test('Current Phase Name is NOT overwritten on resume', () => { + const result = transitionCore(resumeBody(), intent, deps); + assert.strictEqual(stateExtractField(result.content, 'Current Phase Name'), 'Test Phase'); + assert.ok(!result.updated.includes('Current Phase Name')); + }); +}); + +describe('ADR-1769 Phase 1: Current Position section mutation (first-time begin)', () => { + const intent = { kind: 'beginPhase', phaseNumber: 3, phaseName: 'Test Phase', planCount: 5 }; + const deps = { clock: fixedClock, progressProvider: noProgress }; + + test('Current Position Phase line reflects the new phase (EXECUTING)', () => { + const result = transitionCore(firstTimeBody(), intent, deps); + assert.ok(result.updated.includes('Current Position'), + `updated should include Current Position; got ${JSON.stringify(result.updated)}`); + // Verify by extracting Phase from the result content (covers both inline and pipe-table). + // The transition writes "Phase: 3 (Test Phase) — EXECUTING" into ## Current Position. + // stateExtractField returns the first match across the whole content, but the + // **Current Phase:** frontmatter-style line is a different field, so 'Phase' + // extraction finds the Current Position line. + const posPhase = stateExtractField(result.content, 'Phase'); + assert.ok( + /3.*Test Phase.*EXECUTING/.test(posPhase || ''), + `Current Position Phase line should match /3.*Test Phase.*EXECUTING/; got ${JSON.stringify(posPhase)}`, + ); + }); + + test('Current Position Plan line shows "1 of N"', () => { + const result = transitionCore(firstTimeBody(), intent, deps); + const posPlan = stateExtractField(result.content, 'Plan'); + assert.ok( + /1 of 5/.test(posPlan || ''), + `Current Position Plan line should match /1 of 5/; got ${JSON.stringify(posPlan)}`, + ); + }); + + test('Current Position Status line reflects Executing Phase N', () => { + const result = transitionCore(firstTimeBody(), intent, deps); + // 'Status' extraction returns the first match — which is the top-level + // **Status:** line. The Current Position Status line is a different field + // occurrence. Extract from the section to disambiguate. + const { tokenizeHeadings } = require('../gsd-core/bin/lib/markdown-sectionizer.cjs'); + const body = result.content; + const hs = tokenizeHeadings(body); + const posIdx = hs.findIndex(h => h.level === 2 && /^current\s+position$/i.test(h.text)); + assert.notStrictEqual(posIdx, -1, 'Current Position section must exist'); + // Slice the section body and look for the Status line within it. + const h = hs[posIdx]; + const lines = body.split('\n'); + const hl = lines[h.line - 1]; + const bodyStart = h.offset + hl.length + 1; + let bodyEnd = body.length; + for (let j = posIdx + 1; j < hs.length; j++) { + if (hs[j].level >= 2) { bodyEnd = hs[j].offset - 1; break; } + } + const sectionBody = body.slice(bodyStart, bodyEnd); + const sectionStatus = stateExtractField(sectionBody, 'Status'); + assert.ok( + /Executing Phase\s+3/.test(sectionStatus || ''), + `Current Position Status line should match /Executing Phase 3/; got ${JSON.stringify(sectionStatus)}`, + ); + }); +}); + +describe('ADR-1769 Phase 1: Current Position section mutation (resume path)', () => { + const intent = { kind: 'beginPhase', phaseNumber: 3, phaseName: 'Test Phase', planCount: 5 }; + const deps = { clock: fixedClock, progressProvider: noProgress }; + + test('Resume updates only the Last activity line in Current Position (preserves Plan, Phase, Status)', () => { + const result = transitionCore(resumeBody(), intent, deps); + assert.ok(result.updated.includes('Last activity (resume)') || result.updated.includes('Last Activity'), + `resume should update Last activity; got ${JSON.stringify(result.updated)}`); + // Plan line in Current Position should still say "2 of 5" (NOT reset to "1 of 5"). + const posPlan = stateExtractField(result.content, 'Plan'); + assert.ok( + /2 of 5/.test(posPlan || ''), + `resume should preserve Plan "2 of 5"; got ${JSON.stringify(posPlan)}`, + ); + }); +}); + +describe('ADR-1769 Phase 1: property tests (RULESET.TESTS.property-based-testing)', () => { + const deps = { clock: fixedClock, progressProvider: noProgress }; + + test('for any non-negative integer phaseNumber and any STATE.md body with a non-whitespace Status value, beginPhase produces content whose body Status carries "Executing Phase N"', () => { + // Note: filters out whitespace-only statusSuffix because state-document.cjs's + // bold stateReplaceField pattern uses greedy \s* that consumes the trailing + // newline when the value is whitespace-only — a pre-existing bug surfaced + // by this property test, not introduced by ADR-1769. Filed as a follow-up. + fc.assert( + fc.property( + fc.integer({ min: 0, max: 999 }), + fc.string({ minLength: 1 }).filter(s => s.trim().length > 0 && !s.includes('\u0000')), + (phaseNum, statusSuffix) => { + const input = `# Project State\n\n**Status:** ${statusSuffix}\n`; + const result = transitionCore( + input, + { kind: 'beginPhase', phaseNumber: phaseNum, phaseName: null, planCount: null }, + deps, + ); + const bodyStatus = stateExtractField(result.content, 'Status') || ''; + return new RegExp(`Executing Phase\\s+${phaseNum}\\b`).test(bodyStatus); + }, + ), + { numRuns: 100 }, + ); + }); + + test('getFieldClassification own-property lookup always returns null or a valid {source, preservation} row', () => { + const allowedSources = new Set(['body', 'disk', 'external', 'curated', 'free']); + const allowedPreservation = new Set([ + 'derive', + 'preserve-when-unchanged', + 'preserve-always', + 'preserve-if-placeholder', + 'clear', + ]); + fc.assert( + fc.property(fc.string(), (s) => { + const cls = getFieldClassification(s); + if (cls === null) return true; + return allowedSources.has(cls.source) && allowedPreservation.has(cls.preservation); + }), + { numRuns: 200 }, + ); + }); +});