diff --git a/.changeset/proud-ravens-jump.md b/.changeset/proud-ravens-jump.md new file mode 100644 index 000000000..94beacdfd --- /dev/null +++ b/.changeset/proud-ravens-jump.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2229 +--- +**`phase complete` no longer checks the wrong ROADMAP checkbox or writes the plan count into a shipped milestone** — the roadmap mutators ran unanchored and un-milestone-scoped, so they could flip a bullet inside a backticked prose literal or a Backlog entry instead of the closing phase's, and write the plan count into a same-numbered phase in a shipped milestone. The checkbox flip is now line-anchored and both writers are scoped to the current milestone. (#2200) diff --git a/src/phase.cts b/src/phase.cts index 8bdc40cae..57f23c655 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -44,7 +44,7 @@ import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal, getArchivedPhaseDirs } = phaseLocatorMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- roadmap-parser.cjs is an export= CommonJS module import roadmapParserMod = require('./roadmap-parser.cjs'); -const { stripShippedMilestones, extractCurrentMilestone, getMilestonePhaseFilter } = roadmapParserMod; +const { stripShippedMilestones, extractCurrentMilestone, getMilestonePhaseFilter, currentMilestoneRawRanges } = roadmapParserMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-workspace.cjs is an export= CommonJS module import planningWorkspace = require('./planning-workspace.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports -- frontmatter.cjs is an export= CommonJS module @@ -1475,13 +1475,13 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { // so completing an already-checked phase (idempotent re-run) checked the // wrong phase's box. Mirrors the tight pattern used by phase-insert // (`]\\s*(?:\\*\\*)?Phase`). + // #2067/#2200: line-anchored (^ + m flag, optional leading indent) so an + // inline / backticked prose literal cannot match. Milestone-scoped below + // (mutateMilestonePhase) so a Backlog entry or a same-numbered shipped- + // milestone phase cannot be flipped either. const checkboxPattern = new RegExp( - `(-\\s*\\[)[ ](\\]\\s*(?:\\*\\*)?\\s*Phase\\s+${phaseEscaped}${OPTIONAL_PHASE_TAG_SOURCE}[:\\s][^\\n]*)`, - 'i', - ); - roadmapContent = roadmapContent.replace( - checkboxPattern, - `$1x$2 (completed ${today})`, + `^[ \\t]*(-\\s*\\[)[ ](\\]\\s*(?:\\*\\*)?\\s*Phase\\s+${phaseEscaped}${OPTIONAL_PHASE_TAG_SOURCE}[:\\s][^\\n]*)`, + 'im', ); const tableRowPattern = new RegExp( @@ -1522,21 +1522,47 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { `(#{2,4}\\s*Phase\\s+${phaseEscaped}(?:(?!\\n#{1,4}\\s)[\\s\\S])*?\\*\\*Plans:\\*\\*\\s*)[^\\n]+`, 'i', ); - roadmapContent = roadmapContent.replace( - planCountPattern, - `$1${summaryCount}/${planCount} plans complete`, - ); const phaseInfoSummaries = phaseInfo['summaries'] as string[]; - for (const summaryFile of phaseInfoSummaries) { - const planId = summaryFile.replace('-SUMMARY.md', '').replace('SUMMARY.md', ''); - if (!planId) continue; - const planEscaped = escapeRegex(planId); - const planCheckboxPattern = new RegExp( - `(-\\s*\\[) (\\]\\s*(?:\\*\\*)?${planEscaped}(?:\\*\\*)?)`, - 'i', - ); - roadmapContent = (roadmapContent).replace(planCheckboxPattern, '$1x$2'); + + // #2200: apply the phase-checkbox flip, the plan-count write, and the + // per-plan checkbox flips ONLY within the current milestone's region(s) + // (primary section + optional Phase Details section). A bullet/heading in + // a shipped milestone, a Backlog section, or a backticked prose literal is + // outside the window and stays untouched. With no versioned active + // milestone, fall back to whole-content mutation (prior behaviour). + const mutateMilestonePhase = (slice: string): string => { + let s = slice; + s = s.replace(checkboxPattern, `$1x$2 (completed ${today})`); + s = s.replace(planCountPattern, `$1${summaryCount}/${planCount} plans complete`); + for (const summaryFile of phaseInfoSummaries) { + const planId = summaryFile.replace('-SUMMARY.md', '').replace('SUMMARY.md', ''); + if (!planId) continue; + const planEscaped = escapeRegex(planId); + const planCheckboxPattern = new RegExp( + `(-\\s*\\[) (\\]\\s*(?:\\*\\*)?${planEscaped}(?:\\*\\*)?)`, + 'i', + ); + s = s.replace(planCheckboxPattern, '$1x$2'); + } + return s; + }; + + const milestoneRanges = currentMilestoneRawRanges(roadmapContent, cwd); + if (milestoneRanges) { + // Splice later windows first so an earlier window's offsets are not + // shifted by a length-changing mutation in a later window. + const windows = [milestoneRanges.details, milestoneRanges.primary] + .filter((w): w is { start: number; end: number } => w !== null) + .sort((a, b) => b.start - a.start); + for (const w of windows) { + roadmapContent = + roadmapContent.slice(0, w.start) + + mutateMilestonePhase(roadmapContent.slice(w.start, w.end)) + + roadmapContent.slice(w.end); + } + } else { + roadmapContent = mutateMilestonePhase(roadmapContent); } writes.push({ diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index e107a84f2..cfa7d4c69 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -556,6 +556,92 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p return isDirInMilestone as MilestonePhaseFilter; } +/** + * #2200: raw [start,end) offsets of the current milestone's region(s) in ROADMAP + * content, for scoping write-path mutations (phase-checkbox flip, Plans-count + * writer) so they cannot touch a backticked prose literal, a Backlog entry, or a + * same-numbered phase in a shipped milestone. + * + * Mirrors the region selection in `extractCurrentMilestone` (version detection → + * active heading → next milestone boundary → optional Phase Details section). + * Returns null when there is no versioned active milestone; callers then fall + * back to whole-content mutation (the prior behaviour). + * + * NOTE: keep the region logic here in sync with extractCurrentMilestone. + */ +function currentMilestoneRawRanges( + content: string, + cwd?: string, +): { primary: { start: number; end: number }; details: { start: number; end: number } | null } | null { + if (!cwd) return null; + + let version: string | null = null; + try { + const statePath = path.join(planningDir(cwd), 'STATE.md'); + const stateRaw = platformReadSync(statePath); + if (stateRaw !== null) { + const milestoneMatch = stateRaw.match(/^milestone:\s*(.+)/m); + if (milestoneMatch) version = milestoneMatch[1].trim(); + } + } catch { /* ignore */ } + if (!version) { + const inProgressMatch = content.match(/(?:🚧|🔄)\s*\*\*v(\d+\.\d+)\s/); + if (inProgressMatch) version = 'v' + inProgressMatch[1]; + } + if (!version) return null; + + const escapedVersion = escapeRegex(version); + const sectionPattern = new RegExp( + `(^#{1,3}\\s+(?!Phase\\s+\\S).*${escapedVersion}\\b[^\\n]*)`, + 'gmi', + ); + const headingMatches = [...content.matchAll(sectionPattern)]; + if (headingMatches.length === 0) return null; + + const closedMarkerPattern = /\b(?:CLOSED|ARCHIVED|ABANDONED|SHIPPED|FAILED)\b|✅|🗄/i; + const activeMarkerPattern = /\b(?:STARTED|ACTIVE|WIP)\b|in\s+progress|🚧|🔄/i; + const isClosed = (h: string) => closedMarkerPattern.test(h) && !activeMarkerPattern.test(h); + const firstMatch = headingMatches[0]; + const selected = headingMatches.find((m) => !isClosed(m[1])) || firstMatch; + const sectionStart = selected.index ?? 0; + + const computeSectionEnd = (headingText: string, headingStart: number): number => { + const level = (headingText.match(/^(#{1,3})\s/) ?? ['', '#'])[1].length; + const afterHeading = headingStart + headingText.length; + for (const h of tokenizeHeadings(content)) { + if (h.offset <= headingStart) continue; + if (h.offset < afterHeading) continue; + if (h.level > level) continue; + if (/^Phase\s+\S/i.test(h.text)) continue; + if (!/v\d+\.\d+|✅|📋|🚧/i.test(h.text)) continue; + return h.offset; + } + return content.length; + }; + const sectionEnd = computeSectionEnd(selected[0], sectionStart); + + const selectedVersionToken = selected[1].match( + /v\d+(?:\.\d+)+(?:[-.][A-Za-z0-9]+)*/i, + )?.[0]; + const detailsVersionBoundary = selectedVersionToken + ? new RegExp(`${escapeRegex(selectedVersionToken)}(?![\\w.-])`, 'i') + : null; + const detailsMatch = headingMatches.find( + (m) => + /\(Phase\s+Details\)/i.test(m[1]) && + !isClosed(m[1]) && + (!detailsVersionBoundary || detailsVersionBoundary.test(m[1])) && + (m.index ?? 0) >= sectionEnd, + ); + let details: { start: number; end: number } | null = null; + if (detailsMatch) { + const detailsStart = detailsMatch.index ?? 0; + details = { start: detailsStart, end: computeSectionEnd(detailsMatch[0], detailsStart) }; + } + + return { primary: { start: sectionStart, end: sectionEnd }, details }; +} + export = { stripShippedMilestones, extractCurrentMilestone, @@ -563,4 +649,5 @@ export = { getRoadmapPhaseInternal, getMilestoneInfo, getMilestonePhaseFilter, + currentMilestoneRawRanges, }; diff --git a/tests/roadmap-parser.test.cjs b/tests/roadmap-parser.test.cjs index f7bb1510a..493cc9c84 100644 --- a/tests/roadmap-parser.test.cjs +++ b/tests/roadmap-parser.test.cjs @@ -1965,6 +1965,59 @@ describe('feat-3594: roadmap parser does not crash on ANY corpus fixture', () => }); } +// ─── #2200: currentMilestoneRawRanges scopes phase-complete writes ──────────── +// The phase-complete roadmap mutators (checkbox-flip + Plans writer) must mutate +// only within the active milestone so they cannot touch a backticked prose +// literal, a Backlog entry, or a same-numbered phase in a shipped milestone. +// This tests the scoping helper the fix rests on (the mutators' command path is +// covered by the existing phase-complete suite; gsd-test confirms no regression). +{ + const { describe: d3, test: t3 } = require('node:test'); + const a3 = require('node:assert/strict'); + const fs3 = require('node:fs'); + const path3 = require('node:path'); + const { createTempProject: ctp3, cleanup: cu3 } = require('./helpers.cjs'); + const rp3 = require('../gsd-core/bin/lib/roadmap-parser.cjs'); + + d3('#2200 currentMilestoneRawRanges — scopes writes to the active milestone', () => { + t3('the active window contains the active phase bullet, excludes Backlog + prose + shipped', () => { + const tmpDir = ctp3('fix-2200-'); + try { + // Shipped milestone (in a
block) + Backlog + a backticked prose + // literal all come BEFORE the active milestone — the typical layout. + const roadmap = [ + '# Roadmap', '', + '## Backlog', '- [ ] **Phase 1: Some Future Idea**', '', + '> See `- [ ] **Phase 1: Alpha**` in the active milestone.', '', + '
✅ v0.9 Old', + '- [x] **Phase 1: Legacy**', + '### Phase 1: Legacy', + '**Plans:** 9/9 plans complete', + '
', '', + '## v1.0 — Active', '', + '- [ ] **Phase 1: Alpha**', '', + '### Phase 1: Alpha', + '**Plans:** 0/1 plans complete', '', + ].join('\n'); + fs3.writeFileSync(path3.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs3.writeFileSync(path3.join(tmpDir, '.planning', 'STATE.md'), '---\nmilestone: v1.0\ncurrent_phase: 1\n---\n'); + const ranges = rp3.currentMilestoneRawRanges(roadmap, tmpDir); + a3.ok(ranges, 'a versioned active milestone must yield ranges'); + const primary = roadmap.slice(ranges.primary.start, ranges.primary.end); + a3.ok(primary.includes('- [ ] **Phase 1: Alpha**'), 'active phase bullet is inside the window'); + a3.ok(!primary.includes('Some Future Idea'), 'a Backlog entry is outside the active window'); + a3.ok(!primary.includes('See `- [ ]'), 'a backticked prose literal is outside the active window'); + a3.ok(!primary.includes('9/9 plans complete'), 'a shipped milestone plan-count line is outside the active window'); + } finally { + cu3(tmpDir); + } + }); + + t3('returns null without a versioned active milestone (whole-content fallback)', () => { + a3.strictEqual(rp3.currentMilestoneRawRanges('# Roadmap\n- [ ] **Phase 1: X**\n', undefined), null); + }); + }); +} // ─── #2199: bullet/em-dash ROADMAP phase resolution ─────────────────────────── // Self-contained block: phase lookup + milestone filter must accept bullet/ // checkbox entries with an em-dash/en-dash/hyphen/colon separator, not just the