mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 17:45:53 +01:00
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 <noreply@paperclip.ing>
This commit is contained in:
2 files changed
+53
-6
No files matched your search
@@ -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;
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in new issue
Block a user