From ab8453871276b100c36d98d66bf28bcad0fc02fe Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 10 May 2026 17:38:40 -0400 Subject: [PATCH] fix(phase): prevent roadmap renumber collapse --- .../fix-3355-phase-remove-roadmap-renumber.md | 5 ++ get-shit-done/bin/lib/phase.cjs | 57 +++++++++++++++---- tests/phase.test.cjs | 49 ++++++++++++++++ 3 files changed, 99 insertions(+), 12 deletions(-) create mode 100644 .changeset/fix-3355-phase-remove-roadmap-renumber.md diff --git a/.changeset/fix-3355-phase-remove-roadmap-renumber.md b/.changeset/fix-3355-phase-remove-roadmap-renumber.md new file mode 100644 index 000000000..95f7803de --- /dev/null +++ b/.changeset/fix-3355-phase-remove-roadmap-renumber.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3367 +--- +**`phase remove --force` no longer collapses all later ROADMAP phases to the removed phase number** — integer phase removal now renumbers ROADMAP structures in single-pass callbacks, preserving later progress rows/headings and avoiding repeated rewrites of newly generated phase numbers. (#3355) diff --git a/get-shit-done/bin/lib/phase.cjs b/get-shit-done/bin/lib/phase.cjs index 07dea61f7..5ecdf8c18 100644 --- a/get-shit-done/bin/lib/phase.cjs +++ b/get-shit-done/bin/lib/phase.cjs @@ -851,6 +851,26 @@ function renameIntegerPhases(phasesDir, removedInt) { return { renamedDirs, renamedFiles }; } +function decrementRoadmapPhaseNumber(raw, removedInt) { + const num = parseInt(raw, 10); + if (!Number.isInteger(num) || num <= removedInt || num >= 999) return raw; + return String(num - 1); +} + +function decrementRoadmapPhaseToken(raw, removedInt) { + const match = String(raw).match(/^(\d+)(\.\d+)?$/); + if (!match) return raw; + const num = parseInt(match[1], 10); + if (!Number.isInteger(num) || num <= removedInt || num >= 999) return raw; + return `${num - 1}${match[2] || ''}`; +} + +function decrementRoadmapPaddedPhaseNumber(raw, removedInt) { + const num = parseInt(raw, 10); + if (!Number.isInteger(num) || num <= removedInt || num >= 999) return raw; + return String(num - 1).padStart(raw.length, '0'); +} + /** * Remove a phase section from ROADMAP.md and renumber all subsequent integer phases. */ @@ -860,22 +880,35 @@ function updateRoadmapAfterPhaseRemoval(roadmapPath, targetPhase, isDecimal, rem let content = fs.readFileSync(roadmapPath, 'utf-8'); const escaped = escapeRegex(targetPhase); - content = content.replace(new RegExp(`\\n?#{2,4}\\s*Phase\\s+${escaped}\\s*:[\\s\\S]*?(?=\\n#{2,4}\\s+Phase\\s+\\d|$)`, 'i'), ''); + content = content.replace(new RegExp(`\\n?#{2,4}\\s*Phase\\s+${escaped}\\s*:[\\s\\S]*?(?=\\n#{2,4}\\s+Phase\\s+\\d+\\s*:|$)`, 'i'), ''); content = content.replace(new RegExp(`\\n?-\\s*\\[[ x]\\]\\s*.*Phase\\s+${escaped}[:\\s][^\\n]*`, 'gi'), ''); content = content.replace(new RegExp(`\\n?\\|\\s*${escaped}\\.?\\s[^|]*\\|[^\\n]*`, 'gi'), ''); if (!isDecimal) { - const MAX_PHASE = 99; - for (let oldNum = MAX_PHASE; oldNum > removedInt; oldNum--) { - const newNum = oldNum - 1; - const oldStr = String(oldNum), newStr = String(newNum); - const oldPad = oldStr.padStart(2, '0'), newPad = newStr.padStart(2, '0'); - content = content.replace(new RegExp(`(#{2,4}\\s*Phase\\s+)${oldStr}(\\s*:)`, 'gi'), `$1${newStr}$2`); - content = content.replace(new RegExp(`(Phase\\s+)${oldStr}([:\\s])`, 'g'), `$1${newStr}$2`); - content = content.replace(new RegExp(`(? `${prefix}${decrementRoadmapPhaseToken(num, removedInt)}${suffix}` + ); + content = content.replace( + /(-\s*\[[ x]\]\s*.*?Phase\s+)(\d+)(\s*:|\s+)/gi, + (_match, prefix, num, suffix) => `${prefix}${decrementRoadmapPhaseNumber(num, removedInt)}${suffix}` + ); + content = content.replace( + /(\|\s*)(\d+)(\.\s)/g, + (_match, prefix, num, suffix) => `${prefix}${decrementRoadmapPhaseNumber(num, removedInt)}${suffix}` + ); + content = content.replace( + /(? `${decrementRoadmapPaddedPhaseNumber(phaseNum, removedInt)}-${planNum}` + ); + content = content.replace( + /(\*\*Depends on\*\*\s*:\s*Phase\s+)(\d+(?:\.\d+)?)\b/gi, + (_match, prefix, num) => `${prefix}${decrementRoadmapPhaseToken(num, removedInt)}` + ); + content = content.replace( + /(Depends on:\*\*\s*Phase\s+)(\d+(?:\.\d+)?)\b/gi, + (_match, prefix, num) => `${prefix}${decrementRoadmapPhaseToken(num, removedInt)}` + ); } atomicWriteFileSync(roadmapPath, content); diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index fdc9c5970..fac4c2ed2 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -1669,6 +1669,55 @@ describe('phase remove command', () => { // Phase 5 should be renumbered to 4 assert.ok(roadmap.includes('Phase 4: Final'), 'Phase 5 should be renumbered to Phase 4'); }); + + test('bug-3355: integer phase remove renumbers roadmap once without collapsing later phases', () => { + const lines = ['# Roadmap', '', '## Progress', '', '| Phase | Plans | Status | Notes |', '|---|---:|---|---|']; + for (let n = 26; n <= 35; n++) { + lines.push(`| ${n}. Phase ${n} | 0/1 | Planned | - |`); + } + lines.push(''); + for (let n = 26; n <= 35; n++) { + lines.push(`### Phase ${n}: Phase ${n}`); + lines.push(`#### Phase ${n}.1: Phase ${n}.1 follow-up`); + lines.push(`**Goal:** Build phase ${n}`); + lines.push(n % 2 === 0 ? `**Depends on**: Phase ${n - 1}` : `**Depends on:** Phase ${n - 1}`); + lines.push(`Plans: ${String(n).padStart(2, '0')}-01-PLAN.md`); + lines.push(''); + + const phaseDir = path.join(tmpDir, '.planning', 'phases', `${String(n).padStart(2, '0')}-phase-${n}`); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, `${String(n).padStart(2, '0')}-01-PLAN.md`), '# Plan'); + } + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), lines.join('\n')); + + const result = runGsdTools('phase remove 27 --force', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + assert.equal((roadmap.match(/\|\s*27\.\s/g) || []).length, 1, 'progress row 27 appears once'); + assert.equal((roadmap.match(/\|\s*28\.\s/g) || []).length, 1, 'progress row 28 appears once'); + assert.equal((roadmap.match(/\|\s*34\.\s/g) || []).length, 1, 'progress row 34 appears once'); + assert.equal((roadmap.match(/\|\s*35\.\s/g) || []).length, 0, 'old progress row 35 removed by renumber'); + assert.equal((roadmap.match(/^### Phase 27:/gm) || []).length, 1, 'heading 27 appears once'); + assert.equal((roadmap.match(/^### Phase 34:/gm) || []).length, 1, 'heading 34 appears once'); + assert.equal((roadmap.match(/^### Phase 35:/gm) || []).length, 0, 'old heading 35 removed by renumber'); + assert.equal((roadmap.match(/^#### Phase 27\.1:/gm) || []).length, 1, 'decimal heading 27.1 appears once'); + assert.equal((roadmap.match(/^#### Phase 34\.1:/gm) || []).length, 1, 'decimal heading 34.1 appears once'); + assert.equal((roadmap.match(/^#### Phase 35\.1:/gm) || []).length, 0, 'old decimal heading 35.1 removed by renumber'); + assert.equal((roadmap.match(/\*\*Depends on\*\*:\s*Phase\s+28\b/g) || []).length, 1, 'bold depends-on with outside colon is decremented'); + assert.equal((roadmap.match(/\*\*Depends on:\*\*\s*Phase\s+29\b/g) || []).length, 1, 'legacy bold depends-on with inside colon is decremented'); + assert.equal((roadmap.match(/\*\*Depends on:\*\*\s*Phase\s+35\b/g) || []).length, 0, 'old depends-on 35 removed by renumber'); + assert.equal((roadmap.match(/\b27-01-PLAN\.md\b/g) || []).length, 1, 'plan id 27-01 appears once'); + assert.equal((roadmap.match(/\b34-01-PLAN\.md\b/g) || []).length, 1, 'plan id 34-01 appears once'); + assert.equal((roadmap.match(/\b35-01-PLAN\.md\b/g) || []).length, 0, 'old plan id 35-01 removed by renumber'); + + for (let n = 27; n <= 34; n++) { + assert.ok( + fs.existsSync(path.join(tmpDir, '.planning', 'phases', `${String(n).padStart(2, '0')}-phase-${n + 1}`)), + `phase directory ${n} should preserve original phase slug ${n + 1}`, + ); + } + }); }); // ─────────────────────────────────────────────────────────────────────────────