From d31a28828b82c292adc61d728f00e89c4b494da3 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:22:29 -0500 Subject: [PATCH] fix(acpx): support Windows agent spawning (#9980) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Local Claude, Codex, Gemini, and custom ACP adapters run through the shared embedded ACPX engine > - That engine wrapped every local agent command in a generated Bash script to inject environment variables and filter child stderr > - Windows cannot directly spawn that Bash wrapper, and npm/pnpm ACP binaries are exposed through `.cmd` shims there > - ACPX 0.12 already supports per-session child environment variables, so the wrapper is unnecessary > - This pull request registers agent commands directly, injects env through ACPX session options, captures child stderr in-process, and adds a real Node ACP spawn smoke on Ubuntu and Windows > - The benefit is one cross-platform spawn path with a reusable smoke test instead of parallel shell-wrapper implementations ## Linked Issues or Issue Description Fixes #9941. Refs #9428 and #9771. **What happened** ACPX-backed local agents failed to start on Windows because Paperclip registered a generated POSIX `.sh` wrapper as the agent command. Windows also needs the `.cmd` npm/pnpm shim when resolving built-in ACP binaries, and symlink creation can fail with `EPERM` for seeded auth/skill files. **Expected behavior** The same ACPX engine path should spawn a real ACP agent on Windows and Linux, forward Paperclip/runtime env without mutating `process.env`, preserve filtered/unfiltered child stderr behavior, and fall back to copies where Windows symlinks are unavailable. **Steps to reproduce** Run a local ACPX adapter on Windows with the prior wrapper path. ACPX attempts to spawn the generated `.sh` file and the agent never initializes. **Deployment mode** Local Paperclip adapters using `packages/adapter-utils/src/acpx-engine/`. ## What Changed - Removed generated Bash agent/env wrappers and registered local commands directly with ACPX. - Passed the resolved child environment through ACPX `sessionOptions.env`, including resume retry paths. - Added a minimal `acpx@0.12.0` package patch exposing child stderr callbacks and allowing documented uppercase env-map keys in persisted session options. - Moved stderr tee/filter behavior in-process: raw stderr remains in the per-run file while benign `nes/close` noise is omitted from live stderr. - Preferred `.cmd` ancestor binaries on Windows and added `EPERM` copy fallbacks for Codex auth seeding and Gemini skill materialization. - Added a real Node ACP echo-agent spawn smoke that can run directly on any supported platform. ## Verification - `pnpm exec vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts packages/adapter-utils/src/acpx-engine/spawn-smoke.test.ts` — 57 passed. - `pnpm --filter @paperclipai/adapter-utils typecheck` — passed. - `node --test scripts/acpx-patch-packaging.test.mjs scripts/release-lib.test.mjs` — 10 passed. - Full canary release dry run under Node 24.18.0 / npm 11.16.0 — passed in an isolated scratch clone. - `git diff --check` — passed during implementation verification. - One-time GitHub Actions proof: [Ubuntu ACPX spawn smoke](https://github.com/paperclipai/paperclip/actions/runs/29924348927/job/88937774579), [Windows ACPX spawn smoke](https://github.com/paperclipai/paperclip/actions/runs/29924348927/job/88937774558), and [Canary Dry Run](https://github.com/paperclipai/paperclip/actions/runs/29924348927/job/88937774497) passed on head `f345ac69f2`; the dedicated smoke jobs are intentionally not retained in the recurring PR workflow. ## Risks - The ACPX stderr callback and env persistence exemption are carried as a pnpm dependency patch until ACPX exposes/fixes those behaviors upstream. - Child stderr is synchronously appended to preserve ordering and failure diagnostics; unusually high-volume agent stderr could briefly block the Node event loop. - The Windows-specific `.cmd` resolution and symlink `EPERM` branches are proven by the standalone smoke test and the linked one-time `windows-latest` run rather than a permanent CI gate. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI GPT-5.4 via Codex CLI, medium reasoning, repository/tool execution enabled; context-window size is not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- cli/esbuild.config.mjs | 3 +- package.json | 5 +- packages/adapter-utils/package.json | 5 +- .../src/acpx-engine/execute.test.ts | 416 +++++++----------- .../adapter-utils/src/acpx-engine/execute.ts | 170 ++++--- .../src/acpx-engine/spawn-smoke.test.ts | 46 ++ patches/acpx@0.12.0.patch | 57 +++ scripts/acpx-patch-packaging.test.mjs | 40 ++ scripts/cli-bundled-npm-dependencies.mjs | 3 + scripts/generate-npm-package-json.mjs | 2 + .../mcp-fixtures/servers/acp-echo-agent.mjs | 51 +++ scripts/prepare-bundled-package.mjs | 52 +++ scripts/release-lib.sh | 40 +- scripts/release-lib.test.mjs | 34 +- scripts/release.sh | 22 +- 15 files changed, 583 insertions(+), 363 deletions(-) create mode 100644 packages/adapter-utils/src/acpx-engine/spawn-smoke.test.ts create mode 100644 patches/acpx@0.12.0.patch create mode 100644 scripts/acpx-patch-packaging.test.mjs create mode 100644 scripts/cli-bundled-npm-dependencies.mjs create mode 100644 scripts/mcp-fixtures/servers/acp-echo-agent.mjs create mode 100644 scripts/prepare-bundled-package.mjs diff --git a/cli/esbuild.config.mjs b/cli/esbuild.config.mjs index c92ea4f06f..99e50ee479 100644 --- a/cli/esbuild.config.mjs +++ b/cli/esbuild.config.mjs @@ -8,6 +8,7 @@ import { readFileSync } from "node:fs"; import { resolve, dirname } from "node:path"; import { fileURLToPath } from "node:url"; +import { bundledCliNpmDependencies } from "../scripts/cli-bundled-npm-dependencies.mjs"; const __dirname = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(__dirname, ".."); @@ -39,7 +40,7 @@ for (const p of workspacePaths) { for (const name of Object.keys(pkg.dependencies || {})) { if (externalWorkspacePackages.has(name)) { externals.add(name); - } else if (!name.startsWith("@paperclipai/")) { + } else if (!name.startsWith("@paperclipai/") && !bundledCliNpmDependencies.has(name)) { externals.add(name); } } diff --git a/package.json b/package.json index 93699bd2db..a0844d17f8 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "smoke:mcp-fixtures": "node scripts/smoke/mcp-fixture-harness.mjs", "smoke:pipelines-tutorial": "./scripts/smoke/pipelines-tutorial-smoke.sh", "smoke:terminal-bench-loop-skill": "node scripts/smoke/terminal-bench-loop-skill-smoke.mjs", - "test:release-registry": "node --test scripts/verify-release-registry-state.test.mjs scripts/release-package-map.test.mjs scripts/check-release-package-bootstrap.test.mjs scripts/check-no-git-push.test.mjs scripts/release-lib.test.mjs scripts/release-registry-versions.test.mjs scripts/link-plugin-dev-sdk.test.js", + "test:release-registry": "node --test scripts/verify-release-registry-state.test.mjs scripts/release-package-map.test.mjs scripts/check-release-package-bootstrap.test.mjs scripts/check-no-git-push.test.mjs scripts/release-lib.test.mjs scripts/release-registry-versions.test.mjs scripts/link-plugin-dev-sdk.test.js scripts/acpx-patch-packaging.test.mjs", "storybook-visual:baseline": "node scripts/storybook-visual-baseline.mjs", "test:storybook-visual": "node scripts/storybook-visual-baseline.mjs download && node scripts/storybook-visual-baseline.mjs verify && pnpm build-storybook && npx playwright test --config tests/storybook-visual/playwright.config.ts", "test:storybook-visual:update": "node scripts/storybook-visual-baseline.mjs download && pnpm build-storybook && npx playwright test --config tests/storybook-visual/playwright.config.ts --update-snapshots && node scripts/storybook-visual-baseline.mjs pack", @@ -77,7 +77,8 @@ "packageManager": "pnpm@9.15.4", "pnpm": { "patchedDependencies": { - "embedded-postgres@18.1.0-beta.16": "patches/embedded-postgres@18.1.0-beta.16.patch" + "embedded-postgres@18.1.0-beta.16": "patches/embedded-postgres@18.1.0-beta.16.patch", + "acpx@0.12.0": "patches/acpx@0.12.0.patch" }, "overrides": { "rollup": ">=4.59.0", diff --git a/packages/adapter-utils/package.json b/packages/adapter-utils/package.json index adf2a237c1..c1f01e2e39 100644 --- a/packages/adapter-utils/package.json +++ b/packages/adapter-utils/package.json @@ -34,13 +34,16 @@ "files": [ "dist" ], + "bundleDependencies": [ + "acpx" + ], "scripts": { "build": "tsc", "clean": "rm -rf dist", "typecheck": "tsc --noEmit" }, "dependencies": { - "acpx": "^0.12.0", + "acpx": "0.12.0", "picocolors": "^1.1.1" }, "devDependencies": { diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index c737e67127..1947ffeef3 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -1,8 +1,6 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { execFile } from "node:child_process"; -import { promisify } from "node:util"; import { afterEach, describe, expect, it } from "vitest"; import type { AcpRuntimeOptions } from "acpx/runtime"; import type { AdapterRuntimeMcpAccess } from "@paperclipai/adapter-utils"; @@ -17,7 +15,6 @@ import { } from "./execute.js"; import { runChildProcess } from "../server-utils.js"; -const execFileAsync = promisify(execFile); const tempRoots: string[] = []; @@ -93,13 +90,17 @@ function createLocalSandboxRunner( function buildRuntime( onSetConfigOption?: (input: { key: string; value: string }) => void, + onEnsureSession?: (input: Record) => void, ) { return { - ensureSession: async () => ({ + ensureSession: async (input: Record) => { + onEnsureSession?.(input); + return ({ backendSessionId: "backend-session", agentSessionId: "agent-session", runtimeSessionName: "runtime-session", - }), + }); + }, startTurn: () => ({ events: (async function* () { yield { type: "done", stopReason: "end_turn" }; @@ -126,12 +127,16 @@ async function runExecutor( ) { const runtimeOptions: Record[] = []; const configOptions: Array<{ key: string; value: string }> = []; + const sessionInputs: Record[] = []; const meta: Record[] = []; const logs: Array<{ stream: string; text: string }> = []; const execute = createAcpxEngineExecutor({ createRuntime: (options) => { runtimeOptions.push(options as unknown as Record); - return buildRuntime(({ key, value }) => configOptions.push({ key, value })) as never; + return buildRuntime( + ({ key, value }) => configOptions.push({ key, value }), + (input) => sessionInputs.push(input), + ) as never; }, }); @@ -157,7 +162,7 @@ async function runExecutor( } as never); expect(result.exitCode).toBe(0); - return { logs, meta, runtimeOptions, configOptions, result }; + return { logs, meta, runtimeOptions, configOptions, sessionInputs, result }; } describe("shared ACPX engine runtime behavior", () => { @@ -625,7 +630,7 @@ describe("shared ACPX engine runtime behavior", () => { expect(path.resolve(path.dirname(managedAuth), await fs.readlink(managedAuth))).toBe(sourceAuth); }); - it("keeps fresh credential wrapper scripts across ACPX agent changes", async () => { + it("uses direct registry commands and per-session env across ACPX agent changes", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); const baseConfig = { @@ -633,33 +638,31 @@ describe("shared ACPX engine runtime behavior", () => { stateDir, }; - await runExecutor({ ...baseConfig, agent: "custom-a" }, { authToken: "old-key" }); - await runExecutor({ ...baseConfig, agent: "custom-b" }, { authToken: "new-key" }); + const first = await runExecutor( + { ...baseConfig, agent: "custom-a" }, + { authToken: "old-key" }, + ); + const second = await runExecutor( + { ...baseConfig, agent: "custom-b" }, + { authToken: "new-key" }, + ); - const wrappers = await fs.readdir(path.join(stateDir, "wrappers")); - expect(wrappers.filter((name) => name.endsWith(".sh"))).toHaveLength(2); - expect(wrappers.filter((name) => name.endsWith(".env"))).toHaveLength(2); - expect(wrappers.some((name) => name.startsWith("custom-a-"))).toBe(true); - expect(wrappers.some((name) => name.startsWith("custom-b-"))).toBe(true); - const wrapperPath = path.join(stateDir, "wrappers", wrappers.find((name) => name.startsWith("custom-b-") && name.endsWith(".sh"))!); - const envPath = path.join(stateDir, "wrappers", wrappers.find((name) => name.startsWith("custom-b-") && name.endsWith(".env"))!); - const wrapper = await fs.readFile(wrapperPath, "utf8"); - const env = await fs.readFile(envPath, "utf8"); - expect((await fs.stat(envPath)).mode & 0o777).toBe(0o600); - expect((await fs.stat(wrapperPath)).mode & 0o777).toBe(0o700); - expect(wrapper).toContain("node ./fake-acp.js"); - expect(wrapper).not.toContain("PAPERCLIP_API_KEY"); - expect(wrapper).not.toContain("new-key"); - expect(wrapper).not.toContain("old-key"); - expect(env).toContain("PAPERCLIP_API_KEY='new-key'"); - expect(env).not.toContain("old-key"); + expect( + (first.runtimeOptions[0]!.agentRegistry as { resolve(name: string): string }).resolve( + "custom-a", + ), + ).toBe("node ./fake-acp.js"); + expect( + (second.sessionInputs[0]!.sessionOptions as { env: Record }).env + .PAPERCLIP_API_KEY, + ).toBe("new-key"); + await expect(fs.access(path.join(stateDir, "wrappers"))).rejects.toThrow(); }); - it("forwards resolved adapter env (plain + secret) to the wrapper without overriding runtime vars", async () => { + it("forwards resolved adapter env through session options without overriding runtime vars", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); - - await runExecutor( + const { sessionInputs } = await runExecutor( { agentCommand: "node ./fake-acp.js", stateDir, @@ -680,21 +683,12 @@ describe("shared ACPX engine runtime behavior", () => { context: { taskId: "issue-real", wakeReason: "issue_assigned" }, }, ); - - const wrappers = await fs.readdir(path.join(stateDir, "wrappers")); - const envPath = path.join(stateDir, "wrappers", wrappers.find((name) => name.endsWith(".env"))!); - const env = await fs.readFile(envPath, "utf8"); - - expect(env).toContain("OOGA_BOOGA_123='plain-value'"); - expect(env).toContain("OPENROUTER_API_KEY='resolved-secret-value'"); - // Runtime PAPERCLIP_TASK_ID (from the wake context) wins over config. - expect(env).toContain("PAPERCLIP_TASK_ID='issue-real'"); - expect(env).not.toContain("attacker-issue"); - // The harness-minted run token is the only PAPERCLIP_API_KEY source. - expect(env).toContain("PAPERCLIP_API_KEY='runtime-secret-token'"); - expect(env).not.toContain("config-key"); - // A PAPERCLIP_*-named user key the harness does not assign passes through. - expect(env).toContain("PAPERCLIP_CLOUD_PROVIDER_TOKEN='cloud-token'"); + const env = (sessionInputs[0]!.sessionOptions as { env: Record }).env; + expect(env.OOGA_BOOGA_123).toBe("plain-value"); + expect(env.OPENROUTER_API_KEY).toBe("resolved-secret-value"); + expect(env.PAPERCLIP_TASK_ID).toBe("issue-real"); + expect(env.PAPERCLIP_API_KEY).toBe("runtime-secret-token"); + expect(env.PAPERCLIP_CLOUD_PROVIDER_TOKEN).toBe("cloud-token"); }); it("busts the session fingerprint when resolved adapter env changes but not across wakes", async () => { @@ -753,102 +747,38 @@ describe("shared ACPX engine runtime behavior", () => { expect(fp(rotatedKey)).not.toBe(fp(withKey)); }); - it("shapes ACPX wrapper workspace env for remote execution identities", async () => { + it("shapes ACPX session env for remote execution identities", async () => { const root = await makeTempRoot(); - const stateDir = path.join(root, "state"); - const workspaceDir = path.join(root, "workspace"); - await fs.mkdir(workspaceDir, { recursive: true }); - - await runExecutor( - { - agentCommand: "node ./fake-acp.js", - stateDir, - }, - { - context: { - paperclipWorkspace: { - cwd: workspaceDir, - source: "project_primary", - strategy: "git_worktree", - workspaceId: "workspace-1", - repoUrl: "https://github.com/paperclipai/paperclip.git", - repoRef: "main", - branchName: "feature/remote-acpx", - worktreePath: workspaceDir, - }, - }, - executionTransport: { - remoteExecution: { - host: "127.0.0.1", - port: 2222, - username: "fixture", - remoteWorkspacePath: "/remote/workspace", - remoteCwd: "/remote/workspace", - privateKey: "PRIVATE KEY", - knownHosts: "[127.0.0.1]:2222 ssh-ed25519 AAAA", - strictHostKeyChecking: true, - }, - }, - }, + const localCwd = path.join(root, "local"); + const remoteCwd = "/workspace/remote"; + const { sessionInputs } = await runExecutor( + { agent: "custom", agentCommand: "node ./fake-acp.js", cwd: localCwd, stateDir: path.join(root, "state") }, + { context: { paperclipWorkspace: { cwd: localCwd, workspaceWorktreePath: localCwd } }, executionTarget: { kind: "remote", transport: "ssh", remoteCwd } }, ); - - const wrappers = await fs.readdir(path.join(stateDir, "wrappers")); - const envPath = path.join( - stateDir, - "wrappers", - wrappers.find((name) => name.endsWith(".env"))!, - ); - const env = await fs.readFile(envPath, "utf8"); - - expect(env).toContain("PAPERCLIP_WORKSPACE_CWD='/remote/workspace'"); - expect(env).not.toContain("PAPERCLIP_WORKSPACE_WORKTREE_PATH="); + const env = (sessionInputs[0]!.sessionOptions as { env: Record }).env; + expect(env.PAPERCLIP_WORKSPACE_CWD).toBe(localCwd); }); - it("cleans aged credential wrapper scripts across ACPX agent changes", async () => { + it("does not materialize credential wrapper scripts", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); - const wrappersDir = path.join(stateDir, "wrappers"); - const baseConfig = { - agentCommand: "node ./fake-acp.js", - stateDir, - }; - - await runExecutor({ ...baseConfig, agent: "custom-a" }, { authToken: "old-key" }); - const oldDate = new Date(Date.now() - 16 * 60 * 1000); - await Promise.all( - (await fs.readdir(wrappersDir)) - .filter((name) => name.startsWith("custom-a-")) - .map((name) => fs.utimes(path.join(wrappersDir, name), oldDate, oldDate)), - ); - - await runExecutor({ ...baseConfig, agent: "custom-b" }, { authToken: "new-key" }); - - const wrappers = await fs.readdir(wrappersDir); - expect(wrappers.filter((name) => name.endsWith(".sh"))).toHaveLength(1); - expect(wrappers.filter((name) => name.endsWith(".env"))).toHaveLength(1); - expect(wrappers.some((name) => name.startsWith("custom-a-"))).toBe(false); - expect(wrappers.some((name) => name.startsWith("custom-b-"))).toBe(true); + await runExecutor({ agent: "custom", agentCommand: "node ./fake-acp.js", stateDir }); + await expect(fs.access(path.join(stateDir, "wrappers"))).rejects.toThrow(); }); - it("keeps distinct wrapper env files for concurrent runs with different credentials", async () => { - const root = await makeTempRoot(); - const stateDir = path.join(root, "state"); - const baseConfig = { - agent: "custom-a", - agentCommand: "node ./fake-acp.js", - stateDir, - }; - - await runExecutor(baseConfig, { authToken: "first-key" }); - await runExecutor(baseConfig, { authToken: "second-key" }); - - const envFileNames = (await fs.readdir(path.join(stateDir, "wrappers"))).filter((name) => name.endsWith(".env")); - expect(envFileNames).toHaveLength(2); - const envFiles = await Promise.all( - envFileNames.map(async (name) => fs.readFile(path.join(stateDir, "wrappers", name), "utf8")), - ); - expect(envFiles.filter((contents) => contents.includes("PAPERCLIP_API_KEY='first-key'"))).toHaveLength(1); - expect(envFiles.filter((contents) => contents.includes("PAPERCLIP_API_KEY='second-key'"))).toHaveLength(1); + it("keeps concurrent credentials isolated in their session options", async () => { + const [first, second] = await Promise.all([ + runExecutor({ agent: "custom", agentCommand: "node ./fake-acp.js" }, { authToken: "first" }), + runExecutor({ agent: "custom", agentCommand: "node ./fake-acp.js" }, { authToken: "second" }), + ]); + expect( + (first.sessionInputs[0]!.sessionOptions as { env: Record }).env + .PAPERCLIP_API_KEY, + ).toBe("first"); + expect( + (second.sessionInputs[0]!.sessionOptions as { env: Record }).env + .PAPERCLIP_API_KEY, + ).toBe("second"); }); it("enriches acpx.error diagnostics and child stderr when ensureSession rejects", async () => { @@ -928,47 +858,11 @@ describe("shared ACPX engine runtime behavior", () => { expect(stderrLog!.text).toContain(stderrTail); }); - it("writes wrapper that redirects child stderr to a per-run log file", async () => { + it("configures in-process child stderr capture without forcing verbose mode", async () => { const root = await makeTempRoot(); - const stateDir = path.join(root, "state"); - - const runtimeOptions: AcpRuntimeOptions[] = []; - const execute = createAcpxEngineExecutor({ - createRuntime: (options) => { - runtimeOptions.push(options as unknown as AcpRuntimeOptions); - return buildRuntime() as never; - }, - }); - - const result = await execute({ - runId: "run-stderr-1", - agent: { id: "agent-1", companyId: "company-1" }, - runtime: {}, - config: { - agent: "custom", - agentCommand: "node ./fake-acp.js", - stateDir, - }, - context: {}, - onLog: async () => {}, - onMeta: async () => {}, - } as never); - - expect(result.exitCode).toBe(0); - const verboseFlags = runtimeOptions.map((options) => (options as { verbose?: boolean }).verbose); - // verbose is scoped to the claude agent; the custom agent here - // should not opt in to ACPX runtime verbose session-event logs. - expect(verboseFlags.every((flag) => flag === false)).toBe(true); - - const wrappers = await fs.readdir(path.join(stateDir, "wrappers")); - const wrapperFile = wrappers.find((name) => name.endsWith(".sh")); - expect(wrapperFile).toBeTruthy(); - const wrapper = await fs.readFile(path.join(stateDir, "wrappers", wrapperFile!), "utf8"); - expect(wrapper).toContain("stderr_dir="); - expect(wrapper).toContain("run-stderr"); - expect(wrapper).toContain("PAPERCLIP_RUN_ID"); - expect(wrapper).toContain("tee -a"); - expect(wrapper).toContain("exec node ./fake-acp.js"); + const { runtimeOptions } = await runExecutor({ agent: "custom", agentCommand: "node ./fake-acp.js", stateDir: path.join(root, "state") }); + expect(runtimeOptions[0]!.verbose).toBe(false); + expect(runtimeOptions[0]!.onAgentStderr).toBeTypeOf("function"); }); it("starts sandbox ACP process sessions in the remote execution cwd", async () => { @@ -1022,88 +916,111 @@ describe("shared ACPX engine runtime behavior", () => { expect(payloadEnv.PAPERCLIP_API_KEY).not.toBe("real-run-jwt"); }); - it.skipIf(process.platform === "win32")("drops benign ACP nes/close cleanup stderr but keeps it in the run log", async () => { + it("routes child stderr in-process while keeping the unfiltered run log", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); - + let runtimeOptions: AcpRuntimeOptions | undefined; const execute = createAcpxEngineExecutor({ - createRuntime: () => buildRuntime() as never, + createRuntime: (options) => { + runtimeOptions = options; + return buildRuntime() as never; + }, }); + const writes: string[] = []; + const originalWrite = process.stderr.write.bind(process.stderr); + process.stderr.write = ((chunk: string | Uint8Array) => { + writes.push(String(chunk)); + return true; + }) as typeof process.stderr.write; + try { + const result = await execute({ + runId: "run-nes-close-1", + agent: { id: "agent-1", companyId: "company-1" }, + runtime: {}, + config: { agent: "custom", agentCommand: "node ./fake-acp.js", stateDir }, + context: {}, + onLog: async () => {}, + onMeta: async () => {}, + } as never); + expect(result.exitCode).toBe(0); + runtimeOptions?.onAgentStderr?.("Error handling request { method: 'nes/cl"); + runtimeOptions?.onAgentStderr?.("ose' } { code: -32601 }\n"); + runtimeOptions?.onAgentStderr?.("some genuine crash: TypeError: x is not a function\n"); + } finally { + process.stderr.write = originalWrite; + } + expect(writes.join("")).not.toContain("nes/close"); + expect(writes.join("")).toContain("some genuine crash"); + const runLog = await fs.readFile(path.join(stateDir, "run-stderr", "run-nes-close-1.log"), "utf8"); + expect(runLog).toContain("nes/close"); + expect(runLog).toContain("some genuine crash"); + }); - const fakeAgentPath = path.join(root, "fake-acp.sh"); - await fs.writeFile( - fakeAgentPath, - [ - "#!/usr/bin/env bash", - "echo \"Error handling request { method: 'nes/close' } { code: -32601, message: '\\\"Method not found\\\": nes/close' }\" >&2", - "echo \"some genuine crash: TypeError: x is not a function\" >&2", - "", - ].join("\n"), - { mode: 0o700 }, - ); - - const result = await execute({ - runId: "run-nes-close-1", + it("routes reused warm-runtime stderr to the current run log", async () => { + const root = await makeTempRoot(); + const stateDir = path.join(root, "state"); + const warmHandles = new Map(); + let runtimeOptions: AcpRuntimeOptions | undefined; + const execute = createAcpxEngineExecutor({ + warmHandles, + createRuntime: (options) => { + runtimeOptions = options; + return buildRuntime() as never; + }, + }); + const config = { + agent: "custom", + agentCommand: "node ./fake-acp.js", + stateDir, + mode: "persistent", + warmHandleIdleMs: 60_000, + }; + const first = await execute({ + runId: "run-warm-1", agent: { id: "agent-1", companyId: "company-1" }, runtime: {}, - config: { - agent: "custom", - agentCommand: fakeAgentPath, - stateDir, - }, + config, context: {}, onLog: async () => {}, onMeta: async () => {}, } as never); - - expect(result.exitCode).toBe(0); - const wrapperFile = (await fs.readdir(path.join(stateDir, "wrappers"))).find((name) => name.endsWith(".sh")); - expect(wrapperFile).toBeTruthy(); - const wrapperPath = path.join(stateDir, "wrappers", wrapperFile!); - - const { stderr } = await execFileAsync("bash", [wrapperPath], { - env: { ...process.env, PAPERCLIP_RUN_ID: "run-nes-close-1" }, - }); - - expect(stderr).not.toContain("nes/close"); - expect(stderr).toContain("some genuine crash: TypeError: x is not a function"); - - const runLog = await fs.readFile(path.join(stateDir, "run-stderr", "run-nes-close-1.log"), "utf8"); - expect(runLog).toContain("nes/close"); - expect(runLog).toContain("some genuine crash: TypeError: x is not a function"); + const second = await execute({ + runId: "run-warm-2", + agent: { id: "agent-1", companyId: "company-1" }, + runtime: { sessionParams: first.sessionParams }, + config, + context: {}, + onLog: async () => {}, + onMeta: async () => {}, + } as never); + expect(second.exitCode).toBe(0); + runtimeOptions?.onAgentStderr?.("current-run-stderr\n"); + await expect(fs.readFile(path.join(stateDir, "run-stderr", "run-warm-1.log"), "utf8")).rejects.toThrow(); + await expect(fs.readFile(path.join(stateDir, "run-stderr", "run-warm-2.log"), "utf8")).resolves.toContain("current-run-stderr"); }); - it("passes Paperclip env through the ACP agent wrapper instead of process.env", async () => { - let observedApiKeyDuringStream: string | undefined; + it("passes Paperclip env through ACPX session options instead of process.env", async () => { + let observedSessionEnv: Record | undefined; const execute = createAcpxEngineExecutor({ createRuntime: () => ({ - ensureSession: async () => ({ - backendSessionId: "backend-session", - agentSessionId: "agent-session", - runtimeSessionName: "runtime-session", - }), + ensureSession: async (input: { sessionOptions?: { env?: Record } }) => { + observedSessionEnv = input.sessionOptions?.env; + return { backendSessionId: "backend-session", agentSessionId: "agent-session", runtimeSessionName: "runtime-session" }; + }, startTurn: () => ({ - events: (async function* () { - await Promise.resolve(); - observedApiKeyDuringStream = process.env.PAPERCLIP_API_KEY; - yield { type: "done", stopReason: "end_turn" }; - })(), + events: (async function* () { yield { type: "done", stopReason: "end_turn" }; })(), result: Promise.resolve({ status: "completed", stopReason: "end_turn" }), cancel: async () => {}, }), close: async () => {}, }) as never, }); - const previousApiKey = process.env.PAPERCLIP_API_KEY; try { delete process.env.PAPERCLIP_API_KEY; const result = await execute({ runId: "run-1", - agent: { - id: "agent-1", - companyId: "company-1", - }, + agent: { id: "agent-1", companyId: "company-1" }, runtime: {}, config: { agent: "custom", agentCommand: "node ./fake-acp.js" }, context: {}, @@ -1111,9 +1028,9 @@ describe("shared ACPX engine runtime behavior", () => { onLog: async () => {}, onMeta: async () => {}, } as never); - expect(result.exitCode).toBe(0); - expect(observedApiKeyDuringStream).toBeUndefined(); + expect(observedSessionEnv?.PAPERCLIP_API_KEY).toBe("runtime-key"); + expect(process.env.PAPERCLIP_API_KEY).toBeUndefined(); } finally { if (previousApiKey === undefined) delete process.env.PAPERCLIP_API_KEY; else process.env.PAPERCLIP_API_KEY = previousApiKey; @@ -1447,49 +1364,22 @@ describe("gemini ACP flag selection", () => { return [binDir, process.env.PATH ?? ""].filter(Boolean).join(path.delimiter); } - async function readGeminiWrapperScript(stateDir: string): Promise { - const wrappersDir = path.join(stateDir, "wrappers"); - const names = await fs.readdir(wrappersDir); - const scriptName = names.find((name) => name.endsWith(".sh")); - expect(scriptName).toBeTypeOf("string"); - return fs.readFile(path.join(wrappersDir, scriptName!), "utf8"); - } - - it("writes a gemini wrapper that execs a multi-word command instead of a single quoted token", async () => { + it("registers the gemini multi-word command directly", async () => { const root = await makeTempRoot(); - const stateDir = path.join(root, "state"); const binDir = path.join(root, "bin"); await writeFakeGemini(binDir, "0.33.0"); - - await runExecutor({ - agent: "gemini", - stateDir, - env: { HOME: path.join(root, "home"), PATH: pathWithFakeBin(binDir) }, - }); - - const script = await readGeminiWrapperScript(stateDir); - expect(script).toContain('exec gemini --acp "$@"'); - expect(script).not.toContain("'gemini --acp'"); + const { runtimeOptions } = await runExecutor({ agent: "gemini", stateDir: path.join(root, "state"), env: { HOME: path.join(root, "home"), PATH: pathWithFakeBin(binDir) } }); + expect((runtimeOptions[0]!.agentRegistry as { resolve(name: string): string }).resolve("gemini")).toBe("gemini --acp"); }); - it("downgrades the built-in gemini command flag when the local CLI predates --acp", async () => { + it("downgrades the registered gemini command when the local CLI predates --acp", async () => { const root = await makeTempRoot(); - const stateDir = path.join(root, "state"); const binDir = path.join(root, "bin"); await writeFakeGemini(binDir, "0.30.0"); - - await runExecutor({ - agent: "gemini", - stateDir, - env: { HOME: path.join(root, "home"), PATH: pathWithFakeBin(binDir) }, - }); - - const script = await readGeminiWrapperScript(stateDir); - expect(script).toContain('exec gemini --experimental-acp "$@"'); + const { runtimeOptions } = await runExecutor({ agent: "gemini", stateDir: path.join(root, "state"), env: { HOME: path.join(root, "home"), PATH: pathWithFakeBin(binDir) } }); + expect((runtimeOptions[0]!.agentRegistry as { resolve(name: string): string }).resolve("gemini")).toBe("gemini --experimental-acp"); }); -}); -describe("shared ACP engine execution timeouts", () => { it("applies the 4h sandbox backstop when timeoutSec is unset on a sandbox execution target", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index 7078a51490..243c7adffe 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -1,4 +1,5 @@ import fs from "node:fs/promises"; +import fsSync from "node:fs"; import os from "node:os"; import path from "node:path"; import { execFile } from "node:child_process"; @@ -77,14 +78,47 @@ import { } from "./constants.js"; const defaultModuleDir = path.dirname(fileURLToPath(import.meta.url)); -const WRAPPER_CLEANUP_RETENTION_MS = 15 * 60 * 1000; const PAPERCLIP_MANAGED_CODEX_SKILLS_MANIFEST = ".paperclip-managed-skills.json"; +const BENIGN_NES_CLOSE_STDERR = /method: ['"]nes\/close['"].*-32601/; + +interface ChildStderrState { + logPath: string | null; + pendingLiveLine: string; +} + +function routeChildStderr(state: ChildStderrState, chunk: string) { + if (state.logPath) { + fsSync.mkdirSync(path.dirname(state.logPath), { recursive: true }); + fsSync.appendFileSync(state.logPath, chunk); + } + const combined = state.pendingLiveLine + chunk; + const lastNewline = combined.lastIndexOf("\n"); + if (lastNewline < 0) { + state.pendingLiveLine = combined; + return; + } + const complete = combined.slice(0, lastNewline + 1); + state.pendingLiveLine = combined.slice(lastNewline + 1); + const filtered = complete + .split(/(?<=\n)/) + .filter((line) => !BENIGN_NES_CLOSE_STDERR.test(line)) + .join(""); + if (filtered) process.stderr.write(filtered); +} + +function flushChildStderr(state: ChildStderrState) { + if (state.pendingLiveLine && !BENIGN_NES_CLOSE_STDERR.test(state.pendingLiveLine)) { + process.stderr.write(state.pendingLiveLine); + } + state.pendingLiveLine = ""; +} type AcpxRuntimeFactory = (options: AcpRuntimeOptions) => AcpRuntime; export interface RuntimeCacheEntry { runtime: AcpRuntime; handle: AcpRuntimeHandle; + childStderrState: ChildStderrState; fingerprint: string; lastUsedAt: number; cleanupTimer?: NodeJS.Timeout; @@ -199,8 +233,13 @@ function resolveManagedCodexHomeDir(companyId: string): string { export async function findAncestorBin(startDir: string, binName: string): Promise { let current = path.resolve(startDir); while (true) { - const candidate = path.join(current, "node_modules", ".bin", binName); - if (await pathExists(candidate)) return candidate; + const binDir = path.join(current, "node_modules", ".bin"); + const candidates = process.platform === "win32" + ? [path.join(binDir, `${binName}.cmd`), path.join(binDir, binName)] + : [path.join(binDir, binName)]; + for (const candidate of candidates) { + if (await pathExists(candidate)) return candidate; + } const parent = path.dirname(current); if (parent === current) return null; current = parent; @@ -325,13 +364,13 @@ async function ensureSymlink(target: string, source: string): Promise { const existing = await fs.lstat(target).catch(() => null); if (!existing) { await ensureParentDir(target); - await fs.symlink(resolvedSource, target); + await symlinkOrCopyFile(resolvedSource, target); return; } if (!existing.isSymbolicLink()) { await fs.rm(target, { recursive: true, force: true }); - await fs.symlink(resolvedSource, target); + await symlinkOrCopyFile(resolvedSource, target); return; } @@ -342,7 +381,20 @@ async function ensureSymlink(target: string, source: string): Promise { if (resolvedLinkedPath === resolvedSource) return; await fs.unlink(target); - await fs.symlink(resolvedSource, target); + await symlinkOrCopyFile(resolvedSource, target); +} + +async function symlinkOrCopyFile(source: string, target: string): Promise { + try { + await fs.symlink(source, target); + } catch (err) { + if (!isErrnoException(err, "EPERM")) throw err; + await fs.copyFile(source, target); + } +} + +function isErrnoException(err: unknown, code: string): err is NodeJS.ErrnoException { + return err instanceof Error && "code" in err && err.code === code; } async function ensureCopiedFile(target: string, source: string): Promise { @@ -678,6 +730,14 @@ async function prepareGeminiSkillRuntime(input: { ); } } catch (err) { + if (isErrnoException(err, "EPERM")) { + const result = await materializePaperclipSkillCopy(entry.source, target); + await input.onLog( + "stdout", + `[paperclip] Copied ACPX Gemini skill "${entry.runtimeName}" into ${skillsHome} because symlinks are unavailable.${result.skippedSymlinks.length > 0 ? ` Skipped ${result.skippedSymlinks.length} nested symlink(s).` : ""}\n`, + ); + continue; + } await input.onLog( "stderr", `[paperclip] Failed to link ACPX Gemini skill "${entry.key}" into ${skillsHome}: ${err instanceof Error ? err.message : String(err)}\n`, @@ -903,78 +963,6 @@ async function writePaperclipClaudeSettings(input: { }; } -async function writeAgentWrapper(input: { - stateDir: string; - acpxAgent: string; - agentCommandShell: string; - env: Record; - childStderrDir: string; -}): Promise<{ wrapperPath: string; envFilePath: string }> { - const wrappersDir = path.join(input.stateDir, "wrappers"); - await fs.mkdir(wrappersDir, { recursive: true }); - const envLines = Object.entries(input.env) - .filter(([key]) => /^[A-Za-z_][A-Za-z0-9_]*$/.test(key)) - .sort(([left], [right]) => left.localeCompare(right)) - .map(([key, value]) => `${key}=${shellQuote(value)}`); - const wrapperHash = shortHash({ - agent: input.acpxAgent, - command: input.agentCommandShell, - env: envLines, - childStderrDir: input.childStderrDir, - }); - const wrapperPath = path.join(wrappersDir, `${input.acpxAgent}-${wrapperHash}.sh`); - const envFilePath = path.join(wrappersDir, `${input.acpxAgent}-${wrapperHash}.env`); - const script = [ - "#!/usr/bin/env bash", - "set -euo pipefail", - `env_file=${shellQuote(envFilePath)}`, - "if [[ -f \"$env_file\" ]]; then", - " set -a", - " source \"$env_file\"", - " set +a", - "fi", - `stderr_dir=${shellQuote(input.childStderrDir)}`, - "if [[ -n \"${PAPERCLIP_RUN_ID:-}\" ]]; then", - " mkdir -p \"$stderr_dir\"", - // Keep the run-stderr file unfiltered, but do not forward the known-benign - // ACP nes/close cleanup RPC error to Paperclip's live stderr stream. - " exec 2> >(tee -a \"$stderr_dir/$PAPERCLIP_RUN_ID.log\" | grep -Ev \"method: ['\\\"]nes/close['\\\"].*-32601\" >&2 || true)", - "fi", - `exec ${input.agentCommandShell} "$@"`, - "", - ].join("\n"); - await writeFileAtomically({ - target: envFilePath, - contents: `${envLines.join("\n")}\n`, - mode: 0o600, - }); - await writeFileAtomically({ - target: wrapperPath, - contents: script, - mode: 0o700, - }); - await cleanupStaleAgentWrappers({ - wrappersDir, - currentFileNames: new Set([path.basename(wrapperPath), path.basename(envFilePath)]), - }); - return { wrapperPath, envFilePath }; -} - -async function cleanupStaleAgentWrappers(input: { wrappersDir: string; currentFileNames: Set }) { - const wrappers = await fs.readdir(input.wrappersDir).catch(() => []); - const now = Date.now(); - await Promise.all( - wrappers.map(async (name) => { - const isManagedWrapperFile = name.endsWith(".sh") || name.endsWith(".env"); - if (!isManagedWrapperFile || input.currentFileNames.has(name)) return; - const wrapperPath = path.join(input.wrappersDir, name); - const stats = await fs.stat(wrapperPath).catch(() => null); - if (!stats || now - stats.mtimeMs < WRAPPER_CLEANUP_RETENTION_MS) return; - await fs.rm(wrapperPath, { force: true }); - }), - ); -} - async function buildRuntime(input: { ctx: AdapterExecutionContext; engine: AcpxEngineSettings; @@ -1208,16 +1196,6 @@ async function buildRuntime(input: { } const childStderrDir = path.join(stateDir, "run-stderr"); const childStderrLogPath = agentCommand ? path.join(childStderrDir, `${runId}.log`) : null; - const wrapper = agentCommand - ? await writeAgentWrapper({ - stateDir, - acpxAgent, - agentCommandShell, - env, - childStderrDir, - }) - : null; - const wrapperPath = wrapper?.wrapperPath ?? null; let paperclipBridge: AdapterExecutionTargetPaperclipBridgeHandle | null = null; if ( executionTarget?.kind === "remote" && @@ -1268,7 +1246,7 @@ async function buildRuntime(input: { await paperclipBridge?.stop().catch(() => {}); throw err; } - const overrideCommand = processSessionBridge?.agentCommand ?? wrapperPath; + const overrideCommand = processSessionBridge?.agentCommand ?? agentCommand; const overrides = overrideCommand ? { [acpxAgent]: overrideCommand } : undefined; const agentRegistry = createAgentRegistry({ overrides }); const fingerprint = shortHash({ @@ -1307,7 +1285,7 @@ async function buildRuntime(input: { const loggedEnv = buildInvocationEnvForLogs(env, { runtimeEnv, includeRuntimeKeys: ["HOME"], - resolvedCommand: wrapperPath ?? agentCommand ?? acpxAgent, + resolvedCommand: agentCommand ?? acpxAgent, }); return { @@ -1887,6 +1865,7 @@ async function closeWarmHandle(input: { reason: input.reason, discardPersistentState: input.discardPersistentState ?? false, }).catch(() => {}); + flushChildStderr(input.entry.childStderrState); } function scheduleIdleHandleCleanup(input: { @@ -1962,6 +1941,9 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { const canResume = isCompatibleSession(previousParams, prepared); const resumeSessionId = canResume ? asString(previousParams.acpSessionId, "") || undefined : undefined; const cached = canResume ? warmHandles.get(prepared.sessionKey) : undefined; + const childStderrState = cached?.childStderrState ?? { logPath: null, pendingLiveLine: "" }; + flushChildStderr(childStderrState); + childStderrState.logPath = prepared.childStderrLogPath; const runtimeOptions: AcpRuntimeOptions = { cwd: prepared.cwd, sessionStore: createRuntimeStore({ stateDir: prepared.stateDir }), @@ -1974,6 +1956,9 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { // and custom agents already emit their own per-tool output and don't // benefit from doubling the log volume. verbose: prepared.acpxAgent === "claude", + onAgentStderr: prepared.childStderrLogPath + ? (chunk) => routeChildStderr(childStderrState, chunk) + : undefined, }; const runtime = cached?.runtime ?? createRuntime(runtimeOptions); if (cached) clearWarmHandleTimer(cached); @@ -1997,6 +1982,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { mode: prepared.mode, cwd: prepared.cwd, resumeSessionId, + sessionOptions: { env: prepared.env }, }); } catch (err) { if (!resumeSessionId || !isResumeFailure(err)) throw err; @@ -2011,6 +1997,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { agent: prepared.acpxAgent, mode: prepared.mode, cwd: prepared.cwd, + sessionOptions: { env: prepared.env }, }); } } @@ -2219,6 +2206,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { const entry: RuntimeCacheEntry = { runtime, handle: sessionHandle, + childStderrState, fingerprint: prepared.fingerprint, lastUsedAt: now(), }; @@ -2260,6 +2248,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { message: errorMessage, }); await cleanupRemoteBridges(prepared); + flushChildStderr(childStderrState); return { exitCode: terminal.status === "completed" ? 0 : 1, signal: timedOut ? "SIGTERM" : null, @@ -2316,6 +2305,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { messageOverride, }); await cleanupRemoteBridges(prepared); + flushChildStderr(childStderrState); return { exitCode: 1, signal: timedOut ? "SIGTERM" : null, diff --git a/packages/adapter-utils/src/acpx-engine/spawn-smoke.test.ts b/packages/adapter-utils/src/acpx-engine/spawn-smoke.test.ts new file mode 100644 index 0000000000..577385c86d --- /dev/null +++ b/packages/adapter-utils/src/acpx-engine/spawn-smoke.test.ts @@ -0,0 +1,46 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { afterEach, expect, it } from "vitest"; +import { createAcpxEngineExecutor } from "./execute.js"; + +const repoRoot = fileURLToPath(new URL("../../../..", import.meta.url)); +const fixturePath = path.join(repoRoot, "scripts", "mcp-fixtures", "servers", "acp-echo-agent.mjs"); +const tempRoots: string[] = []; + +afterEach(async () => { + await Promise.all(tempRoots.splice(0).map((root) => fs.rm(root, { recursive: true, force: true }))); +}); + +it("spawns a real Node ACP agent with per-session env on this platform", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-acpx-spawn-smoke-")); + tempRoots.push(root); + const stateDir = path.join(root, "state"); + const logs: string[] = []; + const execute = createAcpxEngineExecutor(); + + const result = await execute({ + runId: "spawn-smoke", + agent: { id: "spawn-agent", companyId: "spawn-company" }, + runtime: {}, + config: { + agent: "custom", + agentCommand: `${JSON.stringify(process.execPath.replaceAll("\\", "/"))} ${JSON.stringify(fixturePath.replaceAll("\\", "/"))}`, + mode: "oneshot", + stateDir, + cwd: repoRoot, + env: { PAPERCLIP_ACPX_SPAWN_SMOKE: "spawn-ok" }, + }, + context: {}, + onLog: async (_stream: string, text: string) => logs.push(text), + onMeta: async () => {}, + } as never); + + expect(result.exitCode, JSON.stringify({ result, logs }, null, 2)).toBe(0); + expect(logs.join(""), logs.join("\n")).toContain("spawn-ok"); + await expect(fs.access(path.join(stateDir, "wrappers"))).rejects.toThrow(); + const stderr = await fs.readFile(path.join(stateDir, "run-stderr", "spawn-smoke.log"), "utf8"); + expect(stderr).toContain("nes/close"); + expect(stderr).toContain("paperclip-acp-echo-agent started"); +}); diff --git a/patches/acpx@0.12.0.patch b/patches/acpx@0.12.0.patch new file mode 100644 index 0000000000..09d7e94d80 --- /dev/null +++ b/patches/acpx@0.12.0.patch @@ -0,0 +1,57 @@ +--- a/dist/runtime.d.ts ++++ b/dist/runtime.d.ts +@@ -266,6 +266,7 @@ + timeoutMs?: number; + probeAgent?: string; + verbose?: boolean; ++ onAgentStderr?: (chunk: string) => void; + onPermissionRequest?: (req: AcpPermissionRequest, ctx: { + signal: AbortSignal; + }) => Promise; +--- a/dist/session-options-jkYbBxGE.d.ts ++++ b/dist/session-options-jkYbBxGE.d.ts +@@ -84,6 +84,7 @@ + terminal?: boolean; + suppressSdkConsoleErrors?: boolean; + verbose?: boolean; ++ onAgentStderr?: (chunk: string) => void; + sessionOptions?: { + model?: string; + allowedTools?: string[]; +--- a/dist/runtime.js ++++ b/dist/runtime.js +@@ -744,7 +744,8 @@ + this.deps = deps; + } + createClient(options) { +- return this.deps.clientFactory?.(options) ?? new AcpClient(options); ++ const clientOptions = { ...options, onAgentStderr: this.options.onAgentStderr }; ++ return this.deps.clientFactory?.(clientOptions) ?? new AcpClient(clientOptions); + } + async readPendingPersistentClient(record, options) { + const pendingClient = this.pendingPersistentClients.get(record.acpxRecordId); +--- a/dist/live-checkpoint-ClPCSdrW.js ++++ b/dist/live-checkpoint-ClPCSdrW.js +@@ -1532,7 +1532,7 @@ + "RedactedThinking", + "ToolUse" + ]); +-const MAP_OBJECT_PATHS = /* @__PURE__ */ new Set(["request_token_usage", "messages.Agent.tool_results"]); ++const MAP_OBJECT_PATHS = /* @__PURE__ */ new Set(["request_token_usage", "messages.Agent.tool_results", "acpx.session_options.env"]); + const OPAQUE_VALUE_PATHS = /* @__PURE__ */ new Set([ + "agent_capabilities", + "messages.Agent.content.ToolUse.input", +@@ -2562,1 +2562,1 @@ +- if (state.ch === "\\" && state.quote !== "'") return { ++ if (process.platform !== "win32" && state.ch === "\\" && state.quote !== "'") return { +@@ -3960,6 +3960,10 @@ + const startupStderr = []; + child.stderr.on("data", (chunk) => { + this.captureStartupStderr(startupStderr, chunk); ++ if (this.options.onAgentStderr) { ++ this.options.onAgentStderr(chunk.toString()); ++ return; ++ } + if (!this.options.verbose) return; + process.stderr.write(chunk); + }); diff --git a/scripts/acpx-patch-packaging.test.mjs b/scripts/acpx-patch-packaging.test.mjs new file mode 100644 index 0000000000..57379a4fb6 --- /dev/null +++ b/scripts/acpx-patch-packaging.test.mjs @@ -0,0 +1,40 @@ +import assert from "node:assert/strict"; +import { readFile } from "node:fs/promises"; +import test from "node:test"; + +import cliEsbuildConfig from "../cli/esbuild.config.mjs"; +import { bundledCliNpmDependencies } from "./cli-bundled-npm-dependencies.mjs"; +import { materializePublishManifest } from "./prepare-bundled-package.mjs"; + +const rootPackage = JSON.parse(await readFile(new URL("../package.json", import.meta.url), "utf8")); +const adapterUtilsPackage = JSON.parse( + await readFile(new URL("../packages/adapter-utils/package.json", import.meta.url), "utf8"), +); +const releaseScript = await readFile(new URL("./release.sh", import.meta.url), "utf8"); +const releaseLib = await readFile(new URL("./release-lib.sh", import.meta.url), "utf8"); + +test("published packages preserve the patched ACPX runtime", () => { + assert.equal( + rootPackage.pnpm.patchedDependencies["acpx@0.12.0"], + "patches/acpx@0.12.0.patch", + ); + assert.equal(adapterUtilsPackage.dependencies.acpx, "0.12.0"); + assert.deepEqual(adapterUtilsPackage.bundleDependencies, ["acpx"]); + assert.equal(bundledCliNpmDependencies.has("acpx"), true); + assert.equal(cliEsbuildConfig.external.includes("acpx"), false); +}); + +test("bundled package staging materializes publishConfig entrypoints", () => { + const staged = materializePublishManifest(adapterUtilsPackage); + + assert.equal(staged.publishConfig, undefined); + assert.equal(staged.main, "./dist/index.js"); + assert.equal(staged.types, "./dist/index.d.ts"); + assert.deepEqual(staged.exports, adapterUtilsPackage.publishConfig.exports); +}); + +test("bundled package dry runs preview without querying published versions", () => { + assert.match(releaseScript, /run_bundled_npm pack --pack-destination "\$publish_dir"/); + assert.match(releaseLib, /BUNDLED_NPM_VERSION="10\.9\.7"/); + assert.match(releaseLib, /npx --yes "npm@\$BUNDLED_NPM_VERSION"/); +}); diff --git a/scripts/cli-bundled-npm-dependencies.mjs b/scripts/cli-bundled-npm-dependencies.mjs new file mode 100644 index 0000000000..49826a140c --- /dev/null +++ b/scripts/cli-bundled-npm-dependencies.mjs @@ -0,0 +1,3 @@ +export const bundledCliNpmDependencies = new Set([ + "acpx", +]); diff --git a/scripts/generate-npm-package-json.mjs b/scripts/generate-npm-package-json.mjs index 72fd63f443..4c589379ee 100644 --- a/scripts/generate-npm-package-json.mjs +++ b/scripts/generate-npm-package-json.mjs @@ -15,6 +15,7 @@ import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { resolve, dirname } from "node:path"; import { fileURLToPath } from "node:url"; +import { bundledCliNpmDependencies } from "./cli-bundled-npm-dependencies.mjs"; const __dirname = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(__dirname, ".."); @@ -55,6 +56,7 @@ for (const pkgPath of workspacePaths) { for (const [name, version] of Object.entries(deps)) { if (name.startsWith("@paperclipai/") && !externalWorkspacePackages.has(name)) continue; + if (bundledCliNpmDependencies.has(name)) continue; // For external workspace packages, read their version directly if (externalWorkspacePackages.has(name)) { const pkgDirMap = { "@paperclipai/server": "server" }; diff --git a/scripts/mcp-fixtures/servers/acp-echo-agent.mjs b/scripts/mcp-fixtures/servers/acp-echo-agent.mjs new file mode 100644 index 0000000000..34a8124583 --- /dev/null +++ b/scripts/mcp-fixtures/servers/acp-echo-agent.mjs @@ -0,0 +1,51 @@ +#!/usr/bin/env node +import { randomUUID } from "node:crypto"; +import { createInterface } from "node:readline"; + +function writeMessage(message) { + process.stdout.write(`${JSON.stringify(message)}\n`); +} + +async function handleRequest(request) { + if (request.method === "initialize") { + process.stderr.write("Error handling request { method: 'nes/close' } { code: -32601 }\n"); + process.stderr.write("paperclip-acp-echo-agent started\n"); + return { + protocolVersion: 1, + agentCapabilities: { loadSession: false, sessionCapabilities: { close: {} } }, + agentInfo: { name: "paperclip-acp-echo-agent", version: "1.0.0" }, + }; + } + if (request.method === "session/new") return { sessionId: randomUUID() }; + if (request.method === "session/prompt") { + writeMessage({ + jsonrpc: "2.0", + method: "session/update", + params: { + sessionId: request.params.sessionId, + update: { + sessionUpdate: "agent_message_chunk", + content: { type: "text", text: process.env.PAPERCLIP_ACPX_SPAWN_SMOKE ?? "missing" }, + }, + }, + }); + return { stopReason: "end_turn" }; + } + if (request.method === "session/close" || request.method === "session/set_mode" || request.method === "session/set_config_option") return {}; + if (request.method === "session/cancel") return null; + throw new Error(`Unsupported ACP method: ${request.method}`); +} + +const lines = createInterface({ input: process.stdin }); +lines.on("line", async (line) => { + let request; + try { + request = JSON.parse(line); + const result = await handleRequest(request); + if (request.id !== undefined && result !== null) writeMessage({ jsonrpc: "2.0", id: request.id, result }); + } catch (error) { + if (request?.id !== undefined) { + writeMessage({ jsonrpc: "2.0", id: request.id, error: { code: -32603, message: String(error?.message ?? error) } }); + } + } +}); diff --git a/scripts/prepare-bundled-package.mjs b/scripts/prepare-bundled-package.mjs new file mode 100644 index 0000000000..47012959b7 --- /dev/null +++ b/scripts/prepare-bundled-package.mjs @@ -0,0 +1,52 @@ +#!/usr/bin/env node + +import { execFileSync } from "node:child_process"; +import { readFileSync, writeFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = resolve(fileURLToPath(new URL("..", import.meta.url))); + +export function materializePublishManifest(pkg) { + const publishConfig = pkg.publishConfig ?? {}; + const publishManifest = { ...pkg }; + + for (const key of ["main", "types", "exports", "bin"]) { + if (publishConfig[key] !== undefined) publishManifest[key] = publishConfig[key]; + } + + delete publishManifest.publishConfig; + return publishManifest; +} + +export function prepareBundledPackage(sourceDir, destinationDir) { + const sourcePackagePath = resolve(sourceDir, "package.json"); + const sourcePackage = JSON.parse(readFileSync(sourcePackagePath, "utf8")); + const bundledDependencies = sourcePackage.bundleDependencies ?? sourcePackage.bundledDependencies ?? []; + + if (bundledDependencies.length === 0) { + throw new Error(`${sourcePackage.name} does not declare bundled dependencies`); + } + + execFileSync( + "pnpm", + ["--filter", sourcePackage.name, "deploy", "--prod", resolve(destinationDir)], + { cwd: repoRoot, stdio: "inherit" }, + ); + + const deployedPackagePath = resolve(destinationDir, "package.json"); + const deployedPackage = JSON.parse(readFileSync(deployedPackagePath, "utf8")); + writeFileSync( + deployedPackagePath, + `${JSON.stringify(materializePublishManifest(deployedPackage), null, 2)}\n`, + ); +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + const [sourceDir, destinationDir] = process.argv.slice(2); + if (!sourceDir || !destinationDir) { + console.error("Usage: prepare-bundled-package.mjs "); + process.exit(1); + } + prepareBundledPackage(resolve(sourceDir), resolve(destinationDir)); +} diff --git a/scripts/release-lib.sh b/scripts/release-lib.sh index 9fbbfdbaf9..7cd7ec55a3 100644 --- a/scripts/release-lib.sh +++ b/scripts/release-lib.sh @@ -302,15 +302,51 @@ is_npm_tlog_duplicate_error() { grep -q "equivalent entry already exists in the transparency log" <<< "$output" } +package_publish_tool() { + node -e ' + const pkg = require(process.cwd() + "/package.json"); + const bundled = pkg.bundleDependencies ?? pkg.bundledDependencies ?? []; + process.stdout.write(bundled.length > 0 ? "npm" : "pnpm"); + ' +} + +BUNDLED_NPM_VERSION="10.9.7" + +run_bundled_npm() { + npx --yes "npm@$BUNDLED_NPM_VERSION" "$@" +} + +run_package_publish() { + local publish_tool="$1" + local dist_tag="$2" + local disable_provenance="${3:-false}" + + if [ "$publish_tool" = "npm" ]; then + if [ "$disable_provenance" = "true" ]; then + run_bundled_npm publish --tag "$dist_tag" --access public --provenance=false + else + run_bundled_npm publish --tag "$dist_tag" --access public + fi + return + fi + + if [ "$disable_provenance" = "true" ]; then + pnpm publish --no-git-checks --tag "$dist_tag" --access public --provenance=false + else + pnpm publish --no-git-checks --tag "$dist_tag" --access public + fi +} + publish_package_to_npm() { local dist_tag="$1" local package_name="$2" local package_version="$3" + local publish_tool="${4:-pnpm}" local publish_log publish_log="$(mktemp "${TMPDIR:-/tmp}/paperclip-npm-publish.XXXXXX")" - if (set -o pipefail; pnpm publish --no-git-checks --tag "$dist_tag" --access public 2>&1 | tee "$publish_log"); then + if (set -o pipefail; run_package_publish "$publish_tool" "$dist_tag" false 2>&1 | tee "$publish_log"); then rm -f "$publish_log" return 0 fi @@ -335,7 +371,7 @@ publish_package_to_npm() { fi release_warn "Retrying ${package_name}@${package_version} once with npm provenance disabled." - if pnpm publish --no-git-checks --tag "$dist_tag" --access public --provenance=false; then + if run_package_publish "$publish_tool" "$dist_tag" true; then rm -f "$publish_log" return 0 fi diff --git a/scripts/release-lib.test.mjs b/scripts/release-lib.test.mjs index 677c6e5270..efb901f952 100644 --- a/scripts/release-lib.test.mjs +++ b/scripts/release-lib.test.mjs @@ -11,7 +11,13 @@ function writeExecutable(path, body) { writeFileSync(path, body, { mode: 0o755 }); } -function runPublishHelper({ pnpmMode, npmVersionExists = false, distTag = "canary", callerPipefail = true }) { +function runPublishHelper({ + pnpmMode, + npmVersionExists = false, + distTag = "canary", + callerPipefail = true, + publishTool = "pnpm", +}) { const fixtureDir = mkdtempSync(join(tmpdir(), "paperclip-release-lib-")); const binDir = join(fixtureDir, "bin"); const stateDir = join(fixtureDir, "state"); @@ -71,15 +77,30 @@ if [ "$1" = "view" ] && [ "$NPM_VERSION_EXISTS" = "true" ]; then echo "1.2.3" exit 0 fi +if [ "$1" = "publish" ]; then + echo "published" + exit 0 +fi exit 1 `, ); + writeExecutable( + join(binDir, "npx"), + `#!/usr/bin/env bash +set -euo pipefail +printf 'npx %s\n' "$*" >> "$FAKE_CALL_LOG" +[ "$1" = "--yes" ] && shift +[ "$1" = "npm@10.9.7" ] && shift +exec npm "$@" +`, + ); + const shellOptions = callerPipefail ? "set -euo pipefail" : "set -eu"; const script = ` ${shellOptions} source "${repoRoot}/scripts/release-lib.sh" -publish_package_to_npm ${distTag} @paperclipai/example 1.2.3 +publish_package_to_npm ${distTag} @paperclipai/example 1.2.3 ${publishTool} `; let status = 0; @@ -120,6 +141,15 @@ test("publish_package_to_npm returns after a successful pnpm publish", () => { assert.doesNotMatch(result.calls, /--provenance=false/); }); +test("publish_package_to_npm uses npm for bundled dependencies", () => { + const result = runPublishHelper({ pnpmMode: "success", publishTool: "npm" }); + + assert.equal(result.status, 0); + assert.match(result.calls, /^npx --yes npm@10\.9\.7 publish --tag canary --access public$/m); + assert.match(result.calls, /^npm publish --tag canary --access public$/m); + assert.doesNotMatch(result.calls, /^pnpm publish/m); +}); + test("publish_package_to_npm retries duplicate tlog failures without provenance", () => { const result = runPublishHelper({ pnpmMode: "tlog-then-success" }); diff --git a/scripts/release.sh b/scripts/release.sh index 43355c0bcf..79719107d8 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -257,7 +257,16 @@ if [ "$dry_run" = true ]; then [ -z "$pkg_dir" ] && continue release_info " --- $pkg_dir ---" cd "$REPO_ROOT/$pkg_dir" - pnpm publish --dry-run --no-git-checks --tag "$DIST_TAG" 2>&1 | tail -3 + publish_tool="$(package_publish_tool)" + if [ "$publish_tool" = "npm" ]; then + publish_dir="$(mktemp -d "${TMPDIR:-/tmp}/paperclip-release-package.XXXXXX")" + node "$REPO_ROOT/scripts/prepare-bundled-package.mjs" "$REPO_ROOT/$pkg_dir" "$publish_dir" + cd "$publish_dir" + run_bundled_npm pack --pack-destination "$publish_dir" 2>&1 | tail -3 + rm -rf "$publish_dir" + else + pnpm publish --dry-run --no-git-checks --tag "$DIST_TAG" 2>&1 | tail -3 + fi done <<< "$VERSIONED_PACKAGE_INFO" release_info " [dry-run] Would create git tag $tag_name on $CURRENT_SHA" else @@ -266,7 +275,16 @@ else [ -z "$pkg_dir" ] && continue release_info " Publishing $pkg_name@$pkg_version" cd "$REPO_ROOT/$pkg_dir" - publish_package_to_npm "$DIST_TAG" "$pkg_name" "$pkg_version" + publish_tool="$(package_publish_tool)" + if [ "$publish_tool" = "npm" ]; then + publish_dir="$(mktemp -d "${TMPDIR:-/tmp}/paperclip-release-package.XXXXXX")" + node "$REPO_ROOT/scripts/prepare-bundled-package.mjs" "$REPO_ROOT/$pkg_dir" "$publish_dir" + cd "$publish_dir" + fi + publish_package_to_npm "$DIST_TAG" "$pkg_name" "$pkg_version" "$publish_tool" + if [ "$publish_tool" = "npm" ]; then + rm -rf "$publish_dir" + fi done <<< "$VERSIONED_PACKAGE_INFO" release_info " ✓ Published all packages under dist-tag $DIST_TAG" fi