fix: deduplicate maxNum logic and add nextMilestoneId tests (#177)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

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

@@ -112,13 +112,17 @@ function findMilestoneIds(basePath: string): string[] {
   }
 }
 
-/** Derive the next milestone ID from existing IDs using max-based approach to avoid collisions after deletions. */
-function nextMilestoneId(milestoneIds: string[]): string {
-  const maxNum = milestoneIds.reduce((max, id) => {
+/** Return the highest numeric suffix among milestone IDs (0 when the list is empty or has no numeric IDs). */
+export function maxMilestoneNum(milestoneIds: string[]): number {
+  return milestoneIds.reduce((max, id) => {
     const num = parseInt(id.replace(/^M/, ""), 10);
     return num > max ? num : max;
   }, 0);
-  return `M${String(maxNum + 1).padStart(3, "0")}`;
+}
+
+/** Derive the next milestone ID from existing IDs using max-based approach to avoid collisions after deletions. */
+export function nextMilestoneId(milestoneIds: string[]): string {
+  return `M${String(maxMilestoneNum(milestoneIds) + 1).padStart(3, "0")}`;
 }
 
 // ─── Queue ─────────────────────────────────────────────────────────────────────
@@ -162,12 +166,9 @@ export async function showQueue(
   const existingContext = await buildExistingMilestonesContext(basePath, milestoneIds, state);
 
   // ── Determine next milestone ID ─────────────────────────────────────
-  const maxNum = milestoneIds.reduce((max, id) => {
-    const num = parseInt(id.replace(/^M/, ""), 10);
-    return num > max ? num : max;
-  }, 0);
-  const nextId = nextMilestoneId(milestoneIds);
-  const nextIdPlus1 = `M${String(maxNum + 2).padStart(3, "0")}`;
+  const max = maxMilestoneNum(milestoneIds);
+  const nextId = `M${String(max + 1).padStart(3, "0")}`;
+  const nextIdPlus1 = `M${String(max + 2).padStart(3, "0")}`;
 
   // ── Build preamble ──────────────────────────────────────────────────
   const activePart = state.activeMilestone

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

@@ -0,0 +1,87 @@
+// Tests for nextMilestoneId and maxMilestoneNum — milestone ID generation
+// using max-based approach to avoid collisions after deletions.
+//
+// Sections:
+//   (a) Empty array returns M001
+//   (b) Sequential IDs return next in sequence
+//   (c) IDs with gaps (deletion) use max, not fill
+//   (d) Non-numeric directory names mixed in are ignored
+
+import { nextMilestoneId, maxMilestoneNum } from '../guided-flow.ts';
+
+// ─── Assertion helpers ─────────────────────────────────────────────────────
+
+let passed = 0;
+let failed = 0;
+
+function assertEq<T>(actual: T, expected: T, message: string): void {
+  if (JSON.stringify(actual) === JSON.stringify(expected)) {
+    passed++;
+  } else {
+    failed++;
+    console.error(`  FAIL: ${message} — expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`);
+  }
+}
+
+// ─── Tests ─────────────────────────────────────────────────────────────────
+
+async function main(): Promise<void> {
+  console.log('nextMilestoneId / maxMilestoneNum tests');
+
+  // (a) Empty array → M001
+  {
+    assertEq(maxMilestoneNum([]), 0, 'maxMilestoneNum([]) === 0');
+    assertEq(nextMilestoneId([]), 'M001', 'nextMilestoneId([]) === "M001"');
+  }
+
+  // (b) Sequential IDs → next in sequence
+  {
+    assertEq(
+      nextMilestoneId(['M001', 'M002', 'M003']),
+      'M004',
+      'sequential IDs return M004',
+    );
+    assertEq(maxMilestoneNum(['M001', 'M002', 'M003']), 3, 'max of sequential is 3');
+  }
+
+  // (c) IDs with gaps (deletion scenario) → uses max, not fill
+  {
+    assertEq(
+      nextMilestoneId(['M001', 'M003']),
+      'M004',
+      'gap scenario returns M004, not M002',
+    );
+    assertEq(maxMilestoneNum(['M001', 'M003']), 3, 'max with gap is 3');
+  }
+
+  // (d) Non-numeric directory names mixed in are ignored
+  {
+    assertEq(
+      nextMilestoneId(['M001', 'notes', '.DS_Store', 'M003']),
+      'M004',
+      'non-numeric names ignored, returns M004',
+    );
+    assertEq(
+      maxMilestoneNum(['M001', 'notes', '.DS_Store', 'M003']),
+      3,
+      'max ignores non-numeric entries',
+    );
+  }
+
+  // ═══════════════════════════════════════════════════════════════════════════
+  // Results
+  // ═══════════════════════════════════════════════════════════════════════════
+
+  console.log(`\n${'='.repeat(40)}`);
+  console.log(`Results: ${passed} passed, ${failed} failed`);
+  if (failed > 0) {
+    process.exit(1);
+  } else {
+    console.log('All tests passed');
+  }
+}
+
+main().catch((error) => {
+  console.error(error);
+  process.exit(1);
+});