From 9cebf1955957305949223f7f52fa82a2e0235376 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 30 Mar 2026 16:32:49 -0400 Subject: [PATCH] fix: route allDiscussed and zero-slices paths to queued milestone discussion (#3150) (#3230) The allDiscussed early-return and pendingSlices.length===0 guard in showDiscuss() both hard-returned without checking for queued milestones, blocking users from discussing pending milestones when the active milestone's slices were all discussed or complete. Now both paths check for pending milestones and route to showDiscussQueuedMilestone() first. Co-authored-by: Claude Opus 4.6 --- src/resources/extensions/gsd/guided-flow.ts | 13 +++++- .../tests/discuss-queued-milestones.test.ts | 40 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/resources/extensions/gsd/guided-flow.ts b/src/resources/extensions/gsd/guided-flow.ts index 86a8dfd15..5b56ba378 100644 --- a/src/resources/extensions/gsd/guided-flow.ts +++ b/src/resources/extensions/gsd/guided-flow.ts @@ -627,6 +627,12 @@ export async function showDiscuss( const pendingSlices = normSlices.filter(s => !s.done); if (pendingSlices.length === 0) { + // All slices complete — but queued milestones may still need discussion (#3150) + const pendingMilestones = state.registry.filter(m => m.status === "pending"); + if (pendingMilestones.length > 0) { + await showDiscussQueuedMilestone(ctx, pi, basePath, pendingMilestones); + return; + } ctx.ui.notify("All slices are complete — nothing to discuss.", "info"); return; } @@ -643,9 +649,14 @@ export async function showDiscuss( discussedMap.set(s.id, !!contextFile); } - // If all pending slices are discussed, notify and exit instead of looping + // If all pending slices are discussed, check for queued milestones before exiting (#3150) const allDiscussed = pendingSlices.every(s => discussedMap.get(s.id)); if (allDiscussed) { + const pendingMilestones = state.registry.filter(m => m.status === "pending"); + if (pendingMilestones.length > 0) { + await showDiscussQueuedMilestone(ctx, pi, basePath, pendingMilestones); + return; + } const lockData = readSessionLockData(basePath); const remoteAutoRunning = lockData && lockData.pid !== process.pid && isSessionLockProcessAlive(lockData); const nextStep = remoteAutoRunning diff --git a/src/resources/extensions/gsd/tests/discuss-queued-milestones.test.ts b/src/resources/extensions/gsd/tests/discuss-queued-milestones.test.ts index 98c400f95..63e79f3f6 100644 --- a/src/resources/extensions/gsd/tests/discuss-queued-milestones.test.ts +++ b/src/resources/extensions/gsd/tests/discuss-queued-milestones.test.ts @@ -238,4 +238,44 @@ describe("discuss-queued-milestones (#2307)", () => { "queued milestone picker must label entries with [queued] to distinguish from active", ); }); + + // ─── #3150: allDiscussed early-return must not block queued milestone discussion ── + + test("12. allDiscussed path checks for pending milestones before returning (#3150)", () => { + const source = readGuidedFlowSource(); + + // Extract the allDiscussed block — the if (allDiscussed) { ... } body + const allDiscussedMatch = source.match( + /const allDiscussed = pendingSlices\.every\([\s\S]*?\n if \(allDiscussed\) \{([\s\S]*?)\n \}/, + ); + assert.ok(!!allDiscussedMatch, "allDiscussed guard block must exist in showDiscuss()"); + + if (allDiscussedMatch) { + const body = allDiscussedMatch[1]; + // The fix must check for pending milestones and route to showDiscussQueuedMilestone + assert.ok( + body.includes("pending") && body.includes("showDiscussQueuedMilestone"), + "allDiscussed block must check for pending milestones and call showDiscussQueuedMilestone before returning (#3150)", + ); + } + }); + + test("13. pendingSlices.length===0 path checks for pending milestones before returning (#3150)", () => { + const source = readGuidedFlowSource(); + + // Find the pendingSlices.length === 0 guard block + const zeroSlicesMatch = source.match( + /if \(pendingSlices\.length === 0\) \{([\s\S]*?)\n \}/, + ); + assert.ok(!!zeroSlicesMatch, "pendingSlices.length === 0 guard block must exist in showDiscuss()"); + + if (zeroSlicesMatch) { + const body = zeroSlicesMatch[1]; + // The fix must check for pending milestones and route to showDiscussQueuedMilestone + assert.ok( + body.includes("pending") && body.includes("showDiscussQueuedMilestone"), + "pendingSlices.length===0 block must check for pending milestones and call showDiscussQueuedMilestone before returning (#3150)", + ); + } + }); });