From d858f51a68ec9b04f3c002ae0144b195f4499a4c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Apr 2026 11:15:59 -0400 Subject: [PATCH] fix(phase): update plan count when current milestone is inside
(#2005) (#2029) replaceInCurrentMilestone() locates content by finding the last
in the ROADMAP and only operates on text after that boundary. When the current (in-progress) milestone section is itself wrapped in a
block (the standard /gsd-new-project layout), the phase section's **Plans:** counter lives INSIDE that block. The replacement target ends up in the empty space after the block's closing
, so the regex never matches and the plan count stays at 0/N permanently. Fix: switch the plan count update to use direct .replace() on the full roadmapContent, consistent with the checkbox and progress table updates that already use this pattern. The phase-scoped heading regex (### Phase N: ...) is specific enough to avoid matching archived phases. Adds two regression tests covering: (1) plan count updates inside a
-wrapped current milestone, and (2) phase 2 plan count is not corrupted when completing phase 1. --- get-shit-done/bin/lib/phase.cjs | 13 +- .../bug-2005-phase-complete-details.test.cjs | 180 ++++++++++++++++++ 2 files changed, 190 insertions(+), 3 deletions(-) create mode 100644 tests/bug-2005-phase-complete-details.test.cjs diff --git a/get-shit-done/bin/lib/phase.cjs b/get-shit-done/bin/lib/phase.cjs index 4df2beb60..ecde4d5c8 100644 --- a/get-shit-done/bin/lib/phase.cjs +++ b/get-shit-done/bin/lib/phase.cjs @@ -736,13 +736,20 @@ function cmdPhaseComplete(cwd, phaseNum, raw) { return '|' + cells.join('|') + '|'; }); - // Update plan count in phase section + // Update plan count in phase section. + // Use direct .replace() rather than replaceInCurrentMilestone() so this + // works when the current milestone section is itself inside a
+ // block (the standard /gsd-new-project layout). replaceInCurrentMilestone + // scopes to content after the last
, which misses content inside + // the current milestone's own
wrapper (#2005). + // The phase-scoped heading pattern is specific enough to avoid matching + // archived phases (which belong to different milestones). const planCountPattern = new RegExp( `(#{2,4}\\s*Phase\\s+${phaseEscaped}[\\s\\S]*?\\*\\*Plans:\\*\\*\\s*)[^\\n]+`, 'i' ); - roadmapContent = replaceInCurrentMilestone( - roadmapContent, planCountPattern, + roadmapContent = roadmapContent.replace( + planCountPattern, `$1${summaryCount}/${planCount} plans complete` ); diff --git a/tests/bug-2005-phase-complete-details.test.cjs b/tests/bug-2005-phase-complete-details.test.cjs new file mode 100644 index 000000000..7a98d0e2b --- /dev/null +++ b/tests/bug-2005-phase-complete-details.test.cjs @@ -0,0 +1,180 @@ +/** + * Regression tests for bug #2005 + * + * When the in-progress milestone section is wrapped in a
block + * (the standard /gsd-new-project layout), phase complete silently skips: + * 1. The plan count update (**Plans:** N/M → X/M plans complete) + * 2. Mis-reports is_last_phase and next_phase + * + * Root cause: replaceInCurrentMilestone() uses the last
as the + * boundary, so when the current milestone is itself inside
, the + * replacement target is in the empty space AFTER the current milestone's + * closing
, and the regex never matches anything inside the block. + */ + +'use strict'; + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); +const { execFileSync } = require('node:child_process'); + +const gsdTools = path.resolve(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools.cjs'); + +describe('bug #2005: phase complete updates plan count when milestone is inside
', () => { + let tmpDir; + let planningDir; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2005-')); + planningDir = path.join(tmpDir, '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); + + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ project_code: 'TEST' }) + ); + + fs.writeFileSync( + path.join(planningDir, 'STATE.md'), + '---\ncurrent_phase: 1\nstatus: executing\nmilestone: v2.0\n---\n# State\n' + ); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + test('plan count is updated when current milestone is wrapped in
', () => { + // This is the standard /gsd-new-project layout: every milestone in
+ const phasesDir = path.join(planningDir, 'phases', '01-setup'); + fs.mkdirSync(phasesDir, { recursive: true }); + fs.writeFileSync( + path.join(phasesDir, '01-1-SUMMARY.md'), + '---\nstatus: complete\n---\n# Summary\nDone.\n' + ); + fs.writeFileSync( + path.join(phasesDir, '01-1-PLAN.md'), + '---\nphase: 1\nplan: 1\n---\n# Plan 1\n' + ); + + const roadmapPath = path.join(planningDir, 'ROADMAP.md'); + // Current milestone (v2.0) is wrapped in
+ fs.writeFileSync(roadmapPath, [ + '# Roadmap', + '', + '
', + 'v1.0 (shipped)', + '', + '## v1.0 Phases', + '- [x] **Phase 0: Bootstrap** - shipped', + '', + '
', + '', + '
', + 'v2.0 (in progress)', + '', + '## v2.0 Phases', + '', + '- [ ] **Phase 1: Setup** - initial setup', + '- [ ] **Phase 2: Build** - build features', + '', + '## Progress', + '', + '| Phase | Plans | Status | Completed |', + '|-------|-------|--------|-----------|', + '| 1. Setup | 0/1 | Pending | - |', + '| 2. Build | 0/1 | Pending | - |', + '', + '### Phase 1: Setup', + '', + '**Plans:** 0/1 plans complete', + '', + '### Phase 2: Build', + '', + '**Plans:** 0/1 plans complete', + '', + '
', + ].join('\n')); + + try { + execFileSync('node', [gsdTools, 'phase', 'complete', '1'], { cwd: tmpDir, timeout: 10000 }); + } catch { + // May exit non-zero if STATE.md update fails, but ROADMAP.md update is the target + } + + const result = fs.readFileSync(roadmapPath, 'utf-8'); + + // Plan count must be updated inside the
block + assert.match( + result, + /\*\*Plans:\*\*\s*1\/1 plans complete/, + 'plan count in Phase 1 section must be updated to 1/1 plans complete' + ); + + // Phase 1 checkbox must be checked + assert.match( + result, + /- \[x\] \*\*Phase 1: Setup\*\*/, + 'Phase 1 checkbox must be checked after completion' + ); + + // Phase 2 must be untouched + assert.match( + result, + /- \[ \] \*\*Phase 2: Build\*\*/, + 'Phase 2 checkbox must remain unchecked' + ); + }); + + test('phase complete with all milestones in
does not corrupt phase 2 plan count', () => { + const phasesDir = path.join(planningDir, 'phases', '01-setup'); + fs.mkdirSync(phasesDir, { recursive: true }); + fs.writeFileSync( + path.join(phasesDir, '01-1-SUMMARY.md'), + '---\nstatus: complete\n---\n# Summary\nDone.\n' + ); + fs.writeFileSync( + path.join(phasesDir, '01-1-PLAN.md'), + '---\nphase: 1\nplan: 1\n---\n# Plan 1\n' + ); + + const roadmapPath = path.join(planningDir, 'ROADMAP.md'); + fs.writeFileSync(roadmapPath, [ + '# Roadmap', + '', + '
', + 'v2.0 (in progress)', + '', + '## v2.0 Phases', + '', + '- [ ] **Phase 1: Setup** - initial setup', + '- [ ] **Phase 2: Build** - build features', + '', + '### Phase 1: Setup', + '', + '**Plans:** 0/1 plans complete', + '', + '### Phase 2: Build', + '', + '**Plans:** 0/2 plans complete', + '', + '
', + ].join('\n')); + + try { + execFileSync('node', [gsdTools, 'phase', 'complete', '1'], { cwd: tmpDir, timeout: 10000 }); + } catch {} + + const result = fs.readFileSync(roadmapPath, 'utf-8'); + + // Phase 2's plan count must NOT be touched + assert.match( + result, + /Phase 2: Build[\s\S]*?\*\*Plans:\*\*\s*0\/2 plans complete/, + 'Phase 2 plan count must remain 0/2 (untouched)' + ); + }); +});