diff --git a/.changeset/1591-phase-complete-details-wrapped-checklist.md b/.changeset/1591-phase-complete-details-wrapped-checklist.md new file mode 100644 index 000000000..c6814916b --- /dev/null +++ b/.changeset/1591-phase-complete-details-wrapped-checklist.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 1819 +--- +**`phase.complete` no longer reports a false `is_last_phase` on a `
`-wrapped checkbox checklist (#1591, #1752)** — when the active milestone's phase checklist was written as `- [ ] Phase N:` checkbox items inside a `
` block and the next phase had no directory on disk yet (still in planning), `phase.complete`'s `isLastPhase` roadmap-enumeration fallback used a heading-only pattern (`/#{2,4}\s*Phase…/`) that never matched checkbox items. It returned `is_last_phase: true, next_phase: null` on a mid-milestone phase and — via the milestone-complete cascade — wrongly flipped STATE.md to `Milestone complete` and decremented `progress.total_phases` (e.g. 8 → 7). The pattern now matches both heading-style (`### Phase N:`) and checkbox-list phases (`- [ ] Phase N:` / `- [x] Phase N:`); `extractCurrentMilestone` already surfaces the `
`-wrapped checklist correctly, so no parser change was needed. Only the reproduced `phase.complete` fallback is changed; the heading-only sibling patterns elsewhere in `phase.cts` are untouched. + diff --git a/src/phase.cts b/src/phase.cts index 76ef1a74c..8b463f3e5 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -1648,7 +1648,16 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { if (isLastPhase && roadmapContent !== null) { try { const roadmapForPhases = extractCurrentMilestone(roadmapContent, cwd); - const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:\s*([^\n]+)/gi; + // #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; 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 62c4c90c7..354c07a4c 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -2362,6 +2362,187 @@ describe('phase complete command', () => { assert.ok(state.includes('Milestone complete'), 'status should be milestone complete'); }); + // #1591: when the active milestone's phase checklist is wrapped in a + //
block AND phases are written as `- [ ] Phase N:` checkbox list + // items (not `### Phase N:` headings), phase.complete's next-phase enumerator + // saw no further phases → is_last_phase=true, next_phase=null on a mid- + // milestone phase, and STATE.md was wrongly marked "Milestone complete" with + // total_phases decremented. extractCurrentMilestone correctly surfaces the + //
-wrapped checklist; the defect was the heading-only phasePattern + // at the isLastPhase enumerator not recognizing checkbox-list phase items. + test('#1591:
-wrapped checkbox checklist — mid-milestone phase is NOT last', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# ROADMAP', + '', + '## Phases', + '', + '
', + '✅ v1.0 First (Phases 1–3) — SHIPPED', + '', + '- [x] Phase 1: a', + '- [x] Phase 2: b', + '- [x] Phase 3: c', + '', + '
', + '', + '
', + '🚀 v2.0 Second (Phases 36–38) — IN PLANNING', + '', + '- [x] Phase 36: first (completed)', + '- [ ] Phase 37: second', + '- [ ] Phase 38: third', + '', + '
', + '', + '## Backlog', + '', + '### Phase 999.1: future (BACKLOG)', + '', + ].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 the COMPLETING phase (36) has a directory. Phases 37/38 exist only + // as `- [ ]` checklist items in the ROADMAP — they are not yet started, so + // they have no phase dirs. This is the @Azd325 scenario: the disk-based + // next-phase resolver finds nothing, and the roadmap-enumeration fallback + // (the heading-only phasePattern) is the only path that can find Phase 37. + 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 — Phases 37/38 are still open `- [ ]` (#1591)', + ); + assert.strictEqual( + output.next_phase, + '37', + 'next_phase must resolve to 37 from the
-wrapped checkbox checklist (#1591)', + ); + + // Cascade check: a wrong is_last_phase=true previously wrote "Milestone + // complete" + decremented total_phases. With the fix, the milestone is + // still in progress. + 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" (#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. + // 8 -> 7) and flipped status. Same root cause, distinct symptom. With the + // #1591 fix (is_last_phase=false), the decrement must not occur: with all 8 + // phase dirs on disk, total_phases stays 8 and status does not flip. + test('#1752:
-wrapped checklist — total_phases is NOT decremented on a mid-milestone phase', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# ROADMAP', + '', + '## Phases', + '', + '
', + '✅ v1.0 First (Phases 1–3) — SHIPPED', + '', + '- [x] Phase 1: a', + '- [x] Phase 2: b', + '- [x] Phase 3: c', + '', + '
', + '', + '
', + '🚀 v2.0 Second (Phases 36–43) — IN PLANNING', + '', + '- [x] Phase 36: first (completed)', + '- [ ] Phase 37: second', + '- [ ] Phase 38: third', + '- [ ] Phase 39: fourth', + '', + '
', + '', + ].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', + 'progress:', + ' total_phases: 8', + ' completed_phases: 5', + ' percent: 62', + '---', + '', + '# GSD State', + '', + '**Current Phase:** 36', + '**Status:** Executing Phase 36', + '', + ].join('\n') + ); + // All 8 phase dirs on disk (Phases 36–43) so the disk count is 8 — the + // reporter's real state. Before the #1591 fix, phase.complete 36 returned + // is_last_phase=true (no Phase 37+ heading match) and the milestone-complete + // path DECREMENTED total_phases 8 -> 7. + const names = ['first', 'second', 'third', 'fourth', 'fifth', 'sixth', 'seventh', 'eighth']; + for (let i = 0; i < 8; i++) { + const num = String(36 + i); + const d = path.join(tmpDir, '.planning', 'phases', `${num}-${names[i]}`); + fs.mkdirSync(d, { recursive: true }); + fs.writeFileSync(path.join(d, `${num}-PLAN.md`), '# Plan\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, 'is_last_phase must be false (#1752 cascade root)'); + + 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" (#1752)', + ); + const tpMatch = state.match(/total_phases:\s*(\d+)/); + assert.ok(tpMatch, 'STATE.md must carry a total_phases value after phase.complete'); + assert.notStrictEqual( + parseInt(tpMatch[1], 10), + 7, + 'total_phases must NOT be decremented to 7 — the #1752 cascade of the false is_last_phase', + ); + }); + test('updates REQUIREMENTS.md traceability when phase completes', () => { fs.writeFileSync( path.join(tmpDir, '.planning', 'ROADMAP.md'),