diff --git a/.changeset/3602-phase-remove-slugged-plan-refs.md b/.changeset/3602-phase-remove-slugged-plan-refs.md new file mode 100644 index 000000000..f2795cf3b --- /dev/null +++ b/.changeset/3602-phase-remove-slugged-plan-refs.md @@ -0,0 +1,5 @@ +--- +type: Fixed +issue: 3602 +--- +**`phase remove N` now renumbers slugged plan references in ROADMAP.md** — the plan-reference renumbering regex in `get-shit-done/bin/lib/phase.cjs:updateRoadmapAfterPhaseRemoval` previously only matched compact filenames like `07-01-PLAN.md`. A slug between the plan number and the `-PLAN.md` / `-SUMMARY.md` suffix (e.g. `07-01-cherry-pick-foundation-PLAN.md`) broke the lookahead, so the on-disk file was renamed but the ROADMAP entry kept pointing at the stale `07-01-…` prefix. The suffix lookahead now allows an optional kebab-case slug segment between the number and the canonical suffix. diff --git a/get-shit-done/bin/lib/phase.cjs b/get-shit-done/bin/lib/phase.cjs index 400aa6dcf..24dcba5ba 100644 --- a/get-shit-done/bin/lib/phase.cjs +++ b/get-shit-done/bin/lib/phase.cjs @@ -903,8 +903,17 @@ function updateRoadmapAfterPhaseRemoval(roadmapPath, targetPhase, isDecimal, rem /(\|\s*)(\d+)(\.\s)/g, (_match, prefix, num, suffix) => `${prefix}${decrementRoadmapPhaseNumber(num, removedInt)}${suffix}` ); + // #3602: extend the suffix lookahead so slugged plan filenames like + // `07-01-cherry-pick-foundation-PLAN.md` match too. The previous + // pattern only allowed a compact `-(PLAN|SUMMARY).md` immediately + // after the plan number (or no suffix at all); a slug between the + // number and the `-PLAN.md` / `-SUMMARY.md` suffix made the + // lookahead fail and left the stale `07-01-` prefix in ROADMAP + // text while the on-disk file was already renamed to `06-01-…`. + // The slug segment `(?:-[A-Za-z][A-Za-z0-9-]*)*` allows any number + // of kebab-case tokens before the canonical PLAN/SUMMARY suffix. content = content.replace( - /(? `${decrementRoadmapPaddedPhaseNumber(phaseNum, removedInt)}-${planNum}` ); content = content.replace( diff --git a/tests/bug-3602-phase-remove-renumbers-slugged-plan-refs.test.cjs b/tests/bug-3602-phase-remove-renumbers-slugged-plan-refs.test.cjs new file mode 100644 index 000000000..c64aee867 --- /dev/null +++ b/tests/bug-3602-phase-remove-renumbers-slugged-plan-refs.test.cjs @@ -0,0 +1,179 @@ +/** + * Bug #3602: After `phase remove N`, ROADMAP references to slugged plan + * files (`07-01-cherry-pick-foundation-PLAN.md`) keep their old phase + * prefix while the on-disk file has already been renamed. The + * renumber-references regex in `phase.cjs` only matched compact forms + * (`07-01-PLAN.md`, bare `07-01`) — it did not allow a slug between the + * plan number and the `-PLAN.md` / `-SUMMARY.md` suffix. + * + * Fix: extend the lookahead to allow an optional `-` segment before + * the `-(PLAN|SUMMARY).md` suffix while still preserving the "bare token, + * not part of a longer number/identifier" alternative. + * + * Assertions go through the typed `roadmap get-phase --json` query so no + * test asserts on raw ROADMAP.md text content. + */ + +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +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 { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +function writeRoadmap(tmpDir, body) { + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), body); +} +function writeState(tmpDir, version) { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `---\nmilestone: ${version}\n---\n`, + ); +} +function ensurePhaseDir(tmpDir, name) { + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', name), { recursive: true }); +} +function ensurePlanFile(tmpDir, phaseDirName, planName) { + const p = path.join(tmpDir, '.planning', 'phases', phaseDirName, planName); + fs.mkdirSync(path.dirname(p), { recursive: true }); + fs.writeFileSync(p, '# Plan\n'); +} +function getPhase(tmpDir, phaseNum) { + const r = runGsdTools(['roadmap', 'get-phase', phaseNum, '--json'], tmpDir); + if (!r.success) return { found: false, error: r.error }; + return JSON.parse(r.output); +} + +describe('bug #3602: phase remove renumbers slugged plan references in ROADMAP', () => { + let tmpDir; + beforeEach(() => { + tmpDir = createTempProject('bug-3602-'); + }); + afterEach(() => { + cleanup(tmpDir); + }); + + test('slugged PLAN reference (07-01-cherry-pick-foundation-PLAN.md) is renumbered to 06-01-…', () => { + writeState(tmpDir, 'v1.0.0'); + writeRoadmap( + tmpDir, + [ + '# Roadmap', + '', + '## Current Milestone: v1.0.0', + '', + '### Phase 6: Old Work', + '**Goal:** RemoveThisGoal', + '', + '### Phase 7: New Work', + '**Goal:** Plans: 07-01-cherry-pick-foundation-PLAN.md and 07-02-finish-it-SUMMARY.md', + '', + ].join('\n'), + ); + ensurePhaseDir(tmpDir, '06-old'); + ensurePhaseDir(tmpDir, '07-new'); + ensurePlanFile(tmpDir, '07-new', '07-01-cherry-pick-foundation-PLAN.md'); + ensurePlanFile(tmpDir, '07-new', '07-02-finish-it-SUMMARY.md'); + + const r = runGsdTools(['phase', 'remove', '6'], tmpDir); + assert.ok(r.success, `phase remove failed: ${r.error || r.output}`); + + // Phase 7 → Phase 6 after removal. The renumbered phase's recorded + // goal must reference the renumbered plan filenames (06-01-… and + // 06-02-…), not the stale 07-01-… / 07-02-… form. + const phase6 = getPhase(tmpDir, '6'); + assert.strictEqual(phase6.found, true); + assert.strictEqual(phase6.phase_name, 'New Work'); + assert.ok( + phase6.goal.includes('06-01-cherry-pick-foundation-PLAN.md'), + `Plans reference for slugged PLAN was not renumbered. Goal: ${phase6.goal}`, + ); + assert.ok( + phase6.goal.includes('06-02-finish-it-SUMMARY.md'), + `Plans reference for slugged SUMMARY was not renumbered. Goal: ${phase6.goal}`, + ); + assert.ok( + !phase6.goal.includes('07-01'), + `stale 07-01 prefix remains in ROADMAP. Goal: ${phase6.goal}`, + ); + assert.ok( + !phase6.goal.includes('07-02'), + `stale 07-02 prefix remains in ROADMAP. Goal: ${phase6.goal}`, + ); + }); + + test('compact PLAN/SUMMARY reference (07-01-PLAN.md) still renumbers (#3601 contract preserved)', () => { + writeState(tmpDir, 'v1.0.0'); + writeRoadmap( + tmpDir, + [ + '# Roadmap', + '', + '## Current Milestone: v1.0.0', + '', + '### Phase 6: Old', + '**Goal:** RemoveGoal', + '', + '### Phase 7: New', + '**Goal:** Plans: 07-01-PLAN.md and 07-02-SUMMARY.md', + '', + ].join('\n'), + ); + ensurePhaseDir(tmpDir, '06-old'); + ensurePhaseDir(tmpDir, '07-new'); + ensurePlanFile(tmpDir, '07-new', '07-01-PLAN.md'); + ensurePlanFile(tmpDir, '07-new', '07-02-SUMMARY.md'); + + const r = runGsdTools(['phase', 'remove', '6'], tmpDir); + assert.ok(r.success); + + const phase6 = getPhase(tmpDir, '6'); + assert.strictEqual(phase6.found, true); + assert.ok(phase6.goal.includes('06-01-PLAN.md')); + assert.ok(phase6.goal.includes('06-02-SUMMARY.md')); + assert.ok(!phase6.goal.includes('07-01-PLAN.md')); + assert.ok(!phase6.goal.includes('07-02-SUMMARY.md')); + }); + + test('does NOT renumber values that look like phase-plan tokens but are not (e.g. 2026-01-01 dates)', () => { + // Counter-test: an ISO date `2026-01-01` should NOT be matched by the + // renumber regex even though it superficially looks like NN-NN-NN. + // The negative lookbehind `(?