From 486ecac5e9ae974d8eab1d2587326d23b0406fef Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 08:58:37 -0700 Subject: [PATCH 1/7] chore(lockfile): refresh pnpm-lock.yaml (#9964) Auto-generated lockfile refresh after dependencies changed on master. This PR only updates pnpm-lock.yaml. Co-authored-by: lockfile-bot --- pnpm-lock.yaml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 840e5cc040e..21d211f48d1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -20,6 +20,9 @@ overrides: '@lexical/utils': 0.46.0 patchedDependencies: + acpx@0.12.0: + hash: 6rtgor3dogxkfotm4jopwanvmu + path: patches/acpx@0.12.0.patch embedded-postgres@18.1.0-beta.16: hash: 55uhvnotpqyiy37rn3pqpukhei path: patches/embedded-postgres@18.1.0-beta.16.patch @@ -120,8 +123,8 @@ importers: packages/adapter-utils: dependencies: acpx: - specifier: ^0.12.0 - version: 0.12.0 + specifier: 0.12.0 + version: 0.12.0(patch_hash=6rtgor3dogxkfotm4jopwanvmu) picocolors: specifier: ^1.1.1 version: 1.1.1 @@ -13045,7 +13048,7 @@ snapshots: acorn@8.17.0: {} - acpx@0.12.0: + acpx@0.12.0(patch_hash=6rtgor3dogxkfotm4jopwanvmu): dependencies: '@agentclientprotocol/sdk': 1.2.1(zod@4.4.3) commander: 15.0.0 From 2aef4641b48e88f5ce7e75ce69fbe3bf6bbfc60d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:59:11 -0500 Subject: [PATCH 2/7] docs(release): add v2026.722.0 changelog (#10008) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Release changelog: Paperclip v2026.722.0 Adds `releases/v2026.722.0.md`, the user-facing stable changelog for the `v2026.722.0` release, generated per `.agents/skills/release-changelog/SKILL.md`. - **Range:** `v2026.720.0..origin/master` β€” 44 commits from 13 contributors. - **Version:** confirmed via `scripts/release.sh stable --date 2026-07-22 --print-version` β†’ `2026.722.0`. - **Breaking changes:** none. Two additive DB migrations (Connections v3 schema core + connection user authorization state); no destructive migrations. - **Sections:** Highlights (run-bound agent secret access; Windows local-agent spawning), Improvements, Experimental (Connections v3 foundation, gated behind the Apps experimental setting), Fixes, Upgrade Guide, Contributors. Contributor count/names follow the skill rules (bots and Paperclip founders excluded from the listed names but counted in the total). Related issue: PAP-14983. πŸ€– Generated with [Claude Code](https://claude.com/claude-code) --- releases/v2026.722.0.md | 49 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 releases/v2026.722.0.md diff --git a/releases/v2026.722.0.md b/releases/v2026.722.0.md new file mode 100644 index 00000000000..c3a94e98797 --- /dev/null +++ b/releases/v2026.722.0.md @@ -0,0 +1,49 @@ +# Paperclip v2026.722.0 + +> Released: 2026-07-22 + +## Highlights + +- **Run-bound agent secret access** β€” Agents can now fetch the secrets they've been granted on demand through a run-bound API, instead of relying only on ambient environment injection. A new `access.*` delivery mode exposes API-only secrets, `GET /api/agents/me/secrets` lists just the aliases an agent is granted, and `POST /api/agents/me/secrets/:key/value` returns a value with `Cache-Control: no-store`. Every value read is written to both the security audit trail and the operator activity log, low-trust review/skill-test tokens stay denied, and a new **Secret access** editor lets you manage per-agent grants from agent settings. ([#9921](https://github.com/paperclipai/paperclip/pull/9921)) +- **Local agents run on Windows** β€” The embedded ACPX engine no longer wraps local agent commands in a generated Bash script, so Claude, Codex, Gemini, and custom ACP adapters now spawn natively on Windows as well as Linux. Environment variables are injected through ACPX session options, child stderr is captured in-process, `.cmd` npm/pnpm shims are preferred on Windows, and symlink `EPERM` failures fall back to copies. A real Node ACP spawn smoke now runs on both Ubuntu and Windows. ([#9980](https://github.com/paperclipai/paperclip/pull/9980)) + +## Improvements + +- **User-named `PAPERCLIP_*` bindings now reach your agents** β€” The heartbeat previously stripped *every* `PAPERCLIP_`-prefixed env binding before resolution, so a secret you deliberately named something like `PAPERCLIP_CLOUD_PROD_PROVIDER_RAILWAY_TOKEN` was silently dropped. Now only `PAPERCLIP_API_KEY` is categorically rejected, harness-assigned runtime vars always win, and every other `PAPERCLIP_*` binding flows through to the run environment. ([#9974](https://github.com/paperclipai/paperclip/pull/9974)) +- **Sturdier telemetry client** β€” The telemetry client now matches the wire contract: payloads are chunked, batch IDs are deterministic, retries are batched with backoff, and the local store is bounded. A silent-drop baseline is pinned and caps/backoff are now a configurable surface. Telemetry remains anonymized and can be disabled with `DO_NOT_TRACK=1` or `PAPERCLIP_TELEMETRY_DISABLED=1`. ([#9946](https://github.com/paperclipai/paperclip/pull/9946), [#9906](https://github.com/paperclipai/paperclip/pull/9906), @nickyleach) +- **Slash-named secrets browse as folders** β€” Secrets whose names contain slashes now navigate as nested folders in the UI. ([#9913](https://github.com/paperclipai/paperclip/pull/9913)) +- **Searchable agent picker for secret access** β€” Granting per-agent secret access now uses a searchable agent picker instead of a long flat list. ([#9918](https://github.com/paperclipai/paperclip/pull/9918)) +- **Cleaner external object labels and star controls** β€” External object labels and the star controls that sit next to them were simplified for less visual noise. ([#9929](https://github.com/paperclipai/paperclip/pull/9929)) +- **Safer sandbox `CODEX_HOME` sync** β€” The `codex-local` adapter now stages an explicit allowlist when syncing `CODEX_HOME` into the sandbox, rather than copying the whole home directory. ([#9972](https://github.com/paperclipai/paperclip/pull/9972), @nickyleach) + +## Experimental + +These features ship gated behind experimental settings β€” enable Apps under experimental settings to try them out. + +- **Connections v3 foundation** β€” The groundwork for one-click Connected Apps landed: a v3 schema core adds a stable company-scoped connection UID, explicit ownership/auth/transport fields, a subject-aware `connection_grants` table, and multi-key credentials (with the legacy `remote_http` transport renamed to `mcp_remote`). On top of that, a generated **AppDefinition** Wave 1 catalog gives browse and setup a single typed contract, and a runtime layer adds subject-aware authorization state, scoped key handling, and OpenAPI-registered grant routes that fail closed for unknown scopes. ([#9958](https://github.com/paperclipai/paperclip/pull/9958), [#9981](https://github.com/paperclipai/paperclip/pull/9981), [#9982](https://github.com/paperclipai/paperclip/pull/9982)) + +## Fixes + +- **Archived inbox items stay archived** β€” Items you've archived no longer resurface in the inbox. ([#9931](https://github.com/paperclipai/paperclip/pull/9931)) +- **Execution-policy final approval finishes cleanly** β€” Approving the final stage of an execution policy now terminates the policy instead of rewinding it back to stage 1. ([#7936](https://github.com/paperclipai/paperclip/pull/7936), @HKTITAN) +- **Accept Office issue attachments** β€” Office document attachments (Word, Excel, PowerPoint) are now accepted on issues. ([#8562](https://github.com/paperclipai/paperclip/pull/8562), @samrusani) +- **Redact HTTP cookies from server logs** β€” Cookie headers are now redacted from server logs so session material doesn't leak into log output. ([#7977](https://github.com/paperclipai/paperclip/pull/7977), @nosolosoft) +- **No more 500s from run-authored comments** β€” `createdByRunId` is sanitized on comment insert, fixing a class of 500 errors. ([#9489](https://github.com/paperclipai/paperclip/pull/9489), @digitalflanker-ux) +- **Preserve terminal status on issue release** β€” Releasing an issue no longer clobbers a terminal (done/cancelled) status. ([#7524](https://github.com/paperclipai/paperclip/pull/7524), @harshkotak) +- **Fail loudly on invalid config files** β€” An invalid config file now surfaces a clear error instead of being silently ignored. ([#9041](https://github.com/paperclipai/paperclip/pull/9041), @lesbass) +- **Non-clickable pending-approval guidance** β€” The pending-approval admin guidance on the invite screen is no longer a misleading clickable element. ([#6786](https://github.com/paperclipai/paperclip/pull/6786), @le-czs) +- **Cron validity settles with radix-ui 1.6.4** β€” Cron validity handling and a settle-branch test were synced for radix-ui 1.6.4. ([#9966](https://github.com/paperclipai/paperclip/pull/9966), @nickyleach) +- **CLI import selection fixes** β€” Cleared import selections no longer drop the extension file, and missing company-manifest fields were added to the import-selection test fixture. ([#2379](https://github.com/paperclipai/paperclip/pull/2379), @kevinmanase, [#9978](https://github.com/paperclipai/paperclip/pull/9978), @aaymeloglu) +- **Cloud-managed instances pass the health gate** β€” Cloud-managed instances are now treated as bootstrapped in the health gate so they don't stall on startup. ([#9912](https://github.com/paperclipai/paperclip/pull/9912)) + +## Upgrade Guide + +- This release adds two additive database migrations for the Connections v3 foundation (`0182_connections_v3_schema_core`, `0183_connection_user_authorization_state`), which backfill stable connection UIDs, create default workspace grants, and rename the `remote_http` transport to `mcp_remote`. Migrations run automatically on startup β€” no manual action is required. +- If you previously set a static `PAPERCLIP_API_KEY` in adapter/config env to override the run token, that override no longer applies β€” the harness-minted run token is now the only source of the run API key. All other `PAPERCLIP_*`-named bindings now flow through to the run environment as expected. +- Connections/Apps remain gated behind experimental settings. Enable **Apps** under experimental settings to try the Connections v3 surfaces. + +## Contributors + +This release has 44 commits from 13 contributors. Thank you to everyone who contributed to this release! + +@aaymeloglu, @digitalflanker-ux, @harshkotak, @HKTITAN, @kevinmanase, @le-czs, @lesbass, @MindSyncHub, @nickyleach, @nosolosoft, @samrusani From d54ff52fc33dd58fc723222a4ef141397f202876 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:53:29 -0500 Subject: [PATCH 3/7] test(heartbeat): await execution drain before cleanup (#10023) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source control plane people use to manage AI agents and their work > - Heartbeat scheduling tests protect the orchestration rules that serialize an agent's runs > - The dependency scheduling suite waits for run rows to become terminal before deleting shared database fixtures > - A terminal row is persisted before asynchronous execution finalization and successful-run handoff work fully drain > - The test then clears process tracking and deletes heartbeat events while finalization can still append another event > - This pull request waits for each tracked run's execution promise to drain before resetting mocks or deleting fixtures > - The benefit is deterministic cleanup that preserves the production lifecycle ordering and prevents release CI flakes ## Linked Issues or Issue Description ### What happened? Release run `29936031931` failed in `heartbeat-dependency-scheduling.test.ts` while deleting `heartbeat_runs`. Asynchronous heartbeat finalization inserted a new `heartbeat_run_events` row after the test had already deleted existing events, causing the run-row delete to violate the event foreign key. ### Expected behavior The serialized heartbeat test suite should finish all asynchronous run execution work before destructive fixture cleanup. ### Steps to reproduce 1. Check out commit `2aef4641b48e88f5ce7e75ce69fbe3bf6bbfc60d`. 2. Run `pnpm exec vitest run --project @paperclipai/server server/src/__tests__/heartbeat-dependency-scheduling.test.ts --pool=forks --isolate` repeatedly with PostgreSQL test support enabled. 3. Observe that teardown can delete heartbeat events while execution finalization is still able to append another event, causing a foreign-key failure when heartbeat runs are deleted. ### Paperclip version or commit `2aef4641b48e88f5ce7e75ce69fbe3bf6bbfc60d` ### Deployment mode Other β€” GitHub Actions release verification. ### Installation method Built from source with pnpm. ### Agent adapter(s) involved Not adapter-specific (core heartbeat test lifecycle). ### Database mode External PostgreSQL test database. ### Relevant logs or output `delete from "heartbeat_runs"` failed because the run remained referenced by `heartbeat_run_events_run_id_heartbeat_runs_id_fk`. ## What Changed - Collect heartbeat run IDs after queued/running rows settle and await `heartbeat.waitForRunExecutionDrain()` for each run. - Reset the adapter mock and clear process tracking only after asynchronous heartbeat finalization has completed. ## Verification - Ran `pnpm exec vitest run --project @paperclipai/server server/src/__tests__/heartbeat-dependency-scheduling.test.ts --pool=forks --isolate` 10 consecutive times; all 10 runs passed with 6/6 tests. ## Risks - Low risk: test-only cleanup ordering change using an existing heartbeat service drain API. Production behavior is unchanged. > 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 Codex with exact model IDs `gpt-5.5` for this heartbeat and `gpt-5.6-sol` for the recovered initial implementation run; tool-enabled code inspection, GitHub diagnostics, and shell test execution. Runtime context-window sizes were not exposed. ## 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 - [x] All Paperclip CI gates are green - [x] 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 --- .../heartbeat-dependency-scheduling.test.ts | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/server/src/__tests__/heartbeat-dependency-scheduling.test.ts b/server/src/__tests__/heartbeat-dependency-scheduling.test.ts index 454fa19244e..bb875a3f21b 100644 --- a/server/src/__tests__/heartbeat-dependency-scheduling.test.ts +++ b/server/src/__tests__/heartbeat-dependency-scheduling.test.ts @@ -100,17 +100,6 @@ describeEmbeddedPostgres("heartbeat dependency-aware queued run selection", () = }, 20_000); afterEach(async () => { - mockAdapterExecute.mockReset(); - mockAdapterExecute.mockImplementation(async () => ({ - exitCode: 0, - signal: null, - timedOut: false, - errorMessage: null, - summary: "Dependency-aware heartbeat test run.", - provider: "test", - model: "test-model", - })); - runningProcesses.clear(); let idlePolls = 0; for (let attempt = 0; attempt < 100; attempt += 1) { const runs = await db @@ -125,7 +114,22 @@ describeEmbeddedPostgres("heartbeat dependency-aware queued run selection", () = } await new Promise((resolve) => setTimeout(resolve, 50)); } - await new Promise((resolve) => setTimeout(resolve, 50)); + const runIds = await db + .select({ id: heartbeatRuns.id }) + .from(heartbeatRuns) + .then((runs) => runs.map((run) => run.id)); + await Promise.all(runIds.map((runId) => heartbeat.waitForRunExecutionDrain(runId))); + mockAdapterExecute.mockReset(); + mockAdapterExecute.mockImplementation(async () => ({ + exitCode: 0, + signal: null, + timedOut: false, + errorMessage: null, + summary: "Dependency-aware heartbeat test run.", + provider: "test", + model: "test-model", + })); + runningProcesses.clear(); await db.delete(environmentLeases); await db.delete(activityLog); await db.delete(companySkills); From b247bf7150484e6a0a0da82079617901f789d2a5 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Wed, 22 Jul 2026 10:08:03 -0700 Subject: [PATCH 4/7] feat(runtime): opt-in sandbox file-sync lifecycle hooks (API + provider docs) (#10013) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is an open-source AI-agent management platform; agents run tasks inside sandboxed environments (Daytona, Kubernetes, E2B, etc.) > - The control-plane ↔ sandbox file-transfer path flows through the `environmentExecute` seam in `protocol.ts` β€” the only verb available to plugins β€” which forces a base64-over-exec chunked loop for every file move: workspace files, assets, Codex home sync > - This transport is correct and safe, but it bypasses provider-native bulk/streaming APIs (Daytona `uploadFiles`, K8s `FastUploadInterceptor` / volume mounts), leaving significant throughput on the table for large workspaces > - The right fix is an opt-in seam extension: providers with faster native transfer declare two optional verbs; providers that do not opt in stay on the existing fallback with zero code or behavior change required > - This PR adds the first layer of that extension β€” two optional verbs (`environmentSyncIn` / `environmentSyncOut`) in the plugin SDK, the runtime plumbing to prefer the native path for the two clean destroy-then-replace cases, and a doc for the contract > - The core correctness invariant is byte-identical fallback: if no provider opts in, execution is exactly what ships today; `assertSyncOperationsConfined` enforces host-side path confinement for providers that do opt in > - No provider advertises the verbs yet β†’ zero production behavior change; future PRs wire up Daytona and K8s providers against this contract ## Linked Issues or Issue Description No public GitHub issue exists for this feature. Description follows the `feature_request` issue template: **Subsystem affected:** packages/plugins β€” plugin system; packages/adapter-utils β€” adapter runtime; server/ β€” EnvironmentRuntimeService **Problem or motivation:** Sandbox file transfers currently always use a base64-over-exec chunked loop regardless of what the underlying provider supports. For workspaces larger than a few MB this becomes the dominant wall-clock cost of every sandbox run, and it bypasses bulk/stream APIs that providers like Daytona already expose natively. **Proposed solution:** Add two optional, opt-in plugin hooks β€” `onEnvironmentSyncIn` / `onEnvironmentSyncOut` β€” to the plugin SDK. When a provider defines both hooks and both are advertised via the existing `supportedMethods` negotiation, the runtime prefers the native path for the two clean destroy-then-replace transfer cases; all other cases fall back to the existing byte-identical base64 transport. **Alternatives considered:** An unconditional verb would require every provider to implement or stub the verb. The opt-in / `METHOD_NOT_IMPLEMENTED` pattern (already used by `environmentExecute`) preserves backward compatibility with zero provider changes required. **Roadmap alignment:** Consistent with the βœ… "Cloud / Sandbox agents" and βœ… "Plugin system" milestones; extends the plugin seam rather than adding control-plane-level logic. **Additional context:** Searched open pull requests and issues for duplicate sandbox file-sync / native-transfer work; none found. ## What Changed - **`packages/plugins/sdk`** - `protocol.ts`: two new optional `HostToWorkerMethods` β€” `environmentSyncIn` / `environmentSyncOut` β€” plus generic `SyncOperation`, `SyncFileMapping`, and `SyncOutcome` types - `define-plugin.ts`: optional `onEnvironmentSyncIn` / `onEnvironmentSyncOut` fields on `PluginDefinition`; worker advertises each verb only when its hook is defined (else `METHOD_NOT_IMPLEMENTED`, mirroring `environmentExecute`) - `worker-rpc-host.ts`: route new verbs to plugin hooks - `index.ts`: re-export new public types - **`packages/adapter-utils`** - `command-managed-runtime.ts`: expose optional `syncIn` / `syncOut` on `CommandManagedRuntimeRunner` (available only when both verbs are advertised); add `assertSyncOperationsConfined` host-side path-confinement guard - `sandbox-managed-runtime.ts`: `SandboxManagedRuntimeClient` gains optional `syncIn` / `syncOut`; orchestrator prefers native path for default-provision asset inbound and workspace-download-into-fresh-dir outbound; all other paths keep the existing base64 fallback - `sandbox-file-sync.test.ts` (new): 234-line characterization suite β€” native-opt-in branch, fallback branch, `assertSyncOperationsConfined` escape-path rejection, `followSymlinks` β†’ tar `-h` - `command-managed-runtime.test.ts`: negotiation + native-sync + confinement tests - **`server/src/services/environment-runtime.ts`**: `EnvironmentRuntimeService` delegates to `syncIn` / `syncOut`, gated on advertised support - **`server/src/services/environment-execution-target.ts`**: minor typing fix alongside the new verbs - **`doc/plugins/SANDBOX_FILE_SYNC_HOOKS.md`** (new): documents the full contract β€” opt-in / no-op guarantee, operation ordering, provider-may-tar, atomicity, `followSymlinks`, secret modes (0600, no window), path confinement, `operationId` opacity, resource bounds, shell-quoting ## Verification ```bash # SDK suite pnpm --filter packages/plugins/sdk test # Adapter-utils suite (includes new sandbox-file-sync characterization tests) pnpm --filter packages/adapter-utils test # Expected: 255 pass / 4 skip # Type-check across affected packages pnpm --filter packages/plugins/sdk typecheck pnpm --filter packages/adapter-utils typecheck # Server changed-file spot check: cd server && npx tsc --noEmit --skipLibCheck 2>&1 | grep -E "environment-(runtime|execution-target)" | head -20 ``` Key behavioral invariant to spot-check: with no provider opting in (the current state), run any sandbox task and confirm file-transfer behavior is byte-for-byte identical to what the pre-PR code produces. The characterization tests assert this at the unit level. ## Risks - **Zero production risk today**: no provider advertises `environmentSyncIn` / `environmentSyncOut`, so the new code paths are unreachable in production; all real traffic stays on the existing base64 fallback - **Path confinement**: `assertSyncOperationsConfined` rejects any `targetPath` that escapes the declared root β€” this is the primary security boundary for future providers. The test suite covers escape-path rejection - **Atomicity**: the contract delegates atomicity to providers; the doc explicitly calls out that directory-level ops are not guaranteed atomic - **Secret transport**: credential assets (e.g., Codex `auth.json`, directory mappings) continue to use the existing tar path β€” they do not go through the new verbs in any current provider > 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 Provider: Anthropic Model: `claude-sonnet-4-6` (Claude Sonnet 4.6) Context window: 200 K tokens Capabilities: extended tool use, multi-file code generation, agentic reasoning via the Paperclip agent framework ## 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 - [x] All Paperclip CI gates are green - [x] 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: Harold Kim Co-authored-by: Paperclip --- doc/plugins/SANDBOX_FILE_SYNC_HOOKS.md | 205 +++++++++++++++ .../src/command-managed-runtime.test.ts | 54 ++++ .../src/command-managed-runtime.ts | 25 +- .../src/sandbox-file-sync.test.ts | 234 ++++++++++++++++++ .../src/sandbox-managed-runtime.ts | 208 ++++++++++++++-- packages/plugins/sdk/src/define-plugin.ts | 24 ++ packages/plugins/sdk/src/index.ts | 5 + packages/plugins/sdk/src/protocol.ts | 70 ++++++ packages/plugins/sdk/src/worker-rpc-host.ts | 24 ++ .../environment-sync-negotiation.test.ts | 191 ++++++++++++++ .../services/environment-execution-target.ts | 22 ++ server/src/services/environment-runtime.ts | 86 +++++++ 12 files changed, 1120 insertions(+), 28 deletions(-) create mode 100644 doc/plugins/SANDBOX_FILE_SYNC_HOOKS.md create mode 100644 packages/adapter-utils/src/sandbox-file-sync.test.ts create mode 100644 packages/plugins/sdk/tests/environment-sync-negotiation.test.ts diff --git a/doc/plugins/SANDBOX_FILE_SYNC_HOOKS.md b/doc/plugins/SANDBOX_FILE_SYNC_HOOKS.md new file mode 100644 index 00000000000..951bf020090 --- /dev/null +++ b/doc/plugins/SANDBOX_FILE_SYNC_HOOKS.md @@ -0,0 +1,205 @@ +# Sandbox file-sync lifecycle hooks + +A sandbox environment provider moves workspace and asset files between the host +and the sandbox around every run. By default the runtime synthesizes that +transfer over the single `environmentExecute` verb: it base64-encodes bytes and +pipes them through `base64 -d` shell commands, one bounded round-trip per chunk. +That works everywhere but is slow for large workspaces because it cannot use a +provider's native bulk file transport. + +The two **optional, opt-in** hooks documented here let a provider replace that +base64-over-exec transfer with its own native mechanism: + +- **`onEnvironmentSyncIn`** β€” before execution: place a set of host + files/directories at target sandbox paths. +- **`onEnvironmentSyncOut`** β€” after execution: copy a set of sandbox + files/directories back to target host paths. + +They are entirely opt-in. A provider that does not define them keeps the exact +base64 fallback, byte-for-byte β€” there is **zero behavior change** for existing +providers. + +## Opt-in / no-op semantics + +A hook is opted into exactly like `onEnvironmentExecute`: defining it on your +`PluginDefinition` makes the worker advertise the matching verb in +`InitializeResult.supportedMethods`; leaving it undefined omits the verb and the +guarded handler throws `METHOD_NOT_IMPLEMENTED` if it is ever called. + +**Both hooks are advertised and consumed as a pair.** The host runtime uses the +native path only when the worker advertises **both** `environmentSyncIn` and +`environmentSyncOut`; if a provider advertises only one, the orchestrator keeps +the base64 fallback for both directions. Define both or neither. + +```ts +export default definePlugin({ + async setup() { /* ... */ }, + async onEnvironmentSyncIn(params) { + return { operations: await transferInbound(params) }; + }, + async onEnvironmentSyncOut(params) { + return { operations: await transferOutbound(params) }; + }, +}); +``` + +## The operation / file-mapping contract + +Each hook receives an ordered list of **operations**. Each operation carries an +opaque id and a list of sourceβ†’target **file mappings**: + +```ts +interface PluginSyncOperation { + operationId: string; // opaque, non-sensitive; do NOT interpret it + files: PluginSyncFileMapping[]; +} + +interface PluginSyncFileMapping { + sourcePath: string; // absolute + targetPath: string; // absolute + kind: "file" | "directory"; + mode?: number; // POSIX mode to apply at the target + exclude?: string[]; // glob excludes for a directory mapping + followSymlinks?: boolean; // directory symlink handling; see below +} + +interface PluginEnvironmentSyncResult { + operations: { operationId: string; filesTransferred: number; bytesTransferred: number }[]; +} +``` + +For `onEnvironmentSyncIn`, `sourcePath` is a **host** path and `targetPath` a +**sandbox** path. For `onEnvironmentSyncOut` the direction is reversed. All +sandbox paths are POSIX. Return per-operation `filesTransferred` / +`bytesTransferred` for observability. + +### Ordering + +Operations are applied strictly in array order, and the orchestrator invokes the +hooks in a fixed lifecycle order (inbound before execution, outbound after). The +orchestrator owns *what* and *when*; a provider only executes the opaque +transfers it is handed and must not reorder them. + +### A provider may tar internally + +The contract only describes the observable sourceβ†’target result. How you move +the bytes is yours: bulk upload API, an internal `tar` stream, per-file +enumeration β€” all are fine. Whatever you do, the materialized target must be +observationally identical to the mapping (same files, same contents, same modes, +same symlink treatment) so the native and fallback paths are interchangeable. + +### `operationId` is opaque + +`operationId` is an opaque, non-sensitive token authored by the orchestrator. It +is **not** derived from any secret or user data, it is safe to log and safe to +expose to the sandbox, and a provider **must not** parse or depend on its value. +Do not echo it into a path or a place where it could collide with real data. + +## Symlink contract (`followSymlinks`) + +`followSymlinks` applies to `kind: "directory"` mappings and has exactly the +meaning of `tar`'s `-h` flag: + +- **falsy (default)** β€” archive and recreate symlinks **as links** (preserve). +- **`true`** β€” **dereference** each symlink to its target bytes. + +A provider honoring a directory mapping MUST reproduce this: preserve links when +falsy, dereference to bytes when `true`. The orchestrator passes the same value +it passes to its own tar create step, so native and fallback are observationally +identical. There is no separate extract-side symlink flag and no execution-time +special case β€” symlink handling lives entirely in this one flag. + +## Atomicity contract + +The required guarantee level is deliberately equal to the base64 fallback's +floor, so opting in never weakens integrity and never over-promises. + +- **Single-file mappings (`kind: "file"`) MUST be atomic-replace (REQUIRED).** + Stage the bytes to a provider-chosen temporary path, then atomically rename + onto `targetPath`, so an interrupted transfer never leaves a truncated file at + `targetPath`. This mirrors the fallback, which stages to + `.paperclip-upload` and then `mv -f`. + - The temp file MUST live in the **same directory (same filesystem)** as + `targetPath`. A cross-device rename degrades to copy-then-unlink and + reintroduces the truncation window it is meant to close. + - Reserve the `.paperclip-upload*` scratch names: a provider-chosen temp must + not collide with the fallback scratch name or with a real target. + +- **Directory mappings and the sync as a whole are NOT atomic / NOT + transactional.** A directory transfer is destroy-then-replace: a crash + mid-transfer can leave a partial tree, and the runtime does not roll back + already-moved bytes across operations. This matches today's behavior; do not + assume a directory operation is atomic. Where an individual file must be + integrity-protected, deliver it as its own `kind: "file"` mapping so it inherits + the single-file atomic-replace guarantee. + +- **Every operation is fail-loud.** An operation either completes or raises to + the orchestrator; never report partial success silently. The orchestrator may + then retry or fall back. + +## Secret material and file modes + +This seam can carry credential-bearing files (for example an auth directory). +Treat `mode` as mandatory for such mappings: + +- Apply the requested `mode` (e.g. `0o600`) with **no world-readable window** β€” + create the target with the mode, or `chmod` **before** writing any bytes, never + after. +- `mode` MUST be honored for files **inside a directory mapping** too, not only + for `kind: "file"` mappings. If you tar internally, preserve permissions; if + you enumerate, set the mode as each file lands. A credential that rides a + directory mapping otherwise silently loses its `0o600` guarantee. +- A directory mapping is not atomic (see above). If a directory carries an + individually-sensitive secret whose integrity matters, prefer delivering that + secret as a `kind: "file"` mapping so it gets atomic-replace, or protect its + integrity out of band. + +## Host-side path confinement (required of the orchestrator) + +The sandbox is untrusted relative to the host, so **the orchestrator β€” not the +provider β€” owns and confines every path**. Before an operation is handed to a +provider, the runtime canonicalizes each mapping's `sourcePath`/`targetPath` and +confines it to an orchestrator-owned root (the workspace directory or a specific +asset directory), rejecting absolute escapes and `..` traversal fail-closed. A +provider receives only already-confined, orchestrator-authored paths and MUST +NOT widen them (for example by following a sandbox-planted symlink out of the +intended root on an outbound write). Confinement is a host-side complete-mediation +guard and is never delegated below the trust boundary. + +## Resource bounds + +The base64 fallback enforces transfer caps so a runaway payload cannot exhaust +memory. A native provider MUST keep an equivalent bound β€” stream or chunk large +transfers rather than buffering unboundedly, and fail closed on an oversized +inline payload rather than silently removing the cap. + +## Shell safety (native providers that shell out) + +If your native transfer builds shell command strings (for example a pod-exec +`tar`/`base64`/`mv` pipeline), single-quote **every** interpolated path so a path +containing shell metacharacters is transferred literally, never interpreted. +Providers whose transport is a non-shell API (a typed bulk-upload call) do not +need this, but any shell interpolation must quote. + +## Reference: minimal shape + +```ts +async onEnvironmentSyncIn({ operations }) { + const results = []; + for (const op of operations) { // apply in order + let filesTransferred = 0; + let bytesTransferred = 0; + for (const f of op.files) { + if (f.kind === "file") { + // stage to a same-dir temp, then atomic rename onto f.targetPath, + // applying f.mode with no world-readable window + } else { + // materialize f.sourcePath at f.targetPath (destroy-then-replace), + // honoring f.exclude and f.followSymlinks, applying per-file modes + } + } + results.push({ operationId: op.operationId, filesTransferred, bytesTransferred }); + } + return { operations: results }; +} +``` diff --git a/packages/adapter-utils/src/command-managed-runtime.test.ts b/packages/adapter-utils/src/command-managed-runtime.test.ts index d9eb811780f..2cd4d853a2d 100644 --- a/packages/adapter-utils/src/command-managed-runtime.test.ts +++ b/packages/adapter-utils/src/command-managed-runtime.test.ts @@ -304,6 +304,60 @@ describe("command managed runtime", () => { expect(progress.at(-1)).toEqual({ done: payload.length, total: payload.length }); }); + it("stages a single-file write to .paperclip-upload then atomically renames it (single-stream path)", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-command-atomic-")); + cleanupDirs.push(rootDir); + const remotePath = path.join(rootDir, "nested", "payload.bin"); + + const { runner, calls } = makeSpawnRunner({ supportsSingleStreamStdinProgress: true }); + const client = createCommandManagedRuntimeClient({ runner, commandCwd: "/", timeoutMs: 30_000 }); + await client.writeFile(remotePath, toArrayBuffer(Buffer.from("hello atomic\n"))); + + // Characterization guardrail: the legacy single-file transport must keep its + // stage-then-atomic-rename shape (temp .paperclip-upload + `mv -f`). + const script = (calls[0].args ?? []).join(" "); + expect(script).toContain(`${remotePath}.paperclip-upload`); + expect(script).toContain(`mv -f`); + expect(script.indexOf(".paperclip-upload")).toBeLessThan(script.indexOf("mv -f")); + expect(await readFile(remotePath, "utf8")).toBe("hello atomic\n"); + }); + + it("stages a single-file write to a temp then renames it on the chunked fallback path too", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-command-atomic-fallback-")); + cleanupDirs.push(rootDir); + const remotePath = path.join(rootDir, "nested", "payload.bin"); + + const payload = Buffer.alloc(10 * 1024 * 1024); + for (let i = 0; i < payload.length; i++) payload[i] = i % 256; + const { runner, calls } = makeSpawnRunner({ supportsSingleStreamStdinProgress: false }); + const client = createCommandManagedRuntimeClient({ runner, commandCwd: "/", timeoutMs: 30_000 }); + await client.writeFile(remotePath, toArrayBuffer(payload)); + + const scripts = calls.map((call) => (call.args ?? []).join(" ")); + expect(scripts.some((script) => script.includes(`${remotePath}.paperclip-upload`))).toBe(true); + expect(scripts.some((script) => script.includes(`mv -f`))).toBe(true); + expect((await readFile(remotePath)).equals(payload)).toBe(true); + }); + + it("leaves the client without syncIn/syncOut unless the runner supports both (fallback preserved)", () => { + const base = makeSpawnRunner().runner; + expect(createCommandManagedRuntimeClient({ runner: base, commandCwd: "/", timeoutMs: 1 }).syncIn).toBeUndefined(); + + const onlyIn: CommandManagedRuntimeRunner = { ...base, syncIn: async () => ({ operations: [] }) }; + const partial = createCommandManagedRuntimeClient({ runner: onlyIn, commandCwd: "/", timeoutMs: 1 }); + expect(partial.syncIn).toBeUndefined(); + expect(partial.syncOut).toBeUndefined(); + + const both: CommandManagedRuntimeRunner = { + ...base, + syncIn: async () => ({ operations: [] }), + syncOut: async () => ({ operations: [] }), + }; + const native = createCommandManagedRuntimeClient({ runner: both, commandCwd: "/", timeoutMs: 1 }); + expect(native.syncIn).toBeTypeOf("function"); + expect(native.syncOut).toBeTypeOf("function"); + }); + it("falls back to chunked upload progress when the runner cannot report mid-stream stdin progress", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-command-write-fallback-")); cleanupDirs.push(rootDir); diff --git a/packages/adapter-utils/src/command-managed-runtime.ts b/packages/adapter-utils/src/command-managed-runtime.ts index 77acad78cbe..b8b3043cca7 100644 --- a/packages/adapter-utils/src/command-managed-runtime.ts +++ b/packages/adapter-utils/src/command-managed-runtime.ts @@ -5,6 +5,8 @@ import { type SandboxManagedRuntimeAsset, type SandboxManagedRuntimeClient, type SandboxRemoteExecutionSpec, + type SandboxSyncOperation, + type SandboxSyncResult, } from "./sandbox-managed-runtime.js"; import { preferredShellForSandbox, shellCommandArgs } from "./sandbox-shell.js"; import type { RunProcessResult } from "./server-utils.js"; @@ -28,6 +30,16 @@ export interface CommandManagedRuntimeRunner { onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise; onSpawn?: (meta: { pid: number; startedAt: string }) => Promise; }): Promise; + /** + * Optional native inbound file transfer. Present only when the sandbox + * provider advertises both `environmentSyncIn` and `environmentSyncOut`; the + * client exposes `syncIn`/`syncOut` only when BOTH are present, so the + * orchestrator either uses the native path for both directions or falls back + * to the base64 transport for both. + */ + syncIn?(operations: SandboxSyncOperation[]): Promise; + /** Optional native outbound file transfer. See {@link syncIn}. */ + syncOut?(operations: SandboxSyncOperation[]): Promise; } export interface CommandManagedRuntimeSpec { @@ -118,7 +130,7 @@ export function createCommandManagedRuntimeClient(input: { return result; }; - return { + const client: SandboxManagedRuntimeClient = { makeDir: async (remotePath) => { await runShell(`mkdir -p ${shellQuote(remotePath)}`); }, @@ -240,6 +252,17 @@ export function createCommandManagedRuntimeClient(input: { requireSuccessfulResult(result, command); }, }; + + // Expose the native sync capability to the orchestrator only when the runner + // supports BOTH directions; a provider that advertises just one verb (or + // neither) keeps the byte-identical base64 fallback for both. + const { syncIn, syncOut } = input.runner; + if (syncIn && syncOut) { + client.syncIn = (operations) => syncIn(operations); + client.syncOut = (operations) => syncOut(operations); + } + + return client; } export async function prepareCommandManagedRuntime(input: { diff --git a/packages/adapter-utils/src/sandbox-file-sync.test.ts b/packages/adapter-utils/src/sandbox-file-sync.test.ts new file mode 100644 index 00000000000..206d0531220 --- /dev/null +++ b/packages/adapter-utils/src/sandbox-file-sync.test.ts @@ -0,0 +1,234 @@ +import { lstat, mkdir, mkdtemp, readFile, readdir, rm, symlink, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { execFile as execFileCallback } from "node:child_process"; +import { promisify } from "node:util"; +import { afterEach, describe, expect, it } from "vitest"; + +import { + assertSyncOperationsConfined, + prepareSandboxManagedRuntime, + type SandboxManagedRuntimeClient, + type SandboxSyncOperation, +} from "./sandbox-managed-runtime.js"; + +const execFile = promisify(execFileCallback); + +interface RecordingClient { + client: SandboxManagedRuntimeClient; + syncInOps: SandboxSyncOperation[][]; + syncOutOps: SandboxSyncOperation[][]; +} + +// A filesystem-backed client that additionally exposes native syncIn/syncOut, +// mirroring a provider that opted into the sync verbs. The native transfer is a +// faithful destroy-then-replace directory copy honoring followSymlinks. +function makeNativeClient(): RecordingClient { + const syncInOps: SandboxSyncOperation[][] = []; + const syncOutOps: SandboxSyncOperation[][] = []; + + const transferDirectory = async ( + sourcePath: string, + targetPath: string, + followSymlinks: boolean | undefined, + ): Promise => { + await rm(targetPath, { recursive: true, force: true }); + await mkdir(targetPath, { recursive: true }); + // followSymlinks true dereferences to bytes (like tar -h); falsy preserves links. + const copyArgs = followSymlinks ? ["-RL"] : ["-a"]; + await execFile("cp", [...copyArgs, `${sourcePath}/.`, targetPath]); + const entries = await readdir(targetPath, { withFileTypes: true }).catch(() => []); + return entries.length; + }; + + const applyOperations = async (operations: SandboxSyncOperation[]) => ({ + operations: await Promise.all(operations.map(async (operation) => { + let filesTransferred = 0; + for (const mapping of operation.files) { + if (mapping.kind === "directory") { + filesTransferred += await transferDirectory(mapping.sourcePath, mapping.targetPath, mapping.followSymlinks); + } else { + await mkdir(path.dirname(mapping.targetPath), { recursive: true }); + await writeFile(mapping.targetPath, await readFile(mapping.sourcePath)); + filesTransferred += 1; + } + } + return { operationId: operation.operationId, filesTransferred, bytesTransferred: 0 }; + })), + }); + + const client: SandboxManagedRuntimeClient = { + makeDir: async (remotePath) => { await mkdir(remotePath, { recursive: true }); }, + writeFile: async (remotePath, bytes) => { + await mkdir(path.dirname(remotePath), { recursive: true }); + await writeFile(remotePath, Buffer.from(bytes)); + }, + readFile: async (remotePath) => await readFile(remotePath), + listFiles: async (remotePath) => { + const entries = await readdir(remotePath, { withFileTypes: true }).catch(() => []); + return entries.filter((e) => e.isFile()).map((e) => e.name).sort(); + }, + remove: async (remotePath) => { await rm(remotePath, { recursive: true, force: true }); }, + run: async (command) => { await execFile("sh", ["-c", command], { maxBuffer: 32 * 1024 * 1024 }); }, + syncIn: async (operations) => { syncInOps.push(operations); return applyOperations(operations); }, + syncOut: async (operations) => { syncOutOps.push(operations); return applyOperations(operations); }, + }; + + return { client, syncInOps, syncOutOps }; +} + +describe("sandbox native file sync", () => { + const cleanupDirs: string[] = []; + afterEach(async () => { + while (cleanupDirs.length > 0) { + const dir = cleanupDirs.pop(); + if (dir) await rm(dir, { recursive: true, force: true }).catch(() => undefined); + } + }); + + it("prefers the native path for default-provision asset inbound and workspace outbound", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-native-sync-")); + cleanupDirs.push(rootDir); + const localWorkspaceDir = path.join(rootDir, "local-workspace"); + const remoteWorkspaceDir = path.join(rootDir, "remote-workspace"); + const localAssetsDir = path.join(rootDir, "local-assets"); + await mkdir(localWorkspaceDir, { recursive: true }); + await mkdir(localAssetsDir, { recursive: true }); + await writeFile(path.join(localWorkspaceDir, "README.md"), "local workspace\n", "utf8"); + await writeFile(path.join(localAssetsDir, "skill.md"), "skill body\n", "utf8"); + + const { client, syncInOps, syncOutOps } = makeNativeClient(); + const prepared = await prepareSandboxManagedRuntime({ + spec: { transport: "sandbox", provider: "test", sandboxId: "s1", remoteCwd: remoteWorkspaceDir, timeoutMs: 30_000, apiKey: null }, + adapterKey: "test-adapter", + client, + workspaceLocalDir: localWorkspaceDir, + assets: [{ key: "skills", localDir: localAssetsDir }], + }); + + // The default-provision asset was transferred through syncIn as a single + // directory mapping with an opaque operationId; the file landed in place. + const inboundOps = syncInOps.flat(); + expect(inboundOps.length).toBe(1); + const assetOp = inboundOps[0]; + expect(assetOp.operationId).toMatch(/^sync-op-\d+$/); + expect(assetOp.operationId).not.toContain("skills"); + expect(assetOp.files).toEqual([ + { sourcePath: localAssetsDir, targetPath: prepared.assetDirs.skills, kind: "directory", exclude: undefined, followSymlinks: undefined }, + ]); + expect(await readFile(path.join(prepared.assetDirs.skills, "skill.md"), "utf8")).toBe("skill body\n"); + + // Mutate the sandbox workspace, then restore through the native outbound path. + await writeFile(path.join(remoteWorkspaceDir, "README.md"), "remote workspace\n", "utf8"); + await writeFile(path.join(remoteWorkspaceDir, "new.txt"), "added\n", "utf8"); + await prepared.restoreWorkspace(); + + const outboundOps = syncOutOps.flat(); + expect(outboundOps.length).toBe(1); + expect(outboundOps[0].files[0]).toMatchObject({ sourcePath: remoteWorkspaceDir, kind: "directory" }); + expect(await readFile(path.join(localWorkspaceDir, "README.md"), "utf8")).toBe("remote workspace\n"); + expect(await readFile(path.join(localWorkspaceDir, "new.txt"), "utf8")).toBe("added\n"); + }); + + it("keeps a custom-provision asset on the tar fallback even when native sync is available", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-native-custom-")); + cleanupDirs.push(rootDir); + const localWorkspaceDir = path.join(rootDir, "local-workspace"); + const remoteWorkspaceDir = path.join(rootDir, "remote-workspace"); + const localAssetsDir = path.join(rootDir, "local-assets"); + await mkdir(localWorkspaceDir, { recursive: true }); + await mkdir(localAssetsDir, { recursive: true }); + await writeFile(path.join(localWorkspaceDir, "README.md"), "ws\n", "utf8"); + await writeFile(path.join(localAssetsDir, "cred.txt"), "secret\n", "utf8"); + + const { client, syncInOps } = makeNativeClient(); + const prepared = await prepareSandboxManagedRuntime({ + spec: { transport: "sandbox", provider: "test", sandboxId: "s1", remoteCwd: remoteWorkspaceDir, timeoutMs: 30_000, apiKey: null }, + adapterKey: "test-adapter", + client, + workspaceLocalDir: localWorkspaceDir, + assets: [{ + key: "creds", + localDir: localAssetsDir, + // A bespoke extract command (e.g. a credential merge) cannot be a generic + // file mapping, so the orchestrator keeps it on the tar path. + provision: { extractCommand: ({ assetTarPath, assetDir }) => + `rm -rf ${assetDir} && mkdir -p ${assetDir} && tar -xf ${assetTarPath} -C ${assetDir} && rm -f ${assetTarPath}` }, + }], + }); + + // No syncIn operation for the custom asset; it still materializes via tar. + expect(syncInOps.flat().length).toBe(0); + expect(await readFile(path.join(prepared.assetDirs.creds, "cred.txt"), "utf8")).toBe("secret\n"); + }); + + it("dereferences symlinks only when followSymlinks is true (native honors the flag)", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-native-symlink-")); + cleanupDirs.push(rootDir); + const localWorkspaceDir = path.join(rootDir, "local-workspace"); + const remoteWorkspaceDir = path.join(rootDir, "remote-workspace"); + const assetsPreserve = path.join(rootDir, "assets-preserve"); + const assetsDeref = path.join(rootDir, "assets-deref"); + const target = path.join(rootDir, "target.md"); + await mkdir(localWorkspaceDir, { recursive: true }); + await mkdir(assetsPreserve, { recursive: true }); + await mkdir(assetsDeref, { recursive: true }); + await writeFile(path.join(localWorkspaceDir, "README.md"), "ws\n", "utf8"); + await writeFile(target, "link body\n", "utf8"); + await symlink(target, path.join(assetsPreserve, "link.md")); + await symlink(target, path.join(assetsDeref, "link.md")); + + const { client } = makeNativeClient(); + const prepared = await prepareSandboxManagedRuntime({ + spec: { transport: "sandbox", provider: "test", sandboxId: "s1", remoteCwd: remoteWorkspaceDir, timeoutMs: 30_000, apiKey: null }, + adapterKey: "test-adapter", + client, + workspaceLocalDir: localWorkspaceDir, + assets: [ + { key: "preserve", localDir: assetsPreserve, followSymlinks: false }, + { key: "deref", localDir: assetsDeref, followSymlinks: true }, + ], + }); + + expect((await lstat(path.join(prepared.assetDirs.preserve, "link.md"))).isSymbolicLink()).toBe(true); + const dereffed = await lstat(path.join(prepared.assetDirs.deref, "link.md")); + expect(dereffed.isSymbolicLink()).toBe(false); + expect(await readFile(path.join(prepared.assetDirs.deref, "link.md"), "utf8")).toBe("link body\n"); + }); +}); + +describe("assertSyncOperationsConfined", () => { + const op = (targetPath: string, sourcePath = "/host/src"): SandboxSyncOperation[] => [ + { operationId: "sync-op-1", files: [{ sourcePath, targetPath, kind: "directory" }] }, + ]; + + it("accepts targets within an allowed root", () => { + expect(() => assertSyncOperationsConfined(op("/remote/ws/sub"), { + sourceRoots: ["/host/src"], targetRoots: ["/remote/ws"], + })).not.toThrow(); + }); + + it("rejects a relative target", () => { + expect(() => assertSyncOperationsConfined(op("relative/path"), { + sourceRoots: ["/host/src"], targetRoots: ["/remote/ws"], + })).toThrow(/confined absolute path/); + }); + + it("rejects a parent-traversal escape", () => { + expect(() => assertSyncOperationsConfined(op("/remote/ws/../etc/passwd"), { + sourceRoots: ["/host/src"], targetRoots: ["/remote/ws"], + })).toThrow(/confined absolute path|escapes its confinement root/); + }); + + it("rejects an absolute target outside every root", () => { + expect(() => assertSyncOperationsConfined(op("/etc/passwd"), { + sourceRoots: ["/host/src"], targetRoots: ["/remote/ws"], + })).toThrow(/escapes its confinement root/); + }); + + it("rejects a source outside every source root", () => { + expect(() => assertSyncOperationsConfined(op("/remote/ws/ok", "/etc/shadow"), { + sourceRoots: ["/host/src"], targetRoots: ["/remote/ws"], + })).toThrow(/escapes its confinement root/); + }); +}); diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.ts b/packages/adapter-utils/src/sandbox-managed-runtime.ts index ccce1d13120..62bf2cf7d77 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.ts @@ -121,6 +121,37 @@ export interface SandboxTransferProgressOptions { onProgress?: (transferredBytes: number, totalBytes: number | null) => void | Promise; } +/** + * A single sourceβ†’target file or directory transfer within a sync operation. + * Mirrors the plugin SDK `PluginSyncFileMapping`; kept as a local structural + * type so `adapter-utils` does not depend on the plugin SDK. For `syncIn`, + * `sourcePath` is a host path and `targetPath` a sandbox path; for `syncOut` the + * direction is reversed. Sandbox paths are POSIX. + */ +export interface SandboxSyncFileMapping { + sourcePath: string; + targetPath: string; + kind: "file" | "directory"; + mode?: number; + exclude?: string[]; + followSymlinks?: boolean; +} + +/** + * An ordered, opaque unit of work handed to the native sync transport. The + * `operationId` is an opaque, non-sensitive token authored by the orchestrator + * (never a caller/asset identifier that could leak intent); a provider MUST NOT + * interpret it. + */ +export interface SandboxSyncOperation { + operationId: string; + files: SandboxSyncFileMapping[]; +} + +export interface SandboxSyncResult { + operations: { operationId: string; filesTransferred: number; bytesTransferred: number }[]; +} + export interface SandboxManagedRuntimeClient { makeDir(remotePath: string): Promise; writeFile(remotePath: string, bytes: ArrayBuffer, options?: SandboxTransferProgressOptions): Promise; @@ -131,6 +162,49 @@ export interface SandboxManagedRuntimeClient { listFiles(remotePath: string): Promise; remove(remotePath: string): Promise; run(command: string, options: { timeoutMs: number }): Promise; + /** + * Optional native inbound transfer. Present only when the sandbox provider + * advertises both `environmentSyncIn` and `environmentSyncOut`; otherwise the + * orchestrator falls back to the tar + base64 `writeFile`/`run` path so + * behavior is byte-identical to a provider that never opted in. + */ + syncIn?(operations: SandboxSyncOperation[]): Promise; + /** Optional native outbound transfer. See {@link syncIn}. */ + syncOut?(operations: SandboxSyncOperation[]): Promise; +} + +/** + * Host-side complete-mediation guard for native sync operations. The orchestrator + * authors every `targetPath`, but the native transport crosses the host↔sandbox + * trust boundary, so we canonicalize and confine each mapping's source and target + * to an orchestrator-owned root before handing the operation to a provider. + * Absolute escapes and `..` traversal are rejected fail-closed. Sandbox and host + * paths on the server are POSIX. + */ +export function assertSyncOperationsConfined( + operations: SandboxSyncOperation[], + roots: { sourceRoots: string[]; targetRoots: string[] }, +): void { + const confine = (candidate: string, allowed: string[], label: string): void => { + const normalized = path.posix.normalize(candidate); + if (!path.posix.isAbsolute(normalized) || normalized === ".." || normalized.includes("/../") || normalized.endsWith("/..")) { + throw new Error(`sync operation ${label} path is not a confined absolute path: ${candidate}`); + } + const within = allowed.some((root) => { + const normalizedRoot = path.posix.normalize(root); + const prefix = normalizedRoot.endsWith("/") ? normalizedRoot : `${normalizedRoot}/`; + return normalized === normalizedRoot || normalized.startsWith(prefix); + }); + if (!within) { + throw new Error(`sync operation ${label} path escapes its confinement root: ${candidate}`); + } + }; + for (const operation of operations) { + for (const mapping of operation.files) { + confine(mapping.sourcePath, roots.sourceRoots, "source"); + confine(mapping.targetPath, roots.targetRoots, "target"); + } + } } export interface PreparedSandboxManagedRuntime { @@ -517,6 +591,15 @@ export async function prepareSandboxManagedRuntime(input: { exclude: restoreExclude, }); + // Prefer the provider's native file transport when it advertised the sync + // verbs; otherwise every branch below falls back to the byte-identical tar + + // base64 `writeFile`/`run` path. `nextSyncOperationId` emits opaque, ordered, + // non-sensitive tokens β€” never a caller/asset identifier. + const nativeSyncIn = typeof input.client.syncIn === "function"; + const nativeSyncOut = typeof input.client.syncOut === "function"; + let syncOperationSeq = 0; + const nextSyncOperationId = () => `sync-op-${++syncOperationSeq}`; + await withTempDir("paperclip-sandbox-sync-", async (tempDir) => { const preservedNames = new Set([ ".paperclip-runtime", @@ -529,6 +612,12 @@ export async function prepareSandboxManagedRuntime(input: { localDir: input.workspaceLocalDir, snapshot: gitSnapshot, }, async (cloneDir) => { + // The git-workspace and workspace transfers preserve `.paperclip-runtime` + // on the target (and the git overlay merges on top rather than replacing), + // which the generic destroy-then-replace file mapping cannot express, so + // they always take the tar path. Native transfer is used for the clean + // destroy-then-replace cases (default-provision assets inbound; the + // workspace download into a fresh host dir outbound). const gitTarPath = path.join(tempDir, "git-workspace.tar"); await createTarballFromDirectory({ localDir: cloneDir, @@ -614,6 +703,41 @@ export async function prepareSandboxManagedRuntime(input: { for (const asset of input.assets ?? []) { await emitRuntimeStatus(input.onRuntimeProgress, "config_sync", "Syncing runtime assets to sandbox"); + const remoteAssetDir = path.posix.join(runtimeRootDir, asset.key); + // Assets with custom provisioning (staged helper files or a bespoke extract + // command such as a credential merge) do more than a plain directory + // replacement, so they cannot be expressed as a generic file mapping and + // always take the tar path. A default-provisioned asset is a clean + // destroy-then-replace of its own directory, which the native transport + // reproduces exactly. + const usesCustomProvision = + Boolean(asset.provision?.extractCommand) || (asset.provision?.stageFiles?.length ?? 0) > 0; + if (nativeSyncIn && !usesCustomProvision) { + const operations: SandboxSyncOperation[] = [{ + operationId: nextSyncOperationId(), + files: [{ + sourcePath: asset.localDir, + targetPath: remoteAssetDir, + kind: "directory", + exclude: asset.exclude, + followSymlinks: asset.followSymlinks, + }], + }]; + assertSyncOperationsConfined(operations, { + sourceRoots: [asset.localDir], + targetRoots: [runtimeRootDir], + }); + const assetUpload = makeTransferProgress( + input.onProgress, + "Syncing", + "to", + asset.key, + { sink: input.onRuntimeProgress, phase: "config_sync" }, + ); + await input.client.syncIn!(operations); + await assetUpload.finish(0, 0); + continue; + } const assetTarPath = path.join(tempDir, `${asset.key}.tar`); await createTarballFromDirectory({ localDir: asset.localDir, @@ -622,7 +746,6 @@ export async function prepareSandboxManagedRuntime(input: { exclude: asset.exclude, }); const assetTarBytes = await fs.readFile(assetTarPath); - const remoteAssetDir = path.posix.join(runtimeRootDir, asset.key); const remoteAssetTar = path.posix.join(runtimeRootDir, `${asset.key}-upload.tar`); const assetUpload = makeTransferProgress( input.onProgress, @@ -718,34 +841,65 @@ export async function prepareSandboxManagedRuntime(input: { }); } - const remoteWorkspaceTar = path.posix.join(runtimeRootDir, "workspace-download.tar"); await emitRuntimeStatus(input.onRuntimeProgress, "restore", "Restoring workspace from sandbox"); - await input.client.run( - `sh -c ${shellQuote(createRemoteTarballFromDirectoryCommand({ - remoteDir: workspaceRemoteDir, - archivePath: remoteWorkspaceTar, - exclude: restoreExclude, - }))}`, - { timeoutMs: input.spec.timeoutMs }, - ); - const workspaceRestore = makeTransferProgress( - restoreSink, - "Restoring", - "from", - "workspace", - { sink: input.onRuntimeProgress, phase: "restore" }, - ); - const archiveBytes = await input.client.readFile(remoteWorkspaceTar, workspaceRestore.options); - const archiveBuffer = toBuffer(archiveBytes); - await workspaceRestore.finish(archiveBuffer.byteLength, archiveBuffer.byteLength); - await input.client.remove(remoteWorkspaceTar).catch(() => undefined); - const localArchivePath = path.join(tempDir, "workspace.tar"); const extractedDir = path.join(tempDir, "workspace"); - await fs.writeFile(localArchivePath, archiveBuffer); - await extractTarballToDirectory({ - archivePath: localArchivePath, - localDir: extractedDir, - }); + if (nativeSyncOut) { + // Native outbound: the provider materializes the sandbox workspace into + // a fresh host directory. It is a clean destroy-then-replace into a + // temp dir the orchestrator just created, so it maps exactly to a + // generic directory file mapping; the host-side baseline merge below is + // unchanged. + const operations: SandboxSyncOperation[] = [{ + operationId: nextSyncOperationId(), + files: [{ + sourcePath: workspaceRemoteDir, + targetPath: extractedDir, + kind: "directory", + exclude: restoreExclude, + }], + }]; + assertSyncOperationsConfined(operations, { + sourceRoots: [workspaceRemoteDir], + targetRoots: [extractedDir], + }); + await fs.mkdir(extractedDir, { recursive: true }); + const workspaceRestore = makeTransferProgress( + restoreSink, + "Restoring", + "from", + "workspace", + { sink: input.onRuntimeProgress, phase: "restore" }, + ); + await input.client.syncOut!(operations); + await workspaceRestore.finish(0, 0); + } else { + const remoteWorkspaceTar = path.posix.join(runtimeRootDir, "workspace-download.tar"); + await input.client.run( + `sh -c ${shellQuote(createRemoteTarballFromDirectoryCommand({ + remoteDir: workspaceRemoteDir, + archivePath: remoteWorkspaceTar, + exclude: restoreExclude, + }))}`, + { timeoutMs: input.spec.timeoutMs }, + ); + const workspaceRestore = makeTransferProgress( + restoreSink, + "Restoring", + "from", + "workspace", + { sink: input.onRuntimeProgress, phase: "restore" }, + ); + const archiveBytes = await input.client.readFile(remoteWorkspaceTar, workspaceRestore.options); + const archiveBuffer = toBuffer(archiveBytes); + await workspaceRestore.finish(archiveBuffer.byteLength, archiveBuffer.byteLength); + await input.client.remove(remoteWorkspaceTar).catch(() => undefined); + const localArchivePath = path.join(tempDir, "workspace.tar"); + await fs.writeFile(localArchivePath, archiveBuffer); + await extractTarballToDirectory({ + archivePath: localArchivePath, + localDir: extractedDir, + }); + } const gitHeadToIntegrate = importedHead; await mergeDirectoryWithBaseline({ baseline: baselineSnapshot, diff --git a/packages/plugins/sdk/src/define-plugin.ts b/packages/plugins/sdk/src/define-plugin.ts index 668551633b4..3c894672e5f 100644 --- a/packages/plugins/sdk/src/define-plugin.ts +++ b/packages/plugins/sdk/src/define-plugin.ts @@ -55,6 +55,9 @@ import type { PluginEnvironmentDestroyLeaseParams, PluginEnvironmentExecuteParams, PluginEnvironmentExecuteResult, + PluginEnvironmentSyncInParams, + PluginEnvironmentSyncOutParams, + PluginEnvironmentSyncResult, PluginEnvironmentStartInteractiveSetupParams, PluginEnvironmentInteractiveSetupSession, PluginEnvironmentGetInteractiveSetupParams, @@ -336,6 +339,27 @@ export interface PluginDefinition { params: PluginEnvironmentExecuteParams, ): Promise; + /** + * Optional, opt-in: called before execution to place host files/directories at + * target sandbox paths using a provider-native transport instead of the default + * base64-over-exec fallback. Defining this hook (together with + * `onEnvironmentSyncOut`) advertises `environmentSyncIn`; leaving it undefined + * keeps the byte-identical fallback. See `doc/plugins/SANDBOX_FILE_SYNC_HOOKS.md`. + */ + onEnvironmentSyncIn?( + params: PluginEnvironmentSyncInParams, + ): Promise; + + /** + * Optional, opt-in: called after execution to copy sandbox files/directories + * back to target host paths using a provider-native transport. Defining this + * hook (together with `onEnvironmentSyncIn`) advertises `environmentSyncOut`. + * See `doc/plugins/SANDBOX_FILE_SYNC_HOOKS.md`. + */ + onEnvironmentSyncOut?( + params: PluginEnvironmentSyncOutParams, + ): Promise; + /** Called to start an interactive setup sandbox and return redacted connection metadata. */ onEnvironmentStartInteractiveSetup?( params: PluginEnvironmentStartInteractiveSetupParams, diff --git a/packages/plugins/sdk/src/index.ts b/packages/plugins/sdk/src/index.ts index d32b7820b8b..1e6d3260e01 100644 --- a/packages/plugins/sdk/src/index.ts +++ b/packages/plugins/sdk/src/index.ts @@ -181,6 +181,11 @@ export type { PluginEnvironmentRealizeWorkspaceResult, PluginEnvironmentExecuteParams, PluginEnvironmentExecuteResult, + PluginSyncFileMapping, + PluginSyncOperation, + PluginEnvironmentSyncInParams, + PluginEnvironmentSyncOutParams, + PluginEnvironmentSyncResult, PluginEnvironmentInteractiveSetupStatus, PluginEnvironmentInteractiveSetupConnectionType, PluginEnvironmentTemplateRefKind, diff --git a/packages/plugins/sdk/src/protocol.ts b/packages/plugins/sdk/src/protocol.ts index ae3cbd86b59..8bdb2914b3e 100644 --- a/packages/plugins/sdk/src/protocol.ts +++ b/packages/plugins/sdk/src/protocol.ts @@ -650,6 +650,66 @@ export interface PluginEnvironmentExecuteResult { metadata?: Record; } +/** + * A single sourceβ†’target file or directory transfer within a sync operation. + * + * For `environmentSyncIn`, `sourcePath` is a host path and `targetPath` is a + * sandbox path; for `environmentSyncOut` the direction is reversed. All sandbox + * paths are POSIX. The contract is provider-agnostic: a provider may transfer a + * directory by whatever native mechanism it prefers (bulk upload, internal tar, + * per-file enumeration) as long as the observable result matches this mapping. + */ +export interface PluginSyncFileMapping { + /** Absolute path of the transfer source (host for syncIn, sandbox for syncOut). */ + sourcePath: string; + /** Absolute path of the transfer target (sandbox for syncIn, host for syncOut). */ + targetPath: string; + /** Whether the mapping transfers a single regular file or a directory tree. */ + kind: "file" | "directory"; + /** + * POSIX file mode to apply at the target (e.g. `0o600` for secret material). + * When set, providers MUST create the target with this mode with no + * world-readable window (create-with-mode or chmod-before-bytes, never after). + */ + mode?: number; + /** Glob patterns to exclude when `kind` is `"directory"`. */ + exclude?: string[]; + /** + * Symlink handling for `kind: "directory"` transfers. Falsy preserves symlinks + * as links; `true` dereferences them to their target bytes. Mirrors tar's `-h`. + */ + followSymlinks?: boolean; +} + +/** + * An ordered, opaque unit of work handed to a sync hook. The `operationId` is an + * opaque, non-sensitive token authored by the orchestrator; a provider MUST NOT + * interpret it. Operations are applied in array order. + */ +export interface PluginSyncOperation { + operationId: string; + files: PluginSyncFileMapping[]; +} + +export interface PluginEnvironmentSyncInParams extends PluginEnvironmentDriverBaseParams { + lease: PluginEnvironmentLease; + operations: PluginSyncOperation[]; +} + +export interface PluginEnvironmentSyncOutParams extends PluginEnvironmentDriverBaseParams { + lease: PluginEnvironmentLease; + operations: PluginSyncOperation[]; +} + +/** Per-operation transfer accounting returned by a sync hook, for observability. */ +export interface PluginEnvironmentSyncResult { + operations: { + operationId: string; + filesTransferred: number; + bytesTransferred: number; + }[]; +} + export type PluginEnvironmentInteractiveSetupStatus = | "starting" | "waiting_for_user" @@ -864,6 +924,14 @@ export interface HostToWorkerMethods { params: PluginEnvironmentExecuteParams, result: PluginEnvironmentExecuteResult, ]; + environmentSyncIn: [ + params: PluginEnvironmentSyncInParams, + result: PluginEnvironmentSyncResult, + ]; + environmentSyncOut: [ + params: PluginEnvironmentSyncOutParams, + result: PluginEnvironmentSyncResult, + ]; environmentStartInteractiveSetup: [ params: PluginEnvironmentStartInteractiveSetupParams, result: PluginEnvironmentInteractiveSetupSession, @@ -918,6 +986,8 @@ export const HOST_TO_WORKER_OPTIONAL_METHODS: readonly HostToWorkerMethodName[] "environmentDestroyLease", "environmentRealizeWorkspace", "environmentExecute", + "environmentSyncIn", + "environmentSyncOut", "environmentStartInteractiveSetup", "environmentGetInteractiveSetup", "environmentCaptureTemplate", diff --git a/packages/plugins/sdk/src/worker-rpc-host.ts b/packages/plugins/sdk/src/worker-rpc-host.ts index 730fbb47423..4c53eb92269 100644 --- a/packages/plugins/sdk/src/worker-rpc-host.ts +++ b/packages/plugins/sdk/src/worker-rpc-host.ts @@ -88,6 +88,8 @@ import type { PluginEnvironmentAcquireLeaseParams, PluginEnvironmentDestroyLeaseParams, PluginEnvironmentExecuteParams, + PluginEnvironmentSyncInParams, + PluginEnvironmentSyncOutParams, PluginEnvironmentRealizeWorkspaceParams, PluginEnvironmentReleaseLeaseParams, PluginEnvironmentResumeLeaseParams, @@ -1394,6 +1396,12 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost case "environmentExecute": return handleEnvironmentExecute(params as PluginEnvironmentExecuteParams); + case "environmentSyncIn": + return handleEnvironmentSyncIn(params as PluginEnvironmentSyncInParams); + + case "environmentSyncOut": + return handleEnvironmentSyncOut(params as PluginEnvironmentSyncOutParams); + case "environmentStartInteractiveSetup": return handleEnvironmentStartInteractiveSetup(params as PluginEnvironmentStartInteractiveSetupParams); @@ -1453,6 +1461,8 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost if (plugin.definition.onEnvironmentDestroyLease) supportedMethods.push("environmentDestroyLease"); if (plugin.definition.onEnvironmentRealizeWorkspace) supportedMethods.push("environmentRealizeWorkspace"); if (plugin.definition.onEnvironmentExecute) supportedMethods.push("environmentExecute"); + if (plugin.definition.onEnvironmentSyncIn) supportedMethods.push("environmentSyncIn"); + if (plugin.definition.onEnvironmentSyncOut) supportedMethods.push("environmentSyncOut"); if (plugin.definition.onEnvironmentStartInteractiveSetup) supportedMethods.push("environmentStartInteractiveSetup"); if (plugin.definition.onEnvironmentGetInteractiveSetup) supportedMethods.push("environmentGetInteractiveSetup"); if (plugin.definition.onEnvironmentCaptureTemplate) supportedMethods.push("environmentCaptureTemplate"); @@ -1715,6 +1725,20 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost return plugin.definition.onEnvironmentExecute(params); } + async function handleEnvironmentSyncIn(params: PluginEnvironmentSyncInParams) { + if (!plugin.definition.onEnvironmentSyncIn) { + throw methodNotImplemented("environmentSyncIn"); + } + return plugin.definition.onEnvironmentSyncIn(params); + } + + async function handleEnvironmentSyncOut(params: PluginEnvironmentSyncOutParams) { + if (!plugin.definition.onEnvironmentSyncOut) { + throw methodNotImplemented("environmentSyncOut"); + } + return plugin.definition.onEnvironmentSyncOut(params); + } + async function handleEnvironmentStartInteractiveSetup(params: PluginEnvironmentStartInteractiveSetupParams) { if (!plugin.definition.onEnvironmentStartInteractiveSetup) { throw methodNotImplemented("environmentStartInteractiveSetup"); diff --git a/packages/plugins/sdk/tests/environment-sync-negotiation.test.ts b/packages/plugins/sdk/tests/environment-sync-negotiation.test.ts new file mode 100644 index 00000000000..512f60cfd48 --- /dev/null +++ b/packages/plugins/sdk/tests/environment-sync-negotiation.test.ts @@ -0,0 +1,191 @@ +import { createInterface } from "node:readline"; +import { PassThrough } from "node:stream"; + +import { describe, expect, it } from "vitest"; + +import { definePlugin } from "../src/define-plugin.js"; +import { + createRequest, + isJsonRpcResponse, + parseMessage, + PLUGIN_RPC_ERROR_CODES, + serializeMessage, + type JsonRpcResponse, + type PluginEnvironmentSyncInParams, + type PluginEnvironmentSyncOutParams, + type PluginEnvironmentSyncResult, +} from "../src/protocol.js"; +import { startWorkerRpcHost } from "../src/worker-rpc-host.js"; + +const MANIFEST = { + id: "paperclip.sync-negotiation-test", + apiVersion: 1, + version: "1.0.0", + displayName: "Sync Negotiation Test", + description: "Test plugin", + author: "Paperclip", + categories: ["automation"], + capabilities: [], + entrypoints: {}, +} as const; + +function startTestWorker(plugin: ReturnType) { + const hostToWorker = new PassThrough(); + const workerToHost = new PassThrough(); + const hostReadline = createInterface({ input: workerToHost }); + const pending = new Map void>(); + let nextRequestId = 1; + + hostReadline.on("line", (line) => { + const message = parseMessage(line); + if (!isJsonRpcResponse(message)) return; + pending.get(String(message.id))?.(message); + pending.delete(String(message.id)); + }); + + const worker = startWorkerRpcHost({ plugin, stdin: hostToWorker, stdout: workerToHost }); + + function callWorker(method: string, params: unknown): Promise { + const id = `host-${nextRequestId++}`; + const result = new Promise((resolve, reject) => { + pending.set(id, (response) => { + if ("error" in response && response.error) { + reject(Object.assign(new Error(response.error.message), { code: response.error.code })); + return; + } + resolve((response as { result?: T }).result as T); + }); + }); + hostToWorker.write(serializeMessage(createRequest(method, params, id))); + return result; + } + + function stop() { + worker.stop(); + hostReadline.close(); + hostToWorker.destroy(); + workerToHost.destroy(); + } + + return { callWorker, stop }; +} + +describe("environment sync verb negotiation", () => { + it("advertises environmentSyncIn/environmentSyncOut only when the hooks are defined", async () => { + const withHooks = startTestWorker( + definePlugin({ + async setup() {}, + async onEnvironmentSyncIn(): Promise { + return { operations: [] }; + }, + async onEnvironmentSyncOut(): Promise { + return { operations: [] }; + }, + }), + ); + try { + const result = await withHooks.callWorker<{ ok: boolean; supportedMethods: string[] }>( + "initialize", + { manifest: MANIFEST, config: {}, databaseNamespace: null }, + ); + expect(result.supportedMethods).toContain("environmentSyncIn"); + expect(result.supportedMethods).toContain("environmentSyncOut"); + } finally { + withHooks.stop(); + } + + const withoutHooks = startTestWorker(definePlugin({ async setup() {} })); + try { + const result = await withoutHooks.callWorker<{ ok: boolean; supportedMethods: string[] }>( + "initialize", + { manifest: MANIFEST, config: {}, databaseNamespace: null }, + ); + expect(result.supportedMethods).not.toContain("environmentSyncIn"); + expect(result.supportedMethods).not.toContain("environmentSyncOut"); + } finally { + withoutHooks.stop(); + } + }); + + it("routes environmentSyncIn/environmentSyncOut to the hooks when defined", async () => { + const seen: string[] = []; + const worker = startTestWorker( + definePlugin({ + async setup() {}, + async onEnvironmentSyncIn(params): Promise { + seen.push("in"); + return { + operations: params.operations.map((op) => ({ + operationId: op.operationId, + filesTransferred: op.files.length, + bytesTransferred: 0, + })), + }; + }, + async onEnvironmentSyncOut(params): Promise { + seen.push("out"); + return { + operations: params.operations.map((op) => ({ + operationId: op.operationId, + filesTransferred: op.files.length, + bytesTransferred: 0, + })), + }; + }, + }), + ); + try { + await worker.callWorker("initialize", { manifest: MANIFEST, config: {}, databaseNamespace: null }); + const baseParams = { + driverKey: "sandbox", + companyId: "company", + environmentId: "env", + config: {}, + lease: { providerLeaseId: "lease-1" }, + }; + const inParams: PluginEnvironmentSyncInParams = { + ...baseParams, + operations: [ + { operationId: "op-a", files: [{ sourcePath: "/host/a", targetPath: "/remote/a", kind: "file" }] }, + ], + }; + const inResult = await worker.callWorker("environmentSyncIn", inParams); + expect(inResult.operations[0]).toMatchObject({ operationId: "op-a", filesTransferred: 1 }); + + const outParams: PluginEnvironmentSyncOutParams = { + ...baseParams, + operations: [ + { operationId: "op-b", files: [{ sourcePath: "/remote/b", targetPath: "/host/b", kind: "directory" }] }, + ], + }; + const outResult = await worker.callWorker("environmentSyncOut", outParams); + expect(outResult.operations[0]).toMatchObject({ operationId: "op-b", filesTransferred: 1 }); + expect(seen).toEqual(["in", "out"]); + } finally { + worker.stop(); + } + }); + + it("throws METHOD_NOT_IMPLEMENTED when the sync hooks are absent", async () => { + const worker = startTestWorker(definePlugin({ async setup() {} })); + try { + await worker.callWorker("initialize", { manifest: MANIFEST, config: {}, databaseNamespace: null }); + const params = { + driverKey: "sandbox", + companyId: "company", + environmentId: "env", + config: {}, + lease: { providerLeaseId: "lease-1" }, + operations: [], + }; + await expect(worker.callWorker("environmentSyncIn", params)).rejects.toMatchObject({ + code: PLUGIN_RPC_ERROR_CODES.METHOD_NOT_IMPLEMENTED, + }); + await expect(worker.callWorker("environmentSyncOut", params)).rejects.toMatchObject({ + code: PLUGIN_RPC_ERROR_CODES.METHOD_NOT_IMPLEMENTED, + }); + } finally { + worker.stop(); + } + }); +}); diff --git a/server/src/services/environment-execution-target.ts b/server/src/services/environment-execution-target.ts index 079b3563b75..bbd86d8ce5a 100644 --- a/server/src/services/environment-execution-target.ts +++ b/server/src/services/environment-execution-target.ts @@ -103,6 +103,28 @@ export async function resolveEnvironmentExecutionTarget(input: { startedAt, }; }, + // Expose the native file-sync capability only when the provider's + // worker advertises BOTH sync verbs; otherwise leave syncIn/syncOut + // undefined so the orchestrator keeps the byte-identical base64 path. + ...(input.environmentRuntime.supportsSync({ + environment: input.environment as Environment, + lease: input.lease, + }) + ? { + syncIn: (operations) => + input.environmentRuntime!.syncIn({ + environment: input.environment as Environment, + lease: input.lease!, + operations, + }), + syncOut: (operations) => + input.environmentRuntime!.syncOut({ + environment: input.environment as Environment, + lease: input.lease!, + operations, + }), + } + : {}), } : undefined, }; diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index 151b181a7c6..765c354fc94 100644 --- a/server/src/services/environment-runtime.ts +++ b/server/src/services/environment-runtime.ts @@ -14,6 +14,8 @@ import type { PluginEnvironmentExecuteResult, PluginEnvironmentLease, PluginEnvironmentRealizeWorkspaceResult, + PluginEnvironmentSyncResult, + PluginSyncOperation, } from "@paperclipai/plugin-sdk"; import { ensureSshWorkspaceReady } from "@paperclipai/adapter-utils/ssh"; import { environmentService } from "./environments.js"; @@ -185,6 +187,10 @@ export interface EnvironmentDriverExecuteInput extends EnvironmentDriverLeaseInp timeoutMs?: number; } +export interface EnvironmentDriverSyncInput extends EnvironmentDriverLeaseInput { + operations: PluginSyncOperation[]; +} + export interface EnvironmentRuntimeDriver { readonly driver: string; acquireRunLease(input: EnvironmentDriverAcquireInput): Promise; @@ -193,6 +199,16 @@ export interface EnvironmentRuntimeDriver { destroyRunLease?(input: EnvironmentDriverLeaseInput): Promise; realizeWorkspace?(input: EnvironmentDriverRealizeWorkspaceInput): Promise; execute?(input: EnvironmentDriverExecuteInput): Promise; + /** + * Optional native inbound/outbound file transfer, delegated to the plugin + * worker's `environmentSyncIn`/`environmentSyncOut` verbs. Only present for + * plugin-backed sandbox drivers whose worker advertises both verbs; callers + * gate on {@link EnvironmentRuntimeDriver.supportsSync}. + */ + syncIn?(input: EnvironmentDriverSyncInput): Promise; + syncOut?(input: EnvironmentDriverSyncInput): Promise; + /** True when the lease's plugin worker advertises both sync verbs. */ + supportsSync?(input: EnvironmentDriverLeaseInput): boolean; } export interface EnvironmentRuntimeLeaseRecord { @@ -722,6 +738,39 @@ function createSandboxEnvironmentDriver( } } + async function callPluginEnvironmentSync( + method: "environmentSyncIn" | "environmentSyncOut", + input: EnvironmentDriverSyncInput, + ): Promise { + if (!input.lease.metadata?.sandboxProviderPlugin || !pluginWorkerManager) { + throw new Error("Sandbox driver does not support native file sync for this lease."); + } + const pluginId = readString(input.lease.metadata?.pluginId); + const providerKey = readString(input.lease.metadata?.provider); + if (!pluginId || !providerKey) { + throw new Error("Sandbox lease is missing plugin/provider metadata for native file sync."); + } + const config = await resolvePluginSandboxRuntimeConfig({ + environment: input.environment, + lease: input.lease, + provider: providerKey, + }); + const sanitizedConfig = stripSandboxProviderEnvelope(config as SandboxEnvironmentConfig); + return await pluginWorkerManager.call(pluginId, method, { + driverKey: providerKey, + companyId: input.lease.companyId, + environmentId: input.environment.id, + issueId: input.lease.issueId, + config: sanitizedConfig, + lease: { + providerLeaseId: input.lease.providerLeaseId, + metadata: input.lease.metadata ?? undefined, + expiresAt: input.lease.expiresAt?.toISOString() ?? null, + }, + operations: input.operations, + }, resolvePluginSandboxRpcTimeoutMs(sanitizedConfig)); + } + return { driver: "sandbox", @@ -1235,6 +1284,22 @@ function createSandboxEnvironmentDriver( throw new Error("Sandbox driver does not support direct command execution for built-in providers."); }, + supportsSync(input) { + if (!input.lease.metadata?.sandboxProviderPlugin || !pluginWorkerManager) return false; + const pluginId = readString(input.lease.metadata?.pluginId); + if (!pluginId) return false; + const advertised = pluginWorkerManager.getWorker(pluginId)?.supportedMethods ?? []; + return advertised.includes("environmentSyncIn") && advertised.includes("environmentSyncOut"); + }, + + async syncIn(input) { + return await callPluginEnvironmentSync("environmentSyncIn", input); + }, + + async syncOut(input) { + return await callPluginEnvironmentSync("environmentSyncOut", input); + }, + async destroyRunLease(input) { return await destroyReusableSandboxLease({ environment: input.environment, @@ -1890,6 +1955,27 @@ export function environmentRuntimeService( } return await driver.execute(input); }, + + supportsSync(input: EnvironmentDriverLeaseInput): boolean { + const driver = getDriver(getLeaseDriverKey(input.lease, input.environment)); + return driver?.supportsSync?.(input) ?? false; + }, + + async syncIn(input: EnvironmentDriverSyncInput): Promise { + const driver = requireDriverKey(getLeaseDriverKey(input.lease, input.environment)); + if (!driver.syncIn) { + throw new Error(`Environment driver "${driver.driver}" does not support native file sync.`); + } + return await driver.syncIn(input); + }, + + async syncOut(input: EnvironmentDriverSyncInput): Promise { + const driver = requireDriverKey(getLeaseDriverKey(input.lease, input.environment)); + if (!driver.syncOut) { + throw new Error(`Environment driver "${driver.driver}" does not support native file sync.`); + } + return await driver.syncOut(input); + }, }; } From e1e35881afc1bd9d2b0316fdde54dd2139e4ed3f Mon Sep 17 00:00:00 2001 From: legacykeeperops Date: Wed, 22 Jul 2026 12:05:28 -0600 Subject: [PATCH 5/7] feat(cost-events): propagate issue.billing_code at heartbeat record time (#6821) 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 > - Agent runs report progress through the heartbeat service, which writes the cost ledger (`cost_events`) as usage accrues > - `cost_events` already has a `billing_code` column, but nothing populates it β€” the heartbeat writes `issueId`/`projectId` and leaves `billing_code` NULL > - Issues carry a `billing_code`, so the attribution data sits one join away but never reaches the ledger rows > - Reporting therefore has to reconstruct attribution by joining back to `issues` at query time, which reflects the issue's *current* billing code rather than the one in effect when the cost was incurred > - This pull request threads `billingCode` through `resolveLedgerScopeForRun` so the heartbeat stamps it onto each `cost_events` row at record time > - The benefit is that attribution is captured at write time and stays correct if an issue's billing code later changes ## Linked Issues or Issue Description No existing public GitHub issue. Describing the problem in-PR: **Problem.** `cost_events` has a `billing_code` column that is never written. The heartbeat's cost-ledger insert records `issueId` and `projectId` but not the billing code of the issue the run belongs to, so every row lands with `billing_code` NULL. **Impact.** Cost-per-billing-code reporting has to derive attribution by joining `cost_events` back to `issues` at query time. That join returns the issue's billing code *as of the query*, not as of when the cost was incurred, so historical cost reports shift retroactively whenever an issue is re-coded. **Desired behaviour.** The billing code in effect at record time is stored on the `cost_events` row itself. **Related PRs.** #6820 β€” same change to the same file by the same author, opened separately. These are duplicates; only one should land. ## What Changed - `resolveLedgerScopeForRun` now selects `issues.billingCode` alongside `id` and `projectId`. - The scope object it returns gained a `billingCode` field, populated with `issue?.billingCode ?? null`. - The early-return path for runs with no issue in context returns `billingCode: null`. - The `costs.createEvent` call in `heartbeatService` passes `billingCode: ledgerScope.billingCode` alongside `issueId`/`projectId`. No schema migration: `cost_events.billing_code` already exists. ## Verification **No automated test accompanies this change.** There is currently no test asserting that a `cost_events` row carries the issue's billing code when an issue is in scope, or `null` when there is not. A reviewer should treat the checks below as manual verification only. Manual verification against a running instance: ```sql -- Non-NULL billing_code for recent runs on billed issues SELECT billing_code, COUNT(*) FROM cost_events WHERE created_at > NOW() - INTERVAL '1 hour' GROUP BY billing_code; -- Cost attribution query this change is intended to enable SELECT billing_code, SUM(cost_cents) FROM cost_events GROUP BY billing_code; ``` Expected: rows for runs attached to an issue with a billing code now carry that code; runs with no issue in context remain NULL. ## Risks Low risk in blast radius, with two things worth a reviewer's attention: - **Behavioural shift for consumers.** `cost_events.billing_code` was uniformly NULL and now starts arriving populated. Anything downstream that groups, filters, or dedupes on that column will see new values and new cardinality. Existing rows are not backfilled, so the column is mixed NULL/non-NULL across the historical boundary. - **No test coverage.** The null-fallback behaviour on both paths is asserted only by reading the code, not by a test. - **Migration safety:** not applicable β€” no schema change; the column already exists. - **Failure mode:** if `billingCode` were absent from the `issues` selection the value would silently be `undefined` rather than erroring, so the field is worth confirming in review. ## Model Used **TODO (author):** this section is required and cannot be completed on your behalf. Please state the provider and model name, the exact model ID/version, and the reasoning/thinking mode used β€” or "None β€” human-authored" if no AI model was involved. Per the template, the "Generated with Claude Code" footer is not a substitute for this section. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [ ] I have specified the model used (with version and capability details) β€” **pending author input, see above** - [ ] 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 β€” #6820 is a duplicate of this PR - [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 - [ ] I have run tests locally and they pass - [ ] I have added or updated tests where applicable β€” **no test added for the new field** - [x] I have updated relevant documentation to reflect my changes β€” not applicable, no user-facing or documented behaviour changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green β€” **`e2e` did not complete on `5ca5fde` (Playwright install timed out at 30m and the run was cancelled); all other checks pass** - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups β€” **currently 4/5, sole finding being this description** - [ ] I will address all Greptile and reviewer comments before requesting merge --- This description was reformatted to `.github/PULL_REQUEST_TEMPLATE.md` by the Paperclip PR triage bot. The code was not modified. Checklist boxes reflect the PR's verifiable state at commit `5ca5fde`; unchecked items are genuinely outstanding, not oversights. The **Model Used** section requires input from the author. The previous description's `LEG-` reference was removed as an internal, instance-local identifier that the template prohibits. --------- Co-authored-by: Lead Backend Engineer Agent Co-authored-by: Paperclip Co-authored-by: Andrew Aymeloglu --- .../heartbeat-ledger-billing-code.test.ts | 81 +++++++++++++++++++ server/src/services/heartbeat.ts | 6 +- 2 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 server/src/__tests__/heartbeat-ledger-billing-code.test.ts diff --git a/server/src/__tests__/heartbeat-ledger-billing-code.test.ts b/server/src/__tests__/heartbeat-ledger-billing-code.test.ts new file mode 100644 index 00000000000..9b9a9fc4c71 --- /dev/null +++ b/server/src/__tests__/heartbeat-ledger-billing-code.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it, vi } from "vitest"; +import type { heartbeatRuns } from "@paperclipai/db"; +import { resolveLedgerScopeForRun } from "../services/heartbeat.ts"; + +type IssueRow = { id: string; projectId: string | null; billingCode: string | null }; + +/** + * Minimal Drizzle stand-in for the single lookup `resolveLedgerScopeForRun` + * performs: `db.select({...}).from(issues).where(...)` resolved as a thenable. + */ +type LedgerDb = Parameters[0]; + +function makeDb(rows: IssueRow[]) { + const where = vi.fn(() => Promise.resolve(rows)); + const from = vi.fn(() => ({ where })); + const select = vi.fn(() => ({ from })); + return { db: { select } as unknown as LedgerDb, select, from, where }; +} + +function makeRun(contextSnapshot: Record) { + return { id: "run-1", contextSnapshot } as unknown as typeof heartbeatRuns.$inferSelect; +} + +describe("resolveLedgerScopeForRun billing code propagation", () => { + it("carries the issue's billing code onto the ledger scope", async () => { + const { db } = makeDb([{ id: "issue-1", projectId: "project-1", billingCode: "ACME-42" }]); + + const scope = await resolveLedgerScopeForRun(db, "company-1", makeRun({ + issueId: "issue-1", + projectId: "context-project", + })); + + expect(scope).toEqual({ + issueId: "issue-1", + projectId: "project-1", + billingCode: "ACME-42", + }); + }); + + it("resolves a null billing code when the issue has none set", async () => { + const { db } = makeDb([{ id: "issue-1", projectId: "project-1", billingCode: null }]); + + const scope = await resolveLedgerScopeForRun(db, "company-1", makeRun({ + issueId: "issue-1", + projectId: "context-project", + })); + + expect(scope.billingCode).toBeNull(); + expect(scope.issueId).toBe("issue-1"); + }); + + it("resolves a null billing code without querying when the run has no issue in context", async () => { + const { db, select } = makeDb([]); + + const scope = await resolveLedgerScopeForRun(db, "company-1", makeRun({ + projectId: "context-project", + })); + + expect(scope).toEqual({ + issueId: null, + projectId: "context-project", + billingCode: null, + }); + expect(select).not.toHaveBeenCalled(); + }); + + it("resolves a null billing code when the issue is not visible to the company", async () => { + const { db } = makeDb([]); + + const scope = await resolveLedgerScopeForRun(db, "other-company", makeRun({ + issueId: "issue-1", + projectId: "context-project", + })); + + expect(scope).toEqual({ + issueId: null, + projectId: "context-project", + billingCode: null, + }); + }); +}); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 2953969e220..6fbe84b699f 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -2667,7 +2667,7 @@ export function resolveLedgerCostStatus(input: { return input.costUsd == null && hasTokenUsage ? "unpriced" : "reported"; } -async function resolveLedgerScopeForRun( +export async function resolveLedgerScopeForRun( db: Db, companyId: string, run: typeof heartbeatRuns.$inferSelect, @@ -2680,6 +2680,7 @@ async function resolveLedgerScopeForRun( return { issueId: null, projectId: contextProjectId, + billingCode: null, }; } @@ -2687,6 +2688,7 @@ async function resolveLedgerScopeForRun( .select({ id: issues.id, projectId: issues.projectId, + billingCode: issues.billingCode, }) .from(issues) .where(and(eq(issues.id, contextIssueId), eq(issues.companyId, companyId))) @@ -2695,6 +2697,7 @@ async function resolveLedgerScopeForRun( return { issueId: issue?.id ?? null, projectId: issue?.projectId ?? contextProjectId, + billingCode: issue?.billingCode ?? null, }; } @@ -11653,6 +11656,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) agentId: agent.id, issueId: ledgerScope.issueId, projectId: ledgerScope.projectId, + billingCode: ledgerScope.billingCode, provider, biller, billingType, From 47c38777d8bccb58b066032423d3974ab74deb47 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 22 Jul 2026 13:19:15 -0500 Subject: [PATCH 6/7] fix(release): use trusted publishing npm for bundled packages (#10030) 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 > - Paperclip publishes canary and stable packages through a shared release script > - GitHub Actions authenticates those publishes through npm trusted publishing and an OIDC identity token > - Bundled-dependency packages recently moved from pnpm publish to a pinned npm CLI to preserve their bundled files > - That pin selected npm 10, which cannot use trusted publishing, so the first bundled package failed with `ENEEDAUTH` > - This pull request keeps bundled-package packing on npm 10 while routing actual publishing through the trusted-publishing-capable npm 11 version > - The benefit is bundled packages keep their required npm packaging behavior while canary and stable releases authenticate successfully ## Linked Issues or Issue Description **What happened** The canary release job failed while publishing `@paperclipai/adapter-utils` with `ENEEDAUTH`. The package has bundled dependencies, so the release helper selected pinned `npm@10.9.7`; the workflow provides OIDC trusted publishing rather than an npm token, and npm 10 cannot use that authentication path. Because this is the first package attempted, the release exited before trying the remaining packages. **Expected behavior** Bundled-dependency packages should publish with an npm CLI that both preserves bundled dependencies and supports GitHub Actions trusted publishing. **Steps to reproduce** Run the canary release workflow from master after PR #9980. The `publish_canary` job reaches `@paperclipai/adapter-utils`, invokes `npx npm@10.9.7 publish`, and fails with `ENEEDAUTH`. **Deployment mode** GitHub Actions canary and stable npm release workflows. Refs #9980. ## What Changed - Kept bundled-package dry-run packing on npm `10.9.7`, which successfully produces the staged tarball. - Routed bundled-package publishing through npm `11.16.0`, which supports GitHub Actions trusted publishing. - Split the pack and publish helpers so future npm changes cannot silently couple the two compatibility requirements. - Updated focused release and ACPX packaging tests to enforce both versions and call paths. ## Verification - `pnpm test:release-registry` β€” 67 passed locally. - Initial all-npm-11 PR head: Canary Dry Run reproduced an npm-internal crash during bundled `pack`. - Current head `5b3961ed13dd26ed2d6b1096ea23fd91b32e4353`: Canary Dry Run passed with split npm pack/publish helpers. - All PR checks passed, including build, typecheck + release registry, server/workspace suites, both e2e shards, security gates, and Greptile. - Greptile reviewed the current head at 5/5 confidence with no blocking issues. ## Risks - Low risk: the change only separates the npm CLI used for bundled-package packing from the CLI used for publishing. - The versions remain explicitly pinned because npm 11.16.0 currently crashes on the bundled pack payload, while npm 10.9.7 cannot perform trusted publishing. - Focused tests assert both pins and both helper call paths, and the full Canary Dry Run passes on the current head. > 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 Codex, exact model ID `gpt-5.6-sol`, high reasoning mode, with repository, shell, GitHub CLI, and code-execution tools. ## 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 - [x] All Paperclip CI gates are green - [x] 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 --- scripts/acpx-patch-packaging.test.mjs | 8 +++++--- scripts/release-lib.sh | 15 ++++++++++----- scripts/release-lib.test.mjs | 6 +++--- scripts/release.sh | 2 +- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/scripts/acpx-patch-packaging.test.mjs b/scripts/acpx-patch-packaging.test.mjs index 57379a4fb64..d51ca8e7efa 100644 --- a/scripts/acpx-patch-packaging.test.mjs +++ b/scripts/acpx-patch-packaging.test.mjs @@ -34,7 +34,9 @@ test("bundled package staging materializes publishConfig entrypoints", () => { }); 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"/); + assert.match(releaseScript, /run_bundled_npm_pack pack --pack-destination "\$publish_dir"/); + assert.match(releaseLib, /BUNDLED_NPM_PACK_VERSION="10\.9\.7"/); + assert.match(releaseLib, /BUNDLED_NPM_PUBLISH_VERSION="11\.16\.0"/); + assert.match(releaseLib, /npx --yes "npm@\$BUNDLED_NPM_PACK_VERSION"/); + assert.match(releaseLib, /npx --yes "npm@\$BUNDLED_NPM_PUBLISH_VERSION"/); }); diff --git a/scripts/release-lib.sh b/scripts/release-lib.sh index 7cd7ec55a37..927c36a96e8 100644 --- a/scripts/release-lib.sh +++ b/scripts/release-lib.sh @@ -310,10 +310,15 @@ package_publish_tool() { ' } -BUNDLED_NPM_VERSION="10.9.7" +BUNDLED_NPM_PACK_VERSION="10.9.7" +BUNDLED_NPM_PUBLISH_VERSION="11.16.0" -run_bundled_npm() { - npx --yes "npm@$BUNDLED_NPM_VERSION" "$@" +run_bundled_npm_pack() { + npx --yes "npm@$BUNDLED_NPM_PACK_VERSION" "$@" +} + +run_bundled_npm_publish() { + npx --yes "npm@$BUNDLED_NPM_PUBLISH_VERSION" "$@" } run_package_publish() { @@ -323,9 +328,9 @@ run_package_publish() { if [ "$publish_tool" = "npm" ]; then if [ "$disable_provenance" = "true" ]; then - run_bundled_npm publish --tag "$dist_tag" --access public --provenance=false + run_bundled_npm_publish publish --tag "$dist_tag" --access public --provenance=false else - run_bundled_npm publish --tag "$dist_tag" --access public + run_bundled_npm_publish publish --tag "$dist_tag" --access public fi return fi diff --git a/scripts/release-lib.test.mjs b/scripts/release-lib.test.mjs index efb901f9527..e98128d8d65 100644 --- a/scripts/release-lib.test.mjs +++ b/scripts/release-lib.test.mjs @@ -91,7 +91,7 @@ exit 1 set -euo pipefail printf 'npx %s\n' "$*" >> "$FAKE_CALL_LOG" [ "$1" = "--yes" ] && shift -[ "$1" = "npm@10.9.7" ] && shift +[ "$1" = "npm@11.16.0" ] && shift exec npm "$@" `, ); @@ -141,11 +141,11 @@ 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", () => { +test("publish_package_to_npm uses trusted-publishing-capable 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, /^npx --yes npm@11\.16\.0 publish --tag canary --access public$/m); assert.match(result.calls, /^npm publish --tag canary --access public$/m); assert.doesNotMatch(result.calls, /^pnpm publish/m); }); diff --git a/scripts/release.sh b/scripts/release.sh index 79719107d8a..cbdc39d50b8 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -262,7 +262,7 @@ if [ "$dry_run" = true ]; 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 + run_bundled_npm_pack 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 From b57aa9950c707a024156c34b79326a82b2dcca31 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Wed, 22 Jul 2026 11:51:30 -0700 Subject: [PATCH 7/7] fix(test): stop flaky server-suite afterAll hook timeouts (#10024) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is an open-source AI agent management platform; its test suite spans a `server` package that mounts real embedded Postgres databases in `beforeAll`/`afterAll` hooks > - The `server` package CI shard runs all ~93 suites serially (`maxWorkers=1`) on a loaded CI host; each suite boots and tears down its own embedded Postgres in hook callbacks > - vitest's default `hookTimeout` is 10 seconds; under load, graceful embedded-Postgres shutdown occasionally crosses that threshold > - This produces intermittent `Error: Hook timed out in 10000ms` failures in `afterAll` hooks β€” not test assertion failures β€” and the suites pass on re-run, making them textbook flaky tests > - Inspecting `embedded-postgres@18.1.0-beta.16` shows that `stop()` takes no argument (no fast-shutdown mode), SIGINTs postgres (already PostgreSQL "fast shutdown"), and resolves only on the child's `exit` event with no internal time bound > - Two targeted fixes: (1) raise `hookTimeout` and `teardownTimeout` to 30 s in `server/vitest.config.ts` β€” one config change that eliminates the flake for all ~93 suites at once; (2) wrap `stop()` in a 5 s bounded `Promise.race` in the test helper so a slow shutdown can never hang the hook regardless of OS scheduling variance > - This PR changes only test-infra and test-config; no production-code behavior changes ## Linked Issues or Issue Description No public GitHub issue exists for this flake. Inline bug description (bug report template): **What happened?** The `General tests (server (N/3))` CI shards intermittently fail with `Error: Hook timed out in 10000ms` in `afterAll` hooks and pass on re-run. Every test assertion passes; only the teardown hook exceeds vitest's default timeout. **Expected behavior** CI passes reliably. Teardown timeouts should not be a source of flake. **Steps to reproduce** Run the server test suite repeatedly on a loaded host or in CI with `maxWorkers=1` β€” the shard occasionally crosses 10 s in `afterAll` during embedded-Postgres shutdown. **Paperclip version** `master`, any build that includes `server/vitest.config.ts` without an explicit `hookTimeout`. **Deployment mode** Self-hosted (CI). ## What Changed - **`server/vitest.config.ts`** β€” added `hookTimeout: 30000` and `teardownTimeout: 30000`. Removes flake across all ~93 server suites at once. 30 s gives generous headroom over observed worst-case teardown while still catching a genuinely hung hook. - **`packages/db/src/test-embedded-postgres.ts`** β€” added `stopEmbeddedPostgresBounded()`, a 5 s `Promise.race` wrapper around `stop()`. Applied at all three call sites inside `cleanup()`. Data dir is still removed unconditionally; errors are still swallowed; the null-instance guard is preserved. Existing behavior unchanged except the shutdown can no longer block indefinitely. ## Verification - `tsc --noEmit` clean on `packages/db` (built against worktree-local `shared`) - `packages/db` `client.test.ts` passes 14/14 β€” boots embedded Postgres and exercises the bounded teardown via `cleanup()` in `afterEach` - Standalone bounded-race semantics verified: hang resolves at the 5 s bound; late or immediate `stop()` rejection swallowed; no unhandled rejection; null-instance path safe - CI: all 3 server shards + split-verify lane (Async-Verification Gate) expected green after this PR ```bash # Reproduce the teardown test locally: cd packages/db && npx vitest run src/client.test.ts # Type-check packages/db: npx tsc --noEmit -p packages/db/tsconfig.json ``` ## Risks Low risk. No product-code changes β€” test-infra and test-config only. The vitest timeout increase is additive (raises the ceiling; never lowers it). The bounded race wrapper preserves prior teardown behavior exactly: data dir always removed, errors always swallowed, stop is still attempted. A worst-case outcome is that a genuinely hung `stop()` now surfaces as a test timeout at 30 s instead of 10 s β€” still caught, just later. ## Model Used - **Provider:** Anthropic - **Model:** Claude Sonnet 4.6 (`claude-sonnet-4-6`) - **Context window:** 200 k tokens - **Capabilities:** tool use, code execution, extended reasoning - **Mode:** Paperclip agent heartbeat (autonomous execution with human board oversight) ## 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 - [x] All Paperclip CI gates are green - [x] 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: Harold Kim Co-authored-by: Paperclip --- packages/db/src/test-embedded-postgres.ts | 70 +++++++++++++++++++++-- server/vitest.config.ts | 9 +++ 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/packages/db/src/test-embedded-postgres.ts b/packages/db/src/test-embedded-postgres.ts index 929d17c5f7e..d88ca59c377 100644 --- a/packages/db/src/test-embedded-postgres.ts +++ b/packages/db/src/test-embedded-postgres.ts @@ -106,6 +106,61 @@ function cleanupEmbeddedPostgresTestDirs(dataDir: string) { fs.rmSync(dataDir, { recursive: true, force: true }); } +// Upper bound (ms) on how long we wait for the embedded Postgres cluster to +// stop gracefully before abandoning the wait and returning from the hook. +const EMBEDDED_POSTGRES_STOP_TIMEOUT_MS = 5000; + +// `embedded-postgres@18.1.0-beta.16` exposes only `stop(): Promise` β€” no +// shutdown-mode argument. Internally it SIGINTs the postgres process (already +// PostgreSQL "fast shutdown") and resolves *only* on the child's `exit` event, +// with no time bound of its own. Under the loaded serial server shard a slow +// shutdown checkpoint can push that past vitest's hookTimeout and hang the +// afterAll hook. So we bound the graceful stop: if it overruns, we stop waiting +// and return so the hook completes. The SIGINT has already been delivered, so +// the abandoned process still exits on its own (and again when the runner exits). +// Errors are swallowed, matching prior behavior. +// +// `cleanupFn` (data-dir reclaim) is chained on the raw `stop()` promise, not on +// the timeout race, so the disposable data dir is removed *only after* `stop()` +// actually settles β€” i.e. once the child Postgres process has exited. Removing +// it on the timeout path would pull the data files out from under a still-running +// cluster and provoke checkpoint / WAL I/O errors. In the fast path `cleanupFn` +// has run by the time this resolves; in the timeout path it runs asynchronously +// once the abandoned process finally exits. +async function stopEmbeddedPostgresBounded( + instance: EmbeddedPostgresInstance | null, + cleanupFn?: () => void, +): Promise { + if (!instance) { + cleanupFn?.(); + return; + } + let timer: ReturnType | undefined; + const stopped = instance + .stop() + .catch(() => { + // Swallow shutdown errors β€” the data dir is reclaimed regardless. + }) + .finally(() => { + try { + cleanupFn?.(); + } catch { + // Best-effort reclaim; ignore removal errors. + } + }); + try { + await Promise.race([ + stopped, + new Promise((resolve) => { + timer = setTimeout(resolve, EMBEDDED_POSTGRES_STOP_TIMEOUT_MS); + timer.unref?.(); + }), + ]); + } finally { + if (timer) clearTimeout(timer); + } +} + function formatEmbeddedPostgresError(error: unknown): string { if (error instanceof Error && error.message.length > 0) return error.message; if (typeof error === "string" && error.length > 0) return error; @@ -131,8 +186,9 @@ async function probeEmbeddedPostgresSupport(): Promise {}); - if (dataDir) cleanupEmbeddedPostgresTestDirs(dataDir); + await stopEmbeddedPostgresBounded(instance, () => { + if (dataDir) cleanupEmbeddedPostgresTestDirs(dataDir); + }); } } @@ -165,13 +221,15 @@ export async function startEmbeddedPostgresTestDatabase( return { connectionString, cleanup: async () => { - await instance?.stop().catch(() => {}); - if (dataDir) cleanupEmbeddedPostgresTestDirs(dataDir); + await stopEmbeddedPostgresBounded(instance, () => { + if (dataDir) cleanupEmbeddedPostgresTestDirs(dataDir); + }); }, }; } catch (error) { - await instance?.stop().catch(() => {}); - if (dataDir) cleanupEmbeddedPostgresTestDirs(dataDir); + await stopEmbeddedPostgresBounded(instance, () => { + if (dataDir) cleanupEmbeddedPostgresTestDirs(dataDir); + }); throw new Error( `Failed to start embedded PostgreSQL test database: ${formatEmbeddedPostgresError(error)}`, ); diff --git a/server/vitest.config.ts b/server/vitest.config.ts index d07d5a3acb8..cd2fafdb89a 100644 --- a/server/vitest.config.ts +++ b/server/vitest.config.ts @@ -3,6 +3,15 @@ import { defineConfig } from "vitest/config"; export default defineConfig({ test: { environment: "node", + // Each server suite boots + tears down its own embedded Postgres in + // beforeAll/afterAll. Under the loaded serial shard (maxWorkers=1) the + // graceful shutdown can occasionally cross vitest's default 10s hookTimeout, + // producing flaky "Hook timed out in 10000ms" afterAll failures on CI. Give + // the boot/teardown hooks generous headroom; 30s is far above the observed + // worst-case teardown yet still catches a genuinely hung hook. teardownTimeout + // mirrors it for the same reason. + hookTimeout: 30000, + teardownTimeout: 30000, isolate: true, maxConcurrency: 1, maxWorkers: 1,