From d2fd92f8fc85e458ce77bf25d017eca67074d1a5 Mon Sep 17 00:00:00 2001 From: Lex Christopherson Date: Fri, 13 Mar 2026 10:49:06 -0600 Subject: [PATCH 1/2] fix: use max-based milestone ID generation instead of length+1 (#177) Co-Authored-By: Claude Opus 4.6 --- src/resources/extensions/gsd/guided-flow.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/resources/extensions/gsd/guided-flow.ts b/src/resources/extensions/gsd/guided-flow.ts index 15e7de50d..0259b3063 100644 --- a/src/resources/extensions/gsd/guided-flow.ts +++ b/src/resources/extensions/gsd/guided-flow.ts @@ -112,6 +112,15 @@ 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) => { + const num = parseInt(id.replace(/^M/, ""), 10); + return num > max ? num : max; + }, 0); + return `M${String(maxNum + 1).padStart(3, "0")}`; +} + // ─── Queue ───────────────────────────────────────────────────────────────────── /** @@ -157,7 +166,7 @@ export async function showQueue( const num = parseInt(id.replace(/^M/, ""), 10); return num > max ? num : max; }, 0); - const nextId = `M${String(maxNum + 1).padStart(3, "0")}`; + const nextId = nextMilestoneId(milestoneIds); const nextIdPlus1 = `M${String(maxNum + 2).padStart(3, "0")}`; // ── Build preamble ────────────────────────────────────────────────── @@ -508,7 +517,7 @@ export async function showSmartEntry( } const milestoneIds = findMilestoneIds(basePath); - const nextId = `M${String(milestoneIds.length + 1).padStart(3, "0")}`; + const nextId = nextMilestoneId(milestoneIds); const isFirst = milestoneIds.length === 0; if (isFirst) { @@ -570,7 +579,7 @@ export async function showSmartEntry( if (choice === "new_milestone") { const milestoneIds = findMilestoneIds(basePath); - const nextId = `M${String(milestoneIds.length + 1).padStart(3, "0")}`; + const nextId = nextMilestoneId(milestoneIds); pendingAutoStart = { ctx, pi, basePath, milestoneId: nextId, step: stepMode }; dispatchWorkflow(pi, buildDiscussPrompt(nextId, @@ -638,7 +647,7 @@ export async function showSmartEntry( })); } else if (choice === "skip_milestone") { const milestoneIds = findMilestoneIds(basePath); - const nextId = `M${String(milestoneIds.length + 1).padStart(3, "0")}`; + const nextId = nextMilestoneId(milestoneIds); pendingAutoStart = { ctx, pi, basePath, milestoneId: nextId, step: stepMode }; dispatchWorkflow(pi, buildDiscussPrompt(nextId, `New milestone ${nextId}.`, From 401397362fcf7c112e8680598a9913a144581d66 Mon Sep 17 00:00:00 2001 From: Lex Christopherson Date: Fri, 13 Mar 2026 10:59:10 -0600 Subject: [PATCH 2/2] fix: deduplicate maxNum logic and add nextMilestoneId tests (#177) Co-Authored-By: Claude Opus 4.6 --- src/resources/extensions/gsd/guided-flow.ts | 21 ++--- .../gsd/tests/next-milestone-id.test.ts | 87 +++++++++++++++++++ 2 files changed, 98 insertions(+), 10 deletions(-) create mode 100644 src/resources/extensions/gsd/tests/next-milestone-id.test.ts diff --git a/src/resources/extensions/gsd/guided-flow.ts b/src/resources/extensions/gsd/guided-flow.ts index 0259b3063..3d16194b2 100644 --- a/src/resources/extensions/gsd/guided-flow.ts +++ b/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 diff --git a/src/resources/extensions/gsd/tests/next-milestone-id.test.ts b/src/resources/extensions/gsd/tests/next-milestone-id.test.ts new file mode 100644 index 000000000..6e50d83f0 --- /dev/null +++ b/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(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 { + 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); +});