fix: resolve pending unit promise on all exit paths to prevent orphaned auto-loop (#1774)
handleAgentEnd, pauseAuto, and supervision timer catch blocks could leave the unitPromise unresolved, causing autoLoop to hang permanently on `await unitPromise`. Add resolveAgentEndCancelled() and call it on every exit path that previously skipped resolution. Closes #1666 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
4c3fafd6a6
commit
605fa6803a
6 changed files with 140 additions and 3 deletions
|
|
@ -8,7 +8,7 @@
|
|||
*/
|
||||
|
||||
export { autoLoop } from "./auto/loop.js";
|
||||
export { resolveAgentEnd, isSessionSwitchInFlight, _resetPendingResolve, _setActiveSession } from "./auto/resolve.js";
|
||||
export { resolveAgentEnd, resolveAgentEndCancelled, isSessionSwitchInFlight, _resetPendingResolve, _setActiveSession } from "./auto/resolve.js";
|
||||
export { detectStuck } from "./auto/detect-stuck.js";
|
||||
export { runUnit } from "./auto/run-unit.js";
|
||||
export type { LoopDeps } from "./auto/loop-deps.js";
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import { detectWorkingTreeActivity } from "./auto-supervisor.js";
|
|||
import { closeoutUnit, type CloseoutOptions } from "./auto-unit-closeout.js";
|
||||
import { saveActivityLog } from "./activity-log.js";
|
||||
import { recoverTimedOutUnit, type RecoveryContext } from "./auto-timeout-recovery.js";
|
||||
import { resolveAgentEndCancelled } from "./auto/resolve.js";
|
||||
import type { AutoSession } from "./auto/session.js";
|
||||
|
||||
export interface SupervisionContext {
|
||||
|
|
@ -129,6 +130,8 @@ export function startUnitSupervision(sctx: SupervisionContext): void {
|
|||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
console.error(`[idle-watchdog] Unhandled error: ${message}`);
|
||||
// Unblock any pending unit promise so the auto-loop is not orphaned.
|
||||
resolveAgentEndCancelled();
|
||||
try {
|
||||
ctx.ui.notify(`Idle watchdog error: ${message}`, "warning");
|
||||
} catch { /* best effort */ }
|
||||
|
|
@ -161,6 +164,8 @@ export function startUnitSupervision(sctx: SupervisionContext): void {
|
|||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
console.error(`[hard-timeout] Unhandled error: ${message}`);
|
||||
// Unblock any pending unit promise so the auto-loop is not orphaned.
|
||||
resolveAgentEndCancelled();
|
||||
try {
|
||||
ctx.ui.notify(`Hard timeout error: ${message}`, "warning");
|
||||
} catch { /* best effort */ }
|
||||
|
|
|
|||
|
|
@ -198,7 +198,7 @@ import {
|
|||
postUnitPostVerification,
|
||||
} from "./auto-post-unit.js";
|
||||
import { bootstrapAutoSession, type BootstrapDeps } from "./auto-start.js";
|
||||
import { autoLoop, resolveAgentEnd, isSessionSwitchInFlight, type LoopDeps } from "./auto-loop.js";
|
||||
import { autoLoop, resolveAgentEnd, resolveAgentEndCancelled, isSessionSwitchInFlight, type LoopDeps } from "./auto-loop.js";
|
||||
import {
|
||||
WorktreeResolver,
|
||||
type WorktreeResolverDeps,
|
||||
|
|
@ -719,6 +719,8 @@ export async function pauseAuto(
|
|||
): Promise<void> {
|
||||
if (!s.active) return;
|
||||
clearUnitTimeout();
|
||||
// Unblock any pending unit promise so the auto-loop is not orphaned.
|
||||
resolveAgentEndCancelled();
|
||||
|
||||
s.pausedSessionFile = ctx?.sessionManager?.getSessionFile() ?? null;
|
||||
|
||||
|
|
@ -1133,7 +1135,11 @@ export async function handleAgentEnd(
|
|||
ctx: ExtensionContext,
|
||||
pi: ExtensionAPI,
|
||||
): Promise<void> {
|
||||
if (!s.active || !s.cmdCtx) return;
|
||||
if (!s.active || !s.cmdCtx) {
|
||||
// Even when inactive, resolve any pending promise so the loop is unblocked.
|
||||
resolveAgentEndCancelled();
|
||||
return;
|
||||
}
|
||||
clearUnitTimeout();
|
||||
resolveAgentEnd({ messages: [] });
|
||||
}
|
||||
|
|
|
|||
|
|
@ -68,6 +68,24 @@ export function isSessionSwitchInFlight(): boolean {
|
|||
return _sessionSwitchInFlight;
|
||||
}
|
||||
|
||||
// ─── resolveAgentEndCancelled ─────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Force-resolve the pending unit promise with { status: "cancelled" }.
|
||||
*
|
||||
* Used by pauseAuto, handleAgentEnd early-return, and supervision catch
|
||||
* blocks to ensure the autoLoop is never stuck awaiting a promise that
|
||||
* will never resolve. Safe to call when no resolver is pending (no-op).
|
||||
*/
|
||||
export function resolveAgentEndCancelled(): void {
|
||||
if (_currentResolve) {
|
||||
debugLog("resolveAgentEndCancelled", { status: "resolving-cancelled" });
|
||||
const r = _currentResolve;
|
||||
_currentResolve = null;
|
||||
r({ status: "cancelled" });
|
||||
}
|
||||
}
|
||||
|
||||
// ─── resetPendingResolve (test helper) ───────────────────────────────────────
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -81,3 +81,63 @@ test("handleAgentEnd is a thin compatibility wrapper", () => {
|
|||
"handleAgentEnd must not dispatch recursively",
|
||||
);
|
||||
});
|
||||
|
||||
test("handleAgentEnd early return calls resolveAgentEndCancelled", () => {
|
||||
const source = getAutoTsSource();
|
||||
const fnIdx = source.indexOf("export async function handleAgentEnd");
|
||||
assert.ok(fnIdx > -1, "handleAgentEnd must exist in auto.ts");
|
||||
const fnBlock = source.slice(fnIdx, source.indexOf("\n// ─── ", fnIdx + 100));
|
||||
|
||||
assert.ok(
|
||||
fnBlock.includes("resolveAgentEndCancelled()"),
|
||||
"handleAgentEnd must call resolveAgentEndCancelled on early return to prevent orphaned promises",
|
||||
);
|
||||
});
|
||||
|
||||
test("pauseAuto calls resolveAgentEndCancelled to unblock the loop", () => {
|
||||
const source = getAutoTsSource();
|
||||
const fnIdx = source.indexOf("export async function pauseAuto");
|
||||
assert.ok(fnIdx > -1, "pauseAuto must exist in auto.ts");
|
||||
// Extract the function body (up to the next export or top-level function)
|
||||
const fnBlock = source.slice(fnIdx, source.indexOf("\n/**\n * Build", fnIdx + 100));
|
||||
|
||||
assert.ok(
|
||||
fnBlock.includes("resolveAgentEndCancelled()"),
|
||||
"pauseAuto must call resolveAgentEndCancelled to unblock the auto-loop promise",
|
||||
);
|
||||
});
|
||||
|
||||
test("auto-timers.ts idle watchdog catch calls resolveAgentEndCancelled", () => {
|
||||
const TIMERS_PATH = join(__dirname, "..", "auto-timers.ts");
|
||||
const source = readFileSync(TIMERS_PATH, "utf-8");
|
||||
|
||||
const idleCatchIdx = source.indexOf("[idle-watchdog] Unhandled error");
|
||||
assert.ok(idleCatchIdx > -1, "idle watchdog catch block must exist");
|
||||
// Check that resolveAgentEndCancelled is called near this catch
|
||||
const catchRegion = source.slice(Math.max(0, idleCatchIdx - 200), idleCatchIdx + 200);
|
||||
assert.ok(
|
||||
catchRegion.includes("resolveAgentEndCancelled()"),
|
||||
"idle watchdog catch block must call resolveAgentEndCancelled",
|
||||
);
|
||||
});
|
||||
|
||||
test("auto-timers.ts hard timeout catch calls resolveAgentEndCancelled", () => {
|
||||
const TIMERS_PATH = join(__dirname, "..", "auto-timers.ts");
|
||||
const source = readFileSync(TIMERS_PATH, "utf-8");
|
||||
|
||||
const hardCatchIdx = source.indexOf("[hard-timeout] Unhandled error");
|
||||
assert.ok(hardCatchIdx > -1, "hard timeout catch block must exist");
|
||||
const catchRegion = source.slice(Math.max(0, hardCatchIdx - 200), hardCatchIdx + 200);
|
||||
assert.ok(
|
||||
catchRegion.includes("resolveAgentEndCancelled()"),
|
||||
"hard timeout catch block must call resolveAgentEndCancelled",
|
||||
);
|
||||
});
|
||||
|
||||
test("resolveAgentEndCancelled is exported from auto/resolve.ts", () => {
|
||||
const source = getAutoResolveTsSource();
|
||||
assert.ok(
|
||||
source.includes("export function resolveAgentEndCancelled"),
|
||||
"auto/resolve.ts must export resolveAgentEndCancelled",
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import { resolve } from "node:path";
|
|||
|
||||
import {
|
||||
resolveAgentEnd,
|
||||
resolveAgentEndCancelled,
|
||||
runUnit,
|
||||
autoLoop,
|
||||
detectStuck,
|
||||
|
|
@ -1714,3 +1715,50 @@ test("autoLoop lifecycle: advances through research → plan → execute → ver
|
|||
"dispatched unit types should follow the full lifecycle sequence",
|
||||
);
|
||||
});
|
||||
|
||||
// ─── resolveAgentEndCancelled tests ──────────────────────────────────────────
|
||||
|
||||
test("resolveAgentEndCancelled resolves a pending promise with cancelled status", async () => {
|
||||
_resetPendingResolve();
|
||||
|
||||
const ctx = makeMockCtx();
|
||||
const pi = makeMockPi();
|
||||
const s = makeMockSession();
|
||||
|
||||
const resultPromise = runUnit(ctx, pi, s, "task", "T01", "prompt");
|
||||
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
|
||||
resolveAgentEndCancelled();
|
||||
|
||||
const result = await resultPromise;
|
||||
assert.equal(result.status, "cancelled");
|
||||
assert.equal(result.event, undefined);
|
||||
});
|
||||
|
||||
test("resolveAgentEndCancelled is a no-op when no promise is pending", () => {
|
||||
_resetPendingResolve();
|
||||
|
||||
assert.doesNotThrow(() => {
|
||||
resolveAgentEndCancelled();
|
||||
});
|
||||
});
|
||||
|
||||
test("resolveAgentEndCancelled prevents orphaned promise after abort path", async () => {
|
||||
_resetPendingResolve();
|
||||
|
||||
const ctx = makeMockCtx();
|
||||
const pi = makeMockPi();
|
||||
const s = makeMockSession();
|
||||
|
||||
const resultPromise = runUnit(ctx, pi, s, "task", "T01", "prompt");
|
||||
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
|
||||
// Simulate abort: deactivate session then cancel
|
||||
s.active = false;
|
||||
resolveAgentEndCancelled();
|
||||
|
||||
const result = await resultPromise;
|
||||
assert.equal(result.status, "cancelled");
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue