From 6f80524be1a7fc3122d428486b5d7808af526904 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 27 Jun 2026 14:56:51 -0400 Subject: [PATCH] =?UTF-8?q?refactor(#1784):=20ADR-1769=20Phase=203=20?= =?UTF-8?q?=E2=80=94=20completePhase=20migration=20onto=20Transition=20Mod?= =?UTF-8?q?ule=20(#1785)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate the STATE.md write path inside cmdPhaseComplete (phase.cts) onto the STATE.md Transition Module substrate (ADR-1769, epic #1769). - Add {kind: 'completePhase'} to StateTransitionIntent + completePhaseCore in src/state-transition.cts. Pure body field mutations (Current Phase shape/name, Status, Current Plan, Last Activity + Description, Completed/Total Phases + Progress percent), consulting the field-classification table for touched keys. Roadmap progress injected via a new optional deps.roadmapProvider. - Collapse the ~90-line inline STATE.md transform in cmdPhaseComplete to a transitionCore dispatch. The adapter retains updatePerformanceMetricsSection (section table) + syncStateFrontmatter (disk-scan post-sync) and the multi-file atomic transaction (STATE is committed with ROADMAP/REQUIREMENTS, so readModifyWriteStateMd is not used here). - deriveProgressFromRoadmap/clampPercent/stateReplaceFieldWithFallback move from the phase.cts call site into the pure core (no circular dep: phase-lifecycle and state-document don't import state.cts). - Characterization tests in tests/state-transition.test.cjs pin the field updates, roadmap progress derivation, #1255 frontmatter parity, and the 'Phase:' fallback. All 44 transition + 193 phase tests pass. No user-visible behavior change. cmdPhaseComplete CLI output and 'phase complete' behavior unchanged. Closes #1784 --- docs/adr/1769-state-md-transition-module.md | 6 +- gsd-core/bin/lib/state-transition.cjs | 163 ++++++++++++++ src/phase.cts | 112 +++------- src/state-transition.cts | 213 +++++++++++++++++- tests/state-transition.test.cjs | 231 ++++++++++++++++++++ 5 files changed, 635 insertions(+), 90 deletions(-) diff --git a/docs/adr/1769-state-md-transition-module.md b/docs/adr/1769-state-md-transition-module.md index 1fa4a4b53..e4bea5f36 100644 --- a/docs/adr/1769-state-md-transition-module.md +++ b/docs/adr/1769-state-md-transition-module.md @@ -208,9 +208,9 @@ alongside, leave callbacks — parallel worlds don't converge (ADR-857's failure | Phase | Scope | Closes issue | Bug coverage | |---|---|---|---| | 0 | ADR + CONTEXT.md update | #1769 | — | -| 1 | Substrate + `beginPhase` | TBD | #1255, #1257, #3242 | -| 2 | `advancePlan` | TBD | — | -| 3 | `completePhase` + `phase.cts:1770` | TBD | — | +| 1 | Substrate + `beginPhase` | #1771 | #1255, #1257, #3242 | +| 2 | `advancePlan` | #1782 | — | +| 3 | `completePhase` + `phase.cts:1770` | #1784 | — | | 4 | `plannedPhase` + `milestoneSwitch` | TBD | — | | 5 | `milestoneComplete` + `milestone.cts:352` | TBD | — | | 6 | `patch` | TBD | #1743, #1695 | diff --git a/gsd-core/bin/lib/state-transition.cjs b/gsd-core/bin/lib/state-transition.cjs index 773677b1c..fad501b62 100644 --- a/gsd-core/bin/lib/state-transition.cjs +++ b/gsd-core/bin/lib/state-transition.cjs @@ -23,6 +23,7 @@ const frontmatter = require("./frontmatter.cjs"); const state_document_cjs_1 = require("./state-document.cjs"); const state_document_cjs_2 = require("./state-document.cjs"); const markdown_sectionizer_cjs_1 = require("./markdown-sectionizer.cjs"); +const phase_lifecycle_cjs_1 = require("./phase-lifecycle.cjs"); // eslint-disable-next-line @typescript-eslint/no-require-imports const phaseIdMod = require("./phase-id.cjs"); const { extractFrontmatter, reconstructFrontmatter } = frontmatter; @@ -116,6 +117,8 @@ function transitionCore(content, intent, deps) { return beginPhaseCore(content, intent, deps); case 'advancePlan': return advancePlanCore(content, deps); + case 'completePhase': + return completePhaseCore(content, intent, deps); } } // ---------------------------------------------------------------------------- @@ -489,3 +492,163 @@ function advancePlanCore(content, deps) { data: { advanced: true, previous_plan: currentPlan, current_plan: newPlan, total_plans: totalPlans }, }; } +// ---------------------------------------------------------------------------- +// completePhase — intent implementation (Phase 3) +// ---------------------------------------------------------------------------- +/** + * Apply a `completePhase` transition to STATE.md content. + * + * Migrates the inline STATE.md transform that lived inside `cmdPhaseComplete` + * (phase.cts) onto the substrate. Owns the field-classification-governed body + * mutations: Current Phase (preserving the `of total` shape and phase name), + * Current Phase Name, Status (`Milestone complete` on the last phase, else + * `Ready to plan`), Current Plan (`Not started`), Last Activity + Description, + * and the Completed/Total Phases + Progress percent block (re-derived from the + * roadmap via the injected `roadmapProvider`). + * + * The adapter (`cmdPhaseComplete`) retains two concerns that are NOT pure field + * updates: `updatePerformanceMetricsSection` (a section table upsert) and + * `syncStateFrontmatter` (the disk-scan post-sync). It also retains the + * multi-file atomic transaction (`writePlanningFileSet`) that writes ROADMAP, + * REQUIREMENTS, and STATE together — `readModifyWriteStateMd` is not used here + * because STATE.md is committed atomically with the other two files. + * + * Behavior is byte-for-byte with the pre-migration `phase.cts:1671-1772` block + * (verified by characterization tests in tests/state-transition.test.cjs). + */ +function completePhaseCore(content, intent, deps) { + const updated = []; + const today = deps.clock.today(); + // Consult the field-classification table for the frontmatter keys this + // transition touches (same guard beginPhaseCore applies). A missing row is a + // substrate defect — fail loudly rather than silently re-encoding policy. + for (const fmKey of [ + 'current_phase', + 'current_phase_name', + 'status', + 'current_plan', + 'last_activity', + 'last_activity_desc', + 'progress', + ]) { + const cls = getFieldClassification(fmKey); + if (cls === null) { + throw new Error(`transitionCore completePhase: frontmatter key ${JSON.stringify(fmKey)} is not in FIELD_CLASSIFICATION; ` + + `add a row per ADR-1769 §4 before touching it.`); + } + } + // #1255: body-field replacements operate on body only (frontmatter stripped), + // so the YAML `status:` / `current_phase:` keys cannot shadow the body fields. + 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; + // Current Phase — preserve the existing `of ` shape and the phase name + // in parens (mirrors phase.cts:1675-1697 byte-for-behaviour). + const phaseValue = intent.nextPhaseNum || intent.phaseNum; + const nextPhaseDisplayName = intent.nextPhaseName; + const existingPhaseField = (0, state_document_cjs_1.stateExtractField)(body, 'Current Phase') || (0, state_document_cjs_1.stateExtractField)(body, 'Phase'); + let newPhaseValue = String(phaseValue); + if (existingPhaseField) { + const totalMatch = existingPhaseField.match(/of\s+(\d+)/); + const nameMatch = existingPhaseField.match(/\(([^)]+)\)/); + if (totalMatch) { + const total = totalMatch[1]; + const nameStr = nextPhaseDisplayName + ? ` (${nextPhaseDisplayName})` + : nameMatch + ? ` (${nameMatch[1]})` + : ''; + newPhaseValue = `${phaseValue} of ${total}${nameStr}`; + } + else if (nextPhaseDisplayName) { + newPhaseValue = `${phaseValue} — ${nextPhaseDisplayName}`; + } + } + const phaseAfter = (0, state_document_cjs_1.stateReplaceFieldWithFallback)(body, 'Current Phase', 'Phase', newPhaseValue); + if (phaseAfter !== body) { + body = phaseAfter; + updated.push('Current Phase'); + } + // Current Phase Name — only written when a next-phase display name is known + // (#1743/#1695: classified curated/preserve-always, so an absent name does + // NOT clear an existing curated value). + if (nextPhaseDisplayName) { + const after = (0, state_document_cjs_1.stateReplaceField)(body, 'Current Phase Name', nextPhaseDisplayName); + if (after) { + body = after; + updated.push('Current Phase Name'); + } + } + // Status — `Milestone complete` on the final phase, otherwise `Ready to plan`. + const statusValue = intent.isLastPhase ? 'Milestone complete' : 'Ready to plan'; + const statusAfter = (0, state_document_cjs_1.stateReplaceFieldWithFallback)(body, 'Status', null, statusValue); + if (statusAfter !== body) { + body = statusAfter; + updated.push('Status'); + } + // Current Plan — reset for the next phase. + const planAfter = (0, state_document_cjs_1.stateReplaceFieldWithFallback)(body, 'Current Plan', 'Plan', 'Not started'); + if (planAfter !== body) { + body = planAfter; + updated.push('Current Plan'); + } + // Last Activity — prefer the prose `Last activity:` line (date + narrative) + // when present, else the bold `Last Activity:` date field. + const lastActivityDescription = `Phase ${intent.phaseNum} complete${intent.nextPhaseNum ? `, transitioned to Phase ${intent.nextPhaseNum}` : ''}`; + if (/^Last activity:/m.test(body)) { + const after = (0, state_document_cjs_1.stateReplaceField)(body, 'Last activity', `${today} — ${lastActivityDescription}`); + if (after) { + body = after; + updated.push('Last Activity'); + } + } + else { + const after = (0, state_document_cjs_1.stateReplaceField)(body, 'Last Activity', today); + if (after) { + body = after; + updated.push('Last Activity'); + } + } + const ladAfter = (0, state_document_cjs_1.stateReplaceField)(body, 'Last Activity Description', lastActivityDescription); + if (ladAfter) { + body = ladAfter; + updated.push('Last Activity Description'); + } + // Progress block — re-derive completed/total phases from the roadmap when + // available (milestone-wide source of truth), then recompute the percent. + // Only runs when a Completed Phases field exists (the existing guard). + const completedRaw = (0, state_document_cjs_1.stateExtractField)(body, 'Completed Phases'); + if (completedRaw !== null) { + let newCompleted = parseInt(completedRaw, 10); + let derivedTotalPhases = null; + const roadmapContent = deps.roadmapProvider ? deps.roadmapProvider() : null; + if (roadmapContent) { + const derived = (0, phase_lifecycle_cjs_1.deriveProgressFromRoadmap)(roadmapContent); + if (derived.completedPhases !== null) + newCompleted = derived.completedPhases; + if (derived.totalPhases !== null) + derivedTotalPhases = derived.totalPhases; + } + const completedAfter = (0, state_document_cjs_1.stateReplaceField)(body, 'Completed Phases', String(newCompleted)); + if (completedAfter) { + body = completedAfter; + updated.push('Completed Phases'); + } + const totalRaw = (0, state_document_cjs_1.stateExtractField)(body, 'Total Phases'); + const totalPhases = derivedTotalPhases || (totalRaw ? parseInt(totalRaw, 10) : null); + if (totalPhases && totalPhases > 0) { + const newPercent = (0, phase_lifecycle_cjs_1.clampPercent)(newCompleted, totalPhases); + const progAfter = (0, state_document_cjs_1.stateReplaceField)(body, 'Progress', `${newPercent}%`); + if (progAfter) { + body = progAfter; + updated.push('Progress'); + } + // Inline `percent:` token (frontmatter / progress sub-block). + body = body.replace(/(percent:\s*)\d+/, `$1${newPercent}`); + } + } + return { content: reassemble(body), updated }; +} diff --git a/src/phase.cts b/src/phase.cts index 3e95e08f6..76ef1a74c 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -51,8 +51,8 @@ import frontmatterMod = require('./frontmatter.cjs'); import stateMod = require('./state.cjs'); import { platformWriteSync, platformReadSync, platformEnsureDir, retryRenameSync } from './shell-command-projection.cjs'; import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; -import { deriveProgressFromRoadmap, clampPercent } from './phase-lifecycle.cjs'; import { realClock } from './clock.cjs'; +import { transitionCore } from './state-transition.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- uat-predicate.cjs is an export= CommonJS module import uatPredicate = require('./uat-predicate.cjs'); const { evaluateUatPassed } = uatPredicate; @@ -66,7 +66,6 @@ const { readModifyWriteStateMd, stateExtractField, stateReplaceField, - stateReplaceFieldWithFallback, syncStateFrontmatter, withStateLock, updatePerformanceMetricsSection, @@ -1672,93 +1671,38 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { const originalStateContent = platformReadSync(statePath) || ''; let stateContent = originalStateContent; - const phaseValue = nextPhaseNum || phaseNum; + // ADR-1769 Phase 3: the STATE.md field-update policy (Current Phase + // shape/name, Status, Current Plan, Last Activity + Description, and + // the Completed/Total Phases + Progress percent block) now dispatches + // to the STATE.md Transition Module. The ~90-line inline RMW callback + // that lived here is the pure `completePhaseCore` in + // src/state-transition.cts, backed by the field-classification table. + // `updatePerformanceMetricsSection` + `syncStateFrontmatter` stay in + // this adapter: they are section-table / disk-scan concerns, not + // classified fields, and `syncStateFrontmatter` is the post-sync this + // transaction needs (it does NOT go through readModifyWriteStateMd + // because STATE.md is committed atomically with ROADMAP/REQUIREMENTS). const nextPhaseDisplayName = phaseDisplayNameFromRoadmap(roadmapContent, nextPhaseNum) ?? phaseDisplayNameFromSlug(nextPhaseName); - const existingPhaseField = - stateExtractField(stateContent, 'Current Phase') || - stateExtractField(stateContent, 'Phase'); - let newPhaseValue = String(phaseValue); - if (existingPhaseField) { - const totalMatch = existingPhaseField.match(/of\s+(\d+)/); - const nameMatch = existingPhaseField.match(/\(([^)]+)\)/); - if (totalMatch) { - const total = totalMatch[1]; - const nameStr = nextPhaseDisplayName - ? ` (${nextPhaseDisplayName})` - : nameMatch - ? ` (${nameMatch[1]})` - : ''; - newPhaseValue = `${phaseValue} of ${total}${nameStr}`; - } else if (nextPhaseDisplayName) { - newPhaseValue = `${phaseValue} — ${nextPhaseDisplayName}`; - } - } - stateContent = stateReplaceFieldWithFallback( + const completeResult = transitionCore( stateContent, - 'Current Phase', - 'Phase', - newPhaseValue, + { + kind: 'completePhase', + phaseNum, + nextPhaseNum, + nextPhaseName: nextPhaseDisplayName, + isLastPhase, + planCount, + summaryCount, + }, + { + clock: realClock, + progressProvider: () => null, // completePhase derives progress from the roadmap, not disk + roadmapProvider: () => roadmapContent, + }, ); - - if (nextPhaseDisplayName) { - stateContent = - stateReplaceField(stateContent, 'Current Phase Name', nextPhaseDisplayName) || - stateContent; - } - - stateContent = stateReplaceFieldWithFallback( - stateContent, - 'Status', - null, - isLastPhase ? 'Milestone complete' : 'Ready to plan', - ); - - stateContent = stateReplaceFieldWithFallback( - stateContent, - 'Current Plan', - 'Plan', - 'Not started', - ); - - const lastActivityDescription = `Phase ${phaseNum} complete${nextPhaseNum ? `, transitioned to Phase ${nextPhaseNum}` : ''}`; - if (/^Last activity:/m.test(stateContent)) { - stateContent = - stateReplaceField(stateContent, 'Last activity', `${today} — ${lastActivityDescription}`) || - stateContent; - } else { - stateContent = - stateReplaceField(stateContent, 'Last Activity', today) || - stateContent; - } - - stateContent = - stateReplaceField(stateContent, 'Last Activity Description', lastActivityDescription) || - stateContent; - - const completedRaw = stateExtractField(stateContent, 'Completed Phases'); - if (completedRaw !== null) { - let newCompleted = parseInt(completedRaw, 10); - let derivedTotalPhases: number | null = null; - if (roadmapContent !== null) { - const derived = deriveProgressFromRoadmap(roadmapContent); - if (derived.completedPhases !== null) newCompleted = derived.completedPhases; - if (derived.totalPhases !== null) derivedTotalPhases = derived.totalPhases; - } - stateContent = - stateReplaceField(stateContent, 'Completed Phases', String(newCompleted)) || - stateContent; - - const totalRaw = stateExtractField(stateContent, 'Total Phases'); - const totalPhases = derivedTotalPhases || (totalRaw ? parseInt(totalRaw, 10) : null); - if (totalPhases && totalPhases > 0) { - const newPercent = clampPercent(newCompleted, totalPhases); - stateContent = - stateReplaceField(stateContent, 'Progress', `${newPercent}%`) || stateContent; - stateContent = stateContent.replace(/(percent:\s*)\d+/, `$1${newPercent}`); - } - } + stateContent = completeResult.content; stateContent = updatePerformanceMetricsSection( stateContent, diff --git a/src/state-transition.cts b/src/state-transition.cts index eeae05143..df5204c5e 100644 --- a/src/state-transition.cts +++ b/src/state-transition.cts @@ -16,9 +16,10 @@ // eslint-disable-next-line @typescript-eslint/no-require-imports import frontmatter = require('./frontmatter.cjs'); -import { stateReplaceField, stateExtractField, stateReplaceFieldIfTemplate } from './state-document.cjs'; +import { stateReplaceField, stateExtractField, stateReplaceFieldIfTemplate, stateReplaceFieldWithFallback } from './state-document.cjs'; import { KNOWN_TEMPLATE_DEFAULTS } from './state-document.cjs'; import { tokenizeHeadings } from './markdown-sectionizer.cjs'; +import { deriveProgressFromRoadmap, clampPercent } from './phase-lifecycle.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); @@ -147,12 +148,31 @@ export type ProgressRecord = Record; export type StateTransitionDeps = { progressProvider: () => ProgressRecord | null; clock: { today: () => string; nowIso: () => string }; + /** + * Roadmap content provider for transitions that re-derive milestone-wide + * progress from ROADMAP.md (completePhase). Optional: transitions that don't + * consult the roadmap ignore it. Injected (not imported) so the core stays + * pure and testable without disk I/O. + */ + roadmapProvider?: () => string | null; }; export type StateTransitionIntent = | { kind: 'beginPhase'; phaseNumber: string | number; phaseName: string | null; planCount: number | null } - | { kind: 'advancePlan' }; -// Phases 3–7 add the remaining 8 intent kinds to this discriminated union. + | { kind: 'advancePlan' } + | { + kind: 'completePhase'; + phaseNum: string; + /** Next phase number, or null when completing the final phase. */ + nextPhaseNum: string | null; + /** Display name for the next phase (may be null when unknown). */ + nextPhaseName: string | null; + /** True when this is the final phase in the milestone. */ + isLastPhase: boolean; + planCount: number; + summaryCount: number; + }; +// Phases 4–7 add the remaining intent kinds to this discriminated union. export type StateTransitionResult = { content: string; @@ -185,6 +205,8 @@ export function transitionCore( return beginPhaseCore(content, intent, deps); case 'advancePlan': return advancePlanCore(content, deps); + case 'completePhase': + return completePhaseCore(content, intent, deps); } } @@ -602,3 +624,188 @@ function advancePlanCore(content: string, deps: StateTransitionDeps): StateTrans data: { advanced: true, previous_plan: currentPlan, current_plan: newPlan, total_plans: totalPlans }, }; } + +// ---------------------------------------------------------------------------- +// completePhase — intent implementation (Phase 3) +// ---------------------------------------------------------------------------- + +/** + * Apply a `completePhase` transition to STATE.md content. + * + * Migrates the inline STATE.md transform that lived inside `cmdPhaseComplete` + * (phase.cts) onto the substrate. Owns the field-classification-governed body + * mutations: Current Phase (preserving the `of total` shape and phase name), + * Current Phase Name, Status (`Milestone complete` on the last phase, else + * `Ready to plan`), Current Plan (`Not started`), Last Activity + Description, + * and the Completed/Total Phases + Progress percent block (re-derived from the + * roadmap via the injected `roadmapProvider`). + * + * The adapter (`cmdPhaseComplete`) retains two concerns that are NOT pure field + * updates: `updatePerformanceMetricsSection` (a section table upsert) and + * `syncStateFrontmatter` (the disk-scan post-sync). It also retains the + * multi-file atomic transaction (`writePlanningFileSet`) that writes ROADMAP, + * REQUIREMENTS, and STATE together — `readModifyWriteStateMd` is not used here + * because STATE.md is committed atomically with the other two files. + * + * Behavior is byte-for-byte with the pre-migration `phase.cts:1671-1772` block + * (verified by characterization tests in tests/state-transition.test.cjs). + */ +function completePhaseCore( + content: string, + intent: { + kind: 'completePhase'; + phaseNum: string; + nextPhaseNum: string | null; + nextPhaseName: string | null; + isLastPhase: boolean; + planCount: number; + summaryCount: number; + }, + deps: StateTransitionDeps, +): StateTransitionResult { + const updated: string[] = []; + const today = deps.clock.today(); + + // Consult the field-classification table for the frontmatter keys this + // transition touches (same guard beginPhaseCore applies). A missing row is a + // substrate defect — fail loudly rather than silently re-encoding policy. + for (const fmKey of [ + 'current_phase', + 'current_phase_name', + 'status', + 'current_plan', + 'last_activity', + 'last_activity_desc', + 'progress', + ]) { + const cls = getFieldClassification(fmKey); + if (cls === null) { + throw new Error( + `transitionCore completePhase: frontmatter key ${JSON.stringify(fmKey)} is not in FIELD_CLASSIFICATION; ` + + `add a row per ADR-1769 §4 before touching it.`, + ); + } + } + + // #1255: body-field replacements operate on body only (frontmatter stripped), + // so the YAML `status:` / `current_phase:` keys cannot shadow the body fields. + 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; + + // Current Phase — preserve the existing `of ` shape and the phase name + // in parens (mirrors phase.cts:1675-1697 byte-for-behaviour). + const phaseValue = intent.nextPhaseNum || intent.phaseNum; + const nextPhaseDisplayName = intent.nextPhaseName; + const existingPhaseField = + stateExtractField(body, 'Current Phase') || stateExtractField(body, 'Phase'); + let newPhaseValue = String(phaseValue); + if (existingPhaseField) { + const totalMatch = existingPhaseField.match(/of\s+(\d+)/); + const nameMatch = existingPhaseField.match(/\(([^)]+)\)/); + if (totalMatch) { + const total = totalMatch[1]; + const nameStr = nextPhaseDisplayName + ? ` (${nextPhaseDisplayName})` + : nameMatch + ? ` (${nameMatch[1]})` + : ''; + newPhaseValue = `${phaseValue} of ${total}${nameStr}`; + } else if (nextPhaseDisplayName) { + newPhaseValue = `${phaseValue} — ${nextPhaseDisplayName}`; + } + } + const phaseAfter = stateReplaceFieldWithFallback(body, 'Current Phase', 'Phase', newPhaseValue); + if (phaseAfter !== body) { + body = phaseAfter; + updated.push('Current Phase'); + } + + // Current Phase Name — only written when a next-phase display name is known + // (#1743/#1695: classified curated/preserve-always, so an absent name does + // NOT clear an existing curated value). + if (nextPhaseDisplayName) { + const after = stateReplaceField(body, 'Current Phase Name', nextPhaseDisplayName); + if (after) { + body = after; + updated.push('Current Phase Name'); + } + } + + // Status — `Milestone complete` on the final phase, otherwise `Ready to plan`. + const statusValue = intent.isLastPhase ? 'Milestone complete' : 'Ready to plan'; + const statusAfter = stateReplaceFieldWithFallback(body, 'Status', null, statusValue); + if (statusAfter !== body) { + body = statusAfter; + updated.push('Status'); + } + + // Current Plan — reset for the next phase. + const planAfter = stateReplaceFieldWithFallback(body, 'Current Plan', 'Plan', 'Not started'); + if (planAfter !== body) { + body = planAfter; + updated.push('Current Plan'); + } + + // Last Activity — prefer the prose `Last activity:` line (date + narrative) + // when present, else the bold `Last Activity:` date field. + const lastActivityDescription = `Phase ${intent.phaseNum} complete${intent.nextPhaseNum ? `, transitioned to Phase ${intent.nextPhaseNum}` : ''}`; + if (/^Last activity:/m.test(body)) { + const after = stateReplaceField(body, 'Last activity', `${today} — ${lastActivityDescription}`); + if (after) { + body = after; + updated.push('Last Activity'); + } + } else { + const after = stateReplaceField(body, 'Last Activity', today); + if (after) { + body = after; + updated.push('Last Activity'); + } + } + + const ladAfter = stateReplaceField(body, 'Last Activity Description', lastActivityDescription); + if (ladAfter) { + body = ladAfter; + updated.push('Last Activity Description'); + } + + // Progress block — re-derive completed/total phases from the roadmap when + // available (milestone-wide source of truth), then recompute the percent. + // Only runs when a Completed Phases field exists (the existing guard). + const completedRaw = stateExtractField(body, 'Completed Phases'); + if (completedRaw !== null) { + let newCompleted = parseInt(completedRaw, 10); + let derivedTotalPhases: number | null = null; + const roadmapContent = deps.roadmapProvider ? deps.roadmapProvider() : null; + if (roadmapContent) { + const derived = deriveProgressFromRoadmap(roadmapContent); + if (derived.completedPhases !== null) newCompleted = derived.completedPhases; + if (derived.totalPhases !== null) derivedTotalPhases = derived.totalPhases; + } + const completedAfter = stateReplaceField(body, 'Completed Phases', String(newCompleted)); + if (completedAfter) { + body = completedAfter; + updated.push('Completed Phases'); + } + + const totalRaw = stateExtractField(body, 'Total Phases'); + const totalPhases = derivedTotalPhases || (totalRaw ? parseInt(totalRaw, 10) : null); + if (totalPhases && totalPhases > 0) { + const newPercent = clampPercent(newCompleted, totalPhases); + const progAfter = stateReplaceField(body, 'Progress', `${newPercent}%`); + if (progAfter) { + body = progAfter; + updated.push('Progress'); + } + // Inline `percent:` token (frontmatter / progress sub-block). + body = body.replace(/(percent:\s*)\d+/, `$1${newPercent}`); + } + } + + return { content: reassemble(body), updated }; +} diff --git a/tests/state-transition.test.cjs b/tests/state-transition.test.cjs index 7654eec14..dcdf4fa29 100644 --- a/tests/state-transition.test.cjs +++ b/tests/state-transition.test.cjs @@ -515,3 +515,234 @@ describe('ADR-1769 Phase 2: advancePlan with frontmatter (#1255 pattern — code assert.strictEqual(result.data && result.data.advanced, true); }); }); + +// Shared fixture for completePhase: a STATE.md body mid-execution with the +// progress fields the cmdPhaseComplete transform touches. Mirrors the shape +// state.cts:buildStateFrontmatter emits. +function completePhaseBody() { + return [ + '# Project State', + '', + '**Current Phase:** 3 of 5 (Old Name)', + '**Current Phase Name:** Old Name', + '**Current Plan:** 2', + '**Status:** Executing Phase 3', + '**Last Activity:** 2026-06-20', + '**Last Activity Description:** mid-flight', + '**Completed Phases:** 2', + '**Total Phases:** 5', + '**Progress:** 40%', + 'percent: 40', + '', + ].join('\n'); +} + +// A roadmap with a progress table: 3 of 5 phases Complete → deriveProgressFromRoadmap +// returns { completedPhases: 3, totalPhases: 5 }. +const ROADMAP_3_OF_5 = [ + '## Roadmap', + '', + '| Phase | Title | Status | Completed |', + '| --- | --- | --- | --- |', + '| 1 | A | Complete | 2026-01-01 |', + '| 2 | B | Complete | 2026-02-01 |', + '| 3 | C | Complete | 2026-03-01 |', + '| 4 | D | In Progress | - |', + '| 5 | E | Pending | - |', + '', +].join('\n'); + +describe('ADR-1769 Phase 3: completePhase transition — body field updates', () => { + const deps = { clock: fixedClock, progressProvider: noProgress, roadmapProvider: () => ROADMAP_3_OF_5 }; + + test('Current Phase advances to nextPhaseNum, preserving "of total" and appending the next name', () => { + const intent = { + kind: 'completePhase', + phaseNum: '3', + nextPhaseNum: '4', + nextPhaseName: 'Design Phase', + isLastPhase: false, + planCount: 3, + summaryCount: 3, + }; + const result = transitionCore(completePhaseBody(), intent, deps); + const cp = stateExtractField(result.content, 'Current Phase'); + assert.ok( + /^4 of 5 \(Design Phase\)$/.test(cp || ''), + `Current Phase should be "4 of 5 (Design Phase)"; got ${JSON.stringify(cp)}`, + ); + assert.ok(result.updated.includes('Current Phase')); + }); + + test('Current Phase Name is set to nextPhaseName when provided', () => { + const intent = { + kind: 'completePhase', + phaseNum: '3', + nextPhaseNum: '4', + nextPhaseName: 'Design Phase', + isLastPhase: false, + planCount: 3, + summaryCount: 3, + }; + const result = transitionCore(completePhaseBody(), intent, deps); + assert.strictEqual(stateExtractField(result.content, 'Current Phase Name'), 'Design Phase'); + }); + + test('Status becomes "Ready to plan" when not the last phase', () => { + const result = transitionCore( + completePhaseBody(), + { kind: 'completePhase', phaseNum: '3', nextPhaseNum: '4', nextPhaseName: 'Design Phase', isLastPhase: false, planCount: 3, summaryCount: 3 }, + deps, + ); + assert.strictEqual(stateExtractField(result.content, 'Status'), 'Ready to plan'); + }); + + test('Status becomes "Milestone complete" when isLastPhase is true', () => { + const result = transitionCore( + completePhaseBody(), + { kind: 'completePhase', phaseNum: '5', nextPhaseNum: null, nextPhaseName: null, isLastPhase: true, planCount: 2, summaryCount: 2 }, + deps, + ); + assert.strictEqual(stateExtractField(result.content, 'Status'), 'Milestone complete'); + }); + + test('Current Plan resets to "Not started"', () => { + const result = transitionCore( + completePhaseBody(), + { kind: 'completePhase', phaseNum: '3', nextPhaseNum: '4', nextPhaseName: null, isLastPhase: false, planCount: 3, summaryCount: 3 }, + deps, + ); + assert.strictEqual(stateExtractField(result.content, 'Current Plan'), 'Not started'); + }); + + test('Last Activity Description carries transition narrative', () => { + const result = transitionCore( + completePhaseBody(), + { kind: 'completePhase', phaseNum: '3', nextPhaseNum: '4', nextPhaseName: null, isLastPhase: false, planCount: 3, summaryCount: 3 }, + deps, + ); + assert.strictEqual( + stateExtractField(result.content, 'Last Activity Description'), + 'Phase 3 complete, transitioned to Phase 4', + ); + }); + + test('Last Activity Description has no transition clause when there is no next phase', () => { + const result = transitionCore( + completePhaseBody(), + { kind: 'completePhase', phaseNum: '5', nextPhaseNum: null, nextPhaseName: null, isLastPhase: true, planCount: 2, summaryCount: 2 }, + deps, + ); + assert.strictEqual(stateExtractField(result.content, 'Last Activity Description'), 'Phase 5 complete'); + }); +}); + +describe('ADR-1769 Phase 3: completePhase progress derivation (roadmap)', () => { + const deps = { clock: fixedClock, progressProvider: noProgress, roadmapProvider: () => ROADMAP_3_OF_5 }; + + test('Completed Phases is re-derived from the roadmap progress table', () => { + const result = transitionCore( + completePhaseBody(), + { kind: 'completePhase', phaseNum: '3', nextPhaseNum: '4', nextPhaseName: null, isLastPhase: false, planCount: 3, summaryCount: 3 }, + deps, + ); + assert.strictEqual(stateExtractField(result.content, 'Completed Phases'), '3'); + }); + + test('Progress percent is recomputed and the inline percent: token is updated', () => { + const result = transitionCore( + completePhaseBody(), + { kind: 'completePhase', phaseNum: '3', nextPhaseNum: '4', nextPhaseName: null, isLastPhase: false, planCount: 3, summaryCount: 3 }, + deps, + ); + assert.strictEqual(stateExtractField(result.content, 'Progress'), '60%'); + assert.ok(/percent:\s*60/.test(result.content), `inline percent: token should be 60; content was:\n${result.content}`); + }); + + test('when roadmapProvider yields null, existing Completed Phases / Progress are preserved (no crash)', () => { + const nullDeps = { clock: fixedClock, progressProvider: noProgress, roadmapProvider: () => null }; + const result = transitionCore( + completePhaseBody(), + { kind: 'completePhase', phaseNum: '3', nextPhaseNum: '4', nextPhaseName: null, isLastPhase: false, planCount: 3, summaryCount: 3 }, + nullDeps, + ); + assert.strictEqual(stateExtractField(result.content, 'Completed Phases'), '2'); + assert.strictEqual(stateExtractField(result.content, 'Progress'), '40%'); + }); +}); + +describe('ADR-1769 Phase 3: completePhase edge cases', () => { + const deps = { clock: fixedClock, progressProvider: noProgress, roadmapProvider: () => ROADMAP_3_OF_5 }; + + test('falls back to the "Phase:" field when "Current Phase:" is absent (stateReplaceFieldWithFallback)', () => { + const input = [ + '# Project State', + '', + 'Phase: 3 of 5', + '**Status:** Executing Phase 3', + '**Last Activity:** 2026-06-20', + '**Completed Phases:** 2', + '**Total Phases:** 5', + '**Progress:** 40%', + '', + ].join('\n'); + const result = transitionCore( + input, + { kind: 'completePhase', phaseNum: '3', nextPhaseNum: '4', nextPhaseName: null, isLastPhase: false, planCount: 3, summaryCount: 3 }, + deps, + ); + const phase = stateExtractField(result.content, 'Phase'); + assert.ok(/^4 of 5/.test(phase || ''), `Phase should advance to "4 of 5"; got ${JSON.stringify(phase)}`); + }); + + test('updates body Status, not the YAML status key, when frontmatter is present (#1255)', () => { + const input = [ + '---', + 'status: executing', + 'current_phase: "3"', + '---', + '', + '# Project State', + '', + '**Current Phase:** 3 of 5', + '**Status:** Executing Phase 3', + '**Last Activity:** 2026-06-20', + '**Completed Phases:** 2', + '**Total Phases:** 5', + '**Progress:** 40%', + '', + ].join('\n'); + const result = transitionCore( + input, + { kind: 'completePhase', phaseNum: '3', nextPhaseNum: '4', nextPhaseName: null, isLastPhase: false, planCount: 3, summaryCount: 3 }, + deps, + ); + // Body Status line must read "Ready to plan". + const bodyStatus = stateExtractField(result.content, 'Status'); + assert.strictEqual(bodyStatus, 'Ready to plan'); + // Frontmatter must remain a block and keep its YAML keys (not be mangled). + assert.ok(/^---\r?\n[\s\S]*?\r?\n---/.test(result.content), 'frontmatter block must be preserved'); + const fmLine = result.content.split('\n').find((l) => /^status:/.test(l)); + assert.ok(fmLine && /executing/.test(fmLine), `YAML status key must be untouched; got ${JSON.stringify(fmLine)}`); + }); + + test('when nextPhaseName is absent and Current Phase had no "of total", value is the bare phase number', () => { + const input = [ + '# Project State', + '', + '**Current Phase:** 3', + '**Status:** Executing Phase 3', + '**Last Activity:** 2026-06-20', + '**Completed Phases:** 2', + '**Total Phases:** 5', + '**Progress:** 40%', + '', + ].join('\n'); + const result = transitionCore( + input, + { kind: 'completePhase', phaseNum: '3', nextPhaseNum: '4', nextPhaseName: null, isLastPhase: false, planCount: 3, summaryCount: 3 }, + deps, + ); + assert.strictEqual(stateExtractField(result.content, 'Current Phase'), '4'); + }); +});