From 995715202468d375223ae9bb7f470a61101e070f Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 14 Apr 2026 07:22:56 -0500 Subject: [PATCH] fix(auto-mode): prevent false milestone merge after complete-milestone failure (#4175) When complete-milestone failed verification, auto-mode could end up merging the worktree to main anyway and emit a metadata-only merge warning, creating a misleading near-complete signal while the SUMMARY was never actually written. The blocker-placeholder path for complete-milestone wrote a stub SUMMARY without updating DB status, and stopAuto's SUMMARY-presence check treated the stub as a legitimate completion signal. - auto-post-unit.ts: skip blocker placeholder and pause auto-mode on complete-milestone verification retry exhaustion. - auto-timeout-recovery.ts: same guard for the idle/hard timeout path. - auto.ts: make stopAuto Step 4 DB-authoritative (getMilestone.status === "complete") with SUMMARY-presence fallback only for DB-unavailable legacy projects. Co-Authored-By: Claude Opus 4.6 --- .../extensions/gsd/auto-post-unit.ts | 24 +++++++++++ .../extensions/gsd/auto-timeout-recovery.ts | 17 ++++++++ src/resources/extensions/gsd/auto.ts | 40 ++++++++++++------- 3 files changed, 67 insertions(+), 14 deletions(-) diff --git a/src/resources/extensions/gsd/auto-post-unit.ts b/src/resources/extensions/gsd/auto-post-unit.ts index b0bd77dd2..f9f9ee4f0 100644 --- a/src/resources/extensions/gsd/auto-post-unit.ts +++ b/src/resources/extensions/gsd/auto-post-unit.ts @@ -612,6 +612,30 @@ export async function postUnitPreVerification(pctx: PostUnitContext, opts?: PreV s.verificationRetryCount.set(retryKey, attempt); if (attempt > MAX_VERIFICATION_RETRIES) { + // #4175: For complete-milestone, a blocker placeholder is harmful — + // the stub SUMMARY has no recovery value (milestone is terminal), + // it does not update DB status (so deriveState never advances), + // and it fools stopAuto's presence check into merging a milestone + // that was never legitimately completed. Pause auto-mode with a + // clear single failure signal and preserve the worktree branch. + if (s.currentUnit.type === "complete-milestone") { + debugLog("postUnit", { + phase: "artifact-verify-pause-complete-milestone", + unitType: s.currentUnit.type, + unitId: s.currentUnit.id, + attempt, + maxRetries: MAX_VERIFICATION_RETRIES, + }); + s.verificationRetryCount.delete(retryKey); + s.pendingVerificationRetry = null; + ctx.ui.notify( + `Milestone ${s.currentUnit.id} verification failed after ${MAX_VERIFICATION_RETRIES} retries — worktree branch preserved. Re-run /gsd auto once blockers are resolved.`, + "error", + ); + await pauseAuto(ctx, pi); + return "dispatched"; + } + // Retries exhausted — write a blocker placeholder so the pipeline // can advance past this stuck unit (#2653). debugLog("postUnit", { diff --git a/src/resources/extensions/gsd/auto-timeout-recovery.ts b/src/resources/extensions/gsd/auto-timeout-recovery.ts index 4d62a9fec..fa385fc1b 100644 --- a/src/resources/extensions/gsd/auto-timeout-recovery.ts +++ b/src/resources/extensions/gsd/auto-timeout-recovery.ts @@ -230,6 +230,23 @@ export async function recoverTimedOutUnit( return "recovered"; } + // #4175: For complete-milestone, never write a blocker placeholder — a stub + // SUMMARY has no recovery value (milestone is terminal), it does not update + // DB status, and downstream merge paths can treat the stub as a legitimate + // completion signal. Pause instead so the worktree branch is preserved. + if (unitType === "complete-milestone") { + writeUnitRuntimeRecord(basePath, unitType, unitId, currentUnitStartedAt, { + phase: "paused", + recoveryAttempts: recoveryAttempts + 1, + lastRecoveryReason: reason, + }); + ctx.ui.notify( + `Milestone ${unitId} ${reason}-recovery exhausted ${maxRecoveryAttempts} attempt(s) — worktree branch preserved. Re-run /gsd auto once blockers are resolved.`, + "error", + ); + return "paused"; + } + // Retries exhausted — write a blocker placeholder and advance the pipeline // instead of silently stalling. const placeholder = writeBlockerPlaceholder( diff --git a/src/resources/extensions/gsd/auto.ts b/src/resources/extensions/gsd/auto.ts index 18cad0d22..c5ad7a26c 100644 --- a/src/resources/extensions/gsd/auto.ts +++ b/src/resources/extensions/gsd/auto.ts @@ -186,7 +186,7 @@ import { deregisterSigtermHandler as _deregisterSigtermHandler, detectWorkingTreeActivity, } from "./auto-supervisor.js"; -import { isDbAvailable } from "./gsd-db.js"; +import { isDbAvailable, getMilestone } from "./gsd-db.js"; import { countPendingCaptures } from "./captures.js"; import { clearCmuxSidebar, logCmuxEvent, syncCmuxSidebar } from "../cmux/index.js"; @@ -753,24 +753,36 @@ export async function stopAuto( : { notify: () => {} }; const resolver = buildResolver(); - // Check if the milestone is complete — SUMMARY file is the authoritative signal. + // Check if the milestone is complete. DB status is the authoritative + // signal — only a successful gsd_complete_milestone call flips it to + // "complete" (tools/complete-milestone.ts). SUMMARY file presence is + // NOT sufficient: a blocker placeholder stub or a partial write can + // leave a file behind without the milestone actually being done, + // which previously caused stopAuto to merge a failed milestone and + // emit a misleading metadata-only merge warning (#4175). + // DB-unavailable projects fall back to SUMMARY-file presence. let milestoneComplete = false; try { - const summaryPath = resolveMilestoneFile( - s.originalBasePath || s.basePath, - s.currentMilestoneId, - "SUMMARY", - ); - if (!summaryPath) { - // Also check in the worktree path (SUMMARY may not be synced yet) - const wtSummaryPath = resolveMilestoneFile( - s.basePath, + if (isDbAvailable()) { + const dbRow = getMilestone(s.currentMilestoneId); + milestoneComplete = dbRow?.status === "complete"; + } else { + const summaryPath = resolveMilestoneFile( + s.originalBasePath || s.basePath, s.currentMilestoneId, "SUMMARY", ); - milestoneComplete = wtSummaryPath !== null; - } else { - milestoneComplete = true; + if (!summaryPath) { + // Also check in the worktree path (SUMMARY may not be synced yet) + const wtSummaryPath = resolveMilestoneFile( + s.basePath, + s.currentMilestoneId, + "SUMMARY", + ); + milestoneComplete = wtSummaryPath !== null; + } else { + milestoneComplete = true; + } } } catch (err) { // Non-fatal — fall through to preserveBranch path