From 1ab3cb94e036f5de43e4bad9fd4d345054e8763c Mon Sep 17 00:00:00 2001 From: Dotta Date: Sun, 11 Oct 2026 04:09:40 -0500 Subject: [PATCH] fix: terminate guarded providers on fatal module import --- .../src/cli/acpx-sidecar-input.test.ts | 22 ++++++ .../src/cli/acpx-sidecar-input.ts | 5 ++ .../acpx/installation-integrity.test.ts | 69 +++++++++++++++++++ .../drivers/acpx/installation-integrity.ts | 11 ++- 4 files changed, 106 insertions(+), 1 deletion(-) diff --git a/packages/paperclip-runner/src/cli/acpx-sidecar-input.test.ts b/packages/paperclip-runner/src/cli/acpx-sidecar-input.test.ts index 12c80c49221..4aedd31f729 100644 --- a/packages/paperclip-runner/src/cli/acpx-sidecar-input.test.ts +++ b/packages/paperclip-runner/src/cli/acpx-sidecar-input.test.ts @@ -126,6 +126,28 @@ describe("ACPX sidecar input sequencing", () => { expect(acpxSidecarErrorCode(new Error("wrapped", { cause: error }))).toBe(code); }); + it.each(["OTHER", "UNVERIFIED_MODULE"])("classifies closed import marker %s without private stderr", category => { + const classify = (stderrSummary: string) => acpxSidecarErrorCode(Object.assign(new Error("private-canary"), { + detailCode: "AGENT_STARTUP_FAILED", stderrSummary, exitCode: 1, + })); + expect(classify(`PAPERCLIP_ACPX_IMPORT_FAILURE:${category}\nprivate-canary /secret`)) + .toBe(`AGENT_STARTUP_FAILED.${category}`); + expect(classify(`private-canary PAPERCLIP_ACPX_IMPORT_FAILURE:OTHER\n`)) + .toBe("AGENT_STARTUP_FAILED.EXIT_NONZERO"); + expect(classify(`PAPERCLIP_ACPX_IMPORT_FAILURE:OTHER private-canary\n`)) + .toBe("AGENT_STARTUP_FAILED.EXIT_NONZERO"); + }); + + it.each([ + ["Error [ERR_MODULE_NOT_FOUND]: private-canary", "MODULE_NOT_FOUND"], + ["SyntaxError: private-canary", "SYNTAX_ERROR"], + ])("preserves detailed closed startup category with a generic import marker (%s)", (detail, category) => { + expect(acpxSidecarErrorCode(Object.assign(new Error("private-canary"), { + detailCode: "AGENT_STARTUP_FAILED", + stderrSummary: `PAPERCLIP_ACPX_IMPORT_FAILURE:OTHER\n${detail}`, + }))).toBe(`AGENT_STARTUP_FAILED.${category}`); + }); + it("preserves stable ACPX error identities without copying startup stderr", () => { const missingModule = Object.assign(new Error("provider exited"), { detailCode: "AGENT_STARTUP_FAILED", diff --git a/packages/paperclip-runner/src/cli/acpx-sidecar-input.ts b/packages/paperclip-runner/src/cli/acpx-sidecar-input.ts index 6aeee58ca14..8a07bf7354c 100644 --- a/packages/paperclip-runner/src/cli/acpx-sidecar-input.ts +++ b/packages/paperclip-runner/src/cli/acpx-sidecar-input.ts @@ -88,6 +88,10 @@ function directAcpxSidecarErrorCode(error: Error): string | null { const stderr = typeof details.stderrSummary === "string" ? details.stderrSummary : ""; + // Bootstrap writes this fixed marker before printing private error details and + // terminating. Never copy those details into the public classification. + const importFailure = /(?:^|\n)PAPERCLIP_ACPX_IMPORT_FAILURE:(UNVERIFIED_MODULE|OTHER)(?:\r?\n|$)/.exec(stderr); + if (importFailure?.[1] === "UNVERIFIED_MODULE") return "AGENT_STARTUP_FAILED.UNVERIFIED_MODULE"; if (/ERR_ACPX_UNVERIFIED_MODULE/.test(stderr)) { return "AGENT_STARTUP_FAILED.UNVERIFIED_MODULE"; } @@ -106,6 +110,7 @@ function directAcpxSidecarErrorCode(error: Error): string | null { if (/ERR_INVALID_ARG|invalid argument/i.test(stderr)) { return "AGENT_STARTUP_FAILED.INVALID_ARGUMENT"; } + if (importFailure) return "AGENT_STARTUP_FAILED.OTHER"; if (!stderr.trim()) return "AGENT_STARTUP_FAILED.NO_STDERR"; if (typeof details.signal === "string" && details.signal) { return "AGENT_STARTUP_FAILED.SIGNAL"; 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 391a4c84cc5..93466eed151 100644 --- a/packages/paperclip-runner/src/drivers/acpx/installation-integrity.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/installation-integrity.test.ts @@ -3,6 +3,8 @@ import { constants } from "node:fs"; import { runInNewContext } from "node:vm"; import { fork, type ChildProcess } from "node:child_process"; import { once } from "node:events"; +import { pathToFileURL } from "node:url"; +import { acpxSidecarErrorCode } from "../../cli/acpx-sidecar-input.js"; import { chmod, link, @@ -1707,6 +1709,73 @@ describe("ACPX installation integrity", () => { await expectFailure(child, "requires Linux descriptor-pinned paths"); } }); + it.runIf(process.platform !== "win32").each(["OTHER", "UNVERIFIED_MODULE"] as const)( + "settles a guarded ESM import failure without guardian retirement (%s)", + async (category) => { + const fixture = await installationFixture(); + const outside = join(fixture.root, "private-canary.mjs"); + await writeFile(outside, 'throw new Error("must-not-execute");'); + const command = category === "OTHER" + ? 'throw new Error("guarded-import-fixture-failed private-canary");' + : `import ${JSON.stringify(pathToFileURL(outside).href)};`; + await writeFile(fixture.commandPath, command); + await writeFile(fixture.serverPackageJsonPath, JSON.stringify({ + version: "0.0.33", bin: "bin/server.js", type: "module", + })); + const installation = await verifyQualifiedAcpxInstallation({ + ...fixture.profile, + commandDigest: `sha256:${createHash("sha256").update(command).digest("hex")}`, + }, fixture.resolve); + const credentialHome = join(fixture.root, "credential-home"); + await mkdir(credentialHome, { mode: 0o700 }); + const credential = await stageManagedCodexCredential({ + agentHomeDirectory: credentialHome, + environment: { PAPERCLIP_ACPX_CODEX_AUTH_JSON_SECRET: '{"fixture":true}' }, + }); + const lease = await installation.openCommand(); + const child = lease.spawn([], {}, { + credentialFenceFds: credential.lifetimeFenceFds, + activateCredentialFenceOwner: pid => credential.activateLifetimeOwner(pid), + }); + const exit = once(child, "exit"); + const closed = once(child, "close"); + let stderr = ""; + const failureSeen = new Promise(resolve => child.stderr?.on("data", bytes => { + stderr += String(bytes); + if (stderr.includes("PAPERCLIP_ACPX_IMPORT_FAILURE:")) resolve(); + })); + let timer: ReturnType | undefined; + let startupTimer: ReturnType | undefined; + try { + await Promise.race([failureSeen, new Promise((_, reject) => { + startupTimer = setTimeout(() => reject(new Error("fixture startup marker missing")), 3_000); + })]); + // Bound starts after the exact child error, excluding startup speed. + const settled = await Promise.race([exit.then(() => true), new Promise(resolve => { + timer = setTimeout(() => resolve(false), 500); + })]); + expect(settled).toBe(true); + await closed; + await awaitVerifiedAcpxProviderExit(child); + expect(child.exitCode === 0 && child.signalCode === null).toBe(false); + const code = acpxSidecarErrorCode(Object.assign(new Error("private-canary"), { + detailCode: "AGENT_STARTUP_FAILED", stderrSummary: stderr, + exitCode: child.exitCode, signal: child.signalCode, + })); + expect(code).toBe(`AGENT_STARTUP_FAILED.${category}`); + expect(code).not.toMatch(/canary|\//); + expect(stderr).not.toContain("must-not-execute"); + } finally { + if (timer) clearTimeout(timer); + if (startupTimer) clearTimeout(startupTimer); + if (child.exitCode === null && child.signalCode === null) child.kill("SIGTERM"); + await exit; + await lease.close(); + await credential.close(); + } + }, + ); + it.runIf(process.platform !== "win32")( "rejects incomplete or duplicate provider credential quorum descriptors", async () => { diff --git a/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts b/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts index face6deb3e8..edf2c897bc7 100644 --- a/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts +++ b/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts @@ -1990,7 +1990,16 @@ function snapshotBootstrap(format: AcpxCommandFormat, guarded = false): string { "} finally { fs.closeSync(moduleFd); }", "});", "} });", - "import(target).catch((error) => { console.error(error); process.exitCode = 1; });", + // A guardian stream intentionally keeps the provider alive. A failed module + // import must use the existing owned-group reaper or ACP initialize waits + // until its deadline. process.exit can itself wait on the guardian pipe read. + // Capture these before provider code runs; emit a closed marker synchronously + // so the existing startup-error classifier survives truncated private stderr. + "const writeImportFailure = fs.writeSync.bind(fs);", + guarded + ? "const exitOnImportFailure = reapOnGuardianLoss;" + : "const exitOnImportFailure = process.exit.bind(process);", + 'import(target).catch((error) => { let category = "OTHER"; try { if (error?.code === "ERR_ACPX_UNVERIFIED_MODULE") category = "UNVERIFIED_MODULE"; } catch {} try { writeImportFailure(2, "PAPERCLIP_ACPX_IMPORT_FAILURE:" + category + "\\n"); console.error(error); } finally { exitOnImportFailure(1); } });', ].join(""); }