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 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-03-30 16:32:49 -04:00 committed by GitHub
parent 8b26ec7803
commit 9cebf19559
2 changed files with 52 additions and 1 deletions

View file

@ -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

View file

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