From d966069a788e714b76adcc39aab0e0b26909d89c Mon Sep 17 00:00:00 2001 From: Priya Raman Date: Fri, 28 Aug 2026 01:19:39 +0000 Subject: [PATCH] fix(adapter-utils): skip the remote session close when the duplex channel is already lost The teardown placed a remote session-close call over a duplex control channel the runtime had already latched as lost. The call carries no deadline of its own, so it blocked until the adapter execution timeout released it, leaving the run row read as running long after the outcome was known. The turn-finalize settlement now marks a lost channel with skipRemoteClose, and the end_session step releases the runtime locally without placing the remote call when that flag is set. Known residual: skipping the remote close also skips the vendored runtime's only caller of closeBackendSession, so it keeps a retained client for the run. A duplex loss proves only that the control channel died, not that the backend process died. A separate tracked goal owns that follow-up. Co-authored-by: Paperclip --- .../src/acpx-engine/execute.test.ts | 18 +++++++++++++++ .../adapter-utils/src/acpx-engine/execute.ts | 22 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index 5575b37b46..2149ed2ed3 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -6108,4 +6108,22 @@ describe("ACPX engine sandbox bridge run-disposition seam (fail-closed)", () => fake.emitLoss("provider_exit"); expect(fake.readDisposition().failed).toBe(false); }); + + it("releases the runtime locally and places no remote close call once the duplex channel is lost", async () => { + const sandbox = await setupRemoteSandbox(); + const fake = createFakeBridgeHandle(); + let closeCalls = 0; + const runtime = runtimeWithControlledResult(() => fake.emitLoss("provider_exit")); + // A close call with no deadline of its own would hang forever on a dead + // channel. The test times out if the teardown still places the call. + runtime.close = () => { + closeCalls += 1; + return new Promise(() => {}); + }; + + const result = await runRemote(fake.handle, runtime, sandbox); + + expect(result.errorCode).toBe("duplex_channel_lost"); + expect(closeCalls).toBe(0); + }); }); diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index 29261ea596..209bdc702c 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -2405,6 +2405,10 @@ interface RuntimeSettlementPlan { // Cancel the running turn with this reason before the close (the turn-error // path cancels before it closes). Null on every other path. readonly cancelTurnReason: string | null; + // True when the duplex control channel is already known lost. The settlement + // then releases the runtime locally and places no remote close call, because + // that call has no deadline of its own and would block on the dead channel. + readonly skipRemoteClose: boolean; } function renderPaperclipEnvNote(env: Record): string { @@ -3803,6 +3807,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { dropWarmEntry: true, recordCloseError: true, cancelTurnReason: null, + skipRemoteClose: false, }; await emitPhase("ensure_session", ensureSessionPhaseStart, "failed"); const { classified, message } = await emitAcpxFailure({ @@ -3839,6 +3844,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { dropWarmEntry: false, recordCloseError: true, cancelTurnReason: null, + skipRemoteClose: false, }; await emitPhase("ensure_session", ensureSessionPhaseStart, "failed"); capturedResult = { @@ -3922,6 +3928,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { dropWarmEntry: true, recordCloseError: true, cancelTurnReason: null, + skipRemoteClose: false, }; await emitPhase("configure_session", configureSessionStart, "failed"); const { classified, message } = await emitAcpxFailure({ @@ -4175,6 +4182,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { dropWarmEntry: false, recordCloseError: false, cancelTurnReason: null, + skipRemoteClose: channelLost, }; const errorMessage = timedOut @@ -4313,6 +4321,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { dropWarmEntry: true, recordCloseError: true, cancelTurnReason: preEmitMessage, + skipRemoteClose: false, }; // Emit the failure best-effort. `turnFinalize` must not reject, so a // failing emission never propagates: the settlement owns the teardown, and @@ -4420,12 +4429,25 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { dropWarmEntry: false, recordCloseError: true, cancelTurnReason: null, + skipRemoteClose: false, }; // Cancel a running turn before the close (the turn-error path). if (settlement.cancelTurnReason && activeTurn) { await activeTurn.cancel({ reason: settlement.cancelTurnReason }).catch(() => {}); } const existing = warmHandles.get(prepared.sessionKey); + // The control channel is already known lost, so no remote call can + // reach the backend. Release the local bookkeeping only and place no + // `runtime.close(...)` call — that call has no deadline of its own + // and would block on the dead channel. + if (settlement.skipRemoteClose) { + if (warmHandleMatches(existing, runtime, settlement.handle) && existing) { + clearWarmHandleTimer(existing); + warmHandles.delete(prepared.sessionKey); + flushChildStderr(existing.childStderrState); + } + return; + } if ( settlement.mode === "warm_or_close" && warmHandleMatches(existing, runtime, settlement.handle) &&