fix(runner): contain protocol-failure cleanup rejection

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta 2026-09-12 22:54:18 -05:00
parent b3426bb5ce
commit de64d16f17
3 changed files with 74 additions and 1 deletions

View File

@ -1231,3 +1231,9 @@ An asynchronous remote signal failure, including a sandbox already removed by
the operator, must not crash the controller. Logging that failure must also be
contained. A rejected signal does not prove termination: existing process and
provider monitoring still own stop acknowledgement and cleanup proof.
Protocol-failure handling can begin transport cleanup before the owning runtime
awaits it. That background invocation observes rejection immediately, including
when a remote sandbox has already disappeared. The owner's awaited close still
receives the original failure; containment never fabricates a successful close
or permission to reuse an unverified execution.

View File

@ -0,0 +1,62 @@
import { execFileSync } from "node:child_process";
import { describe, expect, it } from "vitest";
describe("protocol-failure cleanup", () => {
it.each(["immediate", "delayed", "successful"])(
"contains %s background cleanup without hiding its outcome from the owner",
(outcome) => {
// Use Node's fatal unhandled-rejection policy in a separate process.
// A test-runner rejection listener would conceal the controller crash.
const source = `
import assert from "node:assert/strict";
import { CodexHarnessSession } from ${JSON.stringify(new URL("./codex-harness-session.ts", import.meta.url).href)};
const cleanupError = new Error("Sandbox not found during cleanup");
let closePromise;
let cleanupAttempts = 0;
const transport = {
setServerRequestHandler() {},
async *notifications() { throw new Error("Sandbox not found during monitoring"); },
close() {
if (!closePromise) {
cleanupAttempts++;
closePromise = ${JSON.stringify(outcome)} === "successful"
? Promise.resolve()
: ${JSON.stringify(outcome)} === "delayed"
? new Promise((_, reject) => setTimeout(() => reject(cleanupError), 10))
: Promise.reject(cleanupError);
}
return closePromise;
},
};
const session = new CodexHarnessSession({
transport, runId: "run-cleanup", normalizedSessionId: "session-cleanup",
opened: { lineage: { threadId: "thread-cleanup" }, context: {} }, taskEnvelope: {},
conversationMode: "task", resumed: false, activeTurnId: "turn-cleanup",
sourceSequence: 0, now: () => new Date(), runnerInstanceId: "runner-cleanup",
driverKind: "codex", capabilities: {}, goalCapability: "disabled",
goalAvailability: "unavailable", goalReasonCode: null, goalReason: null,
dynamicTools: [],
});
// The ordinary owner may only join cleanup on a later event-loop turn.
await new Promise(resolve => setTimeout(resolve, 40));
assert.equal(session.protocolFailed, true);
assert.equal(session.protocolFailureCode, "notification_transport_failed");
assert.equal(session.activeTurnId, null);
const events = [];
for await (const event of session.eventQueue) events.push(event);
assert.equal(events.filter(event => event.eventType === "session.failed").length, 1);
assert.equal(events.filter(event => event.eventType === "turn.failed").length, 1);
assert.equal(events.some(event => event.eventType === "turn.completed"), false);
session.failProtocol("duplicate_failure", "must not retry cleanup");
if (${JSON.stringify(outcome)} === "successful") await session.close();
else await assert.rejects(session.close(), error => error === cleanupError);
assert.equal(cleanupAttempts, 1);
process.stdout.write("HOST_ALIVE_CLEANUP_OUTCOME_PRESERVED");
`;
expect(execFileSync(process.execPath, [
"--unhandled-rejections=strict", "--import", import.meta.resolve("tsx"),
"--input-type=module", "--eval", source,
], { encoding: "utf8", timeout: 10_000 })).toBe("HOST_ALIVE_CLEANUP_OUTCOME_PRESERVED");
},
);
});

View File

@ -415,7 +415,12 @@ export class CodexSessionState {
}
this.terminal = true;
this.eventQueue.close();
void this.transport.close(`protocol_failure:${code}`);
// Notification failure can initiate cleanup before the owning runtime joins
// it. Observe this background rejection immediately so a deleted remote
// sandbox cannot crash the controller. The transport retains its original
// close promise: the owner's awaited session.close still receives any
// cleanup failure and must not treat it as confirmed termination.
void this.transport.close(`protocol_failure:${code}`).catch(() => undefined);
}
emit(