mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 17:45:53 +01:00
fix(runner): merge origin/master into the ACPX sandbox-abort fix
Combine the base branch's retained-admission-cleanup assertions with the PR branch's explicit admission-start synchronization in the runtime-host tests. Keep the PR branch's resource-typed sandbox acquisition (acquireAbortableAdmissionResource with the "sandbox" resource kind), since it both retains cleanup and removes an orphaned sandbox root, a superset of the base branch's generic retention. Drop the duplicate prepareSandbox dependency field and duplicate runtime-sandbox import that the auto-merge left behind. Rewrite the two sandbox-abort tests the base branch added so they assert against the resource-typed cleanup path instead of the superseded generic one. Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
2 files changed
+292
-88
No files matched your search
@@ -31,8 +31,18 @@ import {
|
||||
} from "./runtime-sandbox.js";
|
||||
|
||||
const temporaryDirectories: string[] = [];
|
||||
const admissionControllers: AbortController[] = [];
|
||||
const pendingAdmissionOpenings = new Set<Promise<void>>();
|
||||
const pendingAdmissionCleanups = new Set<Promise<void>>();
|
||||
|
||||
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)
|
||||
@@ -55,7 +65,7 @@ async function waitForAcpxAdmission<T>(
|
||||
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());
|
||||
@@ -84,9 +94,123 @@ 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 rootGate = deferred<void>();
|
||||
const prepareSandbox: NonNullable<
|
||||
AcpxRuntimeHostDependencies["prepareSandbox"]
|
||||
> = vi.fn((input) =>
|
||||
prepareAcpxRuntimeSandbox(input, {
|
||||
afterRootOwned: () => rootGate.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 = prepareSandbox;
|
||||
|
||||
const unhandled: unknown[] = [];
|
||||
const onUnhandledRejection = (reason: unknown) => {
|
||||
unhandled.push(reason);
|
||||
};
|
||||
process.on("unhandledRejection", onUnhandledRejection);
|
||||
try {
|
||||
const opening = trackAdmissionOpening(
|
||||
AcpxRuntimeHost.open(
|
||||
{
|
||||
...fixture.options,
|
||||
agent: "codex",
|
||||
model: "gpt-5.6-sol",
|
||||
permissionMode: "deny-all",
|
||||
signal: controller.signal,
|
||||
},
|
||||
dependencies,
|
||||
),
|
||||
);
|
||||
await waitForAcpxAdmission(() =>
|
||||
expect(prepareSandbox).toHaveBeenCalledOnce(),
|
||||
);
|
||||
|
||||
controller.abort(cancellation);
|
||||
await expect(opening).rejects.toBe(cancellation);
|
||||
expect(stageCredential).not.toHaveBeenCalled();
|
||||
expect(openRuntime).not.toHaveBeenCalled();
|
||||
expect(fixture.commandClose).not.toHaveBeenCalled();
|
||||
|
||||
rootGate.resolve();
|
||||
const sandbox: AcpxRuntimeSandbox =
|
||||
await prepareSandbox.mock.results[0]!.value;
|
||||
await waitForAcpxAdmission(() =>
|
||||
expect(stat(sandbox.root)).rejects.toMatchObject({ code: "ENOENT" }),
|
||||
);
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
expect(unhandled).toEqual([]);
|
||||
} finally {
|
||||
process.off("unhandledRejection", onUnhandledRejection);
|
||||
}
|
||||
});
|
||||
|
||||
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<void>();
|
||||
const finishSandbox = deferred<void>();
|
||||
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);
|
||||
};
|
||||
|
||||
const unhandled: unknown[] = [];
|
||||
const onUnhandledRejection = (reason: unknown) => {
|
||||
unhandled.push(reason);
|
||||
};
|
||||
process.on("unhandledRejection", onUnhandledRejection);
|
||||
try {
|
||||
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);
|
||||
|
||||
finishSandbox.reject(new Error("sandbox preparation failed after abort"));
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
expect(unhandled).toEqual([]);
|
||||
expect(stageCredential).not.toHaveBeenCalled();
|
||||
expect(openRuntime).not.toHaveBeenCalled();
|
||||
expect(fixture.commandClose).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
process.off("unhandledRejection", onUnhandledRejection);
|
||||
}
|
||||
});
|
||||
|
||||
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 = "";
|
||||
@@ -950,31 +1074,39 @@ describe("ACPX runtime host", () => {
|
||||
it("closes a command lease that resolves after admission is aborted", async () => {
|
||||
const fixture = await hostFixture();
|
||||
const commandAdmission = deferred<VerifiedAcpxCommandLease>();
|
||||
const commandAdmissionStarted = deferred<void>();
|
||||
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 commandAdmissionStarted.promise;
|
||||
await waitForAcpxAdmission(() =>
|
||||
expect(openCommand).toHaveBeenCalledOnce(),
|
||||
);
|
||||
@@ -1013,26 +1145,33 @@ describe("ACPX runtime host", () => {
|
||||
await rm(lateCredentialPath);
|
||||
});
|
||||
const reportRetainedCleanupFailure = vi.fn();
|
||||
const stageCredential = vi.fn(() => credentialAdmission.promise);
|
||||
const credentialAdmissionStarted = deferred<void>();
|
||||
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 credentialAdmissionStarted.promise;
|
||||
await waitForAcpxAdmission(() =>
|
||||
expect(stageCredential).toHaveBeenCalledOnce(),
|
||||
);
|
||||
@@ -1068,6 +1207,7 @@ describe("ACPX runtime host", () => {
|
||||
it("retains managed credentials until an aborted late runtime is closed", async () => {
|
||||
const fixture = await hostFixture();
|
||||
const runtimeAdmission = deferred<AcpxRuntimePort>();
|
||||
const runtimeAdmissionStarted = deferred<void>();
|
||||
const retryClose = deferred<void>();
|
||||
const lateRuntime = runtimePort({
|
||||
onClose: vi
|
||||
@@ -1082,27 +1222,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 runtimeAdmissionStarted.promise;
|
||||
await waitForAcpxAdmission(() =>
|
||||
expect(openRuntime).toHaveBeenCalledOnce(),
|
||||
);
|
||||
@@ -1162,33 +1306,38 @@ describe("ACPX runtime host", () => {
|
||||
it("scrubs credentials after rejected runtime cleanup is proven", async () => {
|
||||
const fixture = await hostFixture();
|
||||
const runtimeAdmission = deferred<AcpxRuntimePort>();
|
||||
const runtimeAdmissionStarted = deferred<void>();
|
||||
const providerCleanup = deferred<void>();
|
||||
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 runtimeAdmissionStarted.promise;
|
||||
await waitForAcpxAdmission(() =>
|
||||
expect(openRuntime).toHaveBeenCalledOnce(),
|
||||
);
|
||||
@@ -1537,6 +1686,7 @@ async function hostFixture() {
|
||||
openCommand: async () => command,
|
||||
}) satisfies VerifiedAcpxInstallation,
|
||||
openRuntime: input.openRuntime,
|
||||
retainAdmissionCleanup: trackAdmissionCleanup,
|
||||
reportRetainedCleanupFailure:
|
||||
input.reportRetainedCleanupFailure ?? vi.fn(),
|
||||
};
|
||||
@@ -1544,6 +1694,33 @@ async function hostFixture() {
|
||||
};
|
||||
}
|
||||
|
||||
function trackedAdmissionController(): AbortController {
|
||||
const controller = new AbortController();
|
||||
admissionControllers.push(controller);
|
||||
return controller;
|
||||
}
|
||||
|
||||
function trackAdmissionOpening<T>(opening: Promise<T>): Promise<T> {
|
||||
trackSettledPromise(pendingAdmissionOpenings, opening);
|
||||
return opening;
|
||||
}
|
||||
|
||||
function trackAdmissionCleanup(cleanup: Promise<void>): void {
|
||||
trackSettledPromise(pendingAdmissionCleanups, cleanup);
|
||||
}
|
||||
|
||||
function trackSettledPromise<T>(
|
||||
pending: Set<Promise<void>>,
|
||||
promise: Promise<T>,
|
||||
): void {
|
||||
const observed = promise.then(
|
||||
() => undefined,
|
||||
() => undefined,
|
||||
);
|
||||
pending.add(observed);
|
||||
void observed.finally(() => pending.delete(observed));
|
||||
}
|
||||
|
||||
function deferred<T>(): {
|
||||
promise: Promise<T>;
|
||||
resolve(value: T): void;
|
||||
|
||||
@@ -256,15 +256,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);
|
||||
@@ -279,10 +282,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");
|
||||
@@ -457,6 +463,7 @@ export class AcpxRuntimeHost {
|
||||
profile,
|
||||
admissionVerificationTimeoutMs,
|
||||
),
|
||||
dependencies.retainAdmissionCleanup,
|
||||
);
|
||||
const observedIdentity: AcpxExpectedSessionIdentity = {
|
||||
kind: "acpx",
|
||||
@@ -652,11 +659,31 @@ export class AcpxRuntimeHost {
|
||||
async function runAbortableAdmissionStage<T>(
|
||||
signal: AbortSignal | undefined,
|
||||
operation: () => Promise<T>,
|
||||
retainCleanup: ((cleanup: Promise<void>) => void) | undefined,
|
||||
): Promise<T> {
|
||||
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<T>(input: {
|
||||
|
||||
Reference in new issue
Block a user