fix(#16): renumber canonical phases above 999 on remove (#241)

.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.

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 `<structured_returns>` 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

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');
 }
 

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.
  *

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(