Merge pull request #2537 from mastertyko/fix/clear-stale-milestone-id-reservations

fix(gsd): clear stale milestone ID reservations at session start

src/resources/extensions/gsd/guided-flow.ts

@@ -35,7 +35,7 @@ import { showProjectInit, offerMigration } from "./init-wizard.js";
 import { validateDirectory } from "./validate-directory.js";
 import { showConfirm } from "../shared/tui.js";
 import { debugLog } from "./debug-logger.js";
-import { findMilestoneIds, nextMilestoneId, reserveMilestoneId, getReservedMilestoneIds } from "./milestone-ids.js";
+import { findMilestoneIds, nextMilestoneId, reserveMilestoneId, getReservedMilestoneIds, clearReservedMilestoneIds } from "./milestone-ids.js";
 import { parkMilestone, discardMilestone } from "./milestone-actions.js";
 import { resolveModelWithFallbacksForUnit } from "./preferences-models.js";
 
@@ -373,6 +373,9 @@ export async function showHeadlessMilestoneCreation(
   basePath: string,
   seedContext: string,
 ): Promise<void> {
+  // Clear stale reservations from previous cancelled sessions (#2488)
+  clearReservedMilestoneIds();
+
   // Ensure .gsd/ is bootstrapped
   bootstrapGsdProject(basePath);
 
@@ -936,6 +939,11 @@ export async function showSmartEntry(
 ): Promise<void> {
   const stepMode = options?.step;
 
+  // ── Clear stale milestone ID reservations from previous cancelled sessions ──
+  // Reservations only need to survive within a single /gsd interaction.
+  // Without this, each cancelled session permanently bumps the next ID. (#2488)
+  clearReservedMilestoneIds();
+
   // ── Directory safety check — refuse to operate in system/home dirs ───
   const dirCheck = validateDirectory(basePath);
   if (dirCheck.severity === "blocked") {

src/resources/extensions/gsd/tests/stale-milestone-id-reservation.test.ts

@@ -0,0 +1,79 @@
+/**
+ * Regression test for #2488: Stale milestone ID reservations inflate next ID
+ * after cancelled /gsd sessions.
+ *
+ * The module-level `reservedMilestoneIds` Set persists across /gsd invocations
+ * within the same Node process. Without clearReservedMilestoneIds() at session
+ * start, each cancelled session permanently bumps the counter by 1.
+ */
+import { describe, test, beforeEach } from "node:test";
+import assert from "node:assert/strict";
+
+import {
+  nextMilestoneId,
+  reserveMilestoneId,
+  getReservedMilestoneIds,
+  clearReservedMilestoneIds,
+} from "../milestone-ids.ts";
+
+describe("stale milestone ID reservation cleanup (#2488)", () => {
+  beforeEach(() => {
+    clearReservedMilestoneIds();
+  });
+
+  test("without cleanup, cancelled sessions inflate the next ID", () => {
+    const diskIds = ["M001", "M002", "M003"];
+
+    // Session 1: user starts /gsd, ID is previewed and reserved, then cancelled
+    const allIds1 = [...new Set([...diskIds, ...getReservedMilestoneIds()])];
+    const preview1 = nextMilestoneId(allIds1);
+    reserveMilestoneId(preview1);
+    assert.equal(preview1, "M004");
+
+    // Session 2: user starts /gsd again — stale reservation still in Set
+    // WITHOUT clearing, the next ID skips M004 (reserved) and goes to M005
+    const allIds2 = [...new Set([...diskIds, ...getReservedMilestoneIds()])];
+    const preview2 = nextMilestoneId(allIds2);
+    assert.equal(preview2, "M005", "without cleanup, ID inflates to M005");
+  });
+
+  test("with cleanup at session start, next ID is correct", () => {
+    const diskIds = ["M001", "M002", "M003"];
+
+    // Session 1: user starts /gsd, ID is previewed and reserved, then cancelled
+    const allIds1 = [...new Set([...diskIds, ...getReservedMilestoneIds()])];
+    const preview1 = nextMilestoneId(allIds1);
+    reserveMilestoneId(preview1);
+    assert.equal(preview1, "M004");
+
+    // Session 2: clear stale reservations first (the fix)
+    clearReservedMilestoneIds();
+
+    // Now the next ID correctly returns M004 again
+    const allIds2 = [...new Set([...diskIds, ...getReservedMilestoneIds()])];
+    const preview2 = nextMilestoneId(allIds2);
+    assert.equal(preview2, "M004", "after cleanup, ID is correctly M004");
+  });
+
+  test("multiple cancelled sessions compound the inflation without cleanup", () => {
+    const diskIds = ["M001", "M002", "M003"];
+
+    // 3 cancelled sessions without cleanup
+    for (let i = 0; i < 3; i++) {
+      const allIds = [...new Set([...diskIds, ...getReservedMilestoneIds()])];
+      const preview = nextMilestoneId(allIds);
+      reserveMilestoneId(preview);
+    }
+
+    // Without cleanup, we're now at M007 instead of M004
+    const allIds = [...new Set([...diskIds, ...getReservedMilestoneIds()])];
+    const next = nextMilestoneId(allIds);
+    assert.equal(next, "M007", "3 cancelled sessions inflate ID by 3");
+
+    // With cleanup, we're back to M004
+    clearReservedMilestoneIds();
+    const allIdsClean = [...new Set([...diskIds, ...getReservedMilestoneIds()])];
+    const nextClean = nextMilestoneId(allIdsClean);
+    assert.equal(nextClean, "M004", "cleanup restores correct next ID");
+  });
+});