From aee09a53ec3ec8480d43b7d01232a115dcaef5eb Mon Sep 17 00:00:00 2001 From: mastertyko <11311479+mastertyko@users.noreply.github.com> Date: Wed, 25 Mar 2026 21:45:11 +0100 Subject: [PATCH] fix(gsd): clear stale milestone ID reservations at session start The module-level reservedMilestoneIds Set persists across /gsd invocations within the same Node process. Each cancelled session reserves an ID that is never claimed, permanently inflating the next milestone number. Starting /gsd 3 times without completing produces M011 instead of M009. Call clearReservedMilestoneIds() at the top of showSmartEntry() and showHeadlessMilestoneCreation() so stale reservations from previous cancelled sessions are discarded before generating new IDs. The function already existed but was never called outside tests. Closes #2488 --- src/resources/extensions/gsd/guided-flow.ts | 10 ++- .../stale-milestone-id-reservation.test.ts | 79 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 src/resources/extensions/gsd/tests/stale-milestone-id-reservation.test.ts diff --git a/src/resources/extensions/gsd/guided-flow.ts b/src/resources/extensions/gsd/guided-flow.ts index c5e757052..f4af061bd 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); @@ -842,6 +845,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"); + }); +});