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
This commit is contained in:
mastertyko 2026-03-25 21:45:11 +01:00
parent 55c8988900
commit aee09a53ec
2 changed files with 88 additions and 1 deletions

View file

@ -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);
@ -842,6 +845,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") {

View file

@ -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");
});
});