From 77e22d9957ec961847cce0dd5dd0ea6ed23fa395 Mon Sep 17 00:00:00 2001 From: Priya Raman Date: Thu, 3 Sep 2026 11:25:16 +0000 Subject: [PATCH] fix(runner): keep a live host retryable when the sandbox claim release fails A live host's close marked itself closed as soon as its other resources shut down, even when the sandbox delete-authority claim release itself then threw. A retried close saw the host already closed and returned early, so the claim was never released again: the marker stayed pinned to this admission forever, even though every real resource had already shut down cleanly. Mark the host closed only once the release itself also succeeds, so a retried close still reaches it. Co-authored-by: Paperclip --- .../src/drivers/acpx/runtime-host.test.ts | 42 +++++++++++++++++++ .../src/drivers/acpx/runtime-host.ts | 17 +++++--- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts b/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts index 5a338898aa2..cd8cb3e8031 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts @@ -900,6 +900,48 @@ describe("ACPX runtime host", () => { await expect(stat(root)).rejects.toMatchObject({ code: "ENOENT" }); }); + it("does not mark itself closed when releasing the sandbox claim fails, so a retry still reaches the release", async () => { + const fixture = await hostFixture(); + const runtime = runtimePort(); + // Hand the host a lookalike sandbox object holding the same real, + // prepared directories, but a distinct reference from the one + // `prepareAcpxRuntimeSandbox` registered as the claim's owner. The + // release call keys its ownership lookup on that exact reference, so + // this copy can never satisfy it: releasing through it always throws, + // deterministically, however many times it is retried. + const dependencies = fixture.dependencies({ + openRuntime: async () => runtime, + }); + dependencies.prepareSandbox = vi.fn(async (input) => ({ + ...(await prepareAcpxRuntimeSandbox(input)), + })); + const host = await AcpxRuntimeHost.open( + { + ...fixture.options, + agent: "codex", + model: "gpt-5.6-sol", + permissionMode: "approve-all", + environment: { + PAPERCLIP_ACPX_CODEX_AUTH_JSON_SECRET: "{}", + }, + }, + dependencies, + ); + + await expect(host.close({ reason: "first close" })).rejects.toThrow( + /cleanup failed/, + ); + expect(runtime.close).toHaveBeenCalledOnce(); + + // The claim release failed, so this host must not consider itself + // closed: a retried close must still reach cleanup, not silently report + // success while the sandbox claim stays pinned forever. + await expect(host.close({ reason: "retry close" })).rejects.toThrow( + /cleanup failed/, + ); + expect(runtime.close).toHaveBeenCalledTimes(2); + }); + it("scrubs credentials only after the exact pending runtime close resolves", async () => { const fixture = await hostFixture(); let resolveRuntimeClose!: () => void; diff --git a/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts b/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts index 07fdfecb7c3..416cf151f7d 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts @@ -664,15 +664,20 @@ export class AcpxRuntimeHost { // a root that is still in use. A retried close reaches this release // once shutdown succeeds. await releaseAcpxRuntimeSandboxRootClaim(this.#sandbox); + // Mark the host closed only once the claim release itself has also + // succeeded. Marking it closed on a failed release would make a + // retried close return early without ever reaching this release + // again, permanently pinning the marker even though every other + // resource has already shut down cleanly. + // Runtime, credential, and command ownership has been relinquished + // even when the provider never acknowledged turn cancellation. + // Preserve that cancellation error for this caller, but make later + // close calls idempotently observe the successfully closed host. + if (this.#activeTurn === activeTurn) this.#activeTurn = null; + this.#closed = true; } catch (error) { errors.push(error); } - // Runtime, credential, and command ownership has been relinquished even - // when the provider never acknowledged turn cancellation. Preserve that - // cancellation error for this caller, but make later close calls - // idempotently observe the successfully closed host. - if (this.#activeTurn === activeTurn) this.#activeTurn = null; - this.#closed = true; } if (errors.length > 0) { throw new AggregateError(errors, "ACPX runtime cleanup failed");