fix: share milestone ID reservation between preview and tool (#1569) (#1802)

src/resources/extensions/gsd/bootstrap/db-tools.ts

@@ -1,7 +1,7 @@
 import { Type } from "@sinclair/typebox";
 import type { ExtensionAPI } from "@gsd/pi-coding-agent";
 
-import { findMilestoneIds, nextMilestoneId } from "../guided-flow.js";
+import { findMilestoneIds, nextMilestoneId, claimReservedId, getReservedMilestoneIds } from "../guided-flow.js";
 import { loadEffectiveGSDPreferences } from "../preferences.js";
 import { ensureDbOpen } from "./dynamic-tools.js";
 
@@ -197,7 +197,6 @@ export function registerDbTools(pi: ExtensionAPI): void {
     },
   });
 
-  const reservedMilestoneIds = new Set<string>();
   pi.registerTool({
     name: "gsd_generate_milestone_id",
     label: "Generate Milestone ID",
@@ -215,15 +214,24 @@ export function registerDbTools(pi: ExtensionAPI): void {
     parameters: Type.Object({}),
     async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) {
       try {
+        // Claim a reserved ID if the guided-flow already previewed one to the user.
+        // This guarantees the ID shown in the UI matches the one materialised on disk.
+        const reserved = claimReservedId();
+        if (reserved) {
+          return {
+            content: [{ type: "text" as const, text: reserved }],
+            details: { operation: "generate_milestone_id", id: reserved, source: "reserved" } as any,
+          };
+        }
+
         const basePath = process.cwd();
         const existingIds = findMilestoneIds(basePath);
         const uniqueEnabled = !!loadEffectiveGSDPreferences()?.preferences?.unique_milestone_ids;
-        const allIds = [...new Set([...existingIds, ...reservedMilestoneIds])];
+        const allIds = [...new Set([...existingIds, ...getReservedMilestoneIds()])];
         const newId = nextMilestoneId(allIds, uniqueEnabled);
-        reservedMilestoneIds.add(newId);
         return {
           content: [{ type: "text" as const, text: newId }],
-          details: { operation: "generate_milestone_id", id: newId, existingCount: existingIds.length, reservedCount: reservedMilestoneIds.size, uniqueEnabled } as any,
+          details: { operation: "generate_milestone_id", id: newId, existingCount: existingIds.length, uniqueEnabled } as any,
         };
       } catch (err) {
         const msg = err instanceof Error ? err.message : String(err);

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

@@ -33,7 +33,7 @@ import { showProjectInit, offerMigration } from "./init-wizard.js";
 import { validateDirectory } from "./validate-directory.js";
 import { showConfirm } from "../shared/mod.js";
 import { debugLog } from "./debug-logger.js";
-import { findMilestoneIds, nextMilestoneId } from "./milestone-ids.js";
+import { findMilestoneIds, nextMilestoneId, reserveMilestoneId, getReservedMilestoneIds } from "./milestone-ids.js";
 import { parkMilestone, discardMilestone } from "./milestone-actions.js";
 import { resolveModelWithFallbacksForUnit } from "./preferences-models.js";
 
@@ -42,6 +42,7 @@ export {
   MILESTONE_ID_RE, generateMilestoneSuffix, nextMilestoneId,
   extractMilestoneSeq, parseMilestoneId, milestoneIdSort,
   maxMilestoneNum, findMilestoneIds,
+  reserveMilestoneId, claimReservedId, getReservedMilestoneIds, clearReservedMilestoneIds,
 } from "./milestone-ids.js";
 export {
   showQueue, handleQueueReorder, showQueueAdd,
@@ -49,6 +50,20 @@ export {
 } from "./guided-flow-queue.js";
 import { getErrorMessage } from "./error-utils.js";
 
+// ─── ID Generation with Reservation ─────────────────────────────────────────
+
+/**
+ * Generate the next milestone ID, accounting for reserved IDs, and reserve it.
+ * Ensures any preview ID shown in the UI matches what `gsd_generate_milestone_id`
+ * will later return.
+ */
+function nextMilestoneIdReserved(existingIds: string[], uniqueEnabled: boolean): string {
+  const allIds = [...new Set([...existingIds, ...getReservedMilestoneIds()])];
+  const id = nextMilestoneId(allIds, uniqueEnabled);
+  reserveMilestoneId(id);
+  return id;
+}
+
 // ─── Commit Instruction Helpers ──────────────────────────────────────────────
 
 /** Build commit instruction for planning prompts. .gsd/ is managed externally and always gitignored. */
@@ -362,7 +377,7 @@ export async function showHeadlessMilestoneCreation(
   // Generate next milestone ID
   const existingIds = findMilestoneIds(basePath);
   const prefs = loadEffectiveGSDPreferences();
-  const nextId = nextMilestoneId(existingIds, prefs?.preferences?.unique_milestone_ids ?? false);
+  const nextId = nextMilestoneIdReserved(existingIds, prefs?.preferences?.unique_milestone_ids ?? false);
 
   // Create milestone directory
   const milestoneDir = join(gsdRoot(basePath), "milestones", nextId, "slices");
@@ -552,7 +567,7 @@ export async function showDiscuss(
     } else if (choice === "skip_milestone") {
       const milestoneIds = findMilestoneIds(basePath);
       const uniqueMilestoneIds = !!loadEffectiveGSDPreferences()?.preferences?.unique_milestone_ids;
-      const nextId = nextMilestoneId(milestoneIds, uniqueMilestoneIds);
+      const nextId = nextMilestoneIdReserved(milestoneIds, uniqueMilestoneIds);
       pendingAutoStart = { ctx, pi, basePath, milestoneId: nextId, step: false };
       await dispatchWorkflow(pi, buildDiscussPrompt(nextId, `New milestone ${nextId}.`, basePath), "gsd-run", ctx, "plan-milestone");
     }
@@ -793,7 +808,7 @@ async function handleMilestoneActions(
   if (choice === "skip") {
     const milestoneIds = findMilestoneIds(basePath);
     const uniqueMilestoneIds = !!loadEffectiveGSDPreferences()?.preferences?.unique_milestone_ids;
-    const nextId = nextMilestoneId(milestoneIds, uniqueMilestoneIds);
+    const nextId = nextMilestoneIdReserved(milestoneIds, uniqueMilestoneIds);
     pendingAutoStart = { ctx, pi, basePath, milestoneId: nextId, step: stepMode };
     await dispatchWorkflow(pi, buildDiscussPrompt(nextId,
       `New milestone ${nextId}.`,
@@ -933,7 +948,7 @@ export async function showSmartEntry(
     }
 
     const uniqueMilestoneIds = !!loadEffectiveGSDPreferences()?.preferences?.unique_milestone_ids;
-    const nextId = nextMilestoneId(milestoneIds, uniqueMilestoneIds);
+    const nextId = nextMilestoneIdReserved(milestoneIds, uniqueMilestoneIds);
     const isFirst = milestoneIds.length === 0;
 
     if (isFirst) {
@@ -996,7 +1011,7 @@ export async function showSmartEntry(
     if (choice === "new_milestone") {
       const milestoneIds = findMilestoneIds(basePath);
       const uniqueMilestoneIds = !!loadEffectiveGSDPreferences()?.preferences?.unique_milestone_ids;
-      const nextId = nextMilestoneId(milestoneIds, uniqueMilestoneIds);
+      const nextId = nextMilestoneIdReserved(milestoneIds, uniqueMilestoneIds);
 
       pendingAutoStart = { ctx, pi, basePath, milestoneId: nextId, step: stepMode };
       await dispatchWorkflow(pi, buildDiscussPrompt(nextId,
@@ -1062,7 +1077,7 @@ export async function showSmartEntry(
     } else if (choice === "skip_milestone") {
       const milestoneIds = findMilestoneIds(basePath);
       const uniqueMilestoneIds = !!loadEffectiveGSDPreferences()?.preferences?.unique_milestone_ids;
-      const nextId = nextMilestoneId(milestoneIds, uniqueMilestoneIds);
+      const nextId = nextMilestoneIdReserved(milestoneIds, uniqueMilestoneIds);
       pendingAutoStart = { ctx, pi, basePath, milestoneId: nextId, step: stepMode };
       await dispatchWorkflow(pi, buildDiscussPrompt(nextId,
         `New milestone ${nextId}.`,
@@ -1146,7 +1161,7 @@ export async function showSmartEntry(
       } else if (choice === "skip_milestone") {
         const milestoneIds = findMilestoneIds(basePath);
         const uniqueMilestoneIds = !!loadEffectiveGSDPreferences()?.preferences?.unique_milestone_ids;
-        const nextId = nextMilestoneId(milestoneIds, uniqueMilestoneIds);
+        const nextId = nextMilestoneIdReserved(milestoneIds, uniqueMilestoneIds);
         pendingAutoStart = { ctx, pi, basePath, milestoneId: nextId, step: stepMode };
         await dispatchWorkflow(pi, buildDiscussPrompt(nextId,
           `New milestone ${nextId}.`,

src/resources/extensions/gsd/milestone-ids.ts

@@ -70,6 +70,44 @@ export function nextMilestoneId(milestoneIds: string[], uniqueEnabled?: boolean)
   return `M${seq}`;
 }
 
+// ─── Reservation ─────────────────────────────────────────────────────────────
+
+/**
+ * Module-level set of milestone IDs that have been previewed/promised to the
+ * user but not yet materialised on disk. Both guided-flow (preview) and
+ * gsd_generate_milestone_id (tool) share this set so the ID shown in the UI
+ * matches the one the tool returns.
+ */
+const reservedMilestoneIds = new Set<string>();
+
+/** Reserve an ID so that subsequent calls to `claimReservedId` / `nextMilestoneId` account for it. */
+export function reserveMilestoneId(id: string): void {
+  reservedMilestoneIds.add(id);
+}
+
+/**
+ * If any IDs have been reserved, shift one out and return it.
+ * Returns `undefined` when the reservation set is empty.
+ */
+export function claimReservedId(): string | undefined {
+  const first = reservedMilestoneIds.values().next().value;
+  if (first !== undefined) {
+    reservedMilestoneIds.delete(first);
+    return first;
+  }
+  return undefined;
+}
+
+/** Return a snapshot of all currently reserved IDs (for merging into the "existing" list). */
+export function getReservedMilestoneIds(): ReadonlySet<string> {
+  return reservedMilestoneIds;
+}
+
+/** Clear all reservations (useful for tests). */
+export function clearReservedMilestoneIds(): void {
+  reservedMilestoneIds.clear();
+}
+
 // ─── Discovery ──────────────────────────────────────────────────────────────
 
 /** Scan the milestones directory and return IDs sorted by queue order (or numeric fallback). */

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

@@ -0,0 +1,73 @@
+// milestone-id-reservation — Verifies that preview IDs from guided-flow
+// match the IDs claimed by gsd_generate_milestone_id via the shared
+// reservation mechanism in milestone-ids.ts.
+//
+// Regression test for #1569.
+
+import { describe, it, beforeEach } from 'node:test';
+import assert from 'node:assert/strict';
+
+import {
+  nextMilestoneId,
+  reserveMilestoneId,
+  claimReservedId,
+  getReservedMilestoneIds,
+  clearReservedMilestoneIds,
+} from '../milestone-ids.ts';
+
+describe('milestone ID reservation (#1569)', () => {
+  beforeEach(() => {
+    clearReservedMilestoneIds();
+  });
+
+  it('claimReservedId returns undefined when nothing is reserved', () => {
+    assert.equal(claimReservedId(), undefined);
+  });
+
+  it('reserved ID is returned by claimReservedId and removed from the set', () => {
+    const id = nextMilestoneId([], true);
+    reserveMilestoneId(id);
+
+    assert.equal(getReservedMilestoneIds().size, 1);
+    assert.equal(claimReservedId(), id);
+    assert.equal(getReservedMilestoneIds().size, 0);
+    // Second claim returns undefined
+    assert.equal(claimReservedId(), undefined);
+  });
+
+  it('reserved IDs are visible in getReservedMilestoneIds', () => {
+    reserveMilestoneId('M001-abc123');
+    reserveMilestoneId('M002-def456');
+    const reserved = getReservedMilestoneIds();
+    assert.equal(reserved.size, 2);
+    assert.ok(reserved.has('M001-abc123'));
+    assert.ok(reserved.has('M002-def456'));
+  });
+
+  it('clearReservedMilestoneIds empties the set', () => {
+    reserveMilestoneId('M001-abc123');
+    clearReservedMilestoneIds();
+    assert.equal(getReservedMilestoneIds().size, 0);
+  });
+
+  it('nextMilestoneId accounts for reserved IDs in sequence numbering', () => {
+    // Simulate: guided-flow previews M001, reserves it
+    const existing: string[] = [];
+    const preview = nextMilestoneId(existing, true);
+    assert.match(preview, /^M001-/);
+    reserveMilestoneId(preview);
+
+    // Now generate the next one accounting for reservations
+    const allIds = [...new Set([...existing, ...getReservedMilestoneIds()])];
+    const second = nextMilestoneId(allIds, true);
+    assert.match(second, /^M002-/);
+  });
+
+  it('claim returns IDs in insertion order (FIFO)', () => {
+    reserveMilestoneId('M001-aaa111');
+    reserveMilestoneId('M002-bbb222');
+    assert.equal(claimReservedId(), 'M001-aaa111');
+    assert.equal(claimReservedId(), 'M002-bbb222');
+    assert.equal(claimReservedId(), undefined);
+  });
+});