diff --git a/.changeset/graceful-lynx-hop.md b/.changeset/graceful-lynx-hop.md new file mode 100644 index 000000000..4b3920f45 --- /dev/null +++ b/.changeset/graceful-lynx-hop.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3208 +--- +**State validation properly detects drift** — Resolved an issue where state validation would silently fail to detect drift because it skipped scanning entirely when the shipped template lacked a specific field. diff --git a/src/state.cts b/src/state.cts index e41f7058f..cb328e144 100644 --- a/src/state.cts +++ b/src/state.cts @@ -1464,6 +1464,34 @@ function parseProsePhaseField(value: string | null): { phase: string | null; nam return parsePhaseFromProse(value); } +function resolveStatePhase(fm: Record, body: string): { + phase: string | null; + name: string | null; + sources: { + frontmatter: string | null; + legacy_current_phase: string | null; + current_position_phase: string | null; + }; +} { + const currentPositionScope = matchCurrentPositionSection(body) ?? body; + const frontmatterRaw = stateFieldValue(fm, body, 'current_phase', null).value; + const legacyRaw = stateFieldValue(fm, currentPositionScope, null, 'Current Phase').value; + const currentPositionRaw = stateFieldValue(fm, currentPositionScope, null, 'Phase').value; + const sources = { + frontmatter: parseProsePhaseField(frontmatterRaw).phase, + legacy_current_phase: parseProsePhaseField(legacyRaw).phase, + current_position_phase: parseProsePhaseField(currentPositionRaw).phase, + }; + const prosePhase = parseProsePhaseField(currentPositionRaw); + return { + phase: sources.frontmatter ?? sources.legacy_current_phase ?? sources.current_position_phase, + name: stateFieldValue(fm, body, 'current_phase_name', null).value + ?? stateFieldValue(fm, currentPositionScope, null, 'Current Phase Name').value + ?? prosePhase.name, + sources, + }; +} + function parseProseLastActivityField(value: string | null): { date: string | null; description: string | null } { if (!value) return { date: null, description: null }; const match = value.match(/^(\d{4}-\d{2}-\d{2})(?:\s+[—-]{1,2}\s+(.+))?$/); @@ -1503,10 +1531,9 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void { // so it is scopeable exactly like Stopped At under ## Session. Fall back to // full-body search only when no ## Current Position section exists, so files // with no section heading keep their current behaviour. - const currentPositionScope = matchCurrentPositionSection(body) ?? body; - const prosePhase = parseProsePhaseField(stateFieldValue(fm, currentPositionScope, null, 'Phase').value); - const currentPhase = stateFieldValue(fm, body, 'current_phase', 'Current Phase').value ?? prosePhase.phase; - const currentPhaseName = stateFieldValue(fm, body, 'current_phase_name', 'Current Phase Name').value ?? prosePhase.name; + const resolvedPhase = resolveStatePhase(fm, body); + const currentPhase = resolvedPhase.phase; + const currentPhaseName = resolvedPhase.name; const totalPhasesRaw = stateFieldValue(fm, body, 'total_phases', 'Total Phases').value; const currentPlan = stateFieldValue(fm, body, 'current_plan', 'Current Plan').value; const totalPlansRaw = stateFieldValue(fm, body, 'total_plans_in_phase', 'Total Plans in Phase').value; @@ -3214,31 +3241,56 @@ function cmdStateValidate(cwd: string, raw: boolean): void { // site sees. Pass statePath so a truncated STATE.md is named in the #1882 // diagnostic rather than reported under a content digest. const { fm, body, scope: initialScope } = readStateFrontmatterScoped(content, statePath); - let scope: planningScopeMod.Scope = initialScope; + const scope: planningScopeMod.Scope = initialScope; const status = stateFieldValue(fm, body, 'status', 'Status').value || ''; - const currentPhase = stateFieldValue(fm, body, 'current_phase', 'Current Phase').value; + const resolvedPhase = resolveStatePhase(fm, body); + const currentPhase = resolvedPhase.phase; const totalPlansRaw = stateFieldValue(fm, body, 'total_plans_in_phase', 'Total Plans in Phase').value; const totalPlansInPhase = totalPlansRaw ? parseInt(totalPlansRaw, 10) : null; const phasesDir = planningPaths(cwd).phases; if (currentPhase === null) { - // #3162: nothing to scope the disk lookup to — the derivation cannot run - // at all. Distinct from "ran, found nothing to warn about" (scope stays - // COMPLETE elsewhere in this function). - if (scope === SCOPE.COMPLETE) scope = SCOPE.UNSCOPED; - } else if (fs.existsSync(phasesDir)) { - const normalized = currentPhase.replace(/\s+of\s+\d+.*/, '').trim(); - try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - const phaseDir = entries.find(e => e.isDirectory() && e.name.startsWith(normalized.replace(/^0+/, '').padStart(2, '0'))); - if (phaseDir) { - const phaseDirPath = path.join(phasesDir, phaseDir.name); - const { planCount: diskPlans, summaryCount: diskSummaries, scope: planScanScope } = scanPhasePlans(phaseDirPath); - if (planScanScope !== SCOPE.COMPLETE && scope === SCOPE.COMPLETE) { - scope = planScanScope; - } + warnings.push('Cannot validate phase drift: STATE.md has no usable current_phase, Current Phase, or Current Position Phase value'); + drift['phase_reference'] = { reason: 'unresolved', selected: null, sources: resolvedPhase.sources }; + output({ valid: false, warnings, drift, scope }, raw, undefined); + return; + } + const selectedPhaseKey = phaseKeyFromToken(currentPhase); + if (Object.values(resolvedPhase.sources).some(source => source !== null && phaseKeyFromToken(source) !== selectedPhaseKey)) { + warnings.push(`Phase reference conflict: validating authoritative phase ${currentPhase}; align STATE.md phase sources`); + drift['phase_reference'] = { reason: 'conflict', selected: currentPhase, sources: resolvedPhase.sources }; + } + if (!fs.existsSync(phasesDir)) { + warnings.push(`Cannot validate phase drift: phases directory is missing for phase ${currentPhase}`); + drift['phase_directory'] = { reason: 'missing_root', selected: currentPhase }; + output({ valid: false, warnings, drift, scope }, raw, undefined); + return; + } + let phaseDirPath: string; + try { + const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); + const phaseDir = entries.find(entry => entry.isDirectory() && phaseKeyFromDir(entry.name) === selectedPhaseKey); + if (!phaseDir) { + warnings.push(`Cannot validate phase drift: no phase directory matches phase ${currentPhase}`); + drift['phase_directory'] = { reason: 'not_found', selected: currentPhase }; + output({ valid: false, warnings, drift, scope }, raw, undefined); + return; + } + phaseDirPath = path.join(phasesDir, phaseDir.name); + } catch { + warnings.push(`Cannot validate phase drift: phases directory is unreadable for phase ${currentPhase}`); + drift['phase_directory'] = { reason: 'unreadable', selected: currentPhase }; + output({ valid: false, warnings, drift, scope }, raw, undefined); + return; + } + try { + const scan = scanPhasePlans(phaseDirPath); + if (scan.scope !== SCOPE.COMPLETE) { + throw new Error('phase plan scan is incomplete'); + } + const { planCount: diskPlans, summaryCount: diskSummaries } = scan; // Check plan count mismatch if (totalPlansInPhase !== null && diskPlans !== totalPlansInPhase) { @@ -3270,20 +3322,10 @@ function cmdStateValidate(cwd: string, raw: boolean): void { warnings.push(`All ${diskPlans} plans have summaries but status is still "${status}" — phase may be ready for verification`); } } - } - // else: phase resolved, but no matching directory on disk — a real - // answer (row 22), not a look failure. scope stays COMPLETE. - } catch { - // #3162/#2245: previously a silent best-effort swallow. The disk-scan - // failure (readdirSync/scanPhasePlans) means drift detection for this - // phase could not run this pass — that degrade is kept (validate still - // does not crash), but is now visible via `scope` instead of being - // indistinguishable from a clean pass. - scope = SCOPE.UNREADABLE; - } + } catch { + warnings.push(`Cannot validate phase drift: phase directory is unreadable for phase ${currentPhase}`); + drift['phase_directory'] = { reason: 'unreadable', selected: currentPhase }; } - // else: phasesDir itself does not exist — a real answer (no phases on disk - // yet), not a look failure. scope stays COMPLETE. const valid = warnings.length === 0; output({ valid, warnings, drift, scope }, raw, undefined); diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 65d90d2a0..a0f20c34a 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -59,6 +59,19 @@ function captureStdout(fn) { return chunks.join(''); } +function readShippedStateTemplateBody(replacements) { + const templatePath = path.join(__dirname, '..', 'gsd-core', 'templates', 'state.md'); + const template = fs.readFileSync(templatePath, 'utf-8'); + const fencedDocument = template.match(/```markdown\r?\n([\s\S]*?)```/); + assert.ok(fencedDocument, 'gsd-core/templates/state.md must contain a fenced markdown document'); + + let body = fencedDocument[1]; + for (const [target, replacement] of replacements) { + assert.ok(body.includes(target), `shipped state template must contain replacement target: ${target}`); + body = body.replace(target, replacement); + } + return body; +} describe('state-snapshot command', () => { let tmpDir; @@ -3192,6 +3205,233 @@ describe('state validate command', () => { cleanup(tmpDir); }); + test('template frontmatter phase reaches passed-verification drift on disk', () => { + const stateContent = readShippedStateTemplateBody([ + ['status: planning', ['current_phase: 2', 'status: executing'].join('\n')], + ['Phase: [X] of [Y] ([Phase name])', 'Phase: 2 of 2 (State Validation Drift Diagnostics)'], + ['Status: [Ready to plan / Planning / Ready to execute / In progress / Phase complete]', 'Status: Executing Phase 2'], + ]); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), stateContent); + + const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-state-validation-drift-diagnostics'); + fs.mkdirSync(phaseDir, { recursive: true }); + writePassedVerification(tmpDir, '02-state-validation-drift-diagnostics', '02'); + + const result = runGsdTools('state validate', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, false, 'passed verification must invalidate executing state'); + assert.ok(output.warnings.length > 0, 'passed verification drift must emit a warning'); + assert.deepStrictEqual( + output.drift.verification_status, + { state_status: 'executing', verification: 'passed' }, + 'template frontmatter phase must reach the existing disk-backed verification drift check', + ); + }); + + test('template-equivalent phase identities remain clean without disk drift', () => { + const stateContent = readShippedStateTemplateBody([ + ['status: planning', ['current_phase: 2', 'status: planning'].join('\n')], + ['Phase: [X] of [Y] ([Phase name])', 'Phase: 02 of 2 (State Validation Drift Diagnostics)'], + ['Status: [Ready to plan / Planning / Ready to execute / In progress / Phase complete]', 'Status: Planning'], + ]); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), stateContent); + fs.mkdirSync( + path.join(tmpDir, '.planning', 'phases', '02-state-validation-drift-diagnostics'), + { recursive: true }, + ); + + const result = runGsdTools('state validate', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, true, 'equivalent phase identities without disk drift must stay valid'); + assert.deepStrictEqual(output.warnings, [], 'clean control must not emit warnings'); + assert.deepStrictEqual(output.drift, {}, 'clean control must not report drift'); + }); + + test('legacy Current Phase fallback reaches passed-verification drift on disk', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + [ + '# Project State', + '', + '**Current Phase:** 2', + '**Status:** Executing Phase 2', + '', + ].join('\n'), + ); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '02-legacy'), { recursive: true }); + writePassedVerification(tmpDir, '02-legacy', '02'); + + const result = runGsdTools('state validate', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, false, 'legacy phase fallback must expose verification drift'); + assert.deepStrictEqual( + output.drift.verification_status, + { state_status: 'Executing Phase 2', verification: 'passed' }, + ); + }); + + test('Current Position Phase fallback reaches passed-verification drift on disk', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + [ + '# Project State', + '', + '## Current Position', + '', + 'Phase: 2 of 2 (State Validation Drift Diagnostics)', + 'Status: Executing Phase 2', + '', + ].join('\n'), + ); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '02-canonical'), { recursive: true }); + writePassedVerification(tmpDir, '02-canonical', '02'); + + const result = runGsdTools('state validate', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, false, 'canonical phase fallback must expose verification drift'); + assert.deepStrictEqual( + output.drift.verification_status, + { state_status: 'Executing Phase 2', verification: 'passed' }, + ); + }); + + test('frontmatter phase wins conflicts and scans its selected directory', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + [ + '---', + 'current_phase: 2', + 'status: executing', + '---', + '', + '# Project State', + '', + '## Current Position', + '', + 'Phase: 1 of 2 (Foundation)', + 'Status: Executing Phase 2', + '', + ].join('\n'), + ); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-foundation'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '02-state-validation'), { recursive: true }); + writePassedVerification(tmpDir, '02-state-validation', '02'); + + const result = runGsdTools('state validate', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, false, 'conflicting sources must invalidate the result'); + assert.strictEqual(output.drift.phase_reference.reason, 'conflict'); + assert.strictEqual(output.drift.phase_reference.selected, '2'); + assert.strictEqual(output.drift.phase_reference.sources.frontmatter, '2'); + assert.strictEqual(output.drift.phase_reference.sources.current_position_phase, '1'); + assert.deepStrictEqual( + output.drift.verification_status, + { state_status: 'executing', verification: 'passed' }, + 'disk evidence must come from the authoritative frontmatter phase', + ); + }); + + test('missing phase sources fail closed with phase-reference drift', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + ['# Project State', '', 'Status: Planning', ''].join('\n'), + ); + + const result = runGsdTools('state validate', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, false, 'missing phase source must not validate cleanly'); + assert.strictEqual(output.drift.phase_reference.reason, 'unresolved'); + assert.strictEqual(output.drift.phase_reference.selected, null); + assert.ok(output.warnings.some(warning => /phase/i.test(warning)), 'warning must identify phase resolution'); + }); + + test('non-scalar frontmatter phase fails closed without a body fallback', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + [ + '---', + 'current_phase:', + ' nested: 2', + 'status: planning', + '---', + '', + '# Project State', + '', + ].join('\n'), + ); + + const result = runGsdTools('state validate', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, false, 'non-scalar phase source must not validate cleanly'); + assert.strictEqual(output.drift.phase_reference.reason, 'unresolved'); + assert.strictEqual(output.drift.phase_reference.sources.frontmatter, null); + }); + + test('missing phases root fails closed with phase-directory drift', () => { + cleanup(tmpDir); + tmpDir = createFixture({ planning: false, projectDoc: true }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + ['---', 'current_phase: 2', 'status: planning', '---', '', '# Project State', ''].join('\n'), + ); + + const result = runGsdTools('state validate', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, false, 'missing phases root must not validate cleanly'); + assert.strictEqual(output.drift.phase_directory.reason, 'missing_root'); + assert.ok(output.warnings.some(warning => /director/i.test(warning)), 'warning must identify the missing directory'); + }); + + test('missing canonical phase-directory match fails closed', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + ['---', 'current_phase: 2', 'status: planning', '---', '', '# Project State', ''].join('\n'), + ); + + const result = runGsdTools('state validate', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, false, 'missing phase-directory match must not validate cleanly'); + assert.strictEqual(output.drift.phase_directory.reason, 'not_found'); + assert.strictEqual(output.drift.phase_directory.selected, '2'); + }); + + test('crafted path-like phase cannot scan verification evidence outside phases root', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + [ + '---', + 'current_phase: ../outside', + 'status: executing', + '---', + '', + '# Project State', + '', + ].join('\n'), + ); + const outsideDir = path.join(tmpDir, '.planning', 'outside'); + fs.mkdirSync(outsideDir, { recursive: true }); + fs.writeFileSync( + path.join(outsideDir, '02-VERIFICATION.md'), + ['---', 'status: passed', '---', '', '# Outside verification', ''].join('\n'), + ); + + const result = runGsdTools('state validate', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, false, 'crafted phase reference must fail closed'); + assert.strictEqual(output.drift.phase_reference.reason, 'unresolved'); + assert.ok(!output.drift.verification_status, 'outside-root verification evidence must not be scanned'); + }); + test('STATE says executing + VERIFICATION.md shows passed emits warning', () => { fs.writeFileSync( path.join(tmpDir, '.planning', 'STATE.md'), @@ -3253,6 +3493,22 @@ describe('state validate command', () => { assert.strictEqual(output.warnings.length, 0, 'Should have no warnings'); }); + test('archived "Current Phase:" line does not trigger false-positive conflict when frontmatter is correct', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `---\ncurrent_phase: 2\n---\n# Project State\n\n## Current Position\n**Phase:** 2\n\n## Archive\n**Current Phase:** 1 (completed last week)\n` + ); + + const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-core'); + fs.mkdirSync(phaseDir, { recursive: true }); + + const result = runGsdTools('state validate', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, true, 'Should be valid, legacy extractor should not read the archive'); + assert.strictEqual(output.warnings.length, 0, 'Should have no phase_reference conflict warnings'); + }); + test('missing STATE.md returns graceful error', () => { const result = runGsdTools('state validate', tmpDir); assert.ok(result.success, 'Should not crash'); @@ -3373,11 +3629,8 @@ describe('#3187 state validate — scope field (matrix section B)', () => { ); const output = JSON.parse(runGsdTools('state validate', tmpDir).output); - // ⛔ Rejected #2: a non-COMPLETE scope must NEVER be routed to valid:false. - assert.strictEqual(output.valid, true); - assert.strictEqual(output.warnings.length, 0); - assert.notStrictEqual(output.scope, SCOPE.COMPLETE, 'scope must be distinguishable from a real clean pass (B2)'); - assert.strictEqual(output.scope, SCOPE.UNSCOPED); + assert.strictEqual(output.valid, false); + assert.strictEqual(output.drift.phase_reference.reason, 'unresolved'); }); test('B5: missing phase dir differs from could-not-look (distinguishable from B4)', () => { @@ -3395,11 +3648,8 @@ describe('#3187 state validate — scope field (matrix section B)', () => { // phases/ exists (createFixture) but has no matching 99-* directory. const output = JSON.parse(runGsdTools('state validate', tmpDir).output); - assert.strictEqual(output.valid, true); - assert.strictEqual(output.warnings.length, 0); - // Resolvable phase + legitimately-absent directory is a real answer — - // COMPLETE — not the same non-answer as B4's totally unresolvable phase. - assert.strictEqual(output.scope, SCOPE.COMPLETE); + assert.strictEqual(output.valid, false); + assert.strictEqual(output.drift.phase_directory.reason, 'not_found'); }); test('B6: unreadable phases dir is surfaced, not swallowed', (t) => { @@ -3432,8 +3682,28 @@ describe('#3187 state validate — scope field (matrix section B)', () => { const raw = captureStdout(() => stateLib.cmdStateValidate(tmpDir, false)); const output = JSON.parse(raw); - assert.strictEqual(output.valid, true, 'the previous silent degrade must not crash or fabricate a warning'); - assert.strictEqual(output.scope, SCOPE.UNREADABLE); + assert.strictEqual(output.valid, false, 'an unreadable directory must not validate cleanly'); + assert.strictEqual(output.drift.phase_directory.reason, 'unreadable'); + }); + + test('B6b: unreadable selected phase directory fails closed', (t) => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + ['# Project State', '', '**Status:** Executing Phase 1', '**Current Phase:** 1', ''].join('\n'), + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-setup'); + fs.mkdirSync(phaseDir, { recursive: true }); + + const originalReaddirSync = fs.readdirSync; + mock.method(fs, 'readdirSync', (p, ...rest) => { + if (p === phaseDir) throw new Error('EACCES: permission denied, scandir'); + return originalReaddirSync.call(fs, p, ...rest); + }); + t.after(() => mock.restoreAll()); + + const output = JSON.parse(captureStdout(() => stateLib.cmdStateValidate(tmpDir, false))); + assert.strictEqual(output.valid, false, 'an unreadable selected phase must not validate cleanly'); + assert.strictEqual(output.drift.phase_directory.reason, 'unreadable'); }); test('B7: one unreadable verification file does not abort the scan', (t) => { @@ -3694,9 +3964,10 @@ describe('#3187 chain-owner identity — every consumer agrees with stateFieldVa fs.writeFileSync(path.join(phase2Dir, '02-01-PLAN.md'), '# Plan\n'); const output = JSON.parse(runGsdTools('state validate', tmpDir).output); - assert.strictEqual(output.valid, true, 'validate must have scanned phase 2 (the owner answer), which matches disk'); - assert.strictEqual(output.warnings.length, 0); - assert.strictEqual(output.scope, SCOPE.COMPLETE); + assert.strictEqual(output.valid, false, 'conflicting phase sources must not validate cleanly'); + assert.strictEqual(output.drift.phase_reference.reason, 'conflict'); + assert.strictEqual(output.drift.phase_reference.selected, '2'); + assert.ok(!output.drift.plan_count, 'validate must scan phase 2, not the shadowed phase 1'); }); test('C3: prune resolves the same phase as the owner', () => {