From d790408aaa08893b9a2aa3c8ca209d6c120fc783 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 4 Apr 2026 07:14:24 -0400 Subject: [PATCH] fix(roadmap): fall back to full ROADMAP.md for backlog phases (#1640) * fix(roadmap): fall back to full ROADMAP.md for backlog and planned phases Closes #1634 Co-Authored-By: Claude Opus 4.6 * fix(roadmap): prevent checklist-only match from blocking full header fallback When the current milestone had a checklist reference to a phase (e.g. `- [ ] **Phase 50: Cleanup**`) but the full `### Phase 50:` header existed in a different milestone, the malformed_roadmap result from the first searchPhaseInContent call short-circuited the `||` operator and prevented the fallback to the full roadmap content. Now a malformed_roadmap result is deferred so the full content search can find the actual header match. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- get-shit-done/bin/lib/roadmap.cjs | 142 ++++++++------- tests/roadmap-phase-fallback.test.cjs | 245 ++++++++++++++++++++++++++ 2 files changed, 327 insertions(+), 60 deletions(-) create mode 100644 tests/roadmap-phase-fallback.test.cjs diff --git a/get-shit-done/bin/lib/roadmap.cjs b/get-shit-done/bin/lib/roadmap.cjs index b0cd2f357..29f2bfde8 100644 --- a/get-shit-done/bin/lib/roadmap.cjs +++ b/get-shit-done/bin/lib/roadmap.cjs @@ -6,6 +6,72 @@ const fs = require('fs'); const path = require('path'); const { escapeRegex, normalizePhaseName, planningPaths, withPlanningLock, output, error, findPhaseInternal, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone } = require('./core.cjs'); +/** + * Search for a phase header (and its section) within the given content string. + * Returns a result object if found (either a full match or a malformed_roadmap + * checklist-only match), or null if the phase is not present at all. + */ +function searchPhaseInContent(content, escapedPhase, phaseNum) { + // Match "## Phase X:", "### Phase X:", or "#### Phase X:" with optional name + const phasePattern = new RegExp( + `#{2,4}\\s*Phase\\s+${escapedPhase}:\\s*([^\\n]+)`, + 'i' + ); + const headerMatch = content.match(phasePattern); + + if (!headerMatch) { + // Fallback: check if phase exists in summary list but missing detail section + const checklistPattern = new RegExp( + `-\\s*\\[[ x]\\]\\s*\\*\\*Phase\\s+${escapedPhase}:\\s*([^*]+)\\*\\*`, + 'i' + ); + const checklistMatch = content.match(checklistPattern); + + if (checklistMatch) { + return { + found: false, + phase_number: phaseNum, + phase_name: checklistMatch[1].trim(), + error: 'malformed_roadmap', + message: `Phase ${phaseNum} exists in summary list but missing "### Phase ${phaseNum}:" detail section. ROADMAP.md needs both formats.` + }; + } + + return null; + } + + const phaseName = headerMatch[1].trim(); + const headerIndex = headerMatch.index; + + // Find the end of this section (next ## or ### phase header, or end of file) + const restOfContent = content.slice(headerIndex); + const nextHeaderMatch = restOfContent.match(/\n#{2,4}\s+Phase\s+\d/i); + const sectionEnd = nextHeaderMatch + ? headerIndex + nextHeaderMatch.index + : content.length; + + const section = content.slice(headerIndex, sectionEnd).trim(); + + // Extract goal if present (supports both **Goal:** and **Goal**: formats) + const goalMatch = section.match(/\*\*Goal(?::\*\*|\*\*:)\s*([^\n]+)/i); + const goal = goalMatch ? goalMatch[1].trim() : null; + + // Extract success criteria as structured array + const criteriaMatch = section.match(/\*\*Success Criteria\*\*[^\n]*:\s*\n((?:\s*\d+\.\s*[^\n]+\n?)+)/i); + const success_criteria = criteriaMatch + ? criteriaMatch[1].trim().split('\n').map(line => line.replace(/^\s*\d+\.\s*/, '').trim()).filter(Boolean) + : []; + + return { + found: true, + phase_number: phaseNum, + phase_name: phaseName, + goal, + success_criteria, + section, + }; +} + function cmdRoadmapGetPhase(cwd, phaseNum, raw) { const roadmapPath = planningPaths(cwd).roadmap; @@ -15,76 +81,32 @@ function cmdRoadmapGetPhase(cwd, phaseNum, raw) { } try { - const content = extractCurrentMilestone(fs.readFileSync(roadmapPath, 'utf-8'), cwd); + const rawContent = fs.readFileSync(roadmapPath, 'utf-8'); + const milestoneContent = extractCurrentMilestone(rawContent, cwd); // Escape special regex chars in phase number, handle decimal const escapedPhase = escapeRegex(phaseNum); - // Match "## Phase X:", "### Phase X:", or "#### Phase X:" with optional name - const phasePattern = new RegExp( - `#{2,4}\\s*Phase\\s+${escapedPhase}:\\s*([^\\n]+)`, - 'i' - ); - const headerMatch = content.match(phasePattern); - - if (!headerMatch) { - // Fallback: check if phase exists in summary list but missing detail section - const checklistPattern = new RegExp( - `-\\s*\\[[ x]\\]\\s*\\*\\*Phase\\s+${escapedPhase}:\\s*([^*]+)\\*\\*`, - 'i' - ); - const checklistMatch = content.match(checklistPattern); - - if (checklistMatch) { - // Phase exists in summary but missing detail section - malformed ROADMAP - output({ - found: false, - phase_number: phaseNum, - phase_name: checklistMatch[1].trim(), - error: 'malformed_roadmap', - message: `Phase ${phaseNum} exists in summary list but missing "### Phase ${phaseNum}:" detail section. ROADMAP.md needs both formats.` - }, raw, ''); - return; - } + // Search the current milestone slice first, then fall back to full roadmap. + // A malformed_roadmap result (checklist-only) from the milestone should not + // block finding a full header match in the wider roadmap content. + const fullContent = stripShippedMilestones(rawContent); + const milestoneResult = searchPhaseInContent(milestoneContent, escapedPhase, phaseNum); + const result = (milestoneResult && !milestoneResult.error) + ? milestoneResult + : searchPhaseInContent(fullContent, escapedPhase, phaseNum) || milestoneResult; + if (!result) { output({ found: false, phase_number: phaseNum }, raw, ''); return; } - const phaseName = headerMatch[1].trim(); - const headerIndex = headerMatch.index; + if (result.error) { + output(result, raw, ''); + return; + } - // Find the end of this section (next ## or ### phase header, or end of file) - const restOfContent = content.slice(headerIndex); - const nextHeaderMatch = restOfContent.match(/\n#{2,4}\s+Phase\s+\d/i); - const sectionEnd = nextHeaderMatch - ? headerIndex + nextHeaderMatch.index - : content.length; - - const section = content.slice(headerIndex, sectionEnd).trim(); - - // Extract goal if present (supports both **Goal:** and **Goal**: formats) - const goalMatch = section.match(/\*\*Goal(?::\*\*|\*\*:)\s*([^\n]+)/i); - const goal = goalMatch ? goalMatch[1].trim() : null; - - // Extract success criteria as structured array - const criteriaMatch = section.match(/\*\*Success Criteria\*\*[^\n]*:\s*\n((?:\s*\d+\.\s*[^\n]+\n?)+)/i); - const success_criteria = criteriaMatch - ? criteriaMatch[1].trim().split('\n').map(line => line.replace(/^\s*\d+\.\s*/, '').trim()).filter(Boolean) - : []; - - output( - { - found: true, - phase_number: phaseNum, - phase_name: phaseName, - goal, - success_criteria, - section, - }, - raw, - section - ); + output(result, raw, result.section); } catch (e) { error('Failed to read ROADMAP.md: ' + e.message); } diff --git a/tests/roadmap-phase-fallback.test.cjs b/tests/roadmap-phase-fallback.test.cjs new file mode 100644 index 000000000..32a044f1d --- /dev/null +++ b/tests/roadmap-phase-fallback.test.cjs @@ -0,0 +1,245 @@ +/** + * GSD Tools Tests - roadmap get-phase fallback to full ROADMAP.md + * + * Covers issue #1634: phases outside the current milestone slice should still + * resolve by falling back to the full ROADMAP.md content. + */ + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +/** + * Helper: write STATE.md with a milestone version so extractCurrentMilestone + * will slice the roadmap to only that milestone's section. + */ +function writeState(tmpDir, version) { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `---\nmilestone: ${version}\n---\n` + ); +} + +describe('roadmap get-phase fallback to full ROADMAP.md (#1634)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('active milestone phase still resolves correctly', () => { + writeState(tmpDir, 'v1.0'); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +## v1.0 Current Release + +### Phase 1: Foundation +**Goal:** Set up project infrastructure + +### Phase 2: API +**Goal:** Build REST API + +## v2.0 Next Release + +### Phase 3: Frontend +**Goal:** Build UI layer +` + ); + + const result = runGsdTools('roadmap get-phase 1', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.equal(output.found, true, 'active milestone phase should be found'); + assert.equal(output.phase_number, '1'); + assert.equal(output.phase_name, 'Foundation'); + assert.equal(output.goal, 'Set up project infrastructure'); + }); + + test('backlog phase outside current milestone resolves via fallback', () => { + writeState(tmpDir, 'v1.0'); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +## v1.0 Current Release + +### Phase 1: Foundation +**Goal:** Set up project infrastructure + +## v2.0 Future Release + +### Phase 999.60: Backlog Cleanup +**Goal:** Clean up technical debt from backlog +` + ); + + const result = runGsdTools('roadmap get-phase 999.60', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.equal(output.found, true, 'backlog phase should be found via fallback'); + assert.equal(output.phase_number, '999.60'); + assert.equal(output.phase_name, 'Backlog Cleanup'); + assert.equal(output.goal, 'Clean up technical debt from backlog'); + }); + + test('future planned milestone phase resolves via fallback', () => { + writeState(tmpDir, 'v1.0'); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +## v1.0 Current Release + +### Phase 1: Foundation +**Goal:** Set up project infrastructure + +## v3.0 Planned Milestone + +### Phase 1025: Advanced Analytics +**Goal:** Build analytics dashboard for enterprise customers + +**Success Criteria** (what must be TRUE): + 1. Dashboard renders in under 2s + 2. Supports 10k concurrent users +` + ); + + const result = runGsdTools('roadmap get-phase 1025', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.equal(output.found, true, 'future milestone phase should be found via fallback'); + assert.equal(output.phase_number, '1025'); + assert.equal(output.phase_name, 'Advanced Analytics'); + assert.equal(output.goal, 'Build analytics dashboard for enterprise customers'); + assert.ok(Array.isArray(output.success_criteria), 'success_criteria should be extracted'); + assert.equal(output.success_criteria.length, 2, 'should have 2 criteria'); + }); + + test('truly missing phase still returns found: false', () => { + writeState(tmpDir, 'v1.0'); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +## v1.0 Current Release + +### Phase 1: Foundation +**Goal:** Set up project infrastructure + +## v2.0 Future Release + +### Phase 5: Mobile +**Goal:** Build mobile app +` + ); + + const result = runGsdTools('roadmap get-phase 9999', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.equal(output.found, false, 'truly missing phase should return found: false'); + assert.equal(output.phase_number, '9999'); + }); + + test('backlog checklist-only phase triggers malformed_roadmap via fallback', () => { + writeState(tmpDir, 'v1.0'); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +## v1.0 Current Release + +### Phase 1: Foundation +**Goal:** Set up project infrastructure + +## v2.0 Backlog + +- [ ] **Phase 50: Cleanup** - Remove old code +` + ); + + const result = runGsdTools('roadmap get-phase 50', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.equal(output.found, false, 'checklist-only phase should not be "found"'); + assert.equal(output.error, 'malformed_roadmap', 'should identify malformed roadmap via fallback'); + assert.ok(output.message.includes('missing'), 'should explain the issue'); + }); + + test('checklist in milestone does not block full header match in wider roadmap', () => { + writeState(tmpDir, 'v1.0'); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +## v1.0 Current Release + +### Phase 1: Foundation +**Goal:** Set up project infrastructure + +- [ ] **Phase 50: Cleanup** - referenced in checklist + +## v2.0 Future Release + +### Phase 50: Cleanup +**Goal:** Remove deprecated modules +` + ); + + const result = runGsdTools('roadmap get-phase 50', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.equal(output.found, true, 'full header in v2.0 should win over checklist in v1.0'); + assert.equal(output.phase_name, 'Cleanup'); + assert.equal(output.goal, 'Remove deprecated modules'); + }); + + test('section extraction from fallback includes correct content boundaries', () => { + writeState(tmpDir, 'v1.0'); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +## v1.0 Current Release + +### Phase 1: Foundation +**Goal:** Set up project infrastructure + +## v2.0 Future Release + +### Phase 10: Database +**Goal:** Schema design and migrations + +This phase covers: +- Schema modeling +- Migration tooling +- Seed data + +### Phase 11: Caching +**Goal:** Add Redis caching layer +` + ); + + const result = runGsdTools('roadmap get-phase 10', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.equal(output.found, true, 'phase 10 should be found via fallback'); + assert.ok(output.section.includes('Schema modeling'), 'section includes description'); + assert.ok(output.section.includes('Seed data'), 'section includes all bullets'); + assert.ok(!output.section.includes('Phase 11'), 'section does not include next phase'); + }); +});