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 <noreply@paperclip.ing>
This commit is contained in:
Priya Raman 2026-08-28 01:19:39 +00:00
parent 4436cf00a2
commit d966069a78
No known key found for this signature in database
GPG Key ID: 4861541D36B2037E
2 changed files with 40 additions and 0 deletions

View File

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

View File

@ -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, string>): 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) &&