diff --git a/.changeset/witty-pumas-caper.md b/.changeset/witty-pumas-caper.md new file mode 100644 index 000000000..f65f267e6 --- /dev/null +++ b/.changeset/witty-pumas-caper.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 4851 +--- +**A superseded plan no longer reads as executed in the ROADMAP** — the roadmap update-plan-progress checkbox tick now skips plans the count already excludes as superseded, so the "N/M plans executed" line and the checkboxes below it always agree. (#4741) + diff --git a/src/roadmap.cts b/src/roadmap.cts index 81bc48c28..52cd41b42 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -1235,9 +1235,17 @@ function cmdRoadmapUpdatePlanProgress(cwd: string, phaseNum: string | null | und } // Mark completed plan checkboxes (e.g. "- [ ] 50-01-PLAN.md", "- [ ] 50-01:", or "- [ ] **50-01**") - for (const summaryFile of phaseInfo!.summaries) { - const planId = summaryFile.replace('-SUMMARY.md', '').replace('SUMMARY.md', ''); - if (!planId) continue; + // #4741: tick only plans the phase's own COUNT still counts. `plans` is the + // superseded-filtered set (#2349 via scanPhasePlans) while `summaries` is + // the raw *-SUMMARY.md listing — a superseded plan can carry a SUMMARY + // (e.g. `status: halted`), and ticking it read as "executed" right under a + // count line that excludes it. The prefix match mirrors the checkbox regex + // below (rows match by planId prefix, which the PLAN-01.md naming shape + // relies on), so non-superseded plans tick exactly as before. + const tickableSummaries = phaseInfo!.summaries + .map((summaryFile) => ({ summaryFile, planId: summaryFile.replace('-SUMMARY.md', '').replace('SUMMARY.md', '') })) + .filter(({ planId }) => planId !== '' && phaseInfo!.plans.some((planFile) => planFile.startsWith(planId))); + for (const { planId } of tickableSummaries) { const planEscaped = escapeRegex(planId); const planCheckboxPattern = new RegExp( `(-\\s*\\[) (\\]\\s*(?:\\*\\*)?${planEscaped}(?:\\*\\*)?)`, @@ -1319,9 +1327,9 @@ function cmdRoadmapUpdatePlanProgress(cwd: string, phaseNum: string | null | und if (withRows !== roadmapContent) { roadmapContent = withRows; // Mark any newly-inserted rows that already have summaries as complete - for (const summaryFile of phaseInfo!.summaries) { - const planId = summaryFile.replace('-SUMMARY.md', '').replace('SUMMARY.md', ''); - if (!planId) continue; + // (#4741: same superseded-filtered tick list as the loop above — a + // pre-existing superseded row must stay unchecked on this path too). + for (const { planId } of tickableSummaries) { const planEscaped = escapeRegex(planId); const planCheckboxPattern = new RegExp( `(-\\s*\\[) (\\]\\s*(?:\\*\\*)?${planEscaped}(?:\\*\\*)?)`, diff --git a/tests/roadmap.test.cjs b/tests/roadmap.test.cjs index 1a83d898e..94188f31b 100644 --- a/tests/roadmap.test.cjs +++ b/tests/roadmap.test.cjs @@ -5117,3 +5117,165 @@ describe('#3957 (epic #3473 B9): no-op decline reports the real condition', () = }); }); }); + +// ─── #4741: a superseded plan must not be ticked from its SUMMARY ──────────── + +describe('roadmap update-plan-progress — superseded plans (#4741)', () => { + // The issue's self-contained fixture: phase 1 with an active plan (01-01) + // and a superseded plan (01-02, `status: superseded` in the PLAN + // frontmatter), each with a SUMMARY file. The count excludes the superseded + // plan (#2349) while the checkbox tick iterated the raw summaries — so the + // ROADMAP read "1/1 plans executed" above two checked rows. + function write4741World(tmpDir, opts = {}) { + const { + supersededPlanStatus = 'superseded', + supersededSummaryStatus = 'halted', + includeSupersededSummary = true, + activeSummaryStatus = 'complete', + roadmap = null, + } = opts; + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-demo'), { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + roadmap ?? [ + '# Roadmap', + '', + '### Phase 1: Demo', + '', + '**Goal:** demo', + '', + '**Plans:** 2 plans', + '', + 'Plans:', + '', + '- [ ] 01-01-PLAN.md — first', + '- [ ] 01-02-PLAN.md — second', + '', + ].join('\n'), + ); + const plan = (num, extra) => + `---\nphase: 01-demo\nplan: ${num}\ntype: execute\nwave: 1\ndepends_on: []\nfiles_modified: []\nautonomous: true${extra ? `\n${extra}` : ''}\n---\n`; + fs.writeFileSync(path.join(tmpDir, '.planning', 'phases', '01-demo', '01-01-PLAN.md'), plan('01')); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'phases', '01-demo', '01-02-PLAN.md'), + plan('02', `status: ${supersededPlanStatus}`), + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'phases', '01-demo', '01-01-SUMMARY.md'), + `---\nphase: 01-demo\nplan: 01\nstatus: ${activeSummaryStatus}\n---\n`, + ); + if (includeSupersededSummary) { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'phases', '01-demo', '01-02-SUMMARY.md'), + `---\nphase: 01-demo\nplan: 02\nstatus: ${supersededSummaryStatus}\n---\n`, + ); + } + } + + function runUpdate(tmpDir) { + const result = runGsdTools('roadmap update-plan-progress 1', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + return JSON.parse(result.output); + } + + test('#4741: a superseded plan is not ticked even when its SUMMARY exists', (t) => { + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + write4741World(tmpDir); + + const output = runUpdate(tmpDir); + assert.strictEqual(output.plan_count, 1, 'the superseded plan is excluded from the count'); + assert.strictEqual(output.summary_count, 1); + + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + assert.ok(roadmap.includes('- [x] 01-01-PLAN.md'), 'the active executed plan ticks'); + assert.ok( + roadmap.includes('- [ ] 01-02-PLAN.md'), + 'the superseded plan must stay unchecked — a plan the tool does not count must not read as executed', + ); + assert.ok(roadmap.includes('1/1 plans executed'), 'the count line agrees with the checkboxes'); + }); + + test('#4741: the superseded filter ignores the SUMMARY\'s own status', (t) => { + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + write4741World(tmpDir, { supersededSummaryStatus: 'complete' }); + + runUpdate(tmpDir); + + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + assert.ok( + roadmap.includes('- [ ] 01-02-PLAN.md'), + 'a superseded plan with a COMPLETE summary still must not read as executed', + ); + }); + + test('#4741: the inserted-rows tick path respects the superseded filter too', (t) => { + // Loop 2 fires when a countable plan's row is MISSING (insertion path): + // pre-existing superseded row + missing countable row. Before the fix the + // insertion path's tick loop iterated the raw summaries and ticked the + // pre-existing superseded row as well. + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + write4741World(tmpDir, { + roadmap: [ + '# Roadmap', + '', + '### Phase 1: Demo', + '', + '**Goal:** demo', + '', + '**Plans:** 2 plans', + '', + 'Plans:', + '', + '- [ ] 01-02-PLAN.md — second', + '', + ].join('\n'), + }); + + const output = runUpdate(tmpDir); + assert.strictEqual(output.plan_count, 1); + + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + assert.ok(roadmap.includes('- [x] 01-01-PLAN.md'), 'the inserted countable row is ticked (it has a summary)'); + assert.ok( + roadmap.includes('- [ ] 01-02-PLAN.md'), + 'the pre-existing superseded row must stay unchecked even on the insertion path', + ); + }); + + test('#4741 negative space: a superseded plan without a summary stays unchecked', (t) => { + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + write4741World(tmpDir, { includeSupersededSummary: false }); + + const output = runUpdate(tmpDir); + assert.strictEqual(output.plan_count, 1); + assert.strictEqual(output.summary_count, 1); + + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + assert.ok(roadmap.includes('- [ ] 01-02-PLAN.md'), 'no summary → no tick (existing behavior preserved)'); + }); + + test('#4741 negative space: a halted summary on an ACTIVE plan still ticks (#2830)', (t) => { + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + // #2830: `status: halted` on a plan that still COUNTS is executed-by-design. + // The active plan's SUMMARY carries `halted` here; the superseded plan's + // row must still stay unchecked in the same document. + write4741World(tmpDir, { activeSummaryStatus: 'halted' }); + + const output = runUpdate(tmpDir); + assert.strictEqual(output.plan_count, 1, 'only the superseded plan is excluded'); + assert.strictEqual(output.summary_count, 1); + + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + assert.ok(roadmap.includes('- [x] 01-01-PLAN.md'), 'a halted summary on an ACTIVE plan ticks (#2830)'); + assert.ok( + roadmap.includes('- [ ] 01-02-PLAN.md'), + 'and the superseded plan stays unchecked in the same document', + ); + assert.ok(roadmap.includes('1/1 plans executed'), 'numbers and checkboxes agree'); + }); +});