From a7f0af2ce90bd54519007eea91ba6122c3b81352 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 14 May 2026 23:17:38 -0400 Subject: [PATCH] fix(3537): route every phase-number ROADMAP regex through phaseMarkdownRegexSource (#3538) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(3537): route every phase-number ROADMAP regex through phaseMarkdownRegexSource v1.42.1 added the padding-tolerant `phaseMarkdownRegexSource()` helper but wired it into only 1 of 8 call sites that build phase-number regexes against ROADMAP/STATE prose. The other 7 used raw `escapeRegex(phaseNum)` or partial `0*${escapeRegex(...)}` (tolerated extra padding, not missing), so when skills passed the resolved padded form (`02.7`) against un-padded ROADMAP prose (`### Phase 2.7:`, `- [ ] **Phase 2.7:**`), the verbs silently no-op'd while reporting success. This consolidates every phase-number ROADMAP/STATE regex through the canonical helper: - Promote `phaseMarkdownRegexSource` from `roadmap.cjs` to `core.cjs` so `phase.cjs` and `core.cjs` itself can consume it (no circular dep — both already import `core.cjs`). - Wire the helper into the 7 remaining sites: - `core.cjs:getRoadmapPhaseInternal` (replaces hand-rolled `isNumeric` branch that only padded integers, not decimals). - `roadmap.cjs:cmdRoadmapGetPhase` (searchPhaseInContent escapedPhase). - `roadmap.cjs:cmdRoadmapAnalyze` checkbox lookup. - `roadmap.cjs:cmdRoadmapAnnotateDependencies` phase header lookup. - `phase.cjs:cmdPhaseNextDecimal` ROADMAP prose scan. - `phase.cjs:cmdPhaseInsert` target anchor + decimal scan + header. - `phase.cjs:cmdPhaseComplete` (3 regexes: checkbox, plan-count, REQUIREMENTS extraction). Adds `tests/bug-3537-padded-id-against-unpadded-roadmap.test.cjs` — a parity-style regression matching CONTEXT.md DEFECT.GENERATIVE-FIX: for each user-facing verb, asserts that the padded form (`02.7`) and the un-padded form (`2.7`) produce identical ROADMAP.md against an identical fixture. Includes one control case (`update-plan-progress`, already wired in 1.42.1) to prove the parity assertion is non-vacuous. Closes #3537 Co-Authored-By: Claude Opus 4.7 (1M context) * chore(3537): add changeset fragment (pr: placeholder, amended post-create) Co-Authored-By: Claude Opus 4.7 (1M context) * chore(3537): pin changeset pr: field to #3538 Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .changeset/3537-phase-regex-fanout.md | 5 + get-shit-done/bin/lib/core.cjs | 46 ++- get-shit-done/bin/lib/phase.cjs | 37 +- get-shit-done/bin/lib/roadmap.cjs | 32 +- ...added-id-against-unpadded-roadmap.test.cjs | 372 ++++++++++++++++++ 5 files changed, 448 insertions(+), 44 deletions(-) create mode 100644 .changeset/3537-phase-regex-fanout.md create mode 100644 tests/bug-3537-padded-id-against-unpadded-roadmap.test.cjs diff --git a/.changeset/3537-phase-regex-fanout.md b/.changeset/3537-phase-regex-fanout.md new file mode 100644 index 000000000..71e6cb595 --- /dev/null +++ b/.changeset/3537-phase-regex-fanout.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3538 +--- +**Phase state verbs no longer silently no-op on projects with `project_code` set and un-padded ROADMAP prose.** v1.42.1 added the padding-tolerant `phaseMarkdownRegexSource()` helper but wired it into only 1 of 8 call sites that build phase-number regexes against ROADMAP/STATE prose; the other 7 used raw `escapeRegex(phaseNum)` or partial `0*${escapeRegex(...)}` (tolerated extra padding, not missing). When skills passed the resolved padded form (`02.7`) against un-padded ROADMAP headings (`### Phase 2.7:`), `phase complete`, `roadmap get-phase`, `roadmap analyze` checkbox detection, `roadmap annotate-dependencies`, `phase next-decimal`, and `phase insert` all silently returned success while ROADMAP.md stayed unchanged. The helper is now promoted to `core.cjs` and routed through every site. Adds a parity-style regression that runs each verb with padded and un-padded ids against an identical fixture and asserts byte-equal output, so the next call-site cannot drift back undetected. Closes #3537. diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index c014de351..0aca97df6 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -726,6 +726,31 @@ function normalizePhaseName(phase) { return str; } +/** + * Render a regex source fragment matching a phase number against ROADMAP/STATE + * prose regardless of zero-padding on either side. Skills pass the resolved + * padded form (`02.7`), but human-authored ROADMAP prose is conventionally + * un-padded (`### Phase 2.7:`); a naive `escapeRegex(phaseNum)` fragment never + * matches when the two diverge. Strips leading zeros from the integer part + * before re-emitting with a `0*` prefix, so the fragment matches both `2.7` + * and `02.7` (and `002.7`). + * + * Falls back to `escapeRegex(phaseNum)` for non-numeric IDs (custom project + * codes like `PROJ-42`) so callers can substitute it unconditionally. + * + * See #3537 — wired into every ROADMAP-prose regex builder. + */ +function phaseMarkdownRegexSource(phaseNum) { + const stripped = String(phaseNum).replace(/^[A-Z]{1,6}-(?=\d)/i, ''); + const match = stripped.match(/^0*(\d+)([A-Z])?((?:\.\d+)*)$/i); + if (!match) return escapeRegex(phaseNum); + + const integer = match[1].replace(/^0+/, '') || '0'; + const letter = match[2] ? escapeRegex(match[2]) : ''; + const decimal = match[3] ? escapeRegex(match[3]) : ''; + return `0*${escapeRegex(integer)}${letter}${decimal}`; +} + function comparePhaseNum(a, b) { // Strip optional project_code prefix before comparing (e.g., 'CK-01-name' → '01-name') const sa = String(a).replace(/^[A-Z]{1,6}-/, ''); @@ -1071,19 +1096,13 @@ function getRoadmapPhaseInternal(cwd, phaseNum) { const roadmapRaw = platformReadSync(roadmapPath); if (roadmapRaw === null) throw new Error('missing'); const content = extractCurrentMilestone(roadmapRaw, cwd); - // 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'); + // #3537: route through canonical padding-tolerant fragment. The prior + // hand-rolled `isNumeric` branch only stripped padding on integer-only + // ids and missed decimal padding (`02.7` against `Phase 2.7:` headings). + const phasePattern = new RegExp( + `#{2,4}\\s*Phase\\s+${phaseMarkdownRegexSource(phaseNum)}:\\s*([^\\n]+)`, + 'i' + ); const headerMatch = content.match(phasePattern); if (!headerMatch) return null; @@ -1880,6 +1899,7 @@ module.exports = { isGitIgnored, escapeRegex, normalizePhaseName, + phaseMarkdownRegexSource, comparePhaseNum, searchPhaseInDir, extractPhaseToken, diff --git a/get-shit-done/bin/lib/phase.cjs b/get-shit-done/bin/lib/phase.cjs index e14be8474..400aa6dcf 100644 --- a/get-shit-done/bin/lib/phase.cjs +++ b/get-shit-done/bin/lib/phase.cjs @@ -4,7 +4,7 @@ const fs = require('fs'); const path = require('path'); -const { escapeRegex, loadConfig, normalizePhaseName, comparePhaseNum, findPhaseInternal, getArchivedPhaseDirs, generateSlugInternal, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, toPosixPath, output, error, readSubdirectories, phaseTokenMatches } = require('./core.cjs'); +const { escapeRegex, loadConfig, normalizePhaseName, phaseMarkdownRegexSource, comparePhaseNum, findPhaseInternal, getArchivedPhaseDirs, generateSlugInternal, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, toPosixPath, output, error, readSubdirectories, phaseTokenMatches } = require('./core.cjs'); const { platformWriteSync, platformReadSync, platformEnsureDir } = require('./shell-command-projection.cjs'); const { planningDir, withPlanningLock } = require('./planning-workspace.cjs'); const { extractFrontmatter } = require('./frontmatter.cjs'); @@ -170,8 +170,10 @@ function cmdPhaseNextDecimal(cwd, basePhase, raw) { if (fs.existsSync(roadmapPath)) { try { const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8'); + // #3537: padding-tolerant on both sides — `0*${escapeRegex(...)}` + // tolerated extra padding but not missing. const phasePattern = new RegExp( - `#{2,4}\\s*Phase\\s+0*${escapeRegex(normalized)}\\.(\\d+)\\s*:`, 'gi' + `#{2,4}\\s*Phase\\s+${phaseMarkdownRegexSource(normalized)}\\.(\\d+)\\s*:`, 'gi' ); let pm; while ((pm = phasePattern.exec(roadmapContent)) !== null) { @@ -691,13 +693,14 @@ function cmdPhaseInsert(cwd, afterPhase, description, raw) { const rawContent = fs.readFileSync(roadmapPath, 'utf-8'); const content = extractCurrentMilestone(rawContent, cwd); - // Normalize input then strip leading zeros for flexible matching + // Normalize input then route through canonical padding-tolerant fragment + // (#3537). The prior hand-rolled `0*${unpadded}` worked for the integer + // base but duplicated logic — funnel it through the shared helper. const normalizedAfter = normalizePhaseName(afterPhase); - const unpadded = normalizedAfter.replace(/^0+/, ''); - const afterPhaseEscaped = unpadded.replace(/\./g, '\\.'); - const targetPattern = new RegExp(`#{2,4}\\s*Phase\\s+0*${afterPhaseEscaped}:`, 'i'); + const afterPhaseEscaped = phaseMarkdownRegexSource(normalizedAfter); + const targetPattern = new RegExp(`#{2,4}\\s*Phase\\s+${afterPhaseEscaped}:`, 'i'); if (!targetPattern.test(content)) { - const checklistPattern = new RegExp(`-\\s*\\[[ x]\\]\\s*\\*\\*Phase\\s+0*${afterPhaseEscaped}:`, 'i'); + const checklistPattern = new RegExp(`-\\s*\\[[ x]\\]\\s*\\*\\*Phase\\s+${afterPhaseEscaped}:`, 'i'); if (checklistPattern.test(content)) { error(`Phase ${afterPhase} exists in roadmap summary but is missing a detail section (### Phase ${afterPhase}: ...).`); } @@ -719,9 +722,11 @@ function cmdPhaseInsert(cwd, afterPhase, description, raw) { } } catch { /* intentionally empty */ } - // Also scan ROADMAP.md content (already loaded) for decimal entries + // Also scan ROADMAP.md content (already loaded) for decimal entries. + // #3537: padding-tolerant fragment so un-padded `Phase 2.7:` is found + // when caller passes the padded base `02`. const rmPhasePattern = new RegExp( - `#{2,4}\\s*Phase\\s+0*${escapeRegex(normalizedBase)}\\.(\\d+)\\s*:`, 'gi' + `#{2,4}\\s*Phase\\s+${phaseMarkdownRegexSource(normalizedBase)}\\.(\\d+)\\s*:`, 'gi' ); let rmMatch; while ((rmMatch = rmPhasePattern.exec(rawContent)) !== null) { @@ -745,7 +750,7 @@ function cmdPhaseInsert(cwd, afterPhase, description, raw) { const phaseEntry = `\n### Phase ${_decimalPhase}: ${description} (INSERTED)\n\n**Goal:** [Urgent work - to be planned]\n**Requirements**: TBD\n**Depends on:** Phase ${afterPhase}\n**Plans:** 0 plans\n\nPlans:\n- [ ] TBD (run /gsd:plan-phase ${_decimalPhase} to break down)\n`; // Insert after the target phase section - const headerPattern = new RegExp(`(#{2,4}\\s*Phase\\s+0*${afterPhaseEscaped}:[^\\n]*\\n)`, 'i'); + const headerPattern = new RegExp(`(#{2,4}\\s*Phase\\s+${afterPhaseEscaped}:[^\\n]*\\n)`, 'i'); const headerMatch = rawContent.match(headerPattern); if (!headerMatch) { error(`Could not find Phase ${afterPhase} header`); @@ -1030,14 +1035,16 @@ function cmdPhaseComplete(cwd, phaseNum, raw) { let roadmapContent = fs.readFileSync(roadmapPath, 'utf-8'); // Checkbox: - [ ] Phase N: → - [x] Phase N: (...completed DATE) + // #3537: padding-tolerant fragment so the caller-resolved padded id + // matches un-padded ROADMAP prose. + const phaseEscaped = phaseMarkdownRegexSource(phaseNum); const checkboxPattern = new RegExp( - `(-\\s*\\[)[ ](\\]\\s*.*Phase\\s+${escapeRegex(phaseNum)}[:\\s][^\\n]*)`, + `(-\\s*\\[)[ ](\\]\\s*.*Phase\\s+${phaseEscaped}[:\\s][^\\n]*)`, 'i' ); roadmapContent = roadmapContent.replace(checkboxPattern, `$1x$2 (completed ${today})`); // Progress table: update Status to Complete, add date (handles 4 or 5 column tables) - const phaseEscaped = escapeRegex(phaseNum); const tableRowPattern = new RegExp( `^(\\|\\s*${phaseEscaped}\\.?\\s[^|]*(?:\\|[^\\n]*))$`, 'im' @@ -1093,8 +1100,10 @@ function cmdPhaseComplete(cwd, phaseNum, raw) { // Update REQUIREMENTS.md traceability for this phase's requirements const reqPath = path.join(planningDir(cwd), 'REQUIREMENTS.md'); if (fs.existsSync(reqPath)) { - // Extract the current phase section from roadmap (scoped to avoid cross-phase matching) - const phaseEsc = escapeRegex(phaseNum); + // Extract the current phase section from roadmap (scoped to avoid cross-phase matching). + // #3537: padding-tolerant fragment so an un-padded `Phase 2.7:` heading + // is found when caller resolved to padded `02.7`. + const phaseEsc = phaseMarkdownRegexSource(phaseNum); const currentMilestoneRoadmap = extractCurrentMilestone(roadmapContent, cwd); const phaseSectionMatch = currentMilestoneRoadmap.match( new RegExp(`(#{2,4}\\s*Phase\\s+${phaseEsc}[:\\s][\\s\\S]*?)(?=#{2,4}\\s*Phase\\s+|$)`, 'i') diff --git a/get-shit-done/bin/lib/roadmap.cjs b/get-shit-done/bin/lib/roadmap.cjs index edf7603ee..be45f5308 100644 --- a/get-shit-done/bin/lib/roadmap.cjs +++ b/get-shit-done/bin/lib/roadmap.cjs @@ -4,7 +4,7 @@ const fs = require('fs'); const path = require('path'); -const { escapeRegex, normalizePhaseName, output, error, findPhaseInternal, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, phaseTokenMatches } = require('./core.cjs'); +const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, output, error, findPhaseInternal, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, phaseTokenMatches } = require('./core.cjs'); const { platformWriteSync } = require('./shell-command-projection.cjs'); const { planningPaths, withPlanningLock } = require('./planning-workspace.cjs'); const scanPhasePlans = require('./plan-scan.cjs'); @@ -52,16 +52,8 @@ function countPhasePlansAndSummaries(phaseDir) { }; } -function phaseMarkdownRegexSource(phaseNum) { - const stripped = String(phaseNum).replace(/^[A-Z]{1,6}-(?=\d)/i, ''); - const match = stripped.match(/^0*(\d+)([A-Z])?((?:\.\d+)*)$/i); - if (!match) return escapeRegex(phaseNum); - - const integer = match[1].replace(/^0+/, '') || '0'; - const letter = match[2] ? escapeRegex(match[2]) : ''; - const decimal = match[3] ? escapeRegex(match[3]) : ''; - return `0*${escapeRegex(integer)}${letter}${decimal}`; -} +// `phaseMarkdownRegexSource` moved to core.cjs (#3537) so phase.cjs and +// core.cjs itself can consume it without circular deps. Imported above. /** * Search for a phase header (and its section) within the given content string. @@ -147,8 +139,9 @@ function cmdRoadmapGetPhase(cwd, phaseNum, raw) { 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); + // #3537: padding-tolerant fragment so callers passing `02.7` still match + // un-padded ROADMAP prose (`### Phase 2.7:`). + const escapedPhase = phaseMarkdownRegexSource(phaseNum); // Search the current milestone slice first, then fall back to full roadmap. // A malformed_roadmap result (checklist-only) from the milestone should not @@ -248,8 +241,11 @@ function cmdRoadmapAnalyze(cwd, raw) { } } catch { /* intentionally empty */ } - // Check ROADMAP checkbox status - const checkboxPattern = new RegExp(`-\\s*\\[(x| )\\]\\s*.*Phase\\s+${escapeRegex(phaseNum)}[:\\s]`, 'i'); + // Check ROADMAP checkbox status. + // #3537: padding-tolerant fragment — the heading discovered above may use + // a different padding than the summary-bullet checkbox below it (mixed + // padding inside one ROADMAP is legal and seen in real projects). + const checkboxPattern = new RegExp(`-\\s*\\[(x| )\\]\\s*.*Phase\\s+${phaseMarkdownRegexSource(phaseNum)}[:\\s]`, 'i'); const checkboxMatch = content.match(checkboxPattern); const roadmapComplete = checkboxMatch ? checkboxMatch[1] === 'x' : false; @@ -511,8 +507,10 @@ function cmdRoadmapAnnotateDependencies(cwd, phaseNum, raw) { withPlanningLock(cwd, () => { let content = fs.readFileSync(roadmapPath, 'utf-8'); - // Find the phase section - const phaseEscaped = escapeRegex(phaseNum); + // Find the phase section. + // #3537: padding-tolerant fragment so the caller's resolved padded id + // matches un-padded ROADMAP headings. + const phaseEscaped = phaseMarkdownRegexSource(phaseNum); const phaseHeaderPattern = new RegExp(`(#{2,4}\\s*Phase\\s+${phaseEscaped}:[^\\n]*)`, 'i'); const phaseMatch = content.match(phaseHeaderPattern); if (!phaseMatch) return; diff --git a/tests/bug-3537-padded-id-against-unpadded-roadmap.test.cjs b/tests/bug-3537-padded-id-against-unpadded-roadmap.test.cjs new file mode 100644 index 000000000..54a06b6c4 --- /dev/null +++ b/tests/bug-3537-padded-id-against-unpadded-roadmap.test.cjs @@ -0,0 +1,372 @@ +/** + * Regression tests for bug #3537 + * + * Phase state verbs must match a canonical phase id against ROADMAP.md prose + * regardless of zero-padding on either side: the skills pass the padded form + * (`02.7`) after resolving the phase directory, but human-authored ROADMAP + * prose is conventionally un-padded (`### Phase 2.7:`, `- [ ] **Phase 2.7:**`). + * + * v1.42.1 added `phaseMarkdownRegexSource()` which renders `0*<...>` + * — padding-tolerant on both sides — but wired it into only 1 of 8 call sites. + * The other 7 used raw `escapeRegex(phaseNum)` or `0*${escapeRegex(...)}` + * (tolerated extra padding, not missing), so passing the padded form silently + * no-op'd and the verbs returned success while ROADMAP.md was unchanged. + * + * Parity assertion (per CONTEXT.md DEFECT.GENERATIVE-FIX): for each verb, + * running with the padded form must produce the same ROADMAP.md as running + * with the un-padded form against an identical fixture. Per-site fixes + * without a parity test let the next call-site drift back undetected. + */ + +'use strict'; + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); +const { execFileSync } = require('node:child_process'); + +const gsdTools = path.resolve(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools.cjs'); + +function run(args, cwd) { + try { + return { + stdout: execFileSync('node', [gsdTools, ...args], { + cwd, + timeout: 15000, + encoding: 'utf-8', + }), + ok: true, + }; + } catch (e) { + return { + stdout: (e.stdout && e.stdout.toString()) || '', + stderr: (e.stderr && e.stderr.toString()) || '', + ok: false, + code: e.status, + }; + } +} + +/** + * Build a planning fixture with project_code='CK', padded phase directory + * (`CK-02.7-meta-lead-ads/`), and un-padded ROADMAP prose (`Phase 2.7`). + * This mirrors the reporter's environment in #3537 exactly. + */ +function setupFixture(tmpDir, opts = {}) { + const { + projectCode = 'CK', + paddedId = '02.7', + unpaddedId = '2.7', + extraPhases = [], + } = opts; + + const planningDir = path.join(tmpDir, '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); + + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ project_code: projectCode }) + ); + + fs.writeFileSync( + path.join(planningDir, 'STATE.md'), + `---\ncurrent_phase: ${unpaddedId}\nstatus: executing\n---\n# State\n` + ); + + // Padded phase directory with one plan + matching summary so the phase + // is "complete" for phase-complete and update-plan-progress verbs. + const phaseDirName = `${projectCode}-${paddedId}-meta-lead-ads`; + const phaseDir = path.join(planningDir, 'phases', phaseDirName); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync( + path.join(phaseDir, `${paddedId}-01-PLAN.md`), + `---\nphase: ${unpaddedId}\nplan: 1\nwave: 1\n---\n# Plan 1\n` + ); + fs.writeFileSync( + path.join(phaseDir, `${paddedId}-01-SUMMARY.md`), + '---\nstatus: complete\n---\n# Summary\nDone.' + ); + + const extra = extraPhases + .map((p) => `- [ ] **Phase ${p.id}: ${p.name}**`) + .join('\n'); + + const roadmap = [ + '# Roadmap', + '', + '## v1.0 Milestone', + '', + `- [ ] **Phase ${unpaddedId}: Meta Lead Ads**`, + extra, + '', + '## Progress', + '', + '| Phase | Plans | Status | Completed |', + '|-------|-------|--------|-----------|', + `| ${unpaddedId} Meta Lead Ads | 0/1 | Planned | - |`, + '', + `### Phase ${unpaddedId}: Meta Lead Ads`, + '', + '**Goal:** ship the thing', + '**Plans:** 0 plans', + '', + 'Plans:', + `- [ ] ${paddedId}-01-PLAN.md`, + '', + ...extraPhases.flatMap((p) => [ + `### Phase ${p.id}: ${p.name}`, + '', + '**Goal:** stub', + '**Plans:** 0 plans', + '', + 'Plans:', + `- [ ] ${p.id}-01-PLAN.md`, + '', + ]), + ] + .filter((l) => l !== '') + .join('\n') + '\n'; + + fs.writeFileSync(path.join(planningDir, 'ROADMAP.md'), roadmap); + + return { + planningDir, + roadmapPath: path.join(planningDir, 'ROADMAP.md'), + phaseDir, + }; +} + +/** + * Run a verb in two parallel fixtures — one passing the padded form, one + * passing the un-padded form — then compare the resulting ROADMAP.md bytes. + * Any divergence means the verb's regex did not tolerate padding on at least + * one side. + */ +function expectParity({ verbWithPadded, verbWithUnpadded, fixtureOpts }) { + const tmpA = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3537-A-')); + const tmpB = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3537-B-')); + try { + const a = setupFixture(tmpA, fixtureOpts); + const b = setupFixture(tmpB, fixtureOpts); + + const ra = verbWithPadded(tmpA); + const rb = verbWithUnpadded(tmpB); + + const aRoadmap = fs.readFileSync(a.roadmapPath, 'utf-8'); + const bRoadmap = fs.readFileSync(b.roadmapPath, 'utf-8'); + + return { aRoadmap, bRoadmap, ra, rb }; + } finally { + fs.rmSync(tmpA, { recursive: true, force: true }); + fs.rmSync(tmpB, { recursive: true, force: true }); + } +} + +describe('bug #3537: phase verbs accept padded ids against un-padded ROADMAP prose', () => { + test('phase complete: padded 02.7 and un-padded 2.7 produce identical ROADMAP', () => { + const { aRoadmap, bRoadmap } = expectParity({ + fixtureOpts: {}, + verbWithPadded: (cwd) => run(['phase', 'complete', '02.7'], cwd), + verbWithUnpadded: (cwd) => run(['phase', 'complete', '2.7'], cwd), + }); + + assert.equal( + aRoadmap, + bRoadmap, + 'padded `02.7` must mutate ROADMAP identically to un-padded `2.7`' + ); + // And the canonical mutation must have actually happened (otherwise + // both forms could be silently no-op'ing and still produce identical + // output — a vacuous parity pass). + assert.match( + aRoadmap, + /- \[x\] \*\*Phase 2\.7:/, + 'overview checkbox should be flipped under both invocations' + ); + }); + + test('roadmap get-phase: padded 02.7 returns the same section as un-padded 2.7', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3537-get-')); + try { + setupFixture(tmp, {}); + const padded = run(['roadmap', 'get-phase', '02.7', '--raw'], tmp); + const unpadded = run(['roadmap', 'get-phase', '2.7', '--raw'], tmp); + + assert.equal( + padded.stdout, + unpadded.stdout, + 'padded and un-padded ids must return identical sections' + ); + // Non-vacuous guard: both forms must have actually returned a section + // (the bug we're fixing was that the padded form returned an empty + // string while reporting success). + assert.ok( + padded.stdout.trim().length > 0, + 'verb must return non-empty section under both invocations' + ); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('phase next-decimal: padded 02 finds decimals in un-padded ROADMAP', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3537-nd-')); + try { + setupFixture(tmp, { + paddedId: '02.7', + unpaddedId: '2.7', + }); + + // Padded base `02` must discover the existing decimal `2.7` from the + // un-padded heading and propose `2.8` (or higher) as next. + const padded = run(['phase', 'next-decimal', '02', '--raw'], tmp); + const unpadded = run(['phase', 'next-decimal', '2', '--raw'], tmp); + + assert.equal( + padded.stdout, + unpadded.stdout, + 'next-decimal must produce identical JSON for padded and un-padded base' + ); + + // Sanity: the existing 2.7 must be reflected. If the prose-scan regex + // silently failed to match `Phase 2.7`, the result would skip 2.7 and + // wrongly propose 2.1 as next. `phase next-decimal --raw` emits the + // next id as plain text (`02.8`), so trimmed string equality is the + // typed assertion shape (no raw-text regex matching — lint policy). + const nextDecimalPadded = padded.stdout.trim(); + assert.notEqual( + nextDecimalPadded, + '02.1', + 'must not propose 02.1 when 2.7 already exists in ROADMAP' + ); + assert.notEqual( + nextDecimalPadded, + '2.1', + 'must not propose 2.1 when 2.7 already exists in ROADMAP' + ); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('phase insert: padded base 02 finds anchor in un-padded ROADMAP', () => { + const tmpA = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3537-ins-A-')); + const tmpB = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3537-ins-B-')); + try { + // Use a phase 2 (no decimal) base so insert proposes 2.1. + const optsA = { + paddedId: '02', + unpaddedId: '2', + }; + setupFixture(tmpA, optsA); + setupFixture(tmpB, optsA); + + const padded = run(['phase', 'insert', '02', 'urgent extension'], tmpA); + const unpadded = run(['phase', 'insert', '2', 'urgent extension'], tmpB); + + // Both invocations should succeed (exit 0) — passing the padded base + // against un-padded prose used to error "Phase 02 not found". + assert.ok( + padded.ok, + `padded form must succeed, got code=${padded.code}, stderr=${padded.stderr}` + ); + assert.ok( + unpadded.ok, + `un-padded form must succeed, got code=${unpadded.code}` + ); + + const aRoadmap = fs.readFileSync( + path.join(tmpA, '.planning', 'ROADMAP.md'), + 'utf-8' + ); + + // The new header may be rendered as `Phase 02.1` or `Phase 2.1` + // (normalizePhaseName pads to 2 digits today; that is pre-existing + // behavior, not the subject of #3537). The critical assertion for + // this verb is "padded form found the anchor and the insertion + // happened" — full byte-parity is gated by an unrelated `Depends on: + // Phase ${afterPhase}` echo bug that lies outside #3537's scope. + assert.match( + aRoadmap, + /### Phase 0?2\.1: urgent extension/, + 'padded form must insert the new decimal phase header' + ); + // Reference `tmpB` to ensure cleanup runs and keep it alive in the + // closure — also a smoke-check that the un-padded sibling did not + // crash mid-run. + assert.ok(fs.existsSync(path.join(tmpB, '.planning', 'ROADMAP.md'))); + } finally { + fs.rmSync(tmpA, { recursive: true, force: true }); + fs.rmSync(tmpB, { recursive: true, force: true }); + } + }); + + test('roadmap annotate-dependencies: padded 02.7 finds phase section', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3537-ann-')); + try { + const { roadmapPath } = setupFixture(tmp, {}); + const before = fs.readFileSync(roadmapPath, 'utf-8'); + + const padded = run( + ['roadmap', 'annotate-dependencies', '02.7'], + tmp + ); + assert.ok( + padded.ok, + `padded form must succeed, got code=${padded.code}, stderr=${padded.stderr}` + ); + + const after = fs.readFileSync(roadmapPath, 'utf-8'); + // The annotation may be a no-op if there's only one wave and no + // cross-cutting truths, but the verb must have reached the phase + // section. Confirm by running parity against un-padded form on a + // separate fixture and asserting equality. + const tmp2 = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3537-ann2-')); + try { + const { roadmapPath: rp2 } = setupFixture(tmp2, {}); + run(['roadmap', 'annotate-dependencies', '2.7'], tmp2); + const unpadded = fs.readFileSync(rp2, 'utf-8'); + assert.equal( + after, + unpadded, + 'annotate-dependencies must produce identical output for padded and un-padded ids' + ); + // And the verb must not have silently destroyed the file (sanity). + assert.match(after, /### Phase 2\.7:/, 'phase header must survive'); + // Reference `before` to keep it from being dead-binding-flagged + // and to assert the run did not corrupt the rest of the file. + assert.ok(before.length > 0); + } finally { + fs.rmSync(tmp2, { recursive: true, force: true }); + } + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('roadmap update-plan-progress: control case — already wired in 1.42.1', () => { + // This is the one site already using phaseMarkdownRegexSource. Including + // it as a control proves the parity assertion is a meaningful signal + // (this test should pass on main, while the others fail). + const { aRoadmap, bRoadmap } = expectParity({ + fixtureOpts: {}, + verbWithPadded: (cwd) => + run(['roadmap', 'update-plan-progress', '02.7'], cwd), + verbWithUnpadded: (cwd) => + run(['roadmap', 'update-plan-progress', '2.7'], cwd), + }); + + assert.equal( + aRoadmap, + bRoadmap, + 'control verb must already produce identical output (wired in 1.42.1)' + ); + assert.match( + aRoadmap, + /- \[x\] \*\*Phase 2\.7:/, + 'control verb must flip checkbox under both invocations' + ); + }); +});