fix: return retry from postUnitPreVerification when artifact verification fails (#1571) (#1782)

When verifyExpectedArtifact returns false for a unit type with a known
expected artifact, postUnitPreVerification now returns "retry" instead
of "continue". This sets pendingVerificationRetry on the session so the
next loop iteration re-dispatches with failure context, preventing
13+ blind re-dispatches of the same failed unit before the stuck-loop
detector kicks in.

Closes #1571

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
TÂCHES 2026-03-21 09:46:27 -06:00 committed by GitHub
parent d14e67cad8
commit 2da1ecfd20
4 changed files with 119 additions and 3 deletions

View file

@ -31,6 +31,7 @@ import {
} from "./worktree.js";
import {
verifyExpectedArtifact,
resolveExpectedArtifactPath,
} from "./auto-recovery.js";
import { writeUnitRuntimeRecord, clearUnitRuntimeRecord } from "./unit-runtime.js";
import { runGSDDoctor, rebuildState, summarizeDoctorIssues } from "./doctor.js";
@ -84,9 +85,12 @@ export interface PostUnitContext {
* Pre-verification processing: parallel worker signal check, cache invalidation,
* auto-commit, doctor run, state rebuild, worktree sync, artifact verification.
*
* Returns "dispatched" if a signal caused stop/pause, "continue" to proceed.
* Returns:
* - "dispatched" a signal caused stop/pause
* - "continue" proceed normally
* - "retry" artifact verification failed, s.pendingVerificationRetry set for loop re-iteration
*/
export async function postUnitPreVerification(pctx: PostUnitContext, opts?: PreVerificationOpts): Promise<"dispatched" | "continue"> {
export async function postUnitPreVerification(pctx: PostUnitContext, opts?: PreVerificationOpts): Promise<"dispatched" | "continue" | "retry"> {
const { s, ctx, pi, buildSnapshotOpts, stopAuto, pauseAuto } = pctx;
// ── Parallel worker signal check ──
@ -347,6 +351,29 @@ export async function postUnitPreVerification(pctx: PostUnitContext, opts?: PreV
} catch (e) {
debugLog("postUnit", { phase: "artifact-verify", error: String(e) });
}
// When artifact verification fails for a unit type that has a known expected
// artifact, return "retry" so the caller re-dispatches with failure context
// instead of blindly re-dispatching the same unit (#1571).
if (!triggerArtifactVerified) {
const hasExpectedArtifact = resolveExpectedArtifactPath(s.currentUnit.type, s.currentUnit.id, s.basePath) !== null;
if (hasExpectedArtifact) {
const retryKey = `${s.currentUnit.type}:${s.currentUnit.id}`;
const attempt = (s.verificationRetryCount.get(retryKey) ?? 0) + 1;
s.verificationRetryCount.set(retryKey, attempt);
s.pendingVerificationRetry = {
unitId: s.currentUnit.id,
failureContext: `Artifact verification failed: expected artifact for ${s.currentUnit.type} "${s.currentUnit.id}" was not found on disk after unit execution (attempt ${attempt}).`,
attempt,
};
debugLog("postUnit", { phase: "artifact-verify-retry", unitType: s.currentUnit.type, unitId: s.currentUnit.id, attempt });
ctx.ui.notify(
`Artifact missing for ${s.currentUnit.type} ${s.currentUnit.id} — retrying (attempt ${attempt})`,
"warning",
);
return "retry";
}
}
} else {
// Hook unit completed — finalize its runtime record
try {

View file

@ -273,7 +273,7 @@ export interface LoopDeps {
postUnitPreVerification: (
pctx: PostUnitContext,
opts?: PreVerificationOpts,
) => Promise<"dispatched" | "continue">;
) => Promise<"dispatched" | "continue" | "retry">;
runPostUnitVerification: (
vctx: VerificationContext,
pauseAuto: (ctx?: ExtensionContext, pi?: ExtensionAPI) => Promise<void>,

View file

@ -1137,6 +1137,17 @@ export async function runFinalize(
});
return { action: "break", reason: "pre-verification-dispatched" };
}
if (preResult === "retry") {
if (sidecarItem) {
// Sidecar artifact retries are skipped — just continue
debugLog("autoLoop", { phase: "sidecar-artifact-retry-skipped", iteration: ic.iteration });
} else {
// s.pendingVerificationRetry was set by postUnitPreVerification.
// Continue the loop — next iteration will inject the retry context into the prompt.
debugLog("autoLoop", { phase: "artifact-verification-retry", iteration: ic.iteration });
return { action: "continue" };
}
}
if (pauseAfterUatDispatch) {
ctx.ui.notify(

View file

@ -1762,3 +1762,81 @@ test("resolveAgentEndCancelled prevents orphaned promise after abort path", asyn
const result = await resultPromise;
assert.equal(result.status, "cancelled");
});
// ─── #1571: artifact verification retry ──────────────────────────────────────
test("autoLoop re-iterates when postUnitPreVerification returns retry (#1571)", async () => {
_resetPendingResolve();
const ctx = makeMockCtx();
ctx.ui.setStatus = () => {};
const pi = makeMockPi();
const s = makeLoopSession();
let preVerifyCallCount = 0;
const deps = makeMockDeps({
deriveState: async () => {
deps.callLog.push("deriveState");
return {
phase: "executing",
activeMilestone: { id: "M001", title: "Test", status: "active" },
activeSlice: { id: "S01", title: "Slice 1" },
activeTask: { id: "T01" },
registry: [{ id: "M001", status: "active" }],
blockers: [],
} as any;
},
postUnitPreVerification: async () => {
deps.callLog.push("postUnitPreVerification");
preVerifyCallCount++;
// First call returns "retry" (artifact missing), second returns "continue"
if (preVerifyCallCount === 1) {
return "retry" as const;
}
return "continue" as const;
},
postUnitPostVerification: async () => {
deps.callLog.push("postUnitPostVerification");
// After the retry succeeds (second iteration), stop the loop
s.active = false;
return "continue" as const;
},
});
const loopPromise = autoLoop(ctx, pi, s, deps);
// First iteration: runUnit completes → preVerification returns "retry" → loop continues
await new Promise((r) => setTimeout(r, 50));
resolveAgentEnd(makeEvent());
// Second iteration: runUnit completes → preVerification returns "continue" → full finalize
await new Promise((r) => setTimeout(r, 50));
resolveAgentEnd(makeEvent());
await loopPromise;
// preVerification should have been called twice (retry + success)
assert.equal(preVerifyCallCount, 2, "preVerification should be called twice");
// When preVerification returns "retry", runPostUnitVerification and
// postUnitPostVerification should be skipped for that iteration.
// So we expect 1 call each (only the second iteration proceeds past pre-verification).
const postVerifyCalls = deps.callLog.filter(
(c: string) => c === "runPostUnitVerification",
);
const postPostVerifyCalls = deps.callLog.filter(
(c: string) => c === "postUnitPostVerification",
);
assert.equal(
postVerifyCalls.length,
1,
"runPostUnitVerification should only be called once (skipped on retry iteration)",
);
assert.equal(
postPostVerifyCalls.length,
1,
"postUnitPostVerification should only be called once (skipped on retry iteration)",
);
});