diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index 4ed31fdcb..66ceb3e64 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -1309,8 +1309,11 @@ function extractCurrentMilestone(content, cwd) { // Milestone headings look like: ## v2.0, ## Roadmap v2.0, ## ✅ v1.0, etc. const headingLevel = sectionMatch[1].match(/^(#{1,3})\s/)[1].length; const restContent = content.slice(sectionStart + sectionMatch[0].length); + // Exclude phase headings (e.g. "### Phase 12: v1.0 Tech-Debt Closure") from + // being treated as milestone boundaries just because they mention vX.Y in + // the title. Phase headings always start with the literal `Phase `. See #2619. const nextMilestonePattern = new RegExp( - `^#{1,${headingLevel}}\\s+(?:.*v\\d+\\.\\d+|✅|📋|🚧)`, + `^#{1,${headingLevel}}\\s+(?!Phase\\s+\\S)(?:.*v\\d+\\.\\d+|✅|📋|🚧)`, 'mi' ); const nextMatch = restContent.match(nextMilestonePattern); diff --git a/sdk/src/query/roadmap.test.ts b/sdk/src/query/roadmap.test.ts index e001e629d..f63a4d460 100644 --- a/sdk/src/query/roadmap.test.ts +++ b/sdk/src/query/roadmap.test.ts @@ -326,6 +326,69 @@ describe('extractCurrentMilestone', () => { expect(result).toContain('Phase 100'); }); + // ─── Bug #2619: phase heading containing vX.Y triggers truncation ───── + it('bug-2619: does not truncate at a phase heading containing vX.Y', async () => { + // A phase title like "Phase 12: v1.0 Tech-Debt Closure" was being treated + // as a milestone boundary because the greedy `.*v(\d+(?:\.\d+)+)` branch + // in nextMilestoneRegex matched any heading with a version literal. + const roadmapWithPhaseVersion = `# ROADMAP + +## Phases + +### 🚧 v1.1 Launch-Ready (In Progress) + +### Phase 11: Structured Logging +**Goal**: Add structured logging + +### Phase 12: v1.0 Tech-Debt Closure +**Goal**: Close out v1.0 debt + +### Phase 19: Security Audit +**Goal**: Full security audit +`; + const state = `---\nmilestone: v1.1\n---\n# State\n`; + await writeFile(join(tmpDir, '.planning', 'STATE.md'), state); + await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmapWithPhaseVersion); + + const result = await extractCurrentMilestone(roadmapWithPhaseVersion, tmpDir); + + // Phase 12 and Phase 19 must both survive — the slice cannot be truncated + // at "### Phase 12: v1.0 Tech-Debt Closure". + expect(result).toContain('### Phase 12: v1.0 Tech-Debt Closure'); + expect(result).toContain('### Phase 19: Security Audit'); + }); + + // ─── Bug #2619 (CodeRabbit follow-up): case-insensitive Phase lookahead ─── + it('bug-2619: does not truncate at PHASE/phase heading containing vX.Y (case-insensitive)', async () => { + // The negative lookahead `(?!Phase\s+\S)` must be case-insensitive so that + // headings like "### PHASE 12: v1.0 Tech-Debt" or "### phase 12: v1.0 …" + // are also excluded from milestone-boundary matching. + const roadmapMixedCase = `# ROADMAP + +## Phases + +### 🚧 v1.1 Launch-Ready (In Progress) + +### PHASE 11: Structured Logging +**Goal**: Add structured logging + +### phase 12: v1.0 Tech-Debt Closure +**Goal**: Close out v1.0 debt + +### Phase 19: Security Audit +**Goal**: Full security audit +`; + const state = `---\nmilestone: v1.1\n---\n# State\n`; + await writeFile(join(tmpDir, '.planning', 'STATE.md'), state); + await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmapMixedCase); + + const result = await extractCurrentMilestone(roadmapMixedCase, tmpDir); + + expect(result).toContain('### PHASE 11: Structured Logging'); + expect(result).toContain('### phase 12: v1.0 Tech-Debt Closure'); + expect(result).toContain('### Phase 19: Security Audit'); + }); + // ─── Bug #2422: same-version sub-heading truncation ─────────────────── it('bug-2422: does not truncate at same-version sub-heading (## v2.0 Phase Details)', async () => { const roadmapWithDetails = `# ROADMAP diff --git a/sdk/src/query/roadmap.ts b/sdk/src/query/roadmap.ts index 984ebbb64..ad139faef 100644 --- a/sdk/src/query/roadmap.ts +++ b/sdk/src/query/roadmap.ts @@ -195,9 +195,14 @@ export async function extractCurrentMilestone(content: string, projectDir: strin const currentVersionMatch = version ? version.match(/v(\d+(?:\.\d+)+)/i) : null; const currentVersionStr = currentVersionMatch ? currentVersionMatch[1] : ''; + // Exclude phase headings (e.g. "### Phase 12: v1.0 Tech-Debt Closure") from + // being treated as milestone boundaries just because they mention vX.Y in + // the title. Phase headings always start with the literal `Phase `. See #2619. const nextMilestoneRegex = new RegExp( - `^#{1,${headingLevel}}\\s+(?:.*v(\\d+(?:\\.\\d+)+)[^\\n]*|.*(?:✅|📋|🚧|🟡))`, - 'gm' + `^#{1,${headingLevel}}\\s+(?!Phase\\s+\\S)(?:.*v(\\d+(?:\\.\\d+)+)[^\\n]*|.*(?:✅|📋|🚧|🟡))`, + // `i` flag ensures the `(?!Phase\s+\S)` lookahead matches PHASE/phase too + // (CodeRabbit follow-up on #2619). + 'gmi' ); let sectionEnd = content.length; @@ -303,7 +308,8 @@ export async function extractNextMilestoneSection( // Look for the next ## milestone heading after the current one. const tail = cleaned.slice(currentMatch.index + currentMatch[0].length); - const nextMilestonePattern = /^##\s+([^\n]*(?:v(\d+(?:\.\d+)+)|✅|🚧|🟡|📋)[^\n]*)$/gim; + // Exclude phase headings — see #2619. + const nextMilestonePattern = /^##\s+(?!Phase\s+\S)([^\n]*(?:v(\d+(?:\.\d+)+)|✅|🚧|🟡|📋)[^\n]*)$/gim; let nextMatch: RegExpExecArray | null; while ((nextMatch = nextMilestonePattern.exec(tail)) !== null) { const heading = nextMatch[1]; diff --git a/tests/roadmap-phase-fallback.test.cjs b/tests/roadmap-phase-fallback.test.cjs index 32a044f1d..1fb6470b1 100644 --- a/tests/roadmap-phase-fallback.test.cjs +++ b/tests/roadmap-phase-fallback.test.cjs @@ -207,6 +207,73 @@ describe('roadmap get-phase fallback to full ROADMAP.md (#1634)', () => { assert.equal(output.goal, 'Remove deprecated modules'); }); + test('extractCurrentMilestone does not truncate on phase heading containing vX.Y (#2619)', () => { + // Regression: phase heading like "### Phase 12: v1.0 Tech-Debt Closure" + // was incorrectly treated as a milestone boundary because the greedy + // `.*v\d+\.\d+` subpattern in nextMilestonePattern matched it. + const core = require('../get-shit-done/bin/lib/core.cjs'); + writeState(tmpDir, 'v1.1'); + const roadmap = `# Roadmap + +## Phases + +### 🚧 v1.1 Launch-Ready (In Progress) + +### Phase 11: Structured Logging +**Goal:** Add structured logging + +### Phase 12: v1.0 Tech-Debt Closure +**Goal:** Close out v1.0 debt + +### Phase 19: Security Audit +**Goal:** Full security audit +`; + const slice = core.extractCurrentMilestone(roadmap, tmpDir); + assert.ok( + slice.includes('### Phase 12: v1.0 Tech-Debt Closure'), + 'slice must include Phase 12 (it lives inside the active milestone)' + ); + assert.ok( + slice.includes('### Phase 19: Security Audit'), + 'slice must include Phase 19 (truncation at Phase 12 would hide it)' + ); + }); + + test('extractCurrentMilestone handles PHASE/phase (case-insensitive) containing vX.Y (#2619 follow-up)', () => { + // CodeRabbit follow-up: the negative lookahead `(?!Phase\s+\S)` must be + // case-insensitive so PHASE/phase variants are also excluded. + const core = require('../get-shit-done/bin/lib/core.cjs'); + writeState(tmpDir, 'v1.1'); + const roadmap = `# Roadmap + +## Phases + +### 🚧 v1.1 Launch-Ready (In Progress) + +### PHASE 11: Structured Logging +**Goal:** Add structured logging + +### phase 12: v1.0 Tech-Debt Closure +**Goal:** Close out v1.0 debt + +### Phase 19: Security Audit +**Goal:** Full security audit +`; + const slice = core.extractCurrentMilestone(roadmap, tmpDir); + assert.ok( + slice.includes('### PHASE 11: Structured Logging'), + 'slice must include PHASE 11 (uppercase)' + ); + assert.ok( + slice.includes('### phase 12: v1.0 Tech-Debt Closure'), + 'slice must include phase 12 (lowercase with vX.Y)' + ); + assert.ok( + slice.includes('### Phase 19: Security Audit'), + 'slice must include Phase 19 (truncation at phase 12 would hide it)' + ); + }); + test('section extraction from fallback includes correct content boundaries', () => { writeState(tmpDir, 'v1.0'); fs.writeFileSync(