diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index 3e035f89..bfcdaad3 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -1334,9 +1334,19 @@ function getRoadmapPhaseInternal(cwd, phaseNum) { try { const content = extractCurrentMilestone(fs.readFileSync(roadmapPath, 'utf-8'), cwd); - const escapedPhase = escapeRegex(phaseNum.toString()); - // Match both numeric (Phase 1:) and custom (Phase PROJ-42:) headers - const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+${escapedPhase}:\\s*([^\\n]+)`, 'i'); + // Strip leading zeros from purely numeric phase numbers so "03" matches "Phase 3:" + // in canonical ROADMAP headings. Non-numeric IDs (e.g. "PROJ-42") are kept as-is. + const normalized = /^\d+$/.test(String(phaseNum)) + ? String(phaseNum).replace(/^0+(?=\d)/, '') + : String(phaseNum); + const escapedPhase = escapeRegex(normalized); + // Match both numeric and custom (Phase PROJ-42:) headers. + // For purely numeric phases allow optional leading zeros so both "Phase 1:" and + // "Phase 01:" are matched regardless of whether the ROADMAP uses padded numbers. + const isNumeric = /^\d+$/.test(String(phaseNum)); + const phasePattern = isNumeric + ? new RegExp(`#{2,4}\\s*Phase\\s+0*${escapedPhase}:\\s*([^\\n]+)`, 'i') + : new RegExp(`#{2,4}\\s*Phase\\s+${escapedPhase}:\\s*([^\\n]+)`, 'i'); const headerMatch = content.match(phasePattern); if (!headerMatch) return null; @@ -1509,6 +1519,50 @@ function getMilestoneInfo(cwd) { try { const roadmap = fs.readFileSync(path.join(planningDir(cwd), 'ROADMAP.md'), 'utf-8'); + // 0. Prefer STATE.md milestone: frontmatter as the authoritative source. + // This prevents falling through to a regex that may match an old heading + // when the active milestone's 🚧 marker is inside a tag without + // **bold** formatting (bug #2409). + let stateVersion = null; + if (cwd) { + try { + const statePath = path.join(planningDir(cwd), 'STATE.md'); + if (fs.existsSync(statePath)) { + const stateRaw = fs.readFileSync(statePath, 'utf-8'); + const m = stateRaw.match(/^milestone:\s*(.+)/m); + if (m) stateVersion = m[1].trim(); + } + } catch { /* intentionally empty */ } + } + + if (stateVersion) { + // Look up the name for this version in ROADMAP.md + const escapedVer = escapeRegex(stateVersion); + // Match heading-format: ## Roadmap v2.9: Name or ## v2.9 Name + const headingMatch = roadmap.match( + new RegExp(`##[^\\n]*${escapedVer}[:\\s]+([^\\n(]+)`, 'i') + ); + if (headingMatch) { + // If the heading line contains ✅ the milestone is already shipped. + // Fall through to normal detection so the NEW active milestone is returned + // instead of the stale shipped one still recorded in STATE.md. + if (!headingMatch[0].includes('✅')) { + return { version: stateVersion, name: headingMatch[1].trim() }; + } + // Shipped milestone — do not early-return; fall through to normal detection below. + } else { + // Match list-format: 🚧 **v2.9 Name** or 🚧 v2.9 Name + const listMatch = roadmap.match( + new RegExp(`🚧\\s*\\*?\\*?${escapedVer}\\s+([^*\\n]+)`, 'i') + ); + if (listMatch) { + return { version: stateVersion, name: listMatch[1].trim() }; + } + // Version found in STATE.md but no name match in ROADMAP — return bare version + return { version: stateVersion, name: 'milestone' }; + } + } + // First: check for list-format roadmaps using 🚧 (in-progress) marker // e.g. "- 🚧 **v2.1 Belgium** — Phases 24-28 (in progress)" // e.g. "- 🚧 **v1.2.1 Tech Debt** — Phases 1-8 (in progress)" @@ -1520,11 +1574,14 @@ function getMilestoneInfo(cwd) { }; } - // Second: heading-format roadmaps — strip shipped milestones in
blocks + // Second: heading-format roadmaps — strip shipped milestones. + //
blocks are stripped by stripShippedMilestones; heading-format ✅ markers + // are excluded by the negative lookahead below so a stale STATE.md version (or any + // shipped ✅ heading) never wins over the first non-shipped milestone heading. const cleaned = stripShippedMilestones(roadmap); - // Extract version and name from the same ## heading for consistency + // Negative lookahead skips headings that contain ✅ (shipped milestone marker). // Supports 2+ segment versions: v1.2, v1.2.1, v2.0.1, etc. - const headingMatch = cleaned.match(/## .*v(\d+(?:\.\d+)+)[:\s]+([^\n(]+)/); + const headingMatch = cleaned.match(/## (?!.*✅).*v(\d+(?:\.\d+)+)[:\s]+([^\n(]+)/); if (headingMatch) { return { version: 'v' + headingMatch[1], diff --git a/tests/core.test.cjs b/tests/core.test.cjs index 16213535..fde86fc1 100644 --- a/tests/core.test.cjs +++ b/tests/core.test.cjs @@ -600,6 +600,108 @@ describe('getMilestoneInfo', () => { assert.strictEqual(info.version, 'v1.0'); assert.strictEqual(info.name, 'milestone'); }); + + // Bug #2409: getMilestoneInfo must prefer STATE.md milestone: field over regex matching + test('uses STATE.md milestone frontmatter when 🚧 is inside tag without bold (bug #2409)', () => { + // STATE.md says v2.9, ROADMAP has 🚧 v2.9 inside (not bolded) — no bold regex match + const roadmap = [ + '# Milestones', + '', + '- ✅ v2.2 Old Features — shipped 2026-04-03', + '- 🚧 v2.9 Full-Pass Verification', + '', + '
', + '🚧 v2.9 Full-Pass Verification & Bug Fixing — IN PROGRESS', + '', + '## Roadmap v2.9: Full-Pass Verification & Bug Fixing', + '', + '### Phase 1: Verification', + '', + '
', + '', + '## Phase Details (v2.2 — Old Features)', + '', + '### Phase 1: Old Stuff', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '---\nmilestone: v2.9\n---\n\n# State\n' + ); + + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.version, 'v2.9', + 'should return v2.9 from STATE.md, not v2.2 from a stale heading match'); + assert.ok(info.name.includes('Full-Pass') || info.name.includes('Verification'), + `name should reference the v2.9 milestone, got: "${info.name}"`); + }); + + test('STATE.md milestone takes precedence over first ## heading match (bug #2409)', () => { + // ROADMAP with multiple ## headings — without STATE.md anchoring, first match wins + const roadmap = [ + '## Phase Details (v1.5–v2.1)', + '', + '## Roadmap v2.2: Old Milestone', + '', + '## Roadmap v2.9: Current Milestone', + '', + '### Phase 1: Alpha', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '---\nmilestone: v2.9\n---\n\n# State\n' + ); + + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.version, 'v2.9', + 'should read v2.9 from STATE.md, not v2.2 from first ## heading'); + assert.strictEqual(info.name, 'Current Milestone'); + }); + + // Bug found in code review of PR #2458: stateVersion early-return doesn't check if shipped + test('falls through to new active milestone when STATE.md version is already shipped (✅ heading)', () => { + // STATE.md still says v1.0 (stale), but v1.0 is marked ✅ in ROADMAP.md. + // getMilestoneInfo must NOT return v1.0; it must fall through and detect v2.0. + const roadmap = [ + '## v1.0 ✅ Initial Release: Done', + '', + '### Phase 1: Setup', + '', + '## v2.0: Active Milestone', + '', + '### Phase 2: Build', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '---\nmilestone: v1.0\n---\n\n# State\n' + ); + + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.version, 'v2.0', + 'should return v2.0 (active milestone), not v1.0 (stale shipped milestone from STATE.md)'); + assert.strictEqual(info.name, 'Active Milestone'); + }); + + test('falls through when STATE.md version matches ✅ heading in alternate position formats', () => { + // ✅ can appear before the version: ## ✅ v1.0 Old Name + const roadmap = [ + '## ✅ v1.0 Old Name', + '', + '## v2.0: New Stuff', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '---\nmilestone: v1.0\n---\n\n# State\n' + ); + + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.version, 'v2.0', + 'should return v2.0, not stale v1.0 with ✅ prefix in heading'); + assert.strictEqual(info.name, 'New Stuff'); + }); }); // ─── searchPhaseInDir ────────────────────────────────────────────────────────── @@ -790,6 +892,41 @@ describe('getRoadmapPhaseInternal', () => { // Should not include Phase 2 content assert.ok(!result.section.includes('Phase 2: API')); }); + + // Bug #2391: zero-padded phase numbers ("03") must match unpadded ROADMAP headings ("Phase 3:") + test('matches zero-padded phase number against unpadded ROADMAP heading (bug #2391)', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '### Phase 3: Rotation Engine\n**Goal**: Build rotation\n**Requirements**: ROTA-01\n' + ); + const result = getRoadmapPhaseInternal(tmpDir, '03'); + assert.ok(result !== null, 'should find the phase with zero-padded input "03"'); + assert.strictEqual(result.found, true); + assert.strictEqual(result.phase_name, 'Rotation Engine'); + assert.strictEqual(result.goal, 'Build rotation'); + }); + + test('matches double-zero-padded phase number against unpadded ROADMAP heading (bug #2391)', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '### Phase 7: Final\n**Goal**: Ship it\n' + ); + const result = getRoadmapPhaseInternal(tmpDir, '007'); + assert.ok(result !== null, 'should find the phase with "007"'); + assert.strictEqual(result.found, true); + assert.strictEqual(result.phase_name, 'Final'); + }); + + test('unpadded lookup still works after fix (regression check)', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '### Phase 3: Rotation Engine\n**Goal**: Build rotation\n' + ); + const result = getRoadmapPhaseInternal(tmpDir, '3'); + assert.ok(result !== null); + assert.strictEqual(result.found, true); + assert.strictEqual(result.phase_name, 'Rotation Engine'); + }); }); // ─── getMilestonePhaseFilter ──────────────────────────────────────────────────── diff --git a/tests/init.test.cjs b/tests/init.test.cjs index 96307891..d5b75ca7 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -422,6 +422,66 @@ describe('init commands ignore archived phases from prior milestones sharing a n }); }); +// ───────────────────────────────────────────────────────────────────────────── +// Bug #2391: zero-padded phase numbers must not bypass archived-phase guard +// ───────────────────────────────────────────────────────────────────────────── + +describe('init plan-phase zero-padded phase number (bug #2391)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + // Current milestone ROADMAP has Phase 3 (unpadded heading) + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '# v2.0 Roadmap\n\n### Phase 3: Rotation Engine + Availability\n**Goal**: Rotation\n**Requirements**: ROTA-01, ROTA-02\n**Plans:** TBD\n' + ); + // Prior milestone archive has a shipped Phase 3 with different content + const archivedDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '03-plant-collection-and-rooms'); + fs.mkdirSync(archivedDir, { recursive: true }); + fs.writeFileSync(path.join(archivedDir, '03-CONTEXT.md'), '# OLD v1.0 Phase 3 context'); + fs.writeFileSync(path.join(archivedDir, '03-RESEARCH.md'), '# OLD v1.0 Phase 3 research'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('zero-padded "03" returns current ROADMAP phase, not archived v1.0 phase', () => { + const result = runGsdTools('init plan-phase 03', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_found, true); + assert.strictEqual(output.phase_name, 'Rotation Engine + Availability', + 'phase_name must come from current ROADMAP.md, not the archived v1.0 phase'); + assert.strictEqual(output.phase_dir, null, + 'phase_dir must be null — current milestone has no directory yet'); + assert.strictEqual(output.has_context, false, + 'has_context must not inherit archived v1.0 artifacts'); + assert.strictEqual(output.has_research, false, + 'has_research must not inherit archived v1.0 artifacts'); + assert.ok(!output.context_path || !output.context_path.includes('v1.0'), + 'context_path must not point at archived v1.0 file'); + assert.strictEqual(output.phase_req_ids, 'ROTA-01, ROTA-02'); + }); + + test('unpadded "3" and zero-padded "03" return identical phase identity', () => { + const result3 = runGsdTools('init plan-phase 3', tmpDir); + const result03 = runGsdTools('init plan-phase 03', tmpDir); + assert.ok(result3.success && result03.success, 'both commands must succeed'); + + const out3 = JSON.parse(result3.output); + const out03 = JSON.parse(result03.output); + assert.strictEqual(out03.phase_name, out3.phase_name, + 'phase_name must be identical regardless of padding'); + assert.strictEqual(out03.phase_slug, out3.phase_slug, + 'phase_slug must be identical regardless of padding'); + assert.strictEqual(out03.phase_req_ids, out3.phase_req_ids, + 'phase_req_ids must be identical regardless of padding'); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // cmdInitTodos (INIT-01) // ─────────────────────────────────────────────────────────────────────────────