From bab72fa2b0933234088c3acc8fabbf8fa2e4aaba Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 30 Jun 2026 21:08:02 -0400 Subject: [PATCH] fix(#1591): match bold checklist phase markers in isLastPhase fallback The #1591 checkbox broadening matched `- [ ] Phase N:` but not the canonical bold form the roadmap template emits (`- [ ] **Phase N: Name**`), so a
-wrapped bold checklist with no next-phase directory still fell through to is_last_phase=true and a false 'Milestone complete'. Allow optional **/__ emphasis after the marker and stop the name capture at emphasis so bold names slug cleanly. Surfaced by adversarial (codex) review. Co-Authored-By: Claude Opus 4.8 (cherry picked from commit ad94b38a69b4d91d2d4f820f288fc8f19d76b5c6) --- src/phase.cts | 21 +++++++------ tests/phase.test.cjs | 70 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 9 deletions(-) diff --git a/src/phase.cts b/src/phase.cts index 8b463f3e5..b6e4a5ae6 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -1649,15 +1649,18 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { try { const roadmapForPhases = extractCurrentMilestone(roadmapContent, cwd); // #1591: match BOTH heading-style phases (`### Phase N:`) AND - // checkbox-list items (`- [ ] Phase N:` / `- [x] Phase N:`). When - // the active milestone's checklist is `- [ ]` items inside a - //
block (and the next phase has no directory yet, so the - // disk-based resolver finds nothing), this roadmap-enumeration - // fallback is the only path that can find the next phase. The prior - // heading-only pattern missed checkbox items → is_last_phase=true on - // a mid-milestone phase. The marker alternation is the only change; - // the number/name captures are unchanged. - const phasePattern = /(?:#{2,4}|-\s*\[[ xX]\])\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:\s*([^\n]+)/gi; + // checkbox-list items, INCLUDING the canonical bold form the roadmap + // template emits (`- [ ] **Phase N: Name**`). When the active + // milestone's checklist is `- [ ]` items inside a
block + // (and the next phase has no directory yet, so the disk-based + // resolver finds nothing), this roadmap-enumeration fallback is the + // only path that can find the next phase. The prior heading-only + // pattern missed checkbox items, and a checkbox-only broadening still + // missed the bold template rows → is_last_phase=true on a mid-milestone + // phase. Allow optional `**`/`__` emphasis after the marker and stop + // the name capture at emphasis so bold names slug cleanly; the number + // capture is unchanged. + const phasePattern = /(?:#{2,4}|-\s*\[[ xX]\])\s*(?:\*\*|__)?\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:\s*([^\n*]+)/gi; let pm: RegExpExecArray | null; while ((pm = phasePattern.exec(roadmapForPhases)) !== null) { if (comparePhaseNum(pm[1], phaseNum) > 0) { diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 354c07a4c..dacecc5ef 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -2455,6 +2455,76 @@ describe('phase complete command', () => { ); }); + // #1591 (bold-checklist follow-up): the roadmap TEMPLATE emits checklist rows + // in the canonical BOLD form `- [ ] **Phase N: Name**`. The initial checkbox + // broadening only matched the un-bolded `- [ ] Phase N:` shape, so the exact + //
-wrapped bold template still fell through to is_last_phase=true. + // Guard the canonical bold form explicitly. + test('#1591:
-wrapped BOLD checkbox checklist — mid-milestone phase is NOT last', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# ROADMAP', + '', + '## Phases', + '', + '
', + '🚀 v2.0 Second (Phases 36–38) — IN PLANNING', + '', + '- [x] **Phase 36: first (completed)** - done', + '- [ ] **Phase 37: second** - one-line description', + '- [ ] **Phase 38: third** - one-line description', + '', + '
', + '', + ].join('\n') + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + [ + '---', + 'gsd_state_version: 1.0', + 'milestone: v2.0', + 'milestone_name: Second', + 'current_phase: "36"', + 'status: executing', + '---', + '', + '# GSD State', + '', + '**Current Phase:** 36', + '**Status:** Executing Phase 36', + '', + ].join('\n') + ); + // Only Phase 36 has a directory; 37/38 are bold `- [ ]` checklist rows only. + const d36 = path.join(tmpDir, '.planning', 'phases', '36-first'); + fs.mkdirSync(d36, { recursive: true }); + fs.writeFileSync(path.join(d36, '36-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(d36, '36-SUMMARY.md'), '# Summary\n'); + + const result = runVerifiedPhaseComplete('phase complete 36', tmpDir); + assert.ok(result.success, `phase complete failed: ${result.error}`); + const output = JSON.parse(result.output); + + assert.strictEqual( + output.is_last_phase, + false, + 'Phase 36 of 36–38 must NOT be last with the canonical BOLD checklist (#1591)', + ); + assert.strictEqual( + output.next_phase, + '37', + 'next_phase must resolve to 37 from the BOLD `- [ ] **Phase N: ...**` checklist (#1591)', + ); + + const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.ok( + !/Milestone complete/i.test(state), + 'a mid-milestone phase must not flip STATE.md to "Milestone complete" — bold checklist (#1591)', + ); + }); + // #1752: the #1591 follow-up — when phase.complete wrongly returned // is_last_phase=true on a
-wrapped mid-milestone checklist, the // milestone-complete cascade also DECREMENTED progress.total_phases (e.g.