diff --git a/.changeset/eager-cranes-gather.md b/.changeset/eager-cranes-gather.md new file mode 100644 index 000000000..db7d52ac0 --- /dev/null +++ b/.changeset/eager-cranes-gather.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3634 +--- +**`state update-progress` no longer writes two different completion percentages in one call** — the verb printed plan throughput (summaries/plans) to stdout and into the body `Progress:` bar, while the same write independently derived the frontmatter `progress.percent` as the deliberate `min(plan, phase)` cap. Mid-phase, when plan throughput runs ahead of phase completion, STATE.md contradicted itself and `state json` disagreed with the command that had just written it — silently, at exit 0. All surfaces now derive from the single canonical computation, and its reported plan counts come from the same milestone window as the percent, so the verb's own output can no longer disagree with itself. When that computation withholds a percent, the verb withholds too rather than substituting a different metric. The min-cap definition is unchanged. (#3583) diff --git a/src/state.cts b/src/state.cts index e47486b1a..c48aba33c 100644 --- a/src/state.cts +++ b/src/state.cts @@ -80,7 +80,6 @@ import { tokenizeHeadings, collectSection, replaceSection } from './markdown-sec import type { HeadingToken } from './markdown-sectionizer.cjs'; import { parseMarkdownTable, updateTableCell, deleteTableRow, insertTableRow, splitTableRow, isDelimiterRow } from './markdown-table.cjs'; import { textEncodingError } from './validate.cjs'; -import { clampPercent } from './phase-lifecycle.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import healthDiagnosticTypesMod = require('./health-diagnostic-types.cjs'); const { SEVERITY, adviseRemedy } = healthDiagnosticTypesMod; @@ -874,14 +873,66 @@ function cmdStateRecordMetric(cwd: string, options: StateRecordMetricOptions, ra output(result, raw, 'true'); } +type UpdateProgressPreview = + | { withheld: true; reason: string } + | { withheld: false; percent: number; completedPlans: number; totalPlans: number }; + +/** + * #3583: computes the write-path percent AND the completed/total plan counts + * reported alongside it from ONE `buildStateFrontmatter` call, so + * `cmdStateUpdateProgress`'s JSON output cannot report a `percent` that + * disagrees with its own `completed`/`total` (`buildStateFrontmatter`'s + * `progress.{percent,completed_plans,total_plans}` all come from the same + * disk scan, scoped to the STORED `milestone:` frontmatter value — #3017). + * Re-deriving completed/total from a second, differently-scoped scan (the + * auto-derived one `cmdStateUpdateProgress` still runs for its own #3217/ + * #3233 withhold gates) is what let the two disagree when the auto-derived + * "current" milestone differs from the stored one. + * + * Perf note: this duplicates buildStateFrontmatter's own `getMilestoneInfo` + * (re-reads/re-parses ROADMAP.md) and `readGitHeadSha` (a `git rev-parse` + * subprocess spawn) — neither is memoized, unlike the phase/plan disk scan + * (`_diskScanCache`), which IS shared with the second `buildStateFrontmatter` + * call `readModifyWriteStateMd` makes below. Both non-cached calls therefore + * run twice per `state update-progress`. + */ +function computeUpdateProgressPreview(statePath: string, cwd: string): UpdateProgressPreview { + const preContent = fs.readFileSync(statePath, 'utf-8'); + const existingFm = extractFrontmatter(preContent, statePath) as Record; + const preBody = stripFrontmatter(preContent); + const storedMilestone = typeof existingFm['milestone'] === 'string' ? existingFm['milestone'] : null; + const builtFm = buildStateFrontmatter(preBody, cwd, storedMilestone, readStoredTotalPhases(existingFm)); + const progress = builtFm['progress'] as Record | undefined; + const percent = progress && typeof progress['percent'] === 'number' ? progress['percent'] : null; + const completedPlans = progress && typeof progress['completed_plans'] === 'number' ? progress['completed_plans'] : null; + const totalPlans = progress && typeof progress['total_plans'] === 'number' ? progress['total_plans'] : null; + // A null percent is REACHABLE beyond the #3217/#3233 withholds the caller + // already applies — buildStateFrontmatter also nulls it via its own #1761 + // milestone-unbounded guard, evaluated from `assertedMilestoneVersion` + // (an independent derivation, including a bare-version-token-in-prose + // fallback) rather than from `storedMilestone`/diskScope, so a STATE.md + // with no explicit `milestone:` field but a bare vX.Y token mentioned in + // ROADMAP prose can pass both of the caller's guards and still land here. + // Falling back to a locally-computed percent would reintroduce the exact + // #3583 defect for that case, so withhold instead — same shape as the + // caller's own no-op guards. + if (percent === null || completedPlans === null || totalPlans === null) { + return { withheld: true, reason: 'progress percent withheld by buildStateFrontmatter — STATE.md left unchanged' }; + } + return { withheld: false, percent, completedPlans, totalPlans }; +} + function cmdStateUpdateProgress(cwd: string, raw: boolean): void { const statePath = planningPaths(cwd).state; if (!fs.existsSync(statePath)) { output({ error: 'STATE.md not found' }, raw, undefined); return; } - // Count summaries across current milestone phases only (outside lock — read-only) + // Auto-derived scan across current-milestone phases (outside lock — read-only). + // Gates the #3217/#3233 withholds below ONLY — the reported completed/total + // counts come from computeUpdateProgressPreview's differently-scoped + // (stored-milestone) scan instead, so percent and completed/total can never + // disagree (#3583, finding 1). const phasesDir = planningPaths(cwd).phases; let totalPlans = 0; - let totalSummaries = 0; let phaseScope: Scope = SCOPE.UNREADABLE; { @@ -894,9 +945,8 @@ function cmdStateUpdateProgress(cwd: string, raw: boolean): void { const { value: phaseDirs, scope } = listMilestonePhaseDirs(phasesDir, { cwd }); phaseScope = scope; for (const dir of phaseDirs) { - const { planCount, summaryCount } = scanPhasePlans(path.join(phasesDir, dir)); + const { planCount } = scanPhasePlans(path.join(phasesDir, dir)); totalPlans += planCount; - totalSummaries += summaryCount; } } @@ -940,15 +990,25 @@ function cmdStateUpdateProgress(cwd: string, raw: boolean): void { return; } - const percent = clampPercent(totalSummaries, totalPlans); + // #3583: percent AND the completed/total counts reported alongside it both + // come from the SAME buildStateFrontmatter call (computeUpdateProgressPreview) + // — never from the auto-derived scan above, which exists only to gate the + // #3217/#3233 withholds and is scoped differently (no stored-milestone + // override), so reusing its counts here could report a percent that + // disagrees with its own completed/total. + const preview = computeUpdateProgressPreview(statePath, cwd); + if (preview.withheld) { + process.stderr.write(`[gsd-tools] WARNING: state update-progress skipped — ${preview.reason}\n`); + output({ updated: false, reason: preview.reason }, raw, 'false'); + return; + } + const { percent, completedPlans: fmCompletedPlans, totalPlans: fmTotalPlans } = preview; const barWidth = 10; const filled = Math.round(percent / 100 * barWidth); const bar = '█'.repeat(filled) + '░'.repeat(barWidth - filled); const progressStr = `[${bar}] ${percent}%`; let updated = false; - const _totalPlans = totalPlans; - const _totalSummaries = totalSummaries; readModifyWriteStateMd(statePath, (content) => { // #2177: match against the BODY only. With /i the patterns below would @@ -981,7 +1041,7 @@ function cmdStateUpdateProgress(cwd: string, raw: boolean): void { }, cwd); if (updated) { - output({ updated: true, percent, completed: _totalSummaries, total: _totalPlans, bar: progressStr }, raw, progressStr); + output({ updated: true, percent, completed: fmCompletedPlans, total: fmTotalPlans, bar: progressStr }, raw, progressStr); } else { output({ updated: false, reason: 'Progress field not found in STATE.md' }, raw, 'false'); } diff --git a/tests/state.test.cjs b/tests/state.test.cjs index c7ca28f81..7ac1d3694 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -60,6 +60,20 @@ function writePassedVerification(tmpDir, phaseDirName, paddedPhase) { ); } +/** + * CONTRIBUTING.md "Prohibited: Raw Text Matching on Test Outputs" — extract + * the body `Progress` field through the repo's own field extractor (never a + * raw substring/regex match against the whole rendered STATE.md) and return + * the parsed percent number, so `state update-progress` body-bar tests + * assert on a typed value instead of the rendered text. + */ +function bodyProgressPercent(stateMdContent) { + const raw = stateDocument.stateExtractField(stateMdContent, 'Progress'); + if (raw === null) return null; + const match = raw.match(/(\d{1,3})%/); + return match ? Number(match[1]) : null; +} + /** * Run `fn` while capturing every fd-1 write a `cmdState*` handler's * `output()` performs (it writes via a raw `fs.writeSync(1, ...)`, never @@ -1861,12 +1875,16 @@ describe('cmdStateUpdateProgress (state update-progress)', () => { const output = JSON.parse(result.output); assert.strictEqual(output.updated, true, 'updated should be true'); - assert.strictEqual(output.percent, 50, 'percent should be 50'); + // #3583: percent is now min(plan_fraction, phase_fraction) — the SAME + // value the frontmatter sync seam derives — not raw plan throughput. + // Plan fraction is 1/2 (50%), but neither phase has a passing + // *-VERIFICATION.md, so completed_phases is 0/2 (0%) and the min caps at 0. + assert.strictEqual(output.percent, 0, 'percent should be 0 (min-capped: 0/2 phases verified)'); assert.strictEqual(output.completed, 1, 'completed should be 1'); assert.strictEqual(output.total, 2, 'total should be 2'); const updated = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); - assert.ok(updated.includes('50%'), 'STATE.md Progress should contain 50%'); + assert.ok(updated.includes('0%'), 'STATE.md Progress should contain 0% (min-capped)'); }); test('#3233: zero plans (0/0) is a no-op — does not clobber the Progress record', () => { @@ -1975,12 +1993,18 @@ describe('cmdStateUpdateProgress (state update-progress)', () => { assert.ok(result.success, `Command failed: ${result.error}`); const out = JSON.parse(result.output); assert.strictEqual(out.updated, true); - assert.strictEqual(out.percent, 50); + // #3583: min-capped, not raw plan throughput — phase 01 has no passing + // *-VERIFICATION.md, so completed_phases is 0/1 and the min caps at 0. + assert.strictEqual(out.percent, 0); const updated = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); - // The body line advanced to 50% AND its descriptive suffix survived. - assert.ok(updated.includes('[█████░░░░░] 50% (1/2 plans complete)'), - 'body Progress line must update to 50% with suffix preserved'); + // The body line advanced to 0% (min-capped) AND its descriptive suffix survived. + // CONTRIBUTING.md "Prohibited: Raw Text Matching" — assert on the field + // extractor's parsed value, not a substring of the whole rendered file. + assert.strictEqual(bodyProgressPercent(updated), 0, 'body Progress line must update to 0% (min-capped)'); + const progressField = stateDocument.stateExtractField(updated, 'Progress'); + assert.ok(progressField && progressField.includes('(1/2 plans complete)'), + 'descriptive suffix must survive on the extracted Progress field'); // The frontmatter block is intact (not mangled by the old \s*-crosses-newline match). assert.ok(updated.includes('total_phases: 1'), 'frontmatter total_phases key must survive'); assert.ok(updated.includes('percent:'), 'frontmatter percent key must survive'); @@ -1991,7 +2015,7 @@ describe('cmdStateUpdateProgress (state update-progress)', () => { path.join(tmpDir, '.planning', 'STATE.md'), '# Project State\n\n**Progress:** [█████░░░░░] 50% (2/4 plans done; blocked on API keys)\n' ); - // 1 of 1 plan complete → 100%. + // 1 of 1 plan summarized, but no passing *-VERIFICATION.md → 0% (min-capped, see below). const phaseDir = path.join(tmpDir, '.planning', 'phases', '01'); fs.mkdirSync(phaseDir, { recursive: true }); fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); @@ -2000,8 +2024,14 @@ describe('cmdStateUpdateProgress (state update-progress)', () => { const result = runGsdTools('state update-progress', tmpDir); assert.ok(result.success); const updated = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); - assert.ok(/\[██████████\] 100% \(2\/4 plans done; blocked on API keys\)/.test(updated), - 'the machine segment updates to 100% while the suffix is preserved verbatim'); + // #3583: min-capped, not raw plan throughput — phase 01 has no passing + // *-VERIFICATION.md, so completed_phases is 0/1 and the min caps at 0 + // even though the single plan is fully summarized. + // CONTRIBUTING.md "Prohibited: Raw Text Matching" — parsed value, not rendered text. + assert.strictEqual(bodyProgressPercent(updated), 0, 'the machine segment updates to 0% (min-capped)'); + const progressField = stateDocument.stateExtractField(updated, 'Progress'); + assert.ok(progressField && progressField.includes('(2/4 plans done; blocked on API keys)'), + 'the descriptive suffix is preserved verbatim on the extracted Progress field'); }); test('#2177 no body Progress: line → updated:false even if frontmatter has a progress: key', () => { @@ -2024,6 +2054,349 @@ describe('cmdStateUpdateProgress (state update-progress)', () => { assert.ok(output.error !== undefined, 'output should have error field'); assert.ok(output.error.includes('STATE.md'), 'error should mention STATE.md'); }); + + // ── #3583: single-percent parity — stdout, body bar, and frontmatter must + // agree, all derived through the SAME computeProgressPercent(min(plan, + // phase)) call the frontmatter sync seam (buildStateFrontmatter) uses. ── + + test('#3583: 3/4 plans done, 1/2 phases verified -> stdout, frontmatter, body bar, and state json all agree at 50', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Project State\n\n**Progress:** [░░░░░░░░░░] 0%\n' + ); + + // Phase 01: fully planned, fully summarized, and verified (passing). + const phase01Dir = path.join(tmpDir, '.planning', 'phases', '01'); + fs.mkdirSync(phase01Dir, { recursive: true }); + fs.writeFileSync(path.join(phase01Dir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phase01Dir, '01-01-SUMMARY.md'), '# Summary\n'); + fs.writeFileSync(path.join(phase01Dir, '01-02-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phase01Dir, '01-02-SUMMARY.md'), '# Summary\n'); + writePassedVerification(tmpDir, '01', '01'); + + // Phase 02: 2 plans, 1 summary (not fully realized), no verification. + const phase02Dir = path.join(tmpDir, '.planning', 'phases', '02'); + fs.mkdirSync(phase02Dir, { recursive: true }); + fs.writeFileSync(path.join(phase02Dir, '02-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phase02Dir, '02-01-SUMMARY.md'), '# Summary\n'); + fs.writeFileSync(path.join(phase02Dir, '02-02-PLAN.md'), '# Plan\n'); + + // 3/4 plans summarized (75%), 1/2 phases verified (50%) -> min = 50. + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.updated, true); + assert.strictEqual(output.percent, 50, 'stdout percent should be min-capped at 50'); + + const updated = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.strictEqual(bodyProgressPercent(updated), 50, 'body bar should read 50%'); + + const jsonResult = runGsdTools('state json', tmpDir); + assert.ok(jsonResult.success, `state json failed: ${jsonResult.error}`); + const jsonOutput = JSON.parse(jsonResult.output); + assert.strictEqual(Number(jsonOutput.progress.percent), 50, 'frontmatter/state json percent should also be 50'); + }); + + test('#3583: 3/4 plans, 0/2 phases verified -> all four surfaces are 0', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Project State\n\n**Progress:** [██████████] 100%\n' + ); + + const phase01Dir = path.join(tmpDir, '.planning', 'phases', '01'); + fs.mkdirSync(phase01Dir, { recursive: true }); + fs.writeFileSync(path.join(phase01Dir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phase01Dir, '01-01-SUMMARY.md'), '# Summary\n'); + fs.writeFileSync(path.join(phase01Dir, '01-02-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phase01Dir, '01-02-SUMMARY.md'), '# Summary\n'); + // No VERIFICATION.md for phase 01. + + const phase02Dir = path.join(tmpDir, '.planning', 'phases', '02'); + fs.mkdirSync(phase02Dir, { recursive: true }); + fs.writeFileSync(path.join(phase02Dir, '02-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phase02Dir, '02-01-SUMMARY.md'), '# Summary\n'); + fs.writeFileSync(path.join(phase02Dir, '02-02-PLAN.md'), '# Plan\n'); + // No VERIFICATION.md for phase 02 either. + + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.updated, true); + assert.strictEqual(output.percent, 0, 'stdout percent should be 0 (0/2 phases verified caps the min)'); + + const updated = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.strictEqual(bodyProgressPercent(updated), 0, 'body bar should read 0%'); + + const jsonResult = runGsdTools('state json', tmpDir); + assert.ok(jsonResult.success, `state json failed: ${jsonResult.error}`); + const jsonOutput = JSON.parse(jsonResult.output); + assert.strictEqual(Number(jsonOutput.progress.percent), 0, 'frontmatter/state json percent should also be 0'); + }); + + test('#3583: fully planned but partially realized ROADMAP still caps — no false 100%', () => { + // ROADMAP declares 4 phases; only 2 have directories on disk, and both + // realized phases are fully summarized AND verified (plan fraction 100%). + // total_phases must still come from the ROADMAP (4), so completed_phases + // (2/4 = 50%) caps the result well under 100%. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + '### Phase 01: First', + '### Phase 02: Second', + '### Phase 03: Third', + '### Phase 04: Fourth', + ].join('\n') + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Project State\n\n**Progress:** [░░░░░░░░░░] 0%\n' + ); + + for (const num of ['01', '02']) { + const phaseDir = path.join(tmpDir, '.planning', 'phases', num); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, `${num}-01-PLAN.md`), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, `${num}-01-SUMMARY.md`), '# Summary\n'); + writePassedVerification(tmpDir, num, num); + } + + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.updated, true); + // Plan fraction is 2/2 = 100%, but completed_phases is 2/4 = 50% against + // the ROADMAP-declared total — the min caps the result at 50, never 100. + assert.strictEqual(output.percent, 50, 'ROADMAP-declared unrealized phases must cap the percent, not report false 100%'); + + const jsonResult = runGsdTools('state json', tmpDir); + assert.ok(jsonResult.success, `state json failed: ${jsonResult.error}`); + const jsonOutput = JSON.parse(jsonResult.output); + assert.strictEqual(Number(jsonOutput.progress.total_phases), 4, 'total_phases should come from the ROADMAP, not just realized dirs'); + assert.strictEqual(Number(jsonOutput.progress.percent), 50, 'frontmatter/state json percent should also cap at 50'); + }); + + test('#3583: all plans done and all phases verified -> 100 everywhere', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Project State\n\n**Progress:** [░░░░░░░░░░] 0%\n' + ); + + for (const num of ['01', '02']) { + const phaseDir = path.join(tmpDir, '.planning', 'phases', num); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, `${num}-01-PLAN.md`), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, `${num}-01-SUMMARY.md`), '# Summary\n'); + writePassedVerification(tmpDir, num, num); + } + + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.updated, true); + assert.strictEqual(output.percent, 100); + + const updated = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.strictEqual(bodyProgressPercent(updated), 100, 'body bar should read 100%'); + + const jsonResult = runGsdTools('state json', tmpDir); + assert.ok(jsonResult.success, `state json failed: ${jsonResult.error}`); + const jsonOutput = JSON.parse(jsonResult.output); + assert.strictEqual(Number(jsonOutput.progress.percent), 100, 'frontmatter/state json percent should also be 100'); + }); + + test('#3583/#3217: non-COMPLETE phase scope withholds before any percent is computed', () => { + // Absent ROADMAP.md is UNREADABLE scope (see beforeEach comment above). + fs.unlinkSync(path.join(tmpDir, '.planning', 'ROADMAP.md')); + + const before = '# Project State\n\n**Progress:** [██████████] 100%\n'; + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), before); + + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + writePassedVerification(tmpDir, '01', '01'); + + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.updated, false, 'non-COMPLETE scope must withhold, not compute a percent'); + assert.ok(/not complete/i.test(String(output.reason)), `should explain the withhold; got: ${output.reason}`); + + const after = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.strictEqual(after, before, 'STATE.md must be unchanged when the phase scope withholds (#3217)'); + }); + + test('#3583 follow-up: buildStateFrontmatter withhold (#1761 milestone-unbounded) is not papered over with plan throughput', () => { + // Neither #3217 (non-COMPLETE phase scope) nor #3233 (zero plans) fires + // here: STATE.md has no explicit `milestone:` field and ROADMAP.md has no + // versioned heading, so this verb's own `phaseScope` guard (and + // buildStateFrontmatter's `diskScope`) both classify as SCOPE.COMPLETE + // (row 3, "free-form legacy roadmap") — the guard above never fires, and + // plans exist on disk. But ROADMAP.md mentions a bare version token + // ("v2.0") in body prose (not a heading, not a 🚧 bullet). + // getMilestoneInfo's own bare-version-token fallback (roadmap-parser.cts) + // picks that up as `assertedMilestoneVersion` — a signal this verb's + // `phaseScope`/`storedMilestone` derivation never sees — and + // buildStateFrontmatter's #1761 guard finds no ROADMAP HEADING matching + // 'v2.0', so it withholds `progress.percent` even though diskScope is + // COMPLETE. Before the fix, this verb fell back to + // clampPercent(totalSummaries, totalPlans) and printed a percent the + // frontmatter never wrote — reintroducing the exact #3583 defect for + // this rarer case. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + 'Target release: v2.0', + '', + '### Phase 1: Foo', + '### Phase 2: Bar', + '', + ].join('\n') + ); + + const before = '# Project State\n\n**Progress:** [██████████] 100%\n'; + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), before); + + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-01-SUMMARY.md'), '# Summary\n'); + + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.updated, false, + 'buildStateFrontmatter withheld a percent (#1761 milestone-unbounded) — the verb must withhold too, not fall back to plan throughput'); + assert.strictEqual(output.percent, undefined, 'no percent may be reported when the frontmatter withheld one'); + assert.ok(/withheld/i.test(String(output.reason)), `should explain the withhold; got: ${output.reason}`); + + const after = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.strictEqual(after, before, 'STATE.md must be unchanged — no bar/percent written when the frontmatter withheld'); + + // Confirm this is genuinely the #1761 path and not #3217/#3233: state json + // must ALSO omit progress.percent for the same reason (proves buildStateFrontmatter + // really withheld here, not merely a divergent local computation). + const jsonResult = runGsdTools('state json --raw', tmpDir); + assert.ok(jsonResult.success, `state json failed: ${jsonResult.error}`); + const jsonOutput = JSON.parse(jsonResult.output); + assert.ok( + jsonOutput.progress === undefined || jsonOutput.progress.percent === undefined, + `state json must also omit percent for this fixture; got progress=${JSON.stringify(jsonOutput.progress)}`, + ); + }); + + test('#3583: derivation parity — completed_phases is verification-passed, not summary parity', () => { + // Phase 01 is fully planned AND fully summarized (a summary-parity + // derivation would call it "complete"), but carries NO passing + // *-VERIFICATION.md. If the verb ever re-derives completed_phases from + // summary parity instead of routing through the same isPhaseComplete + // (verification-passed) owner buildStateFrontmatter uses, this becomes a + // false 100% (plan fraction 2/2 AND a summary-parity phase fraction 1/1 + // both read 100%). The correct min-capped answer is 0, because + // completed_phases is 0/1 under the verification-passed definition. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Project State\n\n**Progress:** [░░░░░░░░░░] 0%\n' + ); + + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-01-SUMMARY.md'), '# Summary\n'); + // Deliberately no VERIFICATION.md: summary-parity says "complete", + // verification-passed says "not complete". + + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.updated, true); + assert.strictEqual( + output.percent, + 0, + 'completed_phases must come from verification-passed status (isPhaseComplete), not summary parity — ' + + 'a summary-parity derivation would wrongly report 100 here' + ); + + const updated = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.strictEqual(bodyProgressPercent(updated), 0, 'body bar must reflect the verification-passed derivation, not summary parity'); + }); + + test('#3583 finding 1: percent and completed/total must come from the SAME (stored-milestone-scoped) window, not a differently-scoped auto-derive', () => { + // Reproduces the exact divergence: `cmdStateUpdateProgress`'s own guard + // scan calls `listMilestonePhaseDirs(phasesDir, { cwd })` with NO + // versionOverride, so it falls back to auto-deriving "current" via + // `extractCurrentMilestoneScoped`, which (per #730/#2947) merges the + // PREAMBLE — everything before the first milestone heading — into its + // window UNLESS the selected milestone's own section already carries a + // `### Phase N:` heading directly (in which case the preamble is + // stripped of phase headings to avoid duplicating them). This milestone + // section deliberately carries ONLY a checklist bullet (no heading of + // its own — the heading lives in the split "(Phase Details)" section), + // so that strip never fires and Phase 03's preamble heading leaks into + // the auto-derived window. `buildStateFrontmatter`'s scan, scoped via + // `versionOverride: storedMilestone` ("v2.0"), calls `sliceMilestoneWindow` + // instead, which never includes the preamble — so it correctly excludes + // Phase 03. Both windows classify SCOPE.COMPLETE (verified directly + // against `listMilestonePhaseDirs` before this test was written), so + // neither the #3217 nor the #1761 withhold intercepts — before the fix, + // this silently produced percent:0 (derived from the correct v2.0-only + // window: 1/1 plan, capped to 0 by the missing phase verification) + // alongside completed:1/total:2 (leaking Phase 03's unsummarized plan + // into the denominator from the auto-derived window) — mutually + // inconsistent in the SAME JSON object. Fixed: completed/total now come + // from the identical buildStateFrontmatter call percent does. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + '### Phase 03: Legacy Preamble Phase', + '', + '## v2.0: Current', + '', + '- [ ] **Phase 2: Feature**', + '', + '## v2.0 (Phase Details)', + '', + '### Phase 2: Feature', + '**Goal:** build it.', + '', + ].join('\n') + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + ['---', 'gsd_state_version: "1.0"', 'milestone: v2.0', 'status: executing', '---', '', + '# Project State', '', '**Progress:** [░░░░░░░░░░] 0%', ''].join('\n') + ); + + const phase02Dir = path.join(tmpDir, '.planning', 'phases', '02'); + fs.mkdirSync(phase02Dir, { recursive: true }); + fs.writeFileSync(path.join(phase02Dir, '02-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phase02Dir, '02-01-SUMMARY.md'), '# Summary\n'); + // Phase 03 belongs to no milestone (preamble-only) and is NOT summarized — + // if it leaks into the reported denominator, completed/total disagree + // with the v2.0-scoped percent. + const phase03Dir = path.join(tmpDir, '.planning', 'phases', '03'); + fs.mkdirSync(phase03Dir, { recursive: true }); + fs.writeFileSync(path.join(phase03Dir, '03-01-PLAN.md'), '# Plan\n'); + + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.updated, true, 'both windows are SCOPE.COMPLETE — neither withhold guard should fire'); + + // The mutual-consistency assertion finding 1 requires: completed/total + // must describe the SAME window percent was computed against (v2.0 only: + // phase 02, 1 plan, not phase-verified → percent 0; NOT phase 03's leaked + // unsummarized plan folded into the denominator). + assert.strictEqual(output.percent, 0, 'v2.0-scoped plan fraction (1/1) is capped to 0 by the missing phase verification'); + assert.strictEqual(output.total, 1, 'total must be v2.0-scoped (phase 02 only) — Phase 03 must not leak in from the auto-derived scan'); + assert.strictEqual(output.completed, 1, 'completed must be v2.0-scoped (phase 02 only)'); + }); }); // ─────────────────────────────────────────────────────────────────────────────