diff --git a/.changeset/1988-roadmap-stray-summary-count.md b/.changeset/1988-roadmap-stray-summary-count.md new file mode 100644 index 000000000..4662bee37 --- /dev/null +++ b/.changeset/1988-roadmap-stray-summary-count.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2016 +--- +**`roadmap update-plan-progress` no longer counts stray non-plan `*-SUMMARY.md` files against phase completion** — remediation/gap-closure summaries (e.g. `30-FIX-CR02-SUMMARY.md`, `30-GAPCLOSURE-SUMMARY.md`) inflated `summary_count`, and once `summary_count >= plan_count` the phase silently flipped to `Complete` (checkbox checked, date stamped) even though several plans had no summary. A new `countMatchedSummaries` helper (core-utils) pairs summaries to plans via the `PLAN→SUMMARY` marker swap + the `-SUMMARY.md` form (layout-agnostic across root, bare, and nested layouts), so only a summary that corresponds to a real plan counts. Wired into `scanPhasePlans` (fixing roadmap listing, state sync, verification, workstream inventory at once) and `cmdRoadmapUpdatePlanProgress`. (#1988) diff --git a/src/core-utils.cts b/src/core-utils.cts index ec0d6a60a..9c02a6d1c 100644 --- a/src/core-utils.cts +++ b/src/core-utils.cts @@ -186,6 +186,45 @@ function extractCanonicalPlanId(filename: string): string { return base; } +/** + * Count summaries that correspond to a real plan (#1988). + * + * A summary counts toward phase completion iff it pairs with an existing plan + * file. This excludes stray non-plan summaries — e.g. `30-FIX-CR02-SUMMARY.md`, + * `30-GAPCLOSURE-SUMMARY.md` — that inflate the raw `*-SUMMARY.md` count and + * silently flip a phase to Complete when plans are actually missing summaries. + * + * Pairing is layout-agnostic. For each plan, up to three candidate summary + * filenames are generated and any match suffices: + * 1. marker swap `PLAN`→`SUMMARY` on the basename — root padded + * (`30-01-PLAN.md`↔`30-01-SUMMARY.md`), nested (`PLAN-01.md`↔ + * `SUMMARY-01.md`, incl. a `plans/` prefix), and bare (`PLAN.md`↔ + * `SUMMARY.md`); + * 2. `-SUMMARY.md` — bare (`PLAN.md`↔`PLAN-SUMMARY.md`) and legacy + * (`14-PLAN-01.md`↔`14-PLAN-01-SUMMARY.md`); + * 3. extended `-PLAN-…`→`--SUMMARY.md` + * (`3-PLAN-01-setup.md`↔`3-01-SUMMARY.md`). + * The swap is applied to the basename only so a lowercase `plans/` dir prefix + * isn't corrupted to `SUMMARYs/…`. + */ +function countMatchedSummaries(planFiles: string[], summaryFiles: string[]): number { + const summarySet = new Set(summaryFiles); + let matched = 0; + for (const plan of planFiles) { + const slashIdx = plan.lastIndexOf('/'); + const dir = slashIdx >= 0 ? plan.slice(0, slashIdx + 1) : ''; + const base = (dir ? plan.slice(dir.length) : plan).replace(/\.md$/i, ''); + const candidates: string[] = [ + dir + base.replace(/PLAN/i, 'SUMMARY') + '.md', + dir + base + '-SUMMARY.md', + ]; + const extended = base.match(/^(\d+)-PLAN-(\d+)/i); + if (extended) candidates.push(dir + extended[1] + '-' + extended[2] + '-SUMMARY.md'); + if (candidates.some((c) => summarySet.has(c))) matched++; + } + return matched; +} + export = { toPosixPath, detectSubRepos, @@ -198,4 +237,5 @@ export = { readSubdirectories, timeAgo, extractCanonicalPlanId, + countMatchedSummaries, }; diff --git a/src/plan-scan.cts b/src/plan-scan.cts index a9a671813..36779b27e 100644 --- a/src/plan-scan.cts +++ b/src/plan-scan.cts @@ -9,6 +9,9 @@ import { existsSync, readdirSync } from 'node:fs'; import { join } from 'node:path'; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import coreUtils = require('./core-utils.cjs'); +const { countMatchedSummaries } = coreUtils; // Excluded derivative files const PLAN_OUTLINE_RE = /-OUTLINE\.md$/i; @@ -82,7 +85,12 @@ function scanPhasePlans(phaseDir: string): PhaseScanResult { const planFiles = rootPlanFiles.concat(nestedPlanFiles); const summaryFiles = rootSummaryFiles.concat(nestedSummaryFiles); const planCount = planFiles.length; - const summaryCount = summaryFiles.length; + // Count only summaries that are the PLAN→SUMMARY partner of an existing plan + // (#1988): stray non-plan summaries (e.g. 30-FIX-CR02-SUMMARY.md, + // 30-GAPCLOSURE-SUMMARY.md) must not inflate summary_count or flip a phase to + // Complete when plans are still missing summaries. summaryFiles (the array) + // still holds every summary on disk for callers that read/list them. + const summaryCount = countMatchedSummaries(planFiles, summaryFiles); return { planCount, diff --git a/src/roadmap.cts b/src/roadmap.cts index 0de363a7a..654493111 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -29,6 +29,9 @@ const { planningPaths, withPlanningLock, findContextMdIn } = planningWorkspace; // eslint-disable-next-line @typescript-eslint/no-require-imports import scanPhasePlans = require('./plan-scan.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports +import coreUtils = require('./core-utils.cjs'); +const { countMatchedSummaries } = coreUtils; +// eslint-disable-next-line @typescript-eslint/no-require-imports import frontmatter = require('./frontmatter.cjs'); const { extractFrontmatter, parseMustHavesBlock } = frontmatter; @@ -491,7 +494,10 @@ function cmdRoadmapUpdatePlanProgress(cwd: string, phaseNum: string | null | und } const planCount = phaseInfo!.plans.length; - const summaryCount = phaseInfo!.summaries.length; + // Count only summaries that pair with a real plan (#1988): stray non-plan + // summaries (30-FIX-CR02-SUMMARY.md, 30-GAPCLOSURE-SUMMARY.md, …) must not + // inflate summary_count and silently flip the phase to Complete. + const summaryCount = countMatchedSummaries(phaseInfo!.plans, phaseInfo!.summaries); if (planCount === 0) { output({ updated: false, reason: 'No plans found', plan_count: 0, summary_count: 0 }, raw, 'no plans'); diff --git a/tests/core-utils.test.cjs b/tests/core-utils.test.cjs index 86ca1cb02..09e78c9e9 100644 --- a/tests/core-utils.test.cjs +++ b/tests/core-utils.test.cjs @@ -529,8 +529,72 @@ describe('extractCanonicalPlanId', () => { // extractCanonicalPlanId operates on a filename string (not a real path). // The function does not sanitize slashes — it strips .md suffixes and // attempts to find phase tokens. The result is a string (no crash). - const result = coreUtils.extractCanonicalPlanId('../../../etc/passwd'); + const result = coreUtils.extractCanonicalPlanId('../../../etc/passwd'); assert.ok(typeof result === 'string'); assert.ok(result.length > 0); }); }); + +// ─── countMatchedSummaries (#1988) ─────────────────────────────────────────── +// Lives in core-utils.test.cjs (not roadmap.test.cjs) so the Stryker core-utils +// shard — which runs only this file — actually covers the helper's mutants. + +describe('countMatchedSummaries — stray non-plan summaries excluded (#1988)', () => { + const { countMatchedSummaries } = coreUtils; + + test('counts only summaries that are the PLAN→SUMMARY partner of a plan', () => { + const plans = ['30-01-PLAN.md', '30-02-PLAN.md', '30-10-PLAN.md']; + const summaries = ['30-01-SUMMARY.md', '30-FIX-CR02-SUMMARY.md', '30-GAPCLOSURE-SUMMARY.md']; + assert.strictEqual(countMatchedSummaries(plans, summaries), 1); + }); + + test('all plans have a partner summary → counts every plan', () => { + const plans = ['01-01-PLAN.md', '01-02-PLAN.md']; + const summaries = ['01-01-SUMMARY.md', '01-02-SUMMARY.md']; + assert.strictEqual(countMatchedSummaries(plans, summaries), 2); + }); + + test('nested layout pairing preserved (plans/PLAN-NN ↔ plans/SUMMARY-NN)', () => { + const plans = ['plans/PLAN-01.md', 'plans/PLAN-02.md']; + const summaries = ['plans/SUMMARY-01.md']; + assert.strictEqual(countMatchedSummaries(plans, summaries), 1); + }); + + test('extended layout pairing (N-PLAN-MM-slug ↔ N-MM-SUMMARY)', () => { + const plans = ['3-PLAN-01-setup.md', '5-PLAN-02-migrations.md']; + const summaries = ['3-01-SUMMARY.md', '5-02-SUMMARY.md']; + assert.strictEqual(countMatchedSummaries(plans, summaries), 2); + }); + + test('extended layout: only the matching plan counts (no cross-pairing)', () => { + const plans = ['3-PLAN-01-setup.md', '3-PLAN-02-seed.md']; + const summaries = ['3-01-SUMMARY.md']; // only plan 01 has a summary + assert.strictEqual(countMatchedSummaries(plans, summaries), 1); + }); + + test('bare PLAN.md ↔ SUMMARY.md', () => { + assert.strictEqual(countMatchedSummaries(['PLAN.md'], ['SUMMARY.md']), 1); + assert.strictEqual(countMatchedSummaries(['PLAN.md'], []), 0); + }); + + test('bare PLAN.md ↔ PLAN-SUMMARY.md (stem-suffix convention)', () => { + assert.strictEqual(countMatchedSummaries(['PLAN.md'], ['PLAN-SUMMARY.md']), 1); + }); + + test('legacy -PLAN- ↔ -PLAN--SUMMARY', () => { + const plans = ['14-PLAN-01.md', '14-PLAN-02.md']; + const summaries = ['14-PLAN-01-SUMMARY.md', '14-PLAN-02-SUMMARY.md']; + assert.strictEqual(countMatchedSummaries(plans, summaries), 2); + }); + + test('stray summaries never count when no plan partners exist', () => { + const plans = ['30-01-PLAN.md']; + const strays = ['30-FIX-CR02-SUMMARY.md', '30-GAPCLOSURE-SUMMARY.md', '30-01-EXTRA-SUMMARY.md']; + assert.strictEqual(countMatchedSummaries(plans, strays), 0); + }); + + test('absolute path (leading slash) still pairs via the dir split', () => { + // Guards the lastIndexOf('/') >= 0 boundary (slash at index 0). + assert.strictEqual(countMatchedSummaries(['/abs/PLAN-01.md'], ['/abs/SUMMARY-01.md']), 1); + }); +}); diff --git a/tests/roadmap.test.cjs b/tests/roadmap.test.cjs index bb5a77d47..83ef41f8d 100644 --- a/tests/roadmap.test.cjs +++ b/tests/roadmap.test.cjs @@ -765,6 +765,55 @@ describe('roadmap update-plan-progress command', () => { assert.strictEqual(output.phases[0].disk_status, 'partial'); }); + test('#1988 — stray non-plan *-SUMMARY.md files do not inflate summary_count or flip phase to Complete', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +- [ ] **Phase 30: Big** - description + +### Phase 30: Big +**Goal:** big goal +**Plans:** TBD + +## Progress + +| Phase | Milestone | Plans Complete | Status | Completed | +|-------|-----------|----------------|--------|-----------| +| 30. Big | v3.0 | 0/4 | Planned | - | +` + ); + + // 4 plans, only 1 has a real summary; plus 3 stray non-plan summaries + // (the exact names from the #1988 report: FIX-CR02, FIX-WR02-04, GAPCLOSURE). + const p = path.join(tmpDir, '.planning', 'phases', '30-big'); + fs.mkdirSync(p, { recursive: true }); + fs.writeFileSync(path.join(p, '30-01-PLAN.md'), '# Plan 1'); + fs.writeFileSync(path.join(p, '30-02-PLAN.md'), '# Plan 2'); + fs.writeFileSync(path.join(p, '30-03-PLAN.md'), '# Plan 3'); + fs.writeFileSync(path.join(p, '30-04-PLAN.md'), '# Plan 4'); + fs.writeFileSync(path.join(p, '30-01-SUMMARY.md'), '# Summary 1'); + // Strays — these must NOT count: + fs.writeFileSync(path.join(p, '30-FIX-CR02-SUMMARY.md'), '# fix summary'); + fs.writeFileSync(path.join(p, '30-FIX-WR02-04-SUMMARY.md'), '# fix summary'); + fs.writeFileSync(path.join(p, '30-GAPCLOSURE-SUMMARY.md'), '# gap closure summary'); + + const result = runGsdTools('roadmap update-plan-progress 30', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.plan_count, 4, 'plan_count is the 4 real plans'); + assert.strictEqual(output.summary_count, 1, 'stray summaries must not inflate the count'); + assert.strictEqual(output.status, 'In Progress', 'must not flip to Complete'); + assert.strictEqual(output.complete, false, 'must not be complete'); + + // The ROADMAP row must NOT show 4/4 Complete. + const roadmapContent = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + assert.ok(roadmapContent.includes('1/4'), 'roadmap row must show the real 1/4 progress'); + assert.ok(!/4\/4\s*\|\s*Complete/.test(roadmapContent), 'must not stamp 4/4 Complete'); + assert.ok(!/\[x\] \*\*Phase 30/.test(roadmapContent), 'phase checkbox must not be checked'); + }); + test('updates progress and checks checkbox on completion', () => { fs.writeFileSync( path.join(tmpDir, '.planning', 'ROADMAP.md'),