diff --git a/src/resources/extensions/gsd/guided-flow.ts b/src/resources/extensions/gsd/guided-flow.ts index c529462b8..30310b3dc 100644 --- a/src/resources/extensions/gsd/guided-flow.ts +++ b/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 { + // Clear stale reservations from previous cancelled sessions (#2488) + clearReservedMilestoneIds(); + // Ensure .gsd/ is bootstrapped bootstrapGsdProject(basePath); @@ -936,6 +939,11 @@ export async function showSmartEntry( ): Promise { 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") { diff --git a/src/resources/extensions/gsd/tests/stale-milestone-id-reservation.test.ts b/src/resources/extensions/gsd/tests/stale-milestone-id-reservation.test.ts new file mode 100644 index 000000000..cfcfbef1a --- /dev/null +++ b/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"); + }); +});