From 81a4d1c0911d96acd5f4ba4f700d05b57e9af685 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 24 May 2026 20:02:44 -0400 Subject: [PATCH] fix(#16): renumber canonical phases above 999 on remove (#241) --- .changeset/gentle-jays-rest.md | 5 ++ agents/gsd-research-synthesizer.md | 2 +- get-shit-done/bin/lib/phase.cjs | 8 ++-- sdk/src/query/phase-lifecycle.ts | 12 ++--- tests/phase.test.cjs | 74 ++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 11 deletions(-) create mode 100644 .changeset/gentle-jays-rest.md diff --git a/.changeset/gentle-jays-rest.md b/.changeset/gentle-jays-rest.md new file mode 100644 index 000000000..b00bc250f --- /dev/null +++ b/.changeset/gentle-jays-rest.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 16 +--- +**Phase remove now correctly renumbers canonical phases above 999** — integer phase removal no longer skips 1000+ phases while still preserving 999.x backlog entries. diff --git a/agents/gsd-research-synthesizer.md b/agents/gsd-research-synthesizer.md index e0ed845f2..918f15241 100644 --- a/agents/gsd-research-synthesizer.md +++ b/agents/gsd-research-synthesizer.md @@ -135,7 +135,7 @@ Identify gaps that couldn't be resolved and need attention during planning. 1. **Use the `Write` tool** to write the file. The `Write` tool is in your `tools:` allowlist; there are no restrictions on it. Do not assume restrictions that the frontmatter does not impose. 2. **Do NOT return the SUMMARY.md content in your response.** Your return message is a brief confirmation (see `` below); the content lives on disk. 3. **Do NOT ask permission to write.** Writing `.planning/research/SUMMARY.md` is the explicit purpose of this agent. Asking the orchestrator to do it instead is a failure mode that can cause downstream `SUMMARY.md not found` failures. -4. **Do NOT use `Bash(cat << 'EOF')` or heredoc** for file creation. Use the `Write` tool. +4. **Do NOT use `Bash(cat << 'EOF')` or heredoc** for file creation. Use the `Write` tool. In short: **never use `Bash(cat << 'EOF')` or heredoc**. 5. **If the Write tool errors,** surface the actual error in your return message. Do not silently fall back to returning content; that hides the failure from the orchestrator. Use template: ~/.claude/get-shit-done/templates/research-project/SUMMARY.md diff --git a/get-shit-done/bin/lib/phase.cjs b/get-shit-done/bin/lib/phase.cjs index 33eaef4f8..b60ea0174 100644 --- a/get-shit-done/bin/lib/phase.cjs +++ b/get-shit-done/bin/lib/phase.cjs @@ -1011,7 +1011,7 @@ function renameIntegerPhases(phasesDir, removedInt) { const m = dir.match(/^(\d+)([A-Z])?(?:\.(\d+))?-(.+)$/i); if (!m) return null; const dirInt = parseInt(m[1], 10); - return (dirInt > removedInt && dirInt < 999) ? { dir, oldInt: dirInt, letter: m[2] ? m[2].toUpperCase() : '', decimal: m[3] ? parseInt(m[3], 10) : null, slug: m[4] } : null; + return (dirInt > removedInt && dirInt !== 999) ? { dir, oldInt: dirInt, letter: m[2] ? m[2].toUpperCase() : '', decimal: m[3] ? parseInt(m[3], 10) : null, slug: m[4] } : null; }) .filter(Boolean) .sort((a, b) => a.oldInt !== b.oldInt ? b.oldInt - a.oldInt : (b.decimal || 0) - (a.decimal || 0)); @@ -1040,7 +1040,7 @@ function renameIntegerPhases(phasesDir, removedInt) { function decrementRoadmapPhaseNumber(raw, removedInt) { const num = parseInt(raw, 10); - if (!Number.isInteger(num) || num <= removedInt || num >= 999) return raw; + if (!Number.isInteger(num) || num <= removedInt || num === 999) return raw; return String(num - 1); } @@ -1048,13 +1048,13 @@ 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; + 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; + if (!Number.isInteger(num) || num <= removedInt || num === 999) return raw; return String(num - 1).padStart(raw.length, '0'); } diff --git a/sdk/src/query/phase-lifecycle.ts b/sdk/src/query/phase-lifecycle.ts index f40e0a621..9792ca4c5 100644 --- a/sdk/src/query/phase-lifecycle.ts +++ b/sdk/src/query/phase-lifecycle.ts @@ -901,7 +901,7 @@ async function renameIntegerPhases( // numbering convention that lives outside the active sequence; renumbering // them would clobber the convention and corrupt downstream lookups. // (bug-2434) - if (dirInt <= removedInt || dirInt >= 999) return null; + if (dirInt <= removedInt || dirInt === 999) return null; return { dir, oldInt: dirInt, @@ -951,14 +951,14 @@ async function renameIntegerPhases( * Skips when: * • not an integer * • num <= removedInt (already-renumbered phases stay put) - * • num >= 999 (backlog/parked-idea numbering range) + * • num === 999 (backlog/parked-idea sentinel) * * Returns the original raw string when the guards trip so the regex pass * leaves dates and unrelated numerics intact. */ function decrementRoadmapPhaseNumber(raw: string, removedInt: number): string { const num = parseInt(raw, 10); - if (!Number.isInteger(num) || num <= removedInt || num >= 999) return raw; + if (!Number.isInteger(num) || num <= removedInt || num === 999) return raw; return String(num - 1); } @@ -971,7 +971,7 @@ function decrementRoadmapPhaseToken(raw: string, removedInt: number): string { 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; + if (!Number.isInteger(num) || num <= removedInt || num === 999) return raw; return `${num - 1}${match[2] || ''}`; } @@ -981,7 +981,7 @@ function decrementRoadmapPhaseToken(raw: string, removedInt: number): string { */ function decrementRoadmapPaddedPhaseNumber(raw: string, removedInt: number): string { const num = parseInt(raw, 10); - if (!Number.isInteger(num) || num <= removedInt || num >= 999) return raw; + if (!Number.isInteger(num) || num <= removedInt || num === 999) return raw; return String(num - 1).padStart(raw.length, '0'); } @@ -999,7 +999,7 @@ function decrementRoadmapPaddedPhaseNumber(raw: string, removedInt: number): str * overlap (bug-3355 — phase 7 → 6 → 5 → ...). * * The CJS pattern uses negative lookbehind/ahead on the padded-prefix regex - * to skip dates and decrement helpers that guard against `num >= 999`. Keep + * to skip dates and decrement helpers that guard against `num === 999`. Keep * this implementation byte-for-byte in lockstep with phase.cjs:880-922 — * deviations are how the three bugs above slipped in. * diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 0b12e6611..8645302cf 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -1878,6 +1878,80 @@ describe('phase remove command', () => { ); }); + test('bug-16: integer phase remove renumbers canonical phases above 999 while preserving 999.x backlog', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +### Phase 1199: Baseline +**Goal:** Before removal +Plans: +- [x] 1199-01-PLAN.md + +### Phase 1200: Remove Me +**Goal:** Target phase +Plans: +- [ ] 1200-01-PLAN.md + +### Phase 1201: Follow Up A +**Goal:** First phase after target +**Depends on:** Phase 1200 +Plans: +- [ ] 1201-01-PLAN.md + +### Phase 1202: Follow Up B +**Goal:** Second phase after target +**Depends on:** Phase 1201 +Plans: +- [ ] 1202-01-PLAN.md + +### Phase 999.1: Backlog Item +**Goal:** Parked backlog item +` + ); + + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '1199-baseline'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '1200-remove-me'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '1201-follow-up-a'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '1202-follow-up-b'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '999.1-backlog-item'), { recursive: true }); + + const result = runGsdTools('phase remove 1200', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + // On-disk phase directories should be decremented by one above removedInt. + assert.ok( + fs.existsSync(path.join(tmpDir, '.planning', 'phases', '1200-follow-up-a')), + '1201-follow-up-a should be renamed to 1200-follow-up-a', + ); + assert.ok( + fs.existsSync(path.join(tmpDir, '.planning', 'phases', '1201-follow-up-b')), + '1202-follow-up-b should be renamed to 1201-follow-up-b', + ); + assert.ok( + !fs.existsSync(path.join(tmpDir, '.planning', 'phases', '1201-follow-up-a')), + 'old 1201-follow-up-a directory should not remain', + ); + assert.ok( + !fs.existsSync(path.join(tmpDir, '.planning', 'phases', '1202-follow-up-b')), + 'old 1202-follow-up-b directory should not remain', + ); + + // Backlog 999.x must remain untouched. + assert.ok( + fs.existsSync(path.join(tmpDir, '.planning', 'phases', '999.1-backlog-item')), + 'backlog directory 999.1-backlog-item must not be renamed', + ); + + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + assert.ok(!roadmap.includes('### Phase 1200: Remove Me'), 'removed phase 1200 section must be gone'); + assert.ok(roadmap.includes('### Phase 1200: Follow Up A'), 'phase 1201 should be renumbered to 1200'); + assert.ok(roadmap.includes('### Phase 1201: Follow Up B'), 'phase 1202 should be renumbered to 1201'); + assert.ok(!roadmap.includes('### Phase 1202: Follow Up B'), 'old phase 1202 heading must not remain'); + assert.ok(roadmap.includes('**Depends on:** Phase 1200'), 'depends-on reference above removed phase should be decremented'); + assert.ok(roadmap.includes('### Phase 999.1: Backlog Item'), 'backlog phase 999.1 heading must not be renumbered'); + }); + test('bug-2435: integer phase remove does not corrupt YYYY-MM-DD dates in ROADMAP.md', () => { // Setup: removing phase 4 from a roadmap containing 2026-04-14 date strings fs.writeFileSync(