From 1a49d2fcfc0eaf3e09d554f6d0499e261d059fe6 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 9 May 2026 11:39:31 -0400 Subject: [PATCH] feat(phase-plans): extract shared scanPhasePlans helper (k014) (#3308) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(phase-plans): red — shared scanPhasePlans contract + parity across call sites (#3262) Co-Authored-By: Claude Sonnet 4.6 * feat(phase-plans): extract shared scanPhasePlans helper (k014) (#3262) Eliminates four divergent copies of the plan-scan algorithm: - roadmap.cjs:countPhasePlansAndSummaries (root call site) - state.cjs:buildStateFrontmatter (1 of 3) - state.cjs:cmdStateValidate (2 of 3) - state.cjs:cmdStateSync (3 of 3) - init.cjs:listPhasePlanFiles / listPhaseSummaryFiles New bin/lib/plan-scan.cjs exports scanPhasePlans(phaseDir) → { planCount, summaryCount, completed, hasNestedPlans, planFiles, summaryFiles } Divergences resolved: - roadmap.cjs used a broad isPlanFile (any .md containing PLAN in name, matching the extended layout 5-PLAN-01-setup.md); canonical helper adopts this wider pattern as the reference implementation. - state.cjs used a strict endsWith(-PLAN.md) filter, missing extended- layout root files; now unified with roadmap.cjs semantics. - init.cjs listPhasePlanFiles used ^PLAN-\d+ for nested, missing the -PLAN-\d+ variant state.cjs also matched; helper includes both. - pre-bounce exclusion broadened to /.pre-bounce.md$/i (any pre-bounce file), not just -PLAN.*\.pre-bounce\.md (roadmap form) or flat .pre-bounce.md (state form). - OUTLINE exclusion broadened to /-OUTLINE\.md$/i to catch both flat (-PLAN-OUTLINE.md) and nested (PLAN-01-OUTLINE.md) forms. Sibling audit: no 5th call site found. phase.cjs:looksLikePlanFile is a diagnostic probe for non-canonical naming (not a counter) — left in place per its distinct purpose. Co-Authored-By: Claude Sonnet 4.6 * chore(changelog): add entry for #3262 scanPhasePlans extraction Co-Authored-By: Claude Sonnet 4.6 * chore(changeset): add changeset fragment for #3262 Co-Authored-By: Claude Sonnet 4.6 * docs(inventory): add plan-scan.cjs row to INVENTORY.md CLI Modules table (#3262) Co-Authored-By: Claude Sonnet 4.6 * fix(3262): update bug-3128 test + INVENTORY counts for plan-scan.cjs - Update tests/bug-3128-roadmap-plan-count-slug-layout.test.cjs to verify that roadmap.cjs delegates to plan-scan.cjs (require check) and that the extended filter lives in plan-scan.cjs as isRootPlanFile with /PLAN/i - Bump docs/INVENTORY.md CLI Modules headline from 46 to 47 (plan-scan.cjs) - Regenerate docs/INVENTORY-MANIFEST.json to include cli_modules/plan-scan.cjs Co-Authored-By: Claude Sonnet 4.6 * fix(3262): migrate two missed call sites to scanPhasePlans (k014) - init.cjs cmdInitExecutePhase: replace inline /-PLAN\.md$/i filter with listPhasePlanFiles(path) to honour nested, extended-layout, OUTLINE and pre-bounce exclusions (CR finding) - state.cjs cmdStateUpdateProgress: replace dual /-PLAN\.md$/i and /-SUMMARY\.md$/i filters with scanPhasePlans() so the progress-bar body field uses the same counts as buildStateFrontmatter frontmatter Co-Authored-By: Claude Sonnet 4.6 * fix(3262): correct INVENTORY-MANIFEST.json to tracked files only Remove 3 untracked local entries from cli_modules so the manifest matches what CI sees (47 tracked .cjs files, not 50 local). Previous regeneration ran against the local filesystem which included cjs-command-router-adapter.cjs, state-document.cjs, and workstream-inventory.cjs (all untracked on this branch). Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/3262-extract-scan-phase-plans.md | 5 + CHANGELOG.md | 4 + docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 3 +- get-shit-done/bin/lib/init.cjs | 24 +- get-shit-done/bin/lib/plan-scan.cjs | 138 ++++++++ get-shit-done/bin/lib/roadmap.cjs | 32 +- get-shit-done/bin/lib/state.cjs | 103 +----- ...28-roadmap-plan-count-slug-layout.test.cjs | 21 +- tests/feat-3262-scan-phase-plans.test.cjs | 307 ++++++++++++++++++ 10 files changed, 495 insertions(+), 143 deletions(-) create mode 100644 .changeset/3262-extract-scan-phase-plans.md create mode 100644 get-shit-done/bin/lib/plan-scan.cjs create mode 100644 tests/feat-3262-scan-phase-plans.test.cjs diff --git a/.changeset/3262-extract-scan-phase-plans.md b/.changeset/3262-extract-scan-phase-plans.md new file mode 100644 index 000000000..40cb094e6 --- /dev/null +++ b/.changeset/3262-extract-scan-phase-plans.md @@ -0,0 +1,5 @@ +--- +type: Enhancement +pr: 3262 +--- +**Shared `scanPhasePlans()` helper extracted from four divergent copies (k014)** — `state.cjs` (3 copies), `roadmap.cjs`, and `init.cjs` each maintained their own plan-scan loop with subtly different regex shapes; divergence caused the plan-count drift that triggered #3257. All four call sites now delegate to `bin/lib/plan-scan.cjs:scanPhasePlans(phaseDir)` which returns `{ planCount, summaryCount, completed, hasNestedPlans, planFiles, summaryFiles }`. The canonical helper adopts roadmap.cjs's broader `isPlanFile` (matching the extended `5-PLAN-01-setup.md` layout gsd-plan-phase writes), adds the `-PLAN-\d+` nested-file variant init.cjs missed, and widens OUTLINE/pre-bounce exclusions to cover both flat and nested forms. (#3262) diff --git a/CHANGELOG.md b/CHANGELOG.md index 025013e41..7a836d715 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased](https://github.com/gsd-build/get-shit-done/compare/v1.41.0...HEAD) +### Enhancement + +- **Shared `scanPhasePlans()` helper extracted to `bin/lib/plan-scan.cjs` (k014)** — four divergent copies of the plan-scan algorithm in `state.cjs`, `roadmap.cjs`, `init.cjs`, and `phase.cjs` are now replaced by a single canonical implementation. Each copy had subtle differences (regex shapes, flat vs nested coverage, pre-bounce exclusion patterns) that caused plan-count drift between the four callers. The helper covers both the flat layout (`*-PLAN.md`, `PLAN.md`) and the nested layout (`plans/PLAN-NN-slug.md`) introduced in #3139, and returns `{ planCount, summaryCount, completed, hasNestedPlans, planFiles, summaryFiles }`. (#3262) + ### Fixed - **`/gsd-discuss-phase` and `/gsd-plan-phase` first-touch creation now apply `project_code` prefix consistently with `phase.add`/`phase.insert`** — projects with `project_code` set in `.planning/config.json` no longer accumulate a two-headed naming convention (`01-foundation/` mixed with `XR-02.1-spike/`). `init.phase-op` and `init.plan-phase` now expose `expected_phase_dir` (with prefix) in their JSON bundle; workflow fallback mkdir calls use this value instead of constructing the path from `padded_phase`+`phase_slug`. `phase.scaffold phase-dir` (CJS and SDK) also fixed. (#3287) diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 4a129e277..ecdec1cb7 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -283,6 +283,7 @@ "phase-command-router.cjs", "phase.cjs", "phases-command-router.cjs", + "plan-scan.cjs", "planning-workspace.cjs", "profile-output.cjs", "profile-pipeline.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 2ea3c8662..47298155c 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -358,7 +358,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (46 shipped) +## CLI Modules (47 shipped) Full listing: `get-shit-done/bin/lib/*.cjs`. @@ -391,6 +391,7 @@ Full listing: `get-shit-done/bin/lib/*.cjs`. | `phase-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools phase` | | `phase.cjs` | Phase directory operations, decimal numbering, plan indexing | | `phases-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools phases` | +| `plan-scan.cjs` | Canonical phase-plan scanner — shared helper for detecting plan and summary files in flat and nested layouts (k014); consumed by `state.cjs`, `roadmap.cjs`, and `init.cjs` | | `planning-workspace.cjs` | Planning path/workstream seam (`planningDir`, `planningPaths`, active-workstream routing, `.planning/.lock` orchestration) | | `profile-output.cjs` | Profile rendering, USER-PROFILE.md and dev-preferences.md generation | | `profile-pipeline.cjs` | User behavioral profiling data pipeline, session file scanning | diff --git a/get-shit-done/bin/lib/init.cjs b/get-shit-done/bin/lib/init.cjs index 9c9d92b4b..068a303f8 100644 --- a/get-shit-done/bin/lib/init.cjs +++ b/get-shit-done/bin/lib/init.cjs @@ -8,6 +8,7 @@ const { execSync } = require('child_process'); const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, normalizePhaseName, toPosixPath, output, error, checkAgentsInstalled, phaseTokenMatches } = require('./core.cjs'); const { planningPaths, planningDir, planningRoot } = require('./planning-workspace.cjs'); const { maskIfSecret } = require('./secrets.cjs'); +const scanPhasePlans = require('./plan-scan.cjs'); // Accept all bold/colon variants of the Requirements header (#2769): // **Requirements:** / **Requirements**: / **Requirements** : render the @@ -15,27 +16,11 @@ const { maskIfSecret } = require('./secrets.cjs'); const REQUIREMENTS_HEADER_RE = /^\*\*Requirements:?\*\*[^\S\n]*:?[^\S\n]*([^\n]*)$/m; function listPhaseSummaryFiles(phaseDir) { - const phaseFiles = fs.readdirSync(phaseDir); - const rootSummaries = phaseFiles.filter(f => f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md'); - const plansDir = path.join(phaseDir, 'plans'); - let nestedSummaries = []; - if (fs.existsSync(plansDir)) { - const files = fs.readdirSync(plansDir); - nestedSummaries = files.filter(f => /^SUMMARY-\d+.*\.md$/i.test(f)); - } - return rootSummaries.concat(nestedSummaries); + return scanPhasePlans(phaseDir).summaryFiles; } function listPhasePlanFiles(phaseDir) { - const phaseFiles = fs.readdirSync(phaseDir); - const rootPlans = phaseFiles.filter(f => f.endsWith('-PLAN.md') || f === 'PLAN.md'); - const plansDir = path.join(phaseDir, 'plans'); - let nestedPlans = []; - if (fs.existsSync(plansDir)) { - const files = fs.readdirSync(plansDir); - nestedPlans = files.filter(f => /^PLAN-\d+.*\.md$/i.test(f)); - } - return rootPlans.concat(nestedPlans); + return scanPhasePlans(phaseDir).planFiles; } function getLatestCompletedMilestone(cwd) { @@ -212,8 +197,7 @@ function cmdInitExecutePhase(cwd, phase, raw, options = {}) { const warnings = []; const phasesPath = planningPaths(cwd).phases; if (phaseInfo && phaseInfo.directory && fs.existsSync(path.join(cwd, phaseInfo.directory))) { - const files = fs.readdirSync(path.join(cwd, phaseInfo.directory)); - const diskPlans = files.filter(f => f.match(/-PLAN\.md$/i)).length; + const diskPlans = listPhasePlanFiles(path.join(cwd, phaseInfo.directory)).length; const totalPlansRaw = stateExtractField(stateContent, 'Total Plans in Phase'); const totalPlansInPhase = totalPlansRaw ? parseInt(totalPlansRaw, 10) : null; if (totalPlansInPhase !== null && diskPlans !== totalPlansInPhase) { diff --git a/get-shit-done/bin/lib/plan-scan.cjs b/get-shit-done/bin/lib/plan-scan.cjs new file mode 100644 index 000000000..6952f419e --- /dev/null +++ b/get-shit-done/bin/lib/plan-scan.cjs @@ -0,0 +1,138 @@ +'use strict'; +/** + * plan-scan — canonical phase-plan scanner (k014) + * + * Single source of truth for detecting plan and summary files in a phase + * directory, replacing four divergent copies in state.cjs, roadmap.cjs, + * init.cjs, and phase.cjs (#3262). + * + * Layout support: + * Flat (pre-#3139): phases//*-PLAN.md, *-SUMMARY.md + * Nested (post-#3139): phases//plans/PLAN--*.md, SUMMARY--*.md + * + * @module plan-scan + */ + +const fs = require('fs'); +const path = require('path'); + +// Excluded derivative files — present alongside real plans but must not be +// counted. OUTLINE exclusion catches both flat (-PLAN-OUTLINE.md) and nested +// (PLAN-NN-OUTLINE.md) forms via a broad -OUTLINE.md$ pattern. The +// pre-bounce pattern is intentionally broad (matches any *.pre-bounce.md) so +// stale bounce files never inflate plan counts (#3257 regression root cause). +const PLAN_OUTLINE_RE = /-OUTLINE\.md$/i; +const PLAN_PRE_BOUNCE_RE = /\.pre-bounce\.md$/i; + +/** + * Determine whether a filename from the flat phase root is a plan file. + * + * Accepts: + * - Bare PLAN.md + * - Canonical padded 01-01-PLAN.md + * - Extended layout 5-PLAN-01-setup.md (the format gsd-plan-phase writes; + * looksLikePlanFile in phase.cjs / isPlanFile in roadmap.cjs) + * + * Rejects: -PLAN-OUTLINE.md, *.pre-bounce.md + */ +function isRootPlanFile(f) { + if (PLAN_OUTLINE_RE.test(f)) return false; + if (PLAN_PRE_BOUNCE_RE.test(f)) return false; + // Canonical suffix or bare name + if (f.endsWith('-PLAN.md') || f === 'PLAN.md') return true; + // Extended layout: any .md that contains PLAN (case-insensitive) in the name + return /\.md$/i.test(f) && /PLAN/i.test(f); +} + +/** + * Determine whether a filename from the nested plans/ subdir is a plan file. + * + * Nested layout names: PLAN-NN-slug.md or N-PLAN-NN-slug.md. + * Excludes OUTLINE and pre-bounce suffixes. + */ +function isNestedPlanFile(f) { + if (PLAN_OUTLINE_RE.test(f)) return false; + if (PLAN_PRE_BOUNCE_RE.test(f)) return false; + return /^PLAN-\d+.*\.md$/i.test(f) || /-PLAN-\d+.*\.md$/i.test(f); +} + +/** + * Determine whether a filename from the flat phase root is a summary file. + */ +function isRootSummaryFile(f) { + return f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md'; +} + +/** + * Determine whether a filename from the nested plans/ subdir is a summary. + */ +function isNestedSummaryFile(f) { + return /^SUMMARY-\d+.*\.md$/i.test(f) || /-SUMMARY-\d+.*\.md$/i.test(f); +} + +/** + * Scan a single phase directory for plan and summary files. + * + * @param {string} phaseDir — absolute path to the phase directory + * @returns {{ + * planCount: number, + * summaryCount: number, + * completed: boolean, + * hasNestedPlans: boolean, + * planFiles: string[], + * summaryFiles: string[], + * }} + */ +function scanPhasePlans(phaseDir) { + let rootFiles; + try { + rootFiles = fs.readdirSync(phaseDir); + } catch { + return { + planCount: 0, + summaryCount: 0, + completed: false, + hasNestedPlans: false, + planFiles: [], + summaryFiles: [], + }; + } + + const rootPlanFiles = rootFiles.filter(isRootPlanFile); + const rootSummaryFiles = rootFiles.filter(isRootSummaryFile); + + let nestedPlanFiles = []; + let nestedSummaryFiles = []; + let hasNestedPlans = false; + + const nestedDir = path.join(phaseDir, 'plans'); + if (fs.existsSync(nestedDir)) { + try { + const nested = fs.readdirSync(nestedDir); + nestedPlanFiles = nested.filter(isNestedPlanFile); + nestedSummaryFiles = nested.filter(isNestedSummaryFile); + hasNestedPlans = nestedPlanFiles.length > 0; + } catch { /* ignore if plans/ is not a readable directory */ } + } + + const planFiles = rootPlanFiles.concat(nestedPlanFiles); + const summaryFiles = rootSummaryFiles.concat(nestedSummaryFiles); + const planCount = planFiles.length; + const summaryCount = summaryFiles.length; + + return { + planCount, + summaryCount, + completed: planCount > 0 && summaryCount >= planCount, + hasNestedPlans, + planFiles, + summaryFiles, + }; +} + +module.exports = scanPhasePlans; +module.exports.scanPhasePlans = scanPhasePlans; +module.exports.isRootPlanFile = isRootPlanFile; +module.exports.isNestedPlanFile = isNestedPlanFile; +module.exports.isRootSummaryFile = isRootSummaryFile; +module.exports.isNestedSummaryFile = isNestedSummaryFile; diff --git a/get-shit-done/bin/lib/roadmap.cjs b/get-shit-done/bin/lib/roadmap.cjs index 53f270618..8e39e2f7e 100644 --- a/get-shit-done/bin/lib/roadmap.cjs +++ b/get-shit-done/bin/lib/roadmap.cjs @@ -6,6 +6,7 @@ const fs = require('fs'); const path = require('path'); const { escapeRegex, normalizePhaseName, output, error, findPhaseInternal, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, phaseTokenMatches, atomicWriteFileSync } = require('./core.cjs'); const { planningPaths, withPlanningLock } = require('./planning-workspace.cjs'); +const scanPhasePlans = require('./plan-scan.cjs'); /** * Coerce an arbitrary YAML scalar/object into a string for cross-cutting @@ -37,31 +38,14 @@ function coerceTruthToString(t) { } function countPhasePlansAndSummaries(phaseDir) { - const phaseFiles = fs.readdirSync(phaseDir); - // Canonical form: *-PLAN.md or PLAN.md. - // Extended form: {N}-PLAN-{NN}-{slug}.md — the layout gsd-plan-phase - // actually writes (e.g. 5-PLAN-01-setup.md). Mirrors the looksLikePlanFile - // logic in phase.cjs (#2893 / #3128). - const PLAN_OUTLINE_RE = /-PLAN-OUTLINE\.md$/i; - const PLAN_PRE_BOUNCE_RE = /-PLAN.*\.pre-bounce\.md$/i; - const isPlanFile = (f) => - (f.endsWith('-PLAN.md') || f === 'PLAN.md') || - (/\.md$/i.test(f) && /PLAN/i.test(f) && !PLAN_OUTLINE_RE.test(f) && !PLAN_PRE_BOUNCE_RE.test(f)); - const rootPlans = phaseFiles.filter(isPlanFile); - const rootSummaries = phaseFiles.filter(f => f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md'); - - let nestedPlans = []; - let nestedSummaries = []; - const plansDir = path.join(phaseDir, 'plans'); - if (fs.existsSync(plansDir)) { - const planFiles = fs.readdirSync(plansDir); - nestedPlans = planFiles.filter(f => /^PLAN-\d+.*\.md$/i.test(f)); - nestedSummaries = planFiles.filter(f => /^SUMMARY-\d+.*\.md$/i.test(f)); - } - + const { planCount, summaryCount } = scanPhasePlans(phaseDir); + // hasContext and hasResearch are not plan-scan concerns — read the directory + // once for the non-plan metadata that cmdRoadmapAnalyze needs. + let phaseFiles = []; + try { phaseFiles = fs.readdirSync(phaseDir); } catch { /* empty */ } return { - planCount: rootPlans.length + nestedPlans.length, - summaryCount: rootSummaries.length + nestedSummaries.length, + planCount, + summaryCount, hasContext: phaseFiles.some(f => f.endsWith('-CONTEXT.md') || f === 'CONTEXT.md'), hasResearch: phaseFiles.some(f => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md'), }; diff --git a/get-shit-done/bin/lib/state.cjs b/get-shit-done/bin/lib/state.cjs index cf0675303..c38952948 100644 --- a/get-shit-done/bin/lib/state.cjs +++ b/get-shit-done/bin/lib/state.cjs @@ -7,6 +7,7 @@ const path = require('path'); const { escapeRegex, loadConfig, getMilestoneInfo, getMilestonePhaseFilter, normalizeMd, output, error, atomicWriteFileSync } = require('./core.cjs'); const { planningDir, planningPaths } = require('./planning-workspace.cjs'); const { extractFrontmatter, reconstructFrontmatter } = require('./frontmatter.cjs'); +const scanPhasePlans = require('./plan-scan.cjs'); // Cache disk scan results from buildStateFrontmatter per cwd per process (#1967). // Avoids re-reading N+1 directories on every state write when the phase structure @@ -458,9 +459,9 @@ function cmdStateUpdateProgress(cwd, raw) { .filter(e => e.isDirectory()).map(e => e.name) .filter(isDirInMilestone); for (const dir of phaseDirs) { - const files = fs.readdirSync(path.join(phasesDir, dir)); - totalPlans += files.filter(f => f.match(/-PLAN\.md$/i)).length; - totalSummaries += files.filter(f => f.match(/-SUMMARY\.md$/i)).length; + const { planCount, summaryCount } = scanPhasePlans(path.join(phasesDir, dir)); + totalPlans += planCount; + totalSummaries += summaryCount; } } @@ -872,39 +873,12 @@ function buildStateFrontmatter(bodyContent, cwd) { let diskTotalSummaries = 0; let diskCompletedPhases = 0; - // Regex constants mirror roadmap.cjs:countPhasePlansAndSummaries (#3257). - const PLAN_OUTLINE_RE = /-PLAN-OUTLINE\.md$/i; - const PLAN_PRE_BOUNCE_RE = /\.pre-bounce\.md$/i; for (const dir of phaseDirs) { const phaseDir = path.join(phasesDir, dir); - const topFiles = fs.readdirSync(phaseDir); - // Canonical flat-layout plan files in phase root. - let plans = topFiles.filter(f => - (f.endsWith('-PLAN.md') || f === 'PLAN.md') && - !PLAN_OUTLINE_RE.test(f) && !PLAN_PRE_BOUNCE_RE.test(f) - ).length; - let summaries = topFiles.filter(f => - f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md' - ).length; - // Nested layout (post-#3139): phases//plans/-PLAN--.md - // Mirrors roadmap.cjs:countPhasePlansAndSummaries — do NOT extract here - // (shared helper is tracked as follow-on for k014). - const nestedPlansDir = path.join(phaseDir, 'plans'); - if (fs.existsSync(nestedPlansDir)) { - try { - const nested = fs.readdirSync(nestedPlansDir); - plans += nested.filter(f => - (/^PLAN-\d+.*\.md$/i.test(f) || /-PLAN-\d+.*\.md$/i.test(f)) && - !PLAN_OUTLINE_RE.test(f) && !PLAN_PRE_BOUNCE_RE.test(f) - ).length; - summaries += nested.filter(f => - /^SUMMARY-\d+.*\.md$/i.test(f) || /-SUMMARY-\d+.*\.md$/i.test(f) - ).length; - } catch { /* ignore if plans/ is not a readable directory */ } - } - diskTotalPlans += plans; - diskTotalSummaries += summaries; - if (plans > 0 && summaries >= plans) diskCompletedPhases++; + const { planCount, summaryCount, completed } = scanPhasePlans(phaseDir); + diskTotalPlans += planCount; + diskTotalSummaries += summaryCount; + if (completed) diskCompletedPhases++; } cached = { totalPhases: isDirInMilestone.phaseCount > 0 @@ -1515,33 +1489,7 @@ function cmdStateValidate(cwd, raw) { 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 files = fs.readdirSync(phaseDirPath); - // Regex constants mirror roadmap.cjs:countPhasePlansAndSummaries (#3257). - const PLAN_OUTLINE_RE_V = /-PLAN-OUTLINE\.md$/i; - const PLAN_PRE_BOUNCE_RE_V = /\.pre-bounce\.md$/i; - let diskPlans = files.filter(f => - (f.endsWith('-PLAN.md') || f === 'PLAN.md') && - !PLAN_OUTLINE_RE_V.test(f) && !PLAN_PRE_BOUNCE_RE_V.test(f) - ).length; - let diskSummaries = files.filter(f => - f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md' - ).length; - // Nested layout (post-#3139): phases//plans/-PLAN--.md - // Mirrors roadmap.cjs:countPhasePlansAndSummaries — do NOT extract here - // (shared helper is tracked as follow-on for k014). - const nestedPlansDirV = path.join(phaseDirPath, 'plans'); - if (fs.existsSync(nestedPlansDirV)) { - try { - const nested = fs.readdirSync(nestedPlansDirV); - diskPlans += nested.filter(f => - (/^PLAN-\d+.*\.md$/i.test(f) || /-PLAN-\d+.*\.md$/i.test(f)) && - !PLAN_OUTLINE_RE_V.test(f) && !PLAN_PRE_BOUNCE_RE_V.test(f) - ).length; - diskSummaries += nested.filter(f => - /^SUMMARY-\d+.*\.md$/i.test(f) || /-SUMMARY-\d+.*\.md$/i.test(f) - ).length; - } catch { /* ignore if plans/ is not a readable directory */ } - } + const { planCount: diskPlans, summaryCount: diskSummaries } = scanPhasePlans(phaseDirPath); // Check plan count mismatch if (totalPlansInPhase !== null && diskPlans !== totalPlansInPhase) { @@ -1550,6 +1498,7 @@ function cmdStateValidate(cwd, raw) { } // Check for VERIFICATION.md + const files = fs.readdirSync(phaseDirPath); const verificationFiles = files.filter(f => f.includes('VERIFICATION') && f.endsWith('.md')); for (const vf of verificationFiles) { try { @@ -1620,40 +1569,12 @@ function cmdStateSync(cwd, options, raw) { let highestIncompletePhaseplanCount = 0; let highestIncompletePhaseSummaryCount = 0; - // Regex constants mirror roadmap.cjs:countPhasePlansAndSummaries (#3257). - const PLAN_OUTLINE_RE_S = /-PLAN-OUTLINE\.md$/i; - const PLAN_PRE_BOUNCE_RE_S = /\.pre-bounce\.md$/i; - for (const dir of entries) { const dirPath = path.join(phasesDir, dir); - const files = fs.readdirSync(dirPath); - // Canonical flat-layout plan files in phase root. - let plans = files.filter(f => - (f.endsWith('-PLAN.md') || f === 'PLAN.md') && - !PLAN_OUTLINE_RE_S.test(f) && !PLAN_PRE_BOUNCE_RE_S.test(f) - ).length; - let summaries = files.filter(f => - f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md' - ).length; - // Nested layout (post-#3139): phases//plans/-PLAN--.md - // Mirrors roadmap.cjs:countPhasePlansAndSummaries — do NOT extract here - // (shared helper is tracked as follow-on for k014). - const nestedPlansDirS = path.join(dirPath, 'plans'); - if (fs.existsSync(nestedPlansDirS)) { - try { - const nested = fs.readdirSync(nestedPlansDirS); - plans += nested.filter(f => - (/^PLAN-\d+.*\.md$/i.test(f) || /-PLAN-\d+.*\.md$/i.test(f)) && - !PLAN_OUTLINE_RE_S.test(f) && !PLAN_PRE_BOUNCE_RE_S.test(f) - ).length; - summaries += nested.filter(f => - /^SUMMARY-\d+.*\.md$/i.test(f) || /-SUMMARY-\d+.*\.md$/i.test(f) - ).length; - } catch { /* ignore if plans/ is not a readable directory */ } - } + const { planCount: plans, summaryCount: summaries, completed } = scanPhasePlans(dirPath); totalDiskPlans += plans; totalDiskSummaries += summaries; - if (plans > 0 && summaries >= plans) diskCompletedPhases++; + if (completed) diskCompletedPhases++; // Track the highest phase with incomplete plans (or any plans) const phaseMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); diff --git a/tests/bug-3128-roadmap-plan-count-slug-layout.test.cjs b/tests/bug-3128-roadmap-plan-count-slug-layout.test.cjs index 69f43cd05..17037b39b 100644 --- a/tests/bug-3128-roadmap-plan-count-slug-layout.test.cjs +++ b/tests/bug-3128-roadmap-plan-count-slug-layout.test.cjs @@ -24,6 +24,7 @@ const path = require('node:path'); const ROOT = path.join(__dirname, '..'); // Require the module under test directly const roadmapLib = path.join(ROOT, 'get-shit-done', 'bin', 'lib', 'roadmap.cjs'); +const planScanLib = path.join(ROOT, 'get-shit-done', 'bin', 'lib', 'plan-scan.cjs'); // We test countPhasePlansAndSummaries indirectly via getManagerInfo since // it is not exported. We build a real phaseDir on disk and call the full @@ -76,16 +77,22 @@ describe('bug #3128: roadmap.cjs plan-count for {N}-PLAN-{NN}-{slug}.md layout', }); test('roadmap.cjs source uses the extended isPlanFile filter', () => { - const src = fs.readFileSync(roadmapLib, 'utf8'); - // Verify the fix is in place: the old simple filter is gone + const roadmapSrc = fs.readFileSync(roadmapLib, 'utf8'); + // Verify the fix is in place: the old simple inline filter is gone from roadmap.cjs assert.ok( - !src.includes("phaseFiles.filter(f => f.endsWith('-PLAN.md') || f === 'PLAN.md')"), - 'Old simple plan filter still present — fix not applied', + !roadmapSrc.includes("phaseFiles.filter(f => f.endsWith('-PLAN.md') || f === 'PLAN.md')"), + 'Old simple plan filter still present in roadmap.cjs — fix not applied', ); - // The fix introduces isPlanFile with PLAN regex + // roadmap.cjs now delegates to plan-scan.cjs via require('./plan-scan.cjs') assert.ok( - src.includes('isPlanFile') && src.includes('/PLAN/i'), - 'isPlanFile with /PLAN/i not found in roadmap.cjs — fix not applied', + roadmapSrc.includes('plan-scan.cjs'), + 'roadmap.cjs does not require plan-scan.cjs — delegation not applied', + ); + // plan-scan.cjs is where the extended plan-file detection logic lives (isRootPlanFile) + const planScanSrc = fs.readFileSync(planScanLib, 'utf8'); + assert.ok( + planScanSrc.includes('isRootPlanFile') && planScanSrc.includes('/PLAN/i'), + 'isRootPlanFile with /PLAN/i not found in plan-scan.cjs — canonical helper missing extended filter', ); }); }); diff --git a/tests/feat-3262-scan-phase-plans.test.cjs b/tests/feat-3262-scan-phase-plans.test.cjs new file mode 100644 index 000000000..fb600f6e7 --- /dev/null +++ b/tests/feat-3262-scan-phase-plans.test.cjs @@ -0,0 +1,307 @@ +/** + * Tests for the shared scanPhasePlans() helper (k014). + * + * Covers: + * - Top-level plans only (flat layout) + * - Top-level + nested layout (post-#3139) + * - Completed-summary detection (summaries >= plans) + * - Ignored files (OUTLINE, pre-bounce, CONTEXT, RESEARCH) + * - Empty phase dir → { planCount: 0, summaryCount: 0 } + * - Parity: helper produces correct counts for mixed flat+nested fixture tree + */ + +'use strict'; + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const os = require('os'); + +// Helper under test — must exist at this path (GREEN phase wires it up) +const scanPhasePlans = require('../get-shit-done/bin/lib/plan-scan.cjs'); + +// --------------------------------------------------------------------------- +// Fixture helpers +// --------------------------------------------------------------------------- + +let tmpDir; + +function phaseDir(name = 'phase') { + const d = path.join(tmpDir, name); + fs.mkdirSync(d, { recursive: true }); + return d; +} + +function touch(dir, ...filenames) { + for (const f of filenames) { + fs.writeFileSync(path.join(dir, f), ''); + } +} + +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-plan-scan-')); +}); + +afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +// --------------------------------------------------------------------------- +// Basic shapes +// --------------------------------------------------------------------------- + +describe('scanPhasePlans — flat layout', () => { + test('empty directory → zero counts', () => { + const dir = phaseDir(); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 0, 'planCount'); + assert.strictEqual(result.summaryCount, 0, 'summaryCount'); + }); + + test('bare PLAN.md counts as one plan', () => { + const dir = phaseDir(); + touch(dir, 'PLAN.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1, 'planCount'); + assert.strictEqual(result.summaryCount, 0, 'summaryCount'); + }); + + test('canonical padded plan file (01-01-PLAN.md)', () => { + const dir = phaseDir(); + touch(dir, '01-01-PLAN.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1, 'planCount'); + }); + + test('canonical padded plan + matching summary → completed', () => { + const dir = phaseDir(); + touch(dir, '01-01-PLAN.md', '01-01-SUMMARY.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1); + assert.strictEqual(result.summaryCount, 1); + assert.strictEqual(result.completed, true, 'phase should be complete when summaries >= plans'); + }); + + test('plan without summary → not completed', () => { + const dir = phaseDir(); + touch(dir, '01-01-PLAN.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.completed, false); + }); + + test('multiple plans all summarized → completed', () => { + const dir = phaseDir(); + touch(dir, '01-01-PLAN.md', '01-02-PLAN.md', '01-01-SUMMARY.md', '01-02-SUMMARY.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 2); + assert.strictEqual(result.summaryCount, 2); + assert.strictEqual(result.completed, true); + }); + + test('bare SUMMARY.md counts as one summary', () => { + const dir = phaseDir(); + touch(dir, 'PLAN.md', 'SUMMARY.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1); + assert.strictEqual(result.summaryCount, 1); + }); + + test('extended-layout root file (5-PLAN-01-setup.md style)', () => { + // roadmap.cjs isPlanFile explicitly matches any .md with PLAN in name at root + // (not just ending with -PLAN.md). The canonical helper must too. + // e.g. gsd-plan-phase writes "5-PLAN-01-setup.md". + const dir = phaseDir(); + // The summary for this file follows the canonical *-SUMMARY.md suffix convention. + touch(dir, '3-PLAN-01-setup.md', '3-01-SUMMARY.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1, 'extended-layout root plan counted'); + assert.strictEqual(result.summaryCount, 1, 'extended-layout root summary counted'); + }); +}); + +// --------------------------------------------------------------------------- +// Ignored files +// --------------------------------------------------------------------------- + +describe('scanPhasePlans — ignored files', () => { + test('PLAN-OUTLINE file is ignored (flat)', () => { + const dir = phaseDir(); + touch(dir, '01-01-PLAN.md', '01-01-PLAN-OUTLINE.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1, 'OUTLINE should not count as a plan'); + }); + + test('pre-bounce file is ignored (flat)', () => { + const dir = phaseDir(); + touch(dir, '01-01-PLAN.md', '01-01-PLAN.pre-bounce.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1, 'pre-bounce should not count as a plan'); + }); + + test('CONTEXT.md is not counted as a plan', () => { + const dir = phaseDir(); + touch(dir, 'PLAN.md', 'CONTEXT.md', '01-01-CONTEXT.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1, 'CONTEXT files should not be plans'); + }); + + test('RESEARCH.md is not counted as a plan', () => { + const dir = phaseDir(); + touch(dir, 'PLAN.md', 'RESEARCH.md', '01-01-RESEARCH.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1, 'RESEARCH files should not be plans'); + }); + + test('VERIFICATION.md is not counted as a plan', () => { + const dir = phaseDir(); + touch(dir, 'PLAN.md', 'VERIFICATION.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1, 'VERIFICATION files should not be plans'); + }); +}); + +// --------------------------------------------------------------------------- +// Nested layout (post-#3139) +// --------------------------------------------------------------------------- + +describe('scanPhasePlans — nested layout', () => { + test('nested PLAN-NN-slug.md files counted', () => { + const dir = phaseDir(); + const plansDir = path.join(dir, 'plans'); + fs.mkdirSync(plansDir); + touch(plansDir, 'PLAN-01-setup.md', 'PLAN-02-impl.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 2, 'nested plans counted'); + assert.strictEqual(result.hasNestedPlans, true, 'hasNestedPlans flag set'); + }); + + test('nested SUMMARY-NN-slug.md files counted', () => { + const dir = phaseDir(); + const plansDir = path.join(dir, 'plans'); + fs.mkdirSync(plansDir); + touch(plansDir, 'PLAN-01-setup.md', 'SUMMARY-01-setup.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1); + assert.strictEqual(result.summaryCount, 1); + assert.strictEqual(result.completed, true); + }); + + test('flat root + nested plans combined', () => { + const dir = phaseDir(); + const plansDir = path.join(dir, 'plans'); + fs.mkdirSync(plansDir); + // root: 1 plan, 1 summary + touch(dir, '01-01-PLAN.md', '01-01-SUMMARY.md'); + // nested: 2 plans, 1 summary + touch(plansDir, 'PLAN-01-setup.md', 'PLAN-02-impl.md', 'SUMMARY-01-setup.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 3, 'root + nested plans'); + assert.strictEqual(result.summaryCount, 2, 'root + nested summaries'); + assert.strictEqual(result.completed, false, 'not all plans have summaries'); + }); + + test('hasNestedPlans is false when plans/ directory absent', () => { + const dir = phaseDir(); + touch(dir, 'PLAN.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.hasNestedPlans, false); + }); + + test('nested OUTLINE files are ignored', () => { + const dir = phaseDir(); + const plansDir = path.join(dir, 'plans'); + fs.mkdirSync(plansDir); + touch(plansDir, 'PLAN-01-setup.md', 'PLAN-01-OUTLINE.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1, 'OUTLINE excluded in nested'); + }); + + test('nested pre-bounce files are ignored', () => { + const dir = phaseDir(); + const plansDir = path.join(dir, 'plans'); + fs.mkdirSync(plansDir); + touch(plansDir, 'PLAN-01-setup.md', 'PLAN-01.pre-bounce.md'); + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1, 'pre-bounce excluded in nested'); + }); + + test('plans/ that is not readable as directory does not throw', () => { + const dir = phaseDir(); + // Create plans/ as a file (unreadable as directory) + fs.writeFileSync(path.join(dir, 'plans'), 'not-a-directory'); + touch(dir, 'PLAN.md'); + // Should not throw + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 1); + assert.strictEqual(result.hasNestedPlans, false); + }); +}); + +// --------------------------------------------------------------------------- +// Parity: helper output shape and mixed fixture +// --------------------------------------------------------------------------- + +describe('scanPhasePlans — call-site parity on mixed fixture', () => { + // Build a fixture tree that exercises both flat and nested layout: + // 01-foundation/ + // 01-01-PLAN.md + // 01-01-SUMMARY.md + // 01-01-PLAN-OUTLINE.md (should be ignored) + // 01-02-PLAN.md + // plans/ + // PLAN-01-setup.md + // SUMMARY-01-setup.md + + function buildMixedPhase() { + const dir = phaseDir('01-foundation'); + const plansDir = path.join(dir, 'plans'); + fs.mkdirSync(plansDir); + touch(dir, '01-01-PLAN.md', '01-01-SUMMARY.md', '01-01-PLAN-OUTLINE.md', '01-02-PLAN.md'); + touch(plansDir, 'PLAN-01-setup.md', 'SUMMARY-01-setup.md'); + return dir; + } + + test('scanPhasePlans() counts match expected values for mixed fixture', () => { + const dir = buildMixedPhase(); + const result = scanPhasePlans(dir); + // flat: 01-01-PLAN.md + 01-02-PLAN.md = 2 (OUTLINE ignored) + // nested: PLAN-01-setup.md = 1 + assert.strictEqual(result.planCount, 3, 'planCount should be 3'); + // flat: 01-01-SUMMARY.md = 1; nested: SUMMARY-01-setup.md = 1 + assert.strictEqual(result.summaryCount, 2, 'summaryCount should be 2'); + assert.strictEqual(result.completed, false, 'not all plans have summaries'); + assert.strictEqual(result.hasNestedPlans, true, 'nested layout present'); + }); + + test('scanPhasePlans() output shape has required fields', () => { + const dir = buildMixedPhase(); + const result = scanPhasePlans(dir); + assert.ok('planCount' in result, 'planCount field present'); + assert.ok('summaryCount' in result, 'summaryCount field present'); + assert.ok('completed' in result, 'completed field present'); + assert.ok('hasNestedPlans' in result, 'hasNestedPlans field present'); + assert.ok('planFiles' in result, 'planFiles field present'); + assert.ok('summaryFiles' in result, 'summaryFiles field present'); + assert.ok(Array.isArray(result.planFiles), 'planFiles is array'); + assert.ok(Array.isArray(result.summaryFiles), 'summaryFiles is array'); + }); + + test('parity baseline: 2 flat + 1 nested plans across all call sites', () => { + // This test documents the exact expected counts for a representative fixture. + // After the GREEN phase ports roadmap.cjs/state.cjs/init.cjs to use + // scanPhasePlans, those call sites delegate here and this assertion is + // the single contract all of them must satisfy. + const dir = phaseDir('02-api'); + touch(dir, '02-01-PLAN.md', '02-02-PLAN.md', '02-01-SUMMARY.md'); + const plansDir = path.join(dir, 'plans'); + fs.mkdirSync(plansDir); + touch(plansDir, 'PLAN-01-impl.md', 'SUMMARY-01-impl.md'); + + const result = scanPhasePlans(dir); + assert.strictEqual(result.planCount, 3, 'helper: 2 flat + 1 nested'); + assert.strictEqual(result.summaryCount, 2, 'helper: 1 flat + 1 nested'); + assert.strictEqual(result.completed, false, '2 summaries < 3 plans'); + assert.strictEqual(result.hasNestedPlans, true, 'plans/ dir exists with plans'); + }); +});