From 34d274f6d3de893f9783e5712ee3e5fa5c8b5038 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 9 Sep 2026 06:37:56 -0500 Subject: [PATCH] fix: retain verified ACPX commands across recovered turns Keep runtime-owned command snapshots reusable until cleanup and observe optional prompt-start rejection without hiding it from callers. Co-Authored-By: Paperclip --- doc/sandbox-work-folders.md | 5 +++ .../acpx/codex-runtime-adapter.test.ts | 24 +++++++++++++ .../src/drivers/acpx/codex-runtime-adapter.ts | 8 ++++- .../acpx/installation-integrity.test.ts | 35 +++++++++++++++++++ .../drivers/acpx/installation-integrity.ts | 24 +++++++++---- .../src/drivers/acpx/runtime-host.test.ts | 1 + .../src/drivers/acpx/runtime-host.ts | 2 +- 7 files changed, 91 insertions(+), 8 deletions(-) diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index 5a420268fa..2a4f5b9dee 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -113,6 +113,11 @@ Native ACPX recovery also admits a provider started lazily by model selection. Selection waits for verified process ownership before accepting the configured model; cleanup and out-of-band process launches remain fenced. This matters when reopening a persisted Codex session after stopping its sandbox. +The host retains the verified command snapshot and its pinned descriptors until +runtime cleanup, so reconnecting for the first turn after model selection can +launch again without reopening a mutable executable path. A failed turn-start +signal remains observable without crashing a sidecar that consumes only the +turn's event stream and result. The new scoped-shell startup setting is not added to an existing unscoped Codex session's protected launch arguments. Its durable provider profile remains unchanged during attachment. diff --git a/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.test.ts b/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.test.ts index 4600763e86..a0924d5355 100644 --- a/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.test.ts @@ -1375,6 +1375,30 @@ describe("Codex ACPX runtime adapter", () => { }); }); + it("keeps a rejected prompt observable without an unhandled rejection for event-stream consumers", async () => { + const runtime = fakeRuntime(); + const failure = new Error("recovered prompt failed"); + vi.mocked(runtime.startTurn).mockImplementation(() => ({ + requestId: "failed-turn", + promptStarted: Promise.reject(failure), + events: { async *[Symbol.asyncIterator]() { throw failure; } }, + result: Promise.reject(failure), + cancel: vi.fn(), closeStream: vi.fn(), + })); + const port = await openCodexAcpxRuntime(openOptions(fakeCommand()), { + createRegistry: () => registry(), createStore: () => store(), + createRuntime: () => runtime, + }); + const turn = port.startTurn({ text: "Resume.", requestId: "failed-turn" }); + // The sidecar consumes events and result; it never awaits promptStarted. + const events = (async () => { for await (const event of turn.events) void event; })(); + await expect(events).rejects.toBe(failure); + await expect(turn.result).rejects.toBe(failure); + await new Promise((resolve) => setTimeout(resolve, 10)); + await expect(turn.promptStarted).rejects.toBe(failure); + await port.close({ reason: "failed prompt observed" }); + }); + it("admits a verified provider that starts with the first recovered turn", async () => { const runtime = fakeRuntime(); const child = fakeChild(); diff --git a/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.ts b/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.ts index e704d64e94..6ca28ca7d6 100644 --- a/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.ts +++ b/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.ts @@ -1102,9 +1102,15 @@ function turnWithVerifiedLifetimeOwnership( finishOwnershipAdmission(), ); void ownershipVerified.catch(() => undefined); + const promptStarted = ownershipVerified.then(() => turn.promptStarted); + // The sidecar consumes the event stream and result without awaiting this + // optional admission signal. Observe its rejection immediately so a failed + // recovered prompt cannot terminate the sidecar as an unhandled rejection. + // Keep the original rejecting promise available to callers that await it. + void promptStarted.catch(() => undefined); return { requestId: turn.requestId, - promptStarted: ownershipVerified.then(() => turn.promptStarted), + promptStarted, events: eventsAfterLifetimeOwnership(turn.events, ownershipVerified), result: ownershipVerified.then(() => turn.result), cancel: (input) => turn.cancel(input), diff --git a/packages/paperclip-runner/src/drivers/acpx/installation-integrity.test.ts b/packages/paperclip-runner/src/drivers/acpx/installation-integrity.test.ts index c978752817..17d355c5d0 100644 --- a/packages/paperclip-runner/src/drivers/acpx/installation-integrity.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/installation-integrity.test.ts @@ -727,6 +727,41 @@ describe("ACPX installation integrity", () => { await expectPinnedOutput(lease.spawn(), "verified"); }); + it("keeps command leases single-use unless reuse is explicitly requested", async () => { + const fixture = await installationFixture(); + const installation = await verifyQualifiedAcpxInstallation(fixture.profile, fixture.resolve); + const lease = await installation.openCommand(); + await expectPinnedOutput(lease.spawn(), "verified"); + expect(() => lease.spawn()).toThrow("Verified ACPX command lease is closed"); + await lease.close(); + }); + + it("reconnects with the same verified bytes after the entry path changes", async () => { + const fixture = await installationFixture(); + const installation = await verifyQualifiedAcpxInstallation(fixture.profile, fixture.resolve); + const lease = await installation.openCommand({ reusable: true }); + try { + await expectPinnedOutput(lease.spawn(), "verified"); + await writeFile(fixture.commandPath, '#!/usr/bin/env node\nprocess.stdout.write("replacement");\n'); + await expectPinnedOutput(lease.spawn(), "verified"); + await expectPinnedOutput(lease.spawn(), "verified"); + } finally { + await lease.close(); + } + expect(() => lease.spawn()).toThrow("Verified ACPX command lease is closed"); + }); + + it("closing a reusable lease does not erase bytes already handed to a child", async () => { + const fixture = await installationFixture(); + const installation = await verifyQualifiedAcpxInstallation(fixture.profile, fixture.resolve); + const lease = await installation.openCommand({ reusable: true }); + const output = expectPinnedOutput(lease.spawn(), "verified"); + await lease.close(); + await output; + await lease.close(); + expect(() => lease.spawn()).toThrow("Verified ACPX command lease is closed"); + }); + it("launches the verified bytes after the open inode is modified", async () => { const fixture = await installationFixture(); const installation = await verifyQualifiedAcpxInstallation( diff --git a/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts b/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts index 02eea23fb2..b23df58a8f 100644 --- a/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts +++ b/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts @@ -408,7 +408,8 @@ export interface VerifiedAcpxInstallation { readonly commandDigest: string; readonly agentServerPackageJsonPath: string; readonly agentRuntimePackageJsonPath: string | null; - openCommand(): Promise; + /** Reusable leases retain verified bytes and descriptors until explicitly closed. */ + openCommand(options?: { reusable?: boolean }): Promise; } export interface VerifiedAcpxCommandLease { @@ -708,7 +709,7 @@ export async function verifyQualifiedAcpxInstallation( commandDigest, agentServerPackageJsonPath: serverPackageJsonPath, agentRuntimePackageJsonPath: runtimePackageJsonPath, - async openCommand(): Promise { + async openCommand(options: { reusable?: boolean } = {}): Promise { const currentDirectory = await openVerifiedCommandDirectory( commandDirectory, "provider", @@ -766,6 +767,7 @@ export async function verifyQualifiedAcpxInstallation( dependencyAncestorFormats, currentRuntimeExecutable, runtimeExecutable?.environmentVariable ?? null, + options.reusable === true, ); } catch (error) { await Promise.all([ @@ -1276,6 +1278,7 @@ function commandLease( providerRuntimeExecutable: FileHandle | null, providerRuntimeEnvironmentVariable: VerifiedAcpxRuntimeExecutable["environmentVariable"] | null, + reusable: boolean, ): VerifiedAcpxCommandLease { let consumed = false; let directoriesReleased = false; @@ -1306,7 +1309,11 @@ function commandLease( lifetime?: VerifiedAcpxProviderLifetime, ): ChildProcess { if (consumed) throw new Error("Verified ACPX command lease is closed"); - consumed = true; + // ACPX may launch once for model selection and reconnect for the turn. + // Reuse only the already verified snapshot and pinned descriptors, never + // the mutable installation path. The runtime host closes this lease. + if (!reusable) consumed = true; + const launchBytes = reusable ? Buffer.from(verifiedBytes) : verifiedBytes; let child: ChildProcess; try { const guarded = lifetime !== undefined; @@ -1467,22 +1474,27 @@ function commandLease( providerGuardianOwnership.set(child, ownership); } } catch (error) { + consumed = true; + launchBytes.fill(0); verifiedBytes.fill(0); releaseDirectoriesBestEffort(); throw error; } - releaseDirectoriesBestEffort(); + if (!reusable) releaseDirectoriesBestEffort(); const sourceInput = child.stdio[COMMAND_SOURCE_FD] as Writable | null; if (sourceInput === null) { + consumed = true; + launchBytes.fill(0); verifiedBytes.fill(0); + releaseDirectoriesBestEffort(); child.kill(); throw new Error("Verified ACPX command source pipe was not created"); } const release = (): void => { - verifiedBytes.fill(0); + launchBytes.fill(0); }; sourceInput.once("error", release); - sourceInput.end(verifiedBytes, release); + sourceInput.end(launchBytes, release); return child; }, close, 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 ef7e956711..23f6ca2698 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts @@ -1219,6 +1219,7 @@ describe("ACPX runtime host", () => { ), ); await commandAdmissionStarted.promise; + expect(openCommand).toHaveBeenCalledWith({ reusable: true }); controller.abort(cancellation); await expect(opening).rejects.toBe(cancellation); diff --git a/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts b/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts index f6ffe0ead5..8cc9452127 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts @@ -364,7 +364,7 @@ export class AcpxRuntimeHost { } command = await acquireAbortableAdmissionResource({ signal: options.signal, - acquire: () => installation.openCommand(), + acquire: () => installation.openCommand({ reusable: true }), resource: "command", releaseLate: (lateCommand) => lateCommand.close(), reportFailure: (failure) =>