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 f5eb29f0bf..8ec2e7cbdc 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts @@ -11,6 +11,7 @@ import type { VerifiedAcpxInstallation, } from "./installation-integrity.js"; import { resolveQualifiedAcpxProfile } from "./qualified-profiles.js"; +import { prepareAcpxRuntimeSandbox } from "./runtime-sandbox.js"; import { AcpxRuntimeHost, type AcpxRuntimeHostDependencies, @@ -20,8 +21,18 @@ import { } from "./runtime-host.js"; const temporaryDirectories: string[] = []; +const admissionControllers: AbortController[] = []; +const pendingAdmissionOpenings = new Set>(); +const pendingAdmissionCleanups = new Set>(); afterEach(async () => { + for (const controller of admissionControllers.splice(0)) { + if (!controller.signal.aborted) { + controller.abort(new Error("ACPX runtime host test cleanup")); + } + } + await Promise.all([...pendingAdmissionOpenings]); + await Promise.all([...pendingAdmissionCleanups]); await Promise.all( temporaryDirectories .splice(0) @@ -32,7 +43,7 @@ afterEach(async () => { describe("ACPX runtime host", () => { it("rejects a pre-aborted admission before acquiring provider resources", async () => { const fixture = await hostFixture(); - const controller = new AbortController(); + const controller = trackedAdmissionController(); const cancellation = new Error("admission cancelled before start"); controller.abort(cancellation); const openRuntime = vi.fn(async () => runtimePort()); @@ -61,9 +72,115 @@ describe("ACPX runtime host", () => { expect(fixture.commandClose).not.toHaveBeenCalled(); }); + it("retains aborted sandbox preparation until its filesystem work settles", async () => { + const fixture = await hostFixture(); + const controller = trackedAdmissionController(); + const cancellation = new Error("sandbox admission cancelled"); + const sandboxStarted = deferred(); + const finishSandbox = deferred(); + const retainedCleanups: Promise[] = []; + const stageCredential = vi.fn(async () => { + throw new Error("credential staging must not start"); + }); + const openRuntime = vi.fn(async () => runtimePort()); + const dependencies = fixture.dependencies({ openRuntime }); + dependencies.stageCredential = stageCredential; + dependencies.prepareSandbox = async (input) => { + sandboxStarted.resolve(undefined); + await finishSandbox.promise; + return await prepareAcpxRuntimeSandbox(input); + }; + dependencies.retainAdmissionCleanup = (cleanup) => { + retainedCleanups.push(cleanup); + trackAdmissionCleanup(cleanup); + }; + const opening = trackAdmissionOpening( + AcpxRuntimeHost.open( + { + ...fixture.options, + agent: "codex", + model: "gpt-5.6-sol", + permissionMode: "deny-all", + signal: controller.signal, + }, + dependencies, + ), + ); + await sandboxStarted.promise; + + controller.abort(cancellation); + await expect(opening).rejects.toBe(cancellation); + expect(retainedCleanups).toHaveLength(2); + let sandboxCleanupSettled = false; + void retainedCleanups[0]!.then( + () => { + sandboxCleanupSettled = true; + }, + () => { + sandboxCleanupSettled = true; + }, + ); + await Promise.resolve(); + expect(sandboxCleanupSettled).toBe(false); + + finishSandbox.resolve(undefined); + await expect(retainedCleanups[0]).resolves.toBeUndefined(); + expect(sandboxCleanupSettled).toBe(true); + expect(stageCredential).not.toHaveBeenCalled(); + expect(openRuntime).not.toHaveBeenCalled(); + expect(fixture.commandClose).not.toHaveBeenCalled(); + }); + + it("settles retained admission work when aborted sandbox preparation rejects", async () => { + const fixture = await hostFixture(); + const controller = trackedAdmissionController(); + const cancellation = new Error("sandbox admission cancelled"); + const sandboxStarted = deferred(); + const finishSandbox = deferred(); + const retainedCleanups: Promise[] = []; + const stageCredential = vi.fn(async () => { + throw new Error("credential staging must not start"); + }); + const openRuntime = vi.fn(async () => runtimePort()); + const dependencies = fixture.dependencies({ openRuntime }); + dependencies.stageCredential = stageCredential; + dependencies.prepareSandbox = async (input) => { + sandboxStarted.resolve(undefined); + await finishSandbox.promise; + return await prepareAcpxRuntimeSandbox(input); + }; + dependencies.retainAdmissionCleanup = (cleanup) => { + retainedCleanups.push(cleanup); + trackAdmissionCleanup(cleanup); + }; + const opening = trackAdmissionOpening( + AcpxRuntimeHost.open( + { + ...fixture.options, + agent: "codex", + model: "gpt-5.6-sol", + permissionMode: "deny-all", + signal: controller.signal, + }, + dependencies, + ), + ); + await sandboxStarted.promise; + + controller.abort(cancellation); + await expect(opening).rejects.toBe(cancellation); + expect(retainedCleanups).toHaveLength(2); + + finishSandbox.reject(new Error("sandbox preparation failed after abort")); + await expect(retainedCleanups[0]).resolves.toBeUndefined(); + expect(stageCredential).not.toHaveBeenCalled(); + expect(openRuntime).not.toHaveBeenCalled(); + expect(fixture.commandClose).not.toHaveBeenCalled(); + }); + it("scrubs credentials when abort wins before the adapter body starts", async () => { const fixture = await hostFixture(); - const controller = new AbortController(); + const controller = trackedAdmissionController(); const cancellation = new Error("runtime admission cancelled before entry"); const createRuntime = vi.fn(); let credentialHome = ""; @@ -919,32 +1036,39 @@ describe("ACPX runtime host", () => { it("closes a command lease that resolves after admission is aborted", async () => { const fixture = await hostFixture(); const commandAdmission = deferred(); + const commandAdmissionStarted = deferred(); const lateCommandClose = vi.fn(async () => undefined); - const openCommand = vi.fn(() => commandAdmission.promise); + const openCommand = vi.fn(() => { + commandAdmissionStarted.resolve(undefined); + return commandAdmission.promise; + }); const openRuntime = vi.fn(async () => runtimePort()); - const controller = new AbortController(); + const controller = trackedAdmissionController(); const cancellation = new Error("command admission cancelled"); const profile = resolveQualifiedAcpxProfile("claude", "claude-sonnet-5"); - const opening = AcpxRuntimeHost.open( - { - ...fixture.options, - agent: "claude", - model: "claude-sonnet-5", - permissionMode: "deny-all", - signal: controller.signal, - }, - { - verifyInstallation: async () => ({ - commandDigest: profile.commandDigest, - agentServerPackageJsonPath: join(fixture.root, "package.json"), - agentRuntimePackageJsonPath: null, - openCommand, - }), - openRuntime, - reportRetainedCleanupFailure: vi.fn(), - }, + const opening = trackAdmissionOpening( + AcpxRuntimeHost.open( + { + ...fixture.options, + agent: "claude", + model: "claude-sonnet-5", + permissionMode: "deny-all", + signal: controller.signal, + }, + { + verifyInstallation: async () => ({ + commandDigest: profile.commandDigest, + agentServerPackageJsonPath: join(fixture.root, "package.json"), + agentRuntimePackageJsonPath: null, + openCommand, + }), + openRuntime, + retainAdmissionCleanup: trackAdmissionCleanup, + reportRetainedCleanupFailure: vi.fn(), + }, + ), ); - await vi.waitFor(() => expect(openCommand).toHaveBeenCalledOnce()); + await commandAdmissionStarted.promise; controller.abort(cancellation); await expect(opening).rejects.toBe(cancellation); @@ -978,27 +1102,33 @@ describe("ACPX runtime host", () => { await rm(lateCredentialPath); }); const reportRetainedCleanupFailure = vi.fn(); - const stageCredential = vi.fn(() => credentialAdmission.promise); + const credentialAdmissionStarted = deferred(); + const stageCredential = vi.fn(() => { + credentialAdmissionStarted.resolve(undefined); + return credentialAdmission.promise; + }); const openRuntime = vi.fn(async () => runtimePort()); - const controller = new AbortController(); + const controller = trackedAdmissionController(); const cancellation = new Error("credential admission cancelled"); - const opening = AcpxRuntimeHost.open( - { - ...fixture.options, - agent: "codex", - model: "gpt-5.6-sol", - permissionMode: "deny-all", - signal: controller.signal, - }, - { - ...fixture.dependencies({ - openRuntime, - reportRetainedCleanupFailure, - }), - stageCredential, - }, + const opening = trackAdmissionOpening( + AcpxRuntimeHost.open( + { + ...fixture.options, + agent: "codex", + model: "gpt-5.6-sol", + permissionMode: "deny-all", + signal: controller.signal, + }, + { + ...fixture.dependencies({ + openRuntime, + reportRetainedCleanupFailure, + }), + stageCredential, + }, + ), ); - await vi.waitFor(() => expect(stageCredential).toHaveBeenCalledOnce()); + await credentialAdmissionStarted.promise; controller.abort(cancellation); await expect(opening).rejects.toBe(cancellation); @@ -1031,6 +1161,7 @@ describe("ACPX runtime host", () => { it("retains managed credentials until an aborted late runtime is closed", async () => { const fixture = await hostFixture(); const runtimeAdmission = deferred(); + const runtimeAdmissionStarted = deferred(); const retryClose = deferred(); const lateRuntime = runtimePort({ onClose: vi @@ -1045,28 +1176,31 @@ describe("ACPX runtime host", () => { receivedSignal = options.signal; credentialHome = options.launchEnvironment.CODEX_HOME!; bridgeUrl = options.mcpServers[0]!.url; + runtimeAdmissionStarted.resolve(undefined); return runtimeAdmission.promise; }); - const controller = new AbortController(); + const controller = trackedAdmissionController(); const cancellation = new Error("runtime admission cancelled"); - const opening = AcpxRuntimeHost.open( - { - ...fixture.options, - agent: "codex", - model: "gpt-5.6-sol", - permissionMode: "deny-all", - environment: { - PAPERCLIP_ACPX_CODEX_AUTH_JSON_SECRET: "{}", + const opening = trackAdmissionOpening( + AcpxRuntimeHost.open( + { + ...fixture.options, + agent: "codex", + model: "gpt-5.6-sol", + permissionMode: "deny-all", + environment: { + PAPERCLIP_ACPX_CODEX_AUTH_JSON_SECRET: "{}", + }, + signal: controller.signal, + semanticTools: { + tools: [], + handler: async () => ({ ok: true }), + }, }, - signal: controller.signal, - semanticTools: { - tools: [], - handler: async () => ({ ok: true }), - }, - }, - fixture.dependencies({ openRuntime }), + fixture.dependencies({ openRuntime }), + ), ); - await vi.waitFor(() => expect(openRuntime).toHaveBeenCalledOnce()); + await runtimeAdmissionStarted.promise; expect(receivedSignal).toBe(controller.signal); controller.abort(cancellation); @@ -1121,34 +1255,38 @@ describe("ACPX runtime host", () => { it("scrubs credentials after rejected runtime cleanup is proven", async () => { const fixture = await hostFixture(); const runtimeAdmission = deferred(); + const runtimeAdmissionStarted = deferred(); const providerCleanup = deferred(); const credentialClose = vi.fn(async () => undefined); - const controller = new AbortController(); + const controller = trackedAdmissionController(); const cancellation = new Error("runtime admission cancelled"); const openRuntime = vi.fn((options: AcpxRuntimePortOpenOptions) => { options.retainFailedAdmissionCleanup(providerCleanup.promise); + runtimeAdmissionStarted.resolve(undefined); return runtimeAdmission.promise; }); - const opening = AcpxRuntimeHost.open( - { - ...fixture.options, - agent: "codex", - model: "gpt-5.6-sol", - permissionMode: "deny-all", - signal: controller.signal, - }, - { - ...fixture.dependencies({ openRuntime }), - stageCredential: async () => ({ - path: join(fixture.root, "auth.json"), - mode: "inline_json", - lifetimeFenceFds: [42, 43], - activateLifetimeOwner: async () => undefined, - close: credentialClose, - }), - }, + const opening = trackAdmissionOpening( + AcpxRuntimeHost.open( + { + ...fixture.options, + agent: "codex", + model: "gpt-5.6-sol", + permissionMode: "deny-all", + signal: controller.signal, + }, + { + ...fixture.dependencies({ openRuntime }), + stageCredential: async () => ({ + path: join(fixture.root, "auth.json"), + mode: "inline_json", + lifetimeFenceFds: [42, 43], + activateLifetimeOwner: async () => undefined, + close: credentialClose, + }), + }, + ), ); - await vi.waitFor(() => expect(openRuntime).toHaveBeenCalledOnce()); + await runtimeAdmissionStarted.promise; controller.abort(cancellation); await expect(opening).rejects.toBe(cancellation); @@ -1245,6 +1383,7 @@ async function hostFixture() { openCommand: async () => command, }) satisfies VerifiedAcpxInstallation, openRuntime: input.openRuntime, + retainAdmissionCleanup: trackAdmissionCleanup, reportRetainedCleanupFailure: input.reportRetainedCleanupFailure ?? vi.fn(), }; @@ -1252,6 +1391,33 @@ async function hostFixture() { }; } +function trackedAdmissionController(): AbortController { + const controller = new AbortController(); + admissionControllers.push(controller); + return controller; +} + +function trackAdmissionOpening(opening: Promise): Promise { + trackSettledPromise(pendingAdmissionOpenings, opening); + return opening; +} + +function trackAdmissionCleanup(cleanup: Promise): void { + trackSettledPromise(pendingAdmissionCleanups, cleanup); +} + +function trackSettledPromise( + pending: Set>, + promise: Promise, +): void { + const observed = promise.then( + () => undefined, + () => undefined, + ); + pending.add(observed); + void observed.finally(() => pending.delete(observed)); +} + function deferred(): { promise: Promise; resolve(value: T): void; diff --git a/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts b/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts index c568b613b1..3c9b54c137 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts @@ -142,6 +142,8 @@ export interface AcpxRuntimeHostDependencies { /** Internal test seam for aborting credential acquisition. */ stageCredential?: typeof stageManagedCodexCredential; openRuntime(options: AcpxRuntimePortOpenOptions): Promise; + /** Internal test seam for deterministic sandbox-admission scheduling. */ + prepareSandbox?: typeof prepareAcpxRuntimeSandbox; /** Internal test seam for the post-handshake admission deadline. */ admissionVerificationTimeoutMs?: number; /** Internal test seam for failed-admission cleanup. */ @@ -251,15 +253,18 @@ export class AcpxRuntimeHost { ); } const profile = resolveQualifiedAcpxProfile(options.agent, options.model); - const binding = await runAbortableAdmissionStage(options.signal, () => - createAcpxRecoveryBinding({ - runtimeDirectory: options.runtimeDirectory, - normalizedSessionId: options.normalizedSessionId, - workingDirectory: options.workingDirectory, - profile, - requestedModel: options.model, - permissionMode: options.permissionMode, - }), + const binding = await runAbortableAdmissionStage( + options.signal, + () => + createAcpxRecoveryBinding({ + runtimeDirectory: options.runtimeDirectory, + normalizedSessionId: options.normalizedSessionId, + workingDirectory: options.workingDirectory, + profile, + requestedModel: options.model, + permissionMode: options.permissionMode, + }), + dependencies.retainAdmissionCleanup, ); if (options.expectedIdentity) { verifyExpectedAcpxIdentity(options.expectedIdentity, binding, null); @@ -274,10 +279,13 @@ export class AcpxRuntimeHost { ); } - const installation = await runAbortableAdmissionStage(options.signal, () => - (dependencies.verifyInstallation ?? verifyQualifiedAcpxInstallation)( - profile, - ), + const installation = await runAbortableAdmissionStage( + options.signal, + () => + (dependencies.verifyInstallation ?? verifyQualifiedAcpxInstallation)( + profile, + ), + dependencies.retainAdmissionCleanup, ); if (installation.commandDigest !== profile.commandDigest) { throw new Error("Verified ACPX installation does not match its profile"); @@ -324,12 +332,15 @@ export class AcpxRuntimeHost { retainRuntimeHostCleanup(ownedCleanup); }; try { - const sandbox = await runAbortableAdmissionStage(options.signal, () => - prepareAcpxRuntimeSandbox({ - binding, - agent: options.agent, - environment: options.environment, - }), + const sandbox = await runAbortableAdmissionStage( + options.signal, + () => + (dependencies.prepareSandbox ?? prepareAcpxRuntimeSandbox)({ + binding, + agent: options.agent, + environment: options.environment, + }), + dependencies.retainAdmissionCleanup, ); if (options.agent === "codex") { credential = await acquireAbortableAdmissionResource({ @@ -445,6 +456,7 @@ export class AcpxRuntimeHost { profile, admissionVerificationTimeoutMs, ), + dependencies.retainAdmissionCleanup, ); const observedIdentity: AcpxExpectedSessionIdentity = { kind: "acpx", @@ -631,11 +643,31 @@ export class AcpxRuntimeHost { async function runAbortableAdmissionStage( signal: AbortSignal | undefined, operation: () => Promise, + retainCleanup: ((cleanup: Promise) => void) | undefined, ): Promise { if (signal === undefined) return await operation(); signal.throwIfAborted(); const pending = Promise.resolve().then(operation); - return await raceAdmissionWithAbort(pending, signal); + try { + return await raceAdmissionWithAbort(pending, signal); + } catch (error) { + if (signal.aborted) { + // Abort may win while sandbox preparation or another non-resource stage + // still owns asynchronous work. Keep that exact operation observed and + // expose it to the embedding lifecycle so filesystem teardown cannot + // remove its session root while it is still making durable writes. + // The aborted opening is already authoritative, and this stage owns no + // provider resource. Its retained promise represents settlement only; + // a late stage rejection must not masquerade as failed resource cleanup. + const cleanup = pending.then( + () => undefined, + () => undefined, + ); + retainRuntimeHostCleanup(cleanup); + retainCleanup?.(cleanup); + } + throw error; + } } async function acquireAbortableAdmissionResource(input: {