From b863ed6de56ae45b4b73eb9d1fe63ff9ccf7ca07 Mon Sep 17 00:00:00 2001 From: j2h4u <39818683+j2h4u@users.noreply.github.com> Date: Mon, 2 Mar 2026 20:27:20 +0500 Subject: [PATCH] fix(state): scope phase counting to current milestone Extract getMilestonePhaseFilter() from milestone.cjs closure into core.cjs as a shared helper. Apply it in buildStateFrontmatter and cmdPhaseComplete so multi-milestone projects count only current milestone's phases instead of all directories on disk. Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 3 + get-shit-done/bin/lib/core.cjs | 30 +++++++++ get-shit-done/bin/lib/phase.cjs | 7 +- get-shit-done/bin/lib/state.cjs | 8 ++- tests/core.test.cjs | 111 ++++++++++++++++++++++++++++++++ tests/phase.test.cjs | 103 +++++++++++++++++++++++++++++ tests/state.test.cjs | 83 ++++++++++++++++++++++++ 7 files changed, 340 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a86f99887..6876974d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Fixed +- Scope phase counting in `buildStateFrontmatter` and `cmdPhaseComplete` to current milestone — multi-milestone projects no longer report inflated total/completed phases + ## [1.22.1] - 2026-03-02 ### Added diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index 6ef6ccb2a..4a1edbeeb 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -409,6 +409,35 @@ function getMilestoneInfo(cwd) { } } +/** + * Returns a filter function that checks whether a phase directory belongs + * to the current milestone based on ROADMAP.md phase headings. + * If no ROADMAP exists or no phases are listed, returns a pass-all filter. + */ +function getMilestonePhaseFilter(cwd) { + const milestonePhaseNums = new Set(); + try { + const roadmap = fs.readFileSync(path.join(cwd, '.planning', 'ROADMAP.md'), 'utf-8'); + const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:/gi; + let m; + while ((m = phasePattern.exec(roadmap)) !== null) { + milestonePhaseNums.add(m[1]); + } + } catch {} + + if (milestonePhaseNums.size === 0) return () => true; + + const normalized = new Set( + [...milestonePhaseNums].map(n => (n.replace(/^0+/, '') || '0').toLowerCase()) + ); + + return function isDirInMilestone(dirName) { + const m = dirName.match(/^0*(\d+[A-Za-z]?(?:\.\d+)*)/); + if (!m) return false; + return normalized.has(m[1].toLowerCase()); + }; +} + module.exports = { MODEL_PROFILES, output, @@ -428,5 +457,6 @@ module.exports = { pathExistsInternal, generateSlugInternal, getMilestoneInfo, + getMilestonePhaseFilter, toPosixPath, }; diff --git a/get-shit-done/bin/lib/phase.cjs b/get-shit-done/bin/lib/phase.cjs index 4e4cbff60..bf37a74fd 100644 --- a/get-shit-done/bin/lib/phase.cjs +++ b/get-shit-done/bin/lib/phase.cjs @@ -4,7 +4,7 @@ const fs = require('fs'); const path = require('path'); -const { escapeRegex, normalizePhaseName, comparePhaseNum, findPhaseInternal, getArchivedPhaseDirs, generateSlugInternal, output, error } = require('./core.cjs'); +const { escapeRegex, normalizePhaseName, comparePhaseNum, findPhaseInternal, getArchivedPhaseDirs, generateSlugInternal, getMilestonePhaseFilter, output, error } = require('./core.cjs'); const { extractFrontmatter } = require('./frontmatter.cjs'); const { writeStateMd } = require('./state.cjs'); @@ -789,8 +789,11 @@ function cmdPhaseComplete(cwd, phaseNum, raw) { let isLastPhase = true; try { + const isDirInMilestone = getMilestonePhaseFilter(cwd); const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - const dirs = entries.filter(e => e.isDirectory()).map(e => e.name).sort((a, b) => comparePhaseNum(a, b)); + const dirs = entries.filter(e => e.isDirectory()).map(e => e.name) + .filter(isDirInMilestone) + .sort((a, b) => comparePhaseNum(a, b)); // Find the next phase directory after current for (const dir of dirs) { diff --git a/get-shit-done/bin/lib/state.cjs b/get-shit-done/bin/lib/state.cjs index 915f51c45..c07e2afca 100644 --- a/get-shit-done/bin/lib/state.cjs +++ b/get-shit-done/bin/lib/state.cjs @@ -4,7 +4,7 @@ const fs = require('fs'); const path = require('path'); -const { loadConfig, getMilestoneInfo, output, error } = require('./core.cjs'); +const { loadConfig, getMilestoneInfo, getMilestonePhaseFilter, output, error } = require('./core.cjs'); const { extractFrontmatter, reconstructFrontmatter } = require('./frontmatter.cjs'); function cmdStateLoad(cwd, raw) { @@ -549,8 +549,10 @@ function buildStateFrontmatter(bodyContent, cwd) { try { const phasesDir = path.join(cwd, '.planning', 'phases'); if (fs.existsSync(phasesDir)) { + const isDirInMilestone = getMilestonePhaseFilter(cwd); const phaseDirs = fs.readdirSync(phasesDir, { withFileTypes: true }) - .filter(e => e.isDirectory()).map(e => e.name); + .filter(e => e.isDirectory()).map(e => e.name) + .filter(isDirInMilestone); let diskTotalPlans = 0; let diskTotalSummaries = 0; let diskCompletedPhases = 0; @@ -563,7 +565,7 @@ function buildStateFrontmatter(bodyContent, cwd) { diskTotalSummaries += summaries; if (plans > 0 && summaries >= plans) diskCompletedPhases++; } - if (totalPhases === null) totalPhases = phaseDirs.length; + totalPhases = phaseDirs.length; completedPhases = diskCompletedPhases; totalPlans = diskTotalPlans; completedPlans = diskTotalSummaries; diff --git a/tests/core.test.cjs b/tests/core.test.cjs index d859269c2..2dd5383a7 100644 --- a/tests/core.test.cjs +++ b/tests/core.test.cjs @@ -22,6 +22,7 @@ const { safeReadFile, pathExistsInternal, getMilestoneInfo, + getMilestonePhaseFilter, getRoadmapPhaseInternal, searchPhaseInDir, findPhaseInternal, @@ -665,3 +666,113 @@ describe('getRoadmapPhaseInternal', () => { assert.ok(!result.section.includes('Phase 2: API')); }); }); + +// ─── getMilestonePhaseFilter ──────────────────────────────────────────────────── + +describe('getMilestonePhaseFilter', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-core-test-')); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true }); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + test('filters directories to only current milestone phases', () => { + // ROADMAP lists only phases 5-7 + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '## Roadmap v2.0: Next Release', + '', + '### Phase 5: Auth', + '**Goal:** Add authentication', + '', + '### Phase 6: Dashboard', + '**Goal:** Build dashboard', + '', + '### Phase 7: Polish', + '**Goal:** Final polish', + ].join('\n') + ); + + // Create phase dirs 1-7 on disk (leftover from previous milestones) + for (let i = 1; i <= 7; i++) { + const padded = String(i).padStart(2, '0'); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', `${padded}-phase-${i}`)); + } + + const filter = getMilestonePhaseFilter(tmpDir); + + // Only phases 5, 6, 7 should match + assert.strictEqual(filter('05-auth'), true); + assert.strictEqual(filter('06-dashboard'), true); + assert.strictEqual(filter('07-polish'), true); + + // Phases 1-4 should NOT match + assert.strictEqual(filter('01-phase-1'), false); + assert.strictEqual(filter('02-phase-2'), false); + assert.strictEqual(filter('03-phase-3'), false); + assert.strictEqual(filter('04-phase-4'), false); + }); + + test('returns pass-all filter when ROADMAP.md is missing', () => { + const filter = getMilestonePhaseFilter(tmpDir); + + assert.strictEqual(filter('01-foundation'), true); + assert.strictEqual(filter('99-anything'), true); + }); + + test('returns pass-all filter when ROADMAP has no phase headings', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '# Roadmap\n\nSome content without phases.\n' + ); + + const filter = getMilestonePhaseFilter(tmpDir); + + assert.strictEqual(filter('01-foundation'), true); + assert.strictEqual(filter('05-api'), true); + }); + + test('handles letter-suffix phases (e.g. 3A)', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '### Phase 3A: Sub-feature\n**Goal:** Sub work\n' + ); + + const filter = getMilestonePhaseFilter(tmpDir); + + assert.strictEqual(filter('03A-sub-feature'), true); + assert.strictEqual(filter('03-main'), false); + assert.strictEqual(filter('04-other'), false); + }); + + test('handles decimal phases (e.g. 5.1)', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '### Phase 5: Main\n**Goal:** Main work\n\n### Phase 5.1: Patch\n**Goal:** Patch work\n' + ); + + const filter = getMilestonePhaseFilter(tmpDir); + + assert.strictEqual(filter('05-main'), true); + assert.strictEqual(filter('05.1-patch'), true); + assert.strictEqual(filter('04-other'), false); + }); + + test('returns false for non-phase directory names', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '### Phase 1: Init\n**Goal:** Start\n' + ); + + const filter = getMilestonePhaseFilter(tmpDir); + + assert.strictEqual(filter('not-a-phase'), false); + assert.strictEqual(filter('.gitkeep'), false); + }); +}); diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index f1888eb3c..7532c3c9b 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -1457,6 +1457,109 @@ describe('letter-suffix phase sorting', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// milestone-scoped next-phase in phase complete +// ───────────────────────────────────────────────────────────────────────────── + +describe('phase complete milestone-scoped next-phase', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('finds next phase within milestone, ignoring prior milestone dirs', () => { + // ROADMAP lists phases 5-6 (current milestone v2.0) + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '## Roadmap v2.0: Release', + '', + '- [ ] Phase 5: Auth', + '- [ ] Phase 6: Dashboard', + '', + '### Phase 5: Auth', + '**Goal:** Add authentication', + '**Plans:** 1 plans', + '', + '### Phase 6: Dashboard', + '**Goal:** Build dashboard', + ].join('\n') + ); + + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# State\n\n**Current Phase:** 05\n**Current Phase Name:** Auth\n**Status:** In progress\n**Current Plan:** 05-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working\n' + ); + + // Disk has dirs 01-06 (01-04 completed from prior milestone) + for (let i = 1; i <= 4; i++) { + const padded = String(i).padStart(2, '0'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', `${padded}-old-phase`); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, `${padded}-01-PLAN.md`), '# Plan'); + fs.writeFileSync(path.join(phaseDir, `${padded}-01-SUMMARY.md`), '# Summary'); + } + + // Phase 5 — completing this one + const p5 = path.join(tmpDir, '.planning', 'phases', '05-auth'); + fs.mkdirSync(p5, { recursive: true }); + fs.writeFileSync(path.join(p5, '05-01-PLAN.md'), '# Plan'); + fs.writeFileSync(path.join(p5, '05-01-SUMMARY.md'), '# Summary'); + + // Phase 6 — next phase in milestone + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '06-dashboard'), { recursive: true }); + + const result = runGsdTools('phase complete 5', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.is_last_phase, false, 'should NOT be last phase — phase 6 is in milestone'); + assert.strictEqual(output.next_phase, '06', 'next phase should be 06'); + }); + + test('detects last phase when only milestone phases are considered', () => { + // ROADMAP lists only phase 5 (current milestone) + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '## Roadmap v2.0: Release', + '', + '### Phase 5: Auth', + '**Goal:** Add authentication', + '**Plans:** 1 plans', + ].join('\n') + ); + + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# State\n\n**Current Phase:** 05\n**Current Phase Name:** Auth\n**Status:** In progress\n**Current Plan:** 05-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working\n' + ); + + // Disk has dirs 01-06 but only 5 is in ROADMAP + for (let i = 1; i <= 6; i++) { + const padded = String(i).padStart(2, '0'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', `${padded}-phase-${i}`); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, `${padded}-01-PLAN.md`), '# Plan'); + fs.writeFileSync(path.join(phaseDir, `${padded}-01-SUMMARY.md`), '# Summary'); + } + + const result = runGsdTools('phase complete 5', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + // Without the fix, dirs 06 on disk would make is_last_phase=false + // With the fix, only phase 5 is in milestone, so it IS the last phase + assert.strictEqual(output.is_last_phase, true, 'should be last phase — only phase 5 is in milestone'); + assert.strictEqual(output.next_phase, null, 'no next phase in milestone'); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // milestone complete command // ───────────────────────────────────────────────────────────────────────────── diff --git a/tests/state.test.cjs b/tests/state.test.cjs index a8b2100e8..264401a98 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -1249,6 +1249,89 @@ describe('cmdStateRecordSession (state record-session)', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// Milestone-scoped phase counting in frontmatter +// ───────────────────────────────────────────────────────────────────────────── + +describe('milestone-scoped phase counting in frontmatter', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('total_phases counts only current milestone phases', () => { + // ROADMAP lists only phases 5-6 (current milestone) + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '## Roadmap v2.0: Next Release', + '', + '### Phase 5: Auth', + '**Goal:** Add authentication', + '', + '### Phase 6: Dashboard', + '**Goal:** Build dashboard', + ].join('\n') + ); + + // Disk has dirs 01-06 (01-04 are leftover from previous milestone) + for (let i = 1; i <= 6; i++) { + const padded = String(i).padStart(2, '0'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', `${padded}-phase-${i}`); + fs.mkdirSync(phaseDir, { recursive: true }); + // Add a plan to each + fs.writeFileSync(path.join(phaseDir, `${padded}-01-PLAN.md`), '# Plan'); + fs.writeFileSync(path.join(phaseDir, `${padded}-01-SUMMARY.md`), '# Summary'); + } + + // Write a STATE.md and trigger a write that will sync frontmatter + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Project State\n\n**Current Phase:** 05\n**Status:** In progress\n' + ); + + const result = runGsdTools('state update Status "Executing"', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + // Read the state json to check frontmatter + const jsonResult = runGsdTools('state json', tmpDir); + assert.ok(jsonResult.success, `state json failed: ${jsonResult.error}`); + + const output = JSON.parse(jsonResult.output); + assert.strictEqual(Number(output.progress.total_phases), 2, 'should count only milestone phases (5 and 6), not all 6'); + assert.strictEqual(Number(output.progress.completed_phases), 2, 'both milestone phases have summaries'); + }); + + test('without ROADMAP counts all phases (pass-all filter)', () => { + // No ROADMAP.md — all phases should be counted + for (let i = 1; i <= 4; i++) { + const padded = String(i).padStart(2, '0'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', `${padded}-phase-${i}`); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, `${padded}-01-PLAN.md`), '# Plan'); + } + + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Project State\n\n**Current Phase:** 01\n**Status:** Planning\n' + ); + + const result = runGsdTools('state update Status "In progress"', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const jsonResult = runGsdTools('state json', tmpDir); + assert.ok(jsonResult.success, `state json failed: ${jsonResult.error}`); + + const output = JSON.parse(jsonResult.output); + assert.strictEqual(Number(output.progress.total_phases), 4, 'without ROADMAP should count all 4 phases'); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // summary-extract command // ─────────────────────────────────────────────────────────────────────────────