mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-10 12:07:09 +02:00
9964b034bbff24e700c8eccf5a8b1fc3daa44bf2
266
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Nicky LeachandPaperclip
|
2862e18484 |
refactor(adapter-utils): remove the retired duplex_v1 sandbox bridge transport (#12171)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The adapter utilities provide sandbox transport paths for agent execution > - The retired `duplex_v1` path remains in host, gateway, and test code after `http2_v1` replaced it > - Retired transport code adds maintenance cost and leaves an unsafe fallback for unknown gateway modes > - This pull request removes the retired path, moves shared `http2_v1` contracts to a leaf module, and closes mode dispatch to a fixed allowlist > - The benefit is a smaller transport surface and explicit failure for unsupported modes ## Linked Issues or Issue Description Refs #12120 The `http2_v1` transport replaced `duplex_v1`, but the retired broker, gateway, constants, and tests remain in the adapter utilities. An unknown bridge mode can also fall through to the queue gateway when a queue directory exists. This change removes the retired code and rejects unsupported modes before gateway selection. ## What Changed - Delete the host `duplex_v1` broker and its transport-only tests. - Delete the in-sandbox duplex gateway and retired mode constants. - Move shared `http2_v1` symbols into `bridge-transport-contract.ts`. - Update the remaining importers and repair their focused tests. - Validate bridge modes against `http2_v1` and `queue_v1` before queue lookup. - Keep `queue_v1`, `duplex-frame-codec.ts`, and duplex telemetry dimensions unchanged. ## Verification - [x] `npx tsc --noEmit -p packages/adapter-utils` passes. - [x] `npx vitest run packages/adapter-utils/src` passes: 48 files and 968 tests pass, with 4 pre-existing platform skips. - [x] Full CI is green on this pull request. - [x] Greptile review is complete and every finding is resolved. ## Risks The change removes an internal transport that no host path selects. The main risk is an overlooked import or test dependency. Targeted typecheck and tests cover the adapter utility package. Full CI must confirm workspace-wide compatibility. ## Model Used Anthropic Claude Sonnet 5 assisted with the implementation, as recorded in the commit. The commit does not record a context-window size or reasoning mode. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
02a984068c |
refactor(adapter-utils): clean up the HTTP/2 bridge request-body bounds (#12166)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent adapters use the HTTP/2 bridge to carry requests and responses > - The bridge has an idle bound and a total-lifetime ceiling for request bodies > - The old renewable lifetime bound re-armed with each DATA chunk and could not act before the idle bound > - The code also repeated the same bounds and rationale in several places > - This pull request removes the unreachable renewable bound, keeps the one-shot ceiling, and simplifies the shared bounds object > - The benefit is clearer protection logic with the same default request-body behavior ## Linked Issues or Issue Description **What existing behavior does this improve?** The HTTP/2 bridge request-body reader uses several repeated bound parameters and comments. One renewable lifetime bound cannot act before the idle bound under the shipped defaults. **Subsystem affected** `packages/adapter-utils/` — HTTP/2 bridge adapter utilities. **Current behavior** The idle bound and renewable lifetime bound both re-arm after each DATA chunk. The renewable bound therefore does not act on its own. The total-lifetime ceiling also shares timer setup with the renewable bound. **Proposed behavior** Remove the renewable lifetime bound. Keep the total-lifetime ceiling as an independent one-shot timer. Pass one bounds object to the bridge call sites and keep tests for the idle bound and total-lifetime ceiling. **Reason and benefit** The change removes unreachable logic and repeated rationale. It keeps the independent total-lifetime protection and makes the bound behavior easier to review. **Breaking changes** The change removes two public constant and option names that repository-wide search found unused outside this implementation. The shipped default behavior does not change. ## What Changed - Remove the renewable request-body lifetime bound and its public names. - Keep the total-lifetime ceiling as a one-shot timer that starts when the body read starts. - Replace repeated bound parameters with one `Http2BridgeBodyBounds` object. - De-duplicate bound rationale comments. - Add shared test helpers and update tests for the idle bound and total-lifetime ceiling. ## Verification - Run `npx tsc --noEmit -p packages/adapter-utils`. - Run `npx vitest run packages/adapter-utils/src/http2-bridge-server.test.ts`. - Wait for the pull request CI checks. - Request the Greptile review and confirm a 5/5 verdict with no open findings. ## Risks The main risk is an incorrect timer lifetime after the renewable timer removal. The one-shot ceiling remains independent, and the updated tests cover its expiry and cleanup paths. The change does not alter the shipped default bounds. ## Model Used OpenAI Codex based on GPT-5. Exact runtime model version is GPT-5. The work used tool calls and code execution for repository inspection and GitHub operations. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
445547c989 |
feat(duplex): run the Daytona sandbox callback bridge over Node HTTP/2 (#12120)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Sandbox providers carry agent work through controlled execution channels > - The Daytona callback bridge uses a bespoke line-framed protocol over its duplex channel > - The bespoke protocol adds framing work and does not use the Node transport that already supports multiplexed streams > - This pull request carries raw bytes across the channel, adds a Node HTTP/2 bridge, and selects it for Daytona > - The benefit is one authenticated, multiplexed callback session with queue_v1 as the bounded fallback ## Linked Issues or Issue Description **Subsystem affected** The packages/plugins Daytona provider and the shared duplex execution path. **Problem or motivation** The Daytona callback bridge uses a bespoke line-framed protocol over the provider duplex channel. This adds protocol work and limits stream handling. **Proposed solution** Carry raw bytes through the cross-layer channel. Add an authenticated Node HTTP/2 host server and sandbox client gateway. Select http2_v1 for Daytona and retain queue_v1 as the fallback. **Alternatives considered** Keep the current duplex_v1 protocol. This keeps the bespoke framing path and does not provide one HTTP/2 session for callback streams. **Roadmap alignment** ROADMAP.md lists Daytona under cloud and sandbox agents. This change improves the shipped Daytona provider path. **Additional context** The branch adds no dependency. Node 24 provides the http2 module. The host token check and canonical path parser remain the single dispatch path. ## What Changed - Carry raw Uint8Array chunks through the adapter, plugin, worker, runtime, and Daytona layers. - Encode bytes as base64 only across the JSON-RPC hop, because JSON has no binary type. - Add the bounded host HTTP/2 server and the in-sandbox HTTP/2 client gateway. - Authenticate every stream with the per-run bridge token before route work. - Parse the path once and reuse the canonical result for route and forwarding work. - Select http2_v1 for Daytona and fall back once to queue_v1 when the client preface is absent. - Add transport, session, stream, and fallback telemetry. - Mark HTTP/2 as the preferred transport and queue_v1 as the soft-deprecated fallback. ## Verification - `npx vitest run packages/adapter-utils/src` — 990 passed and 4 skipped. - `npx vitest run server/src/__tests__/plugin-worker-manager-duplex.test.ts` — 32 passed. - `npx vitest run --config packages/plugins/sandbox-providers/daytona/vitest.config.ts` — 220 passed and 6 skipped. - `npx tsc --noEmit` in `packages/adapter-utils`, `packages/shared`, `packages/plugins/sdk`, and `server` — clean. - No `package.json` or `pnpm-lock.yaml` file changed. - The live Daytona test skips when `DAYTONA_API_KEY` is absent. - The root `npx tsc --noEmit` command has a pre-existing missing `packages/adapters/droid-local` reference on this branch and on `master`. ## Risks - The transport change affects several duplex layers and could expose byte-boundary errors. - A missing HTTP/2 client preface falls back once to queue_v1 and records `preface_missing`. - The host token check and canonical path parser must remain on the shared dispatch path. - The live Daytona test needs `DAYTONA_API_KEY` and does not run in this agent sandbox. ## Model Used OpenAI GPT-5, tool-enabled coding agent with repository inspection, GitHub CLI, and shell execution. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
b6854e61c7 |
refactor(adapter-utils): rename EffectiveSandboxCapabilities to EffectiveExecutionCapabilities (#12119)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The adapter utilities package defines shared types for agent execution targets > - The type name EffectiveSandboxCapabilities describes only one transport > - All execution target drivers return the same resolved capability snapshot > - This pull request gives the snapshot a general name and keeps the old type as a deprecated alias > - The benefit is clearer public vocabulary with source compatibility for current consumers ## Linked Issues or Issue Description **What existing behavior does this improve?** The exported capability snapshot type uses the name `EffectiveSandboxCapabilities`, although local, SSH, sandbox, and plugin drivers return it. **Subsystem affected** The change affects `packages/adapter-utils` and its server consumers. **Current behavior** The public type name points to the sandbox transport. The private parser also uses the sandbox-only name. **Proposed behavior** Use `EffectiveExecutionCapabilities` for the public type and `parseEffectiveExecutionCapabilities` for the private parser. Keep a deprecated alias for the old public type. **Reason and benefit** The new name matches the established execution-target vocabulary. The alias keeps existing type imports working during the migration. **Breaking changes** None. The runtime field, capability flags, parsed shape, and package versions do not change. **Additional context** GitHub search found no duplicate or related open issue or pull request. ## What Changed - Rename the exported interface to `EffectiveExecutionCapabilities`. - Keep `EffectiveSandboxCapabilities` as a deprecated type alias. - Rename the private parser and update its call site and references. - Add a type-level test for the deprecated alias. ## Verification - `npx tsc --noEmit -p packages/adapter-utils` - `npx vitest run packages/adapter-utils/src/execution-target-sandbox.test.ts` - `npx vitest run server/src/__tests__/environment-execution-target-capabilities.test.ts server/src/__tests__/environment-execution-target-duplex.test.ts` - The local checks passed with 133 adapter-utils tests and 31 server tests. - Reviewers can confirm that the runtime field and capability flags stay unchanged. ## Risks Low risk. The alias protects existing type imports. The change does not alter runtime behavior or serialized data. ## Model Used OpenAI Codex, GPT-5, tool use and code execution. The runtime does not expose the context window size. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
d1573244b5 |
refactor: disambiguate the Telemetry and Observability data paths (#12128)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Paperclip records first-party events, OpenTelemetry data, and local run-log events > - The code and documents used one term for these three data paths > - This naming made the required review level unclear > - This pull request names each data path in the module names, documents, and code comments > - The benefit is a clear review rule without a runtime change ## Linked Issues or Issue Description **Issue type** Unclear or confusing. **Where is the issue?** `packages/shared/src/telemetry/README.md`, `doc/observability.md`, `doc/run-log-events.md`, and the duplex instrumentation modules. **What's wrong?** The repository used Telemetry for first-party events, OpenTelemetry data, and local run-log events. This usage made the data path and review level unclear. **Suggested fix** Use Telemetry only for Paperclip first-party events. Use Observability for OpenTelemetry data. Use the run log for rows in `heartbeat_run_events`. Related public pull requests: #8476 and #9672. ## What Changed - Rename the duplex instrumentation modules and identifiers from `Telemetry` to `Observability`. - Move the Observability and run-log contracts out of the Telemetry README. - Add `doc/observability.md` and `doc/run-log-events.md` as the canonical documents. - Add a file-path review rule to `AGENTS.md`. - Correct the remaining code comments that name the wrong data path. - Keep all event names, payloads, database records, spans, configuration keys, environment variables, and runtime paths unchanged. ## Verification - `npx vitest run packages/shared/src/telemetry/readme-contract.test.ts` passes. - `npx vitest run packages/adapter-utils/src/published-exports.test.ts` passes. - `npx vitest run packages/adapter-utils/src/acpx-engine/startup-timing.test.ts` passes with 42 tests. - `pnpm --filter @paperclipai/adapter-utils typecheck` passes. - `pnpm --filter server typecheck` passes. - The old module name does not remain in TypeScript or JSON files, except for the intentional publication guard. - CI and Greptile checks remain pending after PR creation. ## Risks - The old duplex module subpath no longer has a compatibility shim. The board accepted this intentional hard break. - The new duplex module subpath stays blocked from package publication. - The change has no runtime effect. The main risk is an incorrect document or module reference. ## Model Used OpenAI GPT-5 Codex, exact model ID `gpt-5`, with tool use and code review support. ## 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 described the issue in-PR with the documentation issue fields - [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 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 <noreply@paperclip.ing> |
||
|
Devin Foley
|
87d68f476b |
fix: harden the sandbox bridge gateway against crashes and queue wedge (#12060)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents on remote sandbox targets reach the Paperclip API through the sandbox callback bridge: a loopback HTTP gateway inside the sandbox queues request files for a host-side worker > - The gateway process has no supervisor: nothing inside the sandbox respawns it, so a crash leaves a dead loopback port for the rest of the run > - The gateway also never cleaned up request files whose responses never arrived, so a stalled host wedged the queue at its depth cap and every later request got an immediate 503 > - #12052 made the host-side worker survive transient faults; this pull request hardens the other half of the relay > - The benefit is that a gateway fault degrades one request instead of severing the agent from the control plane until run end ## Linked Issues or Issue Description Refs #12052 (host-side worker half of the same relay). Refs #9904 and #8977 (adjacent bridge behavior). No public issue exists for this defect. The description below follows the bug report template. **What happened?** During a staging run, an agent's API calls to the bridge's loopback port began failing at the connection level (curl reported HTTP 000) partway through the run. A dead gateway process is the only mechanism that produces connection-level failures on that port, and nothing restarts it. Separately, request files for timed-out requests stayed in the queue; after 64 accumulated, the gateway answered every request with `503 Bridge request queue is full.` until the run ended. **Expected behavior** An uncaught fault in the gateway must not kill the loopback listener. A request that times out must not leave its file counting toward the queue-depth cap. A queue full of orphaned files must recover instead of rejecting until run end. **Steps to reproduce** 1. Start a remote-sandbox run and stop the host-side bridge worker. 2. Send requests to the gateway until they time out; the request files stay in `requests/`. 3. After 64 such files, every request gets an immediate 503, even after the host recovers. 4. Independently, raise any uncaught exception in the gateway process; the loopback port dies for the rest of the run. ## What Changed - The generated gateway source installs global `uncaughtException` / `unhandledRejection` handlers that log to stderr (already redirected to `logs/bridge.log`) and keep serving. The relay holds no state a fault can corrupt beyond the one request it interrupted. - Survival is gated on readiness: before the gateway has written its readiness file (file mode) or sent its READY frame (duplex mode), the same handlers exit(1) instead. A startup fault (failed bind, failed readiness write) means the process can never serve, and surviving there would only leave an un-ready zombie while the host waits out its readiness poll. - The file gateway attaches an explicit `error` listener to its server and pins the event loop with a keepalive until the bind settles. Newer Node runtimes do not reliably surface a failed bind through `uncaughtException` in this shape: the process can drain and exit 0 before the error event is delivered (reproduced on Node 24/25; Node 22 delivered it). The duplex gateway already had an explicit listener. - A request that times out waiting for the host now deletes its own request file. The host's response write is guarded on that file, so the removal also signals that no caller waits anymore. - At the queue-depth cap, the gateway sweeps request files older than the response deadline (orphans from killed callers or a previous gateway process) before rejecting with 503. - Host-side, `processRequestFile` treats a request file that vanished before the read as the benign caller-gave-up race and skips it quietly instead of escalating into the recovery pass. ## Verification - `npx vitest run packages/adapter-utils/src/sandbox-callback-bridge.test.ts` — 43 passed, verified on both Node 22 and Node 25. - New end-to-end test: with no worker running, a request times out (502), its file is cleaned, and the same gateway then serves a 200 once a worker starts — no wedge, no dead port. - New end-to-end test: with `maxQueueDepth: 1` and a backdated orphan file at the cap, the gateway sweeps the orphan and admits the request instead of answering 503. - New worker test: a request file that vanishes before the read is skipped without a handler call, a response write, or a run-level error. - New generated-source test: spawned directly against an already-occupied port, the gateway exits 1 promptly with the `EADDRINUSE` fault on stderr instead of lingering un-ready (or exiting 0 silently, the pre-existing behavior on Node 24/25). - A pin keeps the crash handlers, the readiness gate, and the sweep in the generated source. - `pnpm --filter @paperclipai/adapter-utils typecheck`. ## Risks - Keeping a Node process alive after `uncaughtException` is normally suspect; here the alternative is a dead loopback port for the rest of the run, and the gateway is a stateless per-request relay. The fault is logged with its stack to `bridge.log`, and survival applies only after readiness — startup faults still fail fast. - Deleting a timed-out request file could race a host that is mid-processing. The host's response write is already guarded on request-file existence, and the new host-side skip treats the vanished file as a no-op, so no duplicate mutation path is introduced. - The stale sweep runs only at the depth cap and only removes files older than the response deadline plus a 2 s grace, so a live caller's file is never swept. - Orphaned response files (host responded after the caller gave up) still linger; that pre-existing minor leak is unchanged here. ## Model Used - Claude Fable 5 (Anthropic), model id `claude-fable-5`, extended thinking enabled, agentic tool use via Claude Code (CLI harness), 200k context window. ## 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 |
||
|
Devin Foley
|
fc9e9b704f |
fix: stop teaching agents to curl literal {id} route templates (#12061)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Adapters inject prompt text that teaches agents how to call the
Paperclip API, including copy-pasteable curl examples
> - Some of those URLs contained brace placeholders like
`/api/issues/{id}/checkout`
> - Agents paste such lines verbatim; the placeholder reaches the server
as `/api/issues/%7Bid%7D` and 404s, and request logs show agents doing
exactly that
> - The acpx engine's API note already avoids this by using
`$PAPERCLIP_TASK_ID`, and its test pins `/api/issues/{id}` out of the
prompt
> - This pull request applies the same standard to the gemini adapter,
the shared prompt template, and the openclaw gateway workflow
> - The benefit is that agents stop burning turns on placeholder 404s
and doc examples stay safe to execute as written
## Linked Issues or Issue Description
No public issue exists for this defect. The description below follows
the bug report template.
**What happened?**
Server request logs show agents issuing `GET /api/issues/%7Bid%7D` — the
literal, percent-encoded text `{id}` — which 404s. The source is adapter
prompt text: the gemini adapter's API note embeds a curl example with
`/api/issues/{id}/checkout` in the URL, the shared agent prompt template
mentions `/api/issues/{issueId}` endpoints, and the harness checkout
notice names `/api/issues/{id}/checkout`. Models copy these strings into
real requests.
**Expected behavior**
URL paths in prompt text must carry environment variables or real ids,
never brace placeholders, in every string an agent might execute
verbatim. Where a placeholder is unavoidable, the prompt must state
explicitly that the literal text must never be sent.
**Steps to reproduce**
1. Give an agent the gemini adapter's API access note.
2. Watch it call `curl ...
"$PAPERCLIP_API_URL/api/issues/{id}/checkout"` as written.
3. The server logs `POST /api/issues/%7Bid%7D/checkout 404`.
## What Changed
- gemini-local's API note curl example now uses `$PAPERCLIP_TASK_ID` and
tells the agent to substitute a real issue id when that variable is
absent — the same convention as the acpx engine's API note.
- The shared agent prompt template (`server-utils.ts`) uses
`$PAPERCLIP_TASK_ID` in its interaction-creation and resume-endpoint
mentions, and the harness checkout notice names `POST
/api/issues/$PAPERCLIP_TASK_ID/checkout`.
- openclaw-gateway's endpoint workflow keeps its `{issueId}`
placeholders — they are defined by its "determine issueId" step — but
now states explicitly that the literal text must never be sent in a URL.
- `server-utils.test.ts` pins the new form and adds negative pins that
keep `/api/issues/{id}` and `/api/issues/{issueId}` out of the shared
prompt template, mirroring the existing acpx-engine negative pin.
- The `confirmation:{issueId}:plan:{revisionId}` idempotency-key
template is untouched: it is a value-construction pattern, not a URL.
## Verification
- `npx vitest run packages/adapter-utils/src/server-utils.test.ts
packages/adapters/gemini-local packages/adapters/openclaw-gateway` — 152
passed. The single failure (`pre-selects gemini-api-key auth in the
managed HOME for sandbox execution`) is a pre-existing
environment-specific failure on the development machine, unrelated to
prompt text; CI is authoritative for it.
- `pnpm --filter @paperclipai/adapter-utils --filter
@paperclipai/adapter-gemini-local --filter
@paperclipai/adapter-openclaw-gateway typecheck`.
## Risks
- Low risk: prompt-text and test changes only; no runtime logic changes.
- Agents that memorized the old example strings keep working — the
routes are unchanged, only the placeholder text in prompts is.
## Model Used
- Claude Fable 5 (Anthropic), model id `claude-fable-5`, extended
thinking enabled, agentic tool use via Claude Code (CLI harness), 200k
context window.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
|
||
|
Devin Foley
|
c7f4bc1300 |
fix: survive transient sandbox exec failures in the callback bridge worker (#12052)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents on remote sandbox targets reach the Paperclip API through the sandbox callback bridge: a loopback gateway inside the sandbox writes request files, and a host-side worker polls them over the provider's exec channel and forwards them to the server > - The worker's poll loop had one terminal catch: a single reset or slow exec ended the relay for the rest of the run > - The in-sandbox gateway kept queueing requests against the dead worker, so every later API call from the agent stranded, including its final status write > - A relay that dies on one transient fault turns a routine provider hiccup into a lost issue disposition > - This pull request restructures the loop so transient faults back off and retry, while the watchdog remains the escalation path for sustained outages > - The benefit is that one flaky exec no longer severs an agent from the control plane mid-run ## Linked Issues or Issue Description Refs #9904. Refs #8977. Both touch adjacent bridge behavior (curl shim, header forwarding); neither addresses worker-loop lifetime. No public issue exists for this defect. The description below follows the bug report template. **What happened?** During a staging run, the host-side bridge worker hit one failed sandbox exec while relaying requests. The poll loop's only catch is terminal: it failed the pending requests and set the worker to settled, with no restart. The agent's later API calls saw connection-level failures or bridge errors until the run ended. Its final `PATCH status: done` was lost, and the missing-disposition recovery had to repair the issue in a corrective run. **Expected behavior** One transient exec failure must not end the relay for the rest of the run. The worker must back off and retry. A sustained outage must still fail queued requests fast through the watchdog. In-flight request semantics (abort plus 504 backstop, retry-safe 503) must not change. **Steps to reproduce** 1. Start a remote-sandbox run and let the provider exec channel reject or stall one call while the bridge worker polls. 2. The worker hits one `listJsonFiles` failure or one request-attempt timeout, and the loop exits through its terminal catch. 3. Every later bridge request strands. The loopback gateway keeps accepting requests that never complete, and after 64 queued files it answers every request with 503 until run end. ## What Changed - `startSandboxCallbackBridgeWorker`'s poll loop now separates three failure domains: - A failed poll backs off exponentially (capped at 5 s) and retries instead of dying. The first failure of a streak still lands on the run trace through the workerFailed span; later repeats only warn. - A failed or hung request attempt runs the same recovery pass the loop previously died on — abort the in-flight handler (its 504 backstop keeps the caller from stranding) and 503 the unclaimed queued requests — and the loop then continues and serves the caller's retry. - A listing where every file already has an in-flight attempt sleeps one poll interval, like an empty listing. The previous immediate re-list was a hot spin: an exec storm against a real channel, and a pure-microtask loop that starved every timer in the process when the queue client resolves synchronously. - The watchdog, the claim/finalize fences, and the stop/drain semantics are unchanged. ## Verification - `npx vitest run packages/adapter-utils/src/sandbox-callback-bridge.test.ts` — 38 passed. - New regression test: a request queued behind three consecutive poll failures is still delivered. - Updated tests: the stalled-poll test now expects recovery (the handler's real 200) instead of a terminal 503; the sustained-outage 503 path remains proven by the dedicated watchdog test; the recovery-503-write-retry test triggers the recovery pass through a hung request read, because a hung poll no longer runs that pass. - `pnpm --filter @paperclipai/adapter-utils typecheck`. ## Risks - Behavior change: a transiently failing poll no longer mass-fails queued requests on the first error. Callers wait through the backoff window, bounded by the existing in-sandbox 30 s response deadline, or the watchdog fails them after 20 s of no successful iteration. This trades fast-but-terminal degradation for recovery. - A hard-down channel now retries every ≤5 s for the rest of the run instead of stopping. Each retry is one exec attempt against a channel that already fails. - In-flight mutation safety is unchanged: the guard map and the claim protocol still prevent a double-applied host mutation, and a guarded file is always finalized by its own attempt or by its 504 backstop. ## Model Used - Claude Fable 5 (Anthropic), model id `claude-fable-5`, extended thinking enabled, agentic tool use via Claude Code (CLI harness), 200k context window. ## 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 |
||
|
Nicky LeachandPaperclip
|
63df7ad2b3 |
feat(login): use the login pseudo-terminal for Codex device login and de-Claude the shared channel (#12020)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent adapters use provider-specific login flows > - Codex device login needs a live pseudo-terminal (PTY), while the shared channel still uses Claude-specific names > - The old streamed-exec path does not provide the prompt transport that Codex needs > - This pull request moves Codex device login to the shared login PTY and removes the dead streamed-exec path > - The benefit is one controlled login transport with fail-closed capability checks and safer credential reads ## Linked Issues or Issue Description **Problem or motivation** Codex device login used a streamed-exec path that did not provide the required prompt transport. The shared login channel also exposed Claude-specific names outside Claude code. **Expected behavior** The host selects a fixed login command from trusted adapter data. Codex login uses the provider login PTY. Providers without that capability fail closed. **Proposed solution** Use a server-controlled session home, create and validate it as a fresh 0700 directory, read credentials from one validated descriptor, and rename shared channel names to the neutral login PTY family. **Alternatives considered** Keep the shared login PTY as the single transport. Do not keep the removed streamed-exec path because it cannot provide the required prompt transport. **Roadmap alignment** This change supports the planned login transport work. It does not add a separate roadmap item. ## What Changed - Route Codex device login through the shared login PTY transport. - Select the login command from a closed internal command key. - Carry a server-controlled session home through the launch contract. - Create and validate the session home as a fresh 0700 directory owned by the login user. - Read the credential file with descriptor-relative, no-follow path walking and final descriptor checks. - Gate the login route and run lease on the provider login PTY capability. - Rename shared channel names to the neutral login PTY family. - Remove the streamed-exec transport value, selector field, driver branch, and related tests. - Hide Codex login in the user interface when the provider lacks the login PTY capability. ## Verification - Server unit suites pass: 89/89. - Adapter-utils suites pass: 262/262. - Codex-local suites pass: 326/326. - Credential-read reader suite passes: 20/20. - Daytona login PTY suite passes: 30/30. - Device-login suites pass: 56/56. - TypeScript checks pass for server, adapter-utils, and UI. - GitHub Actions must pass after pull request creation. - Greptile review must reach 5/5 with no open P2 findings, recommendations, or follow-ups. ## Risks - Providers without a login PTY capability lose Codex login support by design. - The credential read rejects invalid ownership, mode, type, path, and size. - The launch-time sandbox directory race remains outside the threat model because the login runs inside the sandbox and a hostile sandbox already controls its credential. ## Model Used OpenAI Codex, GPT-5, tool use and code review assistance. The exact context window and reasoning mode are not exposed by the runtime. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
16b59c9315 |
feat(adapter-utils): stream duplex bridge bodies as sequenced chunks with receive-side spill (#12006)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent adapters use a duplex bridge to send requests and responses across an isolated boundary > - The bridge held each request body and response body in memory on both ends > - Large bodies can exhaust memory and reduce the safe size of adapter traffic > - This pull request sends receive-side bodies as sequenced chunks and spills large bodies to disk > - The benefit is bounded memory use with strict size, order, and cleanup checks ## Linked Issues or Issue Description **What existing behavior does this improve?** The adapter-utils duplex bridge transports request and response bodies across the sandbox boundary. **Current behavior** The bridge stores each complete body in memory on both ends of the duplex channel. **Proposed behavior** The bridge sends body chunks with sequence checks. The receive side keeps bodies up to 1 MiB in memory and spills larger bodies to a temporary file. **Reason and benefit** This change reduces memory pressure and keeps malformed or oversized input on a terminal error path. **Breaking changes** The duplex frame version changes to version 2. The request and response envelopes now carry bodyByteCount, and body_chunk frames carry the body data. ## What Changed - Add version 2 body_chunk frames with 256 KiB raw slices encoded as canonical base64 text. - Add receive-side memory and spill reassembly with per-channel disk and file limits. - Reject malformed, reordered, oversized, truncated, and non-canonical body chunks. - Stream reassembled request bodies to the host forward handler with a web stream and half-duplex request. - Remove spill files on success, failure, channel death, and startup cleanup. ## Verification - Run the adapter-utils type-check. - Run the adapter-utils duplex test suite. - Run all pull request checks. - Run the Greptile review and confirm a 5/5 score with no open findings. ## Risks The wire format changes from version 1 to version 2. Older bridge peers cannot use this protocol. The receive path adds temporary file operations and cleanup paths. The implementation fails closed when a body violates size or sequence rules. ## Model Used OpenAI GPT-5 Codex, tool-enabled coding agent. The exact context-window size and reasoning mode are not exposed by the runtime. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
05b35d4669 |
feat(duplex): bound aggregate duplex route resource consumption with a process-owned byte ledger (#12003)
## Thinking Path > - Paperclip runs AI agents through adapters and sandboxed execution targets. > - Duplex routes retain bytes across route data, broker messages, decoder buffers, and readiness replay. > - Per-route limits bound each route but do not bound the total retained bytes across many routes. > - A process-owned ledger must charge each retained buffer before allocation and release the charge during cleanup. > - This pull request adds the aggregate ledger, connects it to host and sandbox duplex paths, and adds route coverage. > - The benefit is a fail-closed process-wide byte limit that keeps concurrent duplex work within a safe resource budget. ## Linked Issues or Issue Description **Subsystem affected** This change affects packages/adapter-utils and server duplex orchestration. **Problem or motivation** Many routes can each stay below their per-route limits while their combined retained bytes exceed a safe process budget. **Proposed solution** Add a process-owned aggregate byte ledger. Charge route data, broker bytes, decoder buffers, and readiness replay bytes before allocation. Release each charge during cleanup. Use a separate sandbox_process decoder cap for the in-sandbox path. **Alternatives considered** Keep only per-route limits. This does not bound the combined process use. Set a fixed limit at one call site. This misses retained bytes in other duplex paths. **Roadmap alignment** This is a tightly scoped reliability and resource-safety improvement. It does not duplicate a roadmap feature. **Additional context** The aggregate ceiling uses a safe 256 MiB default. An invalid override falls back to that default and reports the rejected value. ## What Changed - Add a process-owned aggregate byte ledger for duplex route resource use. - Charge and release route data, broker forward and response bytes, decoder buffers, and readiness replay bytes. - Bound host-to-worker pending writes and standard input transport bytes. - Add a separate decoder cap for the sandbox_process path. - Make invalid aggregate-ceiling overrides fall back to the safe default without host startup failure. - Add adapter-utils and server tests for charging, release, rejection, cleanup, and many-route aggregate limits. ## Verification - pnpm --filter @paperclipai/adapter-utils exec tsc --noEmit - pnpm --filter @paperclipai/server exec tsc --noEmit - Run the focused adapter-utils duplex ledger and execution-target tests. - Run the server aggregate-ledger route test. - Confirm all required pull request checks pass on this branch. ## Risks The ledger touches several duplex buffer paths. A missed release could reduce later capacity until process restart. The tests cover charge, release, rejection, cleanup, and route aggregation. The change uses a safe default when configuration input is invalid. ## Model Used OpenAI GPT-5 Codex. The runtime model ID and context window are not exposed to this task. The model used tool calls, shell commands, and code review workflow support. ## 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 issue references) - [x] My branch name describes the change 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
c5050396c7 |
fix(daytona-duplex): chunk host-to-sandbox writes and make a transport close legible (#11986)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Paperclip runs agent work through adapters and sandbox providers > - The Daytona duplex path sends host input through a provider pseudo-terminal WebSocket > - Large messages exceed the provider limit, and a transport close can look like a process exit > - This pull request chunks UTF-8 input and carries transport-close state through the duplex path > - The benefit is reliable large input and accurate loss reporting ## Linked Issues or Issue Description **What happened?** The Daytona duplex path sent a full input payload as one WebSocket message. A payload above the provider limit closed the channel. The wait path also mapped a non-numeric exit result to a process exit without exit data. **Expected behavior** The provider must receive large input as ordered UTF-8 chunks. A transport close without exit data must record `transport_closed`, while a numeric exit must record `provider_exit`. **Steps to reproduce** 1. Start a Daytona duplex session. 2. Send an input payload larger than 65536 bytes. 3. Observe that one message closes the provider channel. 4. End a session without a numeric exit code. 5. Observe that the loss reason reports a process exit. **Paperclip version or commit** Commit `1761e79ec9097c65d94f90a8ba20416f8ab718a6`. **Deployment mode** Built from source with the Daytona sandbox provider. ## What Changed - Add a shared UTF-8 byte chunker with a 32768-byte cap. - Route both Daytona pseudo-terminal write paths through the chunker. - Preserve multi-byte UTF-8 sequences across read-side chunks. - Carry an explicit `transportClosed` state through the worker and host wait paths. - Record `transport_closed` for a reason-less transport close and `provider_exit` for a numeric exit. - Keep orderly completion suppression for both exit paths. ## Verification - The Daytona plugin suite passes 194 tests. - The adapter-utils broker, codec, and telemetry suites pass 73 tests. - The plugin SDK duplex and worker RPC host suites pass 37 tests. - The server plugin worker manager duplex suite passes 78 tests. - The execution target sandbox and ACPX execute suites pass 257 tests. - TypeScript checks pass for adapter-utils, plugin SDK, server, and the standalone Daytona plugin. ## Risks The chunk size adds a loop for large input payloads. The 32768-byte cap stays below the provider limit. The optional loss field preserves compatibility for other providers. ## Model Used OpenAI Codex, GPT-5, extended reasoning, tool use, and code execution. The runtime does not expose a separate context-window value. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
141b815294 |
fix(adapter-utils): enforce the duplex frame size bound on encode in both codec copies (#11983)
## Thinking Path > - Paperclip uses adapter utilities to move bounded messages between agent processes. > - The duplex frame codec encodes and decodes these messages. > - The decoder rejects frames above the documented byte limit. > - The encoder did not apply the same limit before it sent a frame. > - This mismatch let a sender write a frame that the peer rejected after transmission. > - This pull request applies the same byte limit to both codec copies and keeps the broker channel open. > - The benefit is a local error with stable request telemetry instead of a channel loss. ## Linked Issues or Issue Description No public GitHub issue exists for this change. The problem follows the bug report fields below. **What happened?** The duplex encoder could write a frame larger than `DEFAULT_MAX_DUPLEX_FRAME_BYTES`. The peer decoder then rejected the frame after transmission. In the WebSocket 1009 case, this closed the channel and reported a process exit. **Expected behavior** The encoder should reject an oversized frame before it writes bytes. The gateway should return HTTP 413. The broker should return a bounded terminal response and keep other requests active. **Steps to reproduce** 1. Encode a duplex frame above `DEFAULT_MAX_DUPLEX_FRAME_BYTES`. 2. Send the frame through the gateway or broker. 3. Observe that the old path writes the frame or drops the channel after peer rejection. **Paperclip version or commit** Reproduced from the `master` development line before this change. **Deployment mode** Local dev (`pnpm dev`). ## What Changed - Add `encodeDuplexFrameChecked` to the host and embedded gateway codecs. - Measure encoded JSON bytes without the trailing newline. - Return a typed `frame_too_large` result without throwing. - Return HTTP 413 for oversized gateway requests without writing a frame. - Share one frame bound between broker decode and encode checks. - Return a bounded, non-retryable terminal response for oversized broker responses. - Add encode vectors to the shared wire-compatibility fixture. ## Verification - Run `pnpm --filter @paperclipai/adapter-utils typecheck`. - Run `npx vitest run packages/adapter-utils/src/duplex-frame-codec.test.ts packages/adapter-utils/src/duplex-bridge-broker.test.ts packages/adapter-utils/src/execution-target-sandbox.test.ts`. - Confirm the oversized-response broker test keeps the channel open and serves the other in-flight request. - Confirm the gateway test returns HTTP 413 and keeps the channel open. ## Risks The encoder now rejects oversized frames before transmission. This changes an unsafe write into a typed local error. The broker and gateway keep existing frame limits and affect only oversized frames. ## Model Used OpenAI Codex, GPT-5, tool use and code execution. The model assisted with review and repository operations. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
cc42a67e7e |
fix(adapter-utils): extend the duplex fail-closed run disposition to the CLI lane (#11966)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Paperclip runs agents through adapter execution lanes > - Duplex adapters can lose their control channel before a process completes > - The ACP lane already fails closed, but the CLI lane can report false success > - This pull request applies the same completion rule to the CLI lane and shares the loss code > - The benefit is consistent failure reporting when a duplex channel closes during a run ## Linked Issues or Issue Description **What happened?** A CLI-lane duplex run can lose its control channel before clean process completion. The run can then report `succeeded` with exit code 0 and no error code. **Expected behavior** The execution target must fail closed when the channel dies before clean completion. It must return exit code 1, the typed `duplex_channel_lost` error code, and a short stderr note. **Steps to reproduce** 1. Start a duplex adapter run through the CLI execution lane. 2. Close the duplex control channel before the process completes cleanly. 3. Inspect the run result and error code. **Paperclip version or commit** Commit `5e01523d4eb6df4a20a0bddd05374c9c42225203`. **Deployment mode** Built from source. **Installation method** Built from source with pnpm. **Agent adapter(s) involved** Claude Code, Codex, Cursor, Gemini, Kimi, OpenCode, and Pi local adapters. **Database mode** Not database-related. ## What Changed - Add an optional `errorCode` field to `RunProcessResult`. - Add a one-read completion seam to the execution target process options. - Fail closed when a duplex channel dies before clean process completion. - Add `settleRunDisposition()` to atomically read and mark orderly completion. - Share the typed duplex loss error code across the ACP and CLI lanes. - Mark non-success terminal results as orderly completion before teardown. - Wire the seam through the seven duplex adapters. - Add regression tests for channel loss, clean completion, and non-clean terminal results. ## Verification - `npx vitest run packages/adapter-utils/src/execution-target-sandbox.test.ts` — 118 passed. - `npx vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts -t "sandbox duplex run-disposition seam"` — 4 passed. - The author confirmed a clean type-check for `@paperclipai/adapter-utils` and the seven duplex adapter packages. - Pre-existing environment failures remain outside this change. They include `EACCES mkdir '/srv/paperclip'` and remote file-size setup failures. ## Risks The change alters terminal status for CLI duplex runs that lose control before clean completion. The typed error code and stderr note keep the failure visible. The broker marks failed, cancelled, and timed-out results as orderly completion to prevent false loss events during teardown. ## Model Used OpenAI Codex, GPT-5, tool use and code execution, with the standard GPT-5 context window. The model assisted with the implementation and test work. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
10d2781a29 |
feat(sandbox): add the duplex bridge broker, gated transport selection, and fixed observability (#11769)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Sandbox adapters provide controlled execution for untrusted provider environments. > - The sandbox channel needs one persistent duplex transport with strict host control. > - The transport must remain off unless the instance setting and provider capability both allow it. > - The host must detect loss, bound resource use, and expose only safe telemetry. > - This pull request adds the broker, gated selection, kill-switch wiring, fixed observability, and real-process proof. > - The benefit is safer sandbox execution with bounded failure behavior and inspectable transport results. ## Linked Issues or Issue Description No public issue exists for this change. The related pull requests are #11738 and #11750. **Problem or motivation** The sandbox duplex channel needs a host-controlled broker, strict transport gates, bounded provider input, and safe loss telemetry. Without these controls, a provider can cause replay, resource growth, unsafe endpoint selection, or data exposure through telemetry. **Proposed solution** Add a host broker with nested time limits, request limits, one-shot loss, and per-id deduplication. Select duplex transport only when the instance setting and provider capability both equal true. Assign the endpoint and nonce on the host. Reject invalid readiness data and use the file bridge on failure. Add fixed redacted telemetry and a real-process end-to-end test harness. **Alternatives considered** Keep the file bridge as the only transport. This avoids new channel behavior but does not provide persistent duplex operation for supported sandbox providers. **Roadmap alignment** This change supports the Cloud / Sandbox agents section in ROADMAP.md. ## What Changed - Add the duplex bridge broker with bounded forward, response, and gateway wait budgets. - Bound concurrent requests, lifetime requests, and request-id bytes before retention or forwarding. - Select duplex transport only when both required gates are true. - Assign the loopback port and nonce on the host and enforce a liveness-only READY frame. - Fall back to the file bridge after invalid readiness, contamination, bind failure, or timeout. - Carry the kill switch through the server, acpx engine, and six local adapters. - Add fixed, redacted duplex telemetry with a provider allowlist. - Add a real-process end-to-end harness for readiness, round trips, loss, and teardown. - Add regression coverage for limits, loss, UTF-8 splits, concurrency, and telemetry dimensions. ## Verification - Adapter-utils, server, and Daytona typechecks pass locally. - Adapter-utils tests pass, including the codec, broker, execution-target sandbox, and real-process harness. - Server kill-switch tests pass. - Live Daytona tests pass with the required provider key and skip without that key. - The root pnpm-lock.yaml file has no diff. - The branch contains ten commits after origin/master. ## Risks - Duplex transport remains disabled unless both gates equal true. - A provider remains an untrusted boundary and needs least-privilege credentials and quotas. - The server telemetry recorder stays deferred; the default recorder does nothing. - A provider that pre-binds the host port causes a fail-closed fallback to the file bridge. - The change adds no database migration and changes no root lockfile. ## Model Used OpenAI GPT-5, exact model family GPT-5, large context window, reasoning, and tool use. The model assisted with Git handoff validation and PR preparation. The implementation commits came from the engineering worktree. ## 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/... or 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 - [x] I searched the GitHub PR list for similar PRs and confirmed this is not a duplicate --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Devin Foley
|
adfbe2d4b9 |
feat(environments): refer to the managed default environment by name, not the sandbox driver key (#11838)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Managed deployments provision a platform-managed default environment
for agent runs; the UI shows this environment in selectors, the agent
form, run details, and the environments page
> - Those surfaces append the raw driver key to the environment name, so
users see labels like "Paperclip Computer (sandbox)", "Paperclip
Computer · sandbox", and fallback copy such as "Managed sandbox" and
"The sandbox has no ready authentication"
> - "sandbox" is infrastructure vocabulary, not the product name of the
environment; showing it next to the managed environment's name is
confusing and off-brand
> - This pull request renders platform-managed environments by name
alone and rewords the sandbox-phrased copy, while user-created
environments keep the driver suffix so mixed lists stay distinguishable
> - The benefit is that the default environment reads as one clear
product name everywhere, and self-hosted users lose nothing: their own
environments still show the driver
## Linked Issues or Issue Description
**What existing behavior does this improve?**
Display of the platform-managed default environment across the UI.
**Subsystem affected**
UI (environment selectors, agent config form, environments page, agents
page, run details) and the claude-local/codex-local adapter auth checks.
**Current behavior**
The agent form labels the inherited default environment as "Name
(sandbox)". Environment selectors and the environments list render "Name
· sandbox". The agents page describes the environment as "<provider>
sandbox provider". The agent form's fallback label is "Managed sandbox".
Adapter auth checks say "The sandbox has no ready authentication for
this adapter."
**Proposed behavior**
Platform-managed environment rows (`metadata.managedByPaperclip`) render
their name alone. The fallback label is "Paperclip Computer". The agents
page describes managed environments as "Managed by Paperclip". Run
details omit the driver suffix for sandbox-driver environments (the
adjacent Provider entry already identifies the mechanism). Adapter auth
checks say "This environment has no ready authentication for this
adapter."
**Reason and benefit**
The managed environment carries a product name. Appending the raw driver
key ("sandbox") to it is noise and contradicts the product naming.
User-created environments keep the driver suffix, so mixed lists stay
distinguishable.
**Breaking changes**
None. Message text of the auth check is not read programmatically; the
UI keys off `ADAPTER_AUTH_MISSING_CHECK_CODE`. Rows without the managed
marker render exactly as before.
## What Changed
- New `environmentDisplayLabel` helper in
`ui/src/lib/managed-sandbox-environment.ts`: managed rows → name alone;
other rows → "Name · driver".
- `AgentConfigForm`: inherited-default label uses the helper; fallback
copy "Managed sandbox" → "Paperclip Computer"; environment options use
the helper.
- `ProjectProperties`, `CompanyEnvironments`: environment selector
options use the helper; the environments-list row hides the driver
suffix on managed rows; the managed detail page's fallback description
no longer says "sandbox".
- `Agents` page: managed environments are described as "Managed by
Paperclip" instead of "<provider> sandbox provider".
- `CommentThread` run details: the driver suffix is omitted for
sandbox-driver environments.
- claude-local and codex-local adapters: auth-missing check message/hint
reworded from "sandbox" to "environment" (ACP and environment-test
paths); claude-local probe/effort/login hints reworded the same way.
- Run status lines: "Syncing workspace to sandbox", "Exporting git
changes from sandbox", "Starting adapter in sandbox", and friends now
say "environment"; "Finalizing sandbox workspace" → "Finalizing
workspace". Templated transfer-progress lines map the `sandbox`
transport key to "environment" for display (`runtime-progress.ts`).
- Agent form sign-in panel: "Sign in to the sandbox" → "Sign in to the
environment"; "Authenticated. The sandbox has credentials now." → "…The
environment has credentials now."
- Feature catalog + instance settings card: "Managed Sandbox Only" →
"Managed Environment Only" (setting key unchanged; the card keeps its
alphabetical slot).
- Server agents routes: execution-target failure and test-identity copy
no longer say "sandbox"; workspace-mode label "Cloud sandbox" → "Cloud
environment".
- Tests: new `environmentDisplayLabel` unit cases; new `AgentConfigForm`
render case asserting the managed default renders without "(sandbox)" or
"· sandbox"; status-line assertions updated across adapter-utils, server
heartbeat/live-run, and UI chat suites.
## Verification
- `pnpm --filter @paperclipai/ui typecheck` — clean.
- `pnpm --filter @paperclipai/adapter-claude-local typecheck` and
`--filter @paperclipai/adapter-codex-local typecheck` — clean.
- `vitest run` for `managed-sandbox-environment.test.ts`,
`AgentConfigForm.render.test.tsx`, `CompanyEnvironments.test.tsx`,
`Agents.test.tsx`, `CommentThread.test.tsx`, `NewAgent.test.tsx` — all
green (118 tests across the two runs).
## Risks
Low risk. Cosmetic label changes only; no data or API changes. Rows
without `metadata.managedByPaperclip` render exactly as before, so
self-hosted deployments with their own environments see no change. The
only self-hosted-visible wording changes are the adapter auth-check
message and the driver suffix omission on sandbox-driver rows in run
details.
## Model Used
- Claude (Anthropic) — claude-fable-5 (Claude Fable 5), Claude Code CLI,
extended thinking, tool use.
## 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 (no
docs reference these labels)
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
|
||
|
Nicky LeachandPaperclip
|
38d8f37172 |
fix(build): enforce Node 24 across Paperclip (#11792)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Paperclip runs across the CLI, server, adapters, plugins, CI, and container images. > - These surfaces declared different Node.js versions from 20 through 24. > - A newer `@types/node` major can expose APIs that the supported runtime does not provide. > - Node.js 20 is no longer a suitable project baseline, and Node.js 24 is the current LTS line. > - This pull request sets Node.js 24.11.0 as one repository-wide baseline, adds a drift check, and gives users actionable startup guidance when their runtime is too old. > - The benefit is one clear runtime contract for development, release, installation, and published packages. ## Linked Issues or Issue Description Refs #2734 Refs #11727 Refs #739 ## What Changed - Require Node.js 24.11.0 or newer in all 42 package manifests and runtime checks. - Use Node.js 24 in GitHub Actions, Docker images, smoke images, sandbox setup, portable installs, and esbuild targets. - Align every direct `@types/node` declaration on `^24.0.0`. - Prevent Dependabot from opening major `@types/node` upgrades without a matching runtime decision. - Add `.nvmrc` and a CI policy check for Node version drift. - Update ACP version gates, tests, and user documentation for the new minimum. - Print a non-blocking warning on CLI and server startup when Node is unsupported, with remediation through a version manager or the documented downloaded `install.sh` workflow. - Deduplicate that warning when `paperclipai run` boots the CLI and server in the same process. ## Verification - `node scripts/check-node-version-policy.mjs` - `node --check scripts/check-node-version-policy.mjs` - `node --check cli/esbuild.config.mjs` - `node --check scripts/generate-npm-package-json.mjs` - `bash -n scripts/install.sh scripts/test-install-sh-docker.sh scripts/e2e-install-lifecycle.sh` - Parsed all 42 package manifests and confirmed `engines.node` is `>=24.11.0`. - `git diff --check` - `vitest run packages/adapter-utils/src/sandbox-install-command.test.ts` passed with 3 tests. - `vitest run cli/src/node-version.test.ts` passed with 4 tests. - Directly exercised the shared warning helper for unsupported-version messaging and same-process deduplication. - The focused exe.dev suite could not resolve the locally unbuilt plugin SDK from this isolated worktree. A full offline workspace install was also blocked because the package-manager signature verifier requires registry access. The full suite was not run locally; draft CI performs a clean install and evaluates the wider impact. ## Risks - This is a breaking runtime change for users, plugins, and deployments that still use Node.js 20 or 22. - Published workspace packages will now produce an engine warning or failure in strict package managers on older Node.js releases. - Node.js 24 can reveal dependency, native module, Playwright, or agent CLI compatibility issues in CI. - The bootstrap installer now installs Node.js 24 when the current runtime is older than 24.11.0. - The portable sandbox fallback is pinned to Node.js 24.11.0 and depends on that upstream tarball remaining available. - Unsupported runtimes continue booting after a warning, so a later incompatibility can still fail at its point of use. - The CLI and server share the warning policy through the published `@paperclipai/shared` package; packaging checks must keep that subpath export available. - This PR does not commit `pnpm-lock.yaml` because repository policy assigns lockfile generation to CI. > 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 based on GPT-5. The exact deployment ID and context window are not exposed in this session. Reasoning, repository tools, shell execution, and GitHub tools were enabled. ## 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 <noreply@paperclip.ing> |
||
|
nearfolk
|
c2cfd55e97 |
fix: exclude thought text from automatic issue comments (#11801)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The heartbeat system records agent runs and can add a run summary to an issue. > - The ACPX engine receives output text and internal thought text as separate streams. > - The default summary strategy joined both streams and could publish internal text in an issue comment. > - Paperclip already has final-output segmentation for run summaries. > - This pull request makes final-output-only summaries mandatory and removes the configuration bypass. > - The benefit is that automatic issue comments contain the intended final message instead of internal execution text. ## Linked Issues or Issue Description Refs #11761 **What happened?** The ACPX engine used the full summary strategy when an adapter did not set `summaryStrategy`. That strategy joined all text deltas, including thought-stream text and intermediate narration. The heartbeat finalizer could then store that summary as an issue comment. **Expected behavior** An automatic issue comment must use only the final output segment. Configuration must not allow thought-stream text or intermediate narration into that summary. **Steps to reproduce** 1. Run an ACPX adapter without a configured `summaryStrategy`. 2. Emit an output delta, a thought delta, a tool call, and a final output delta. 3. Read the generated run summary. 4. Observe that the old default included all text deltas. **Paperclip version or commit** `54b8bec44417511c623999613f9f1006f8af0517` **Deployment mode** Built from source with a local ACPX adapter. ## What Changed - Limit ACPX run summaries to the final non-empty output segment. - Ignore the legacy full-summary setting so configuration cannot bypass containment. - Update regression tests for the safe default and an attempted unsafe override. ## Verification - Observed the new guard fail before the implementation change because the summary contained thought text. - Ran `pnpm exec vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts -t "defaults run summaries to the final output segment without thought text|does not allow configuration to include thought text in run summaries"`. Result: 2 passed. - Ran `pnpm exec vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts`. Result: 130 passed. - Ran `pnpm --filter @paperclipai/adapter-utils typecheck`. Result: passed. ## Risks - Run summaries are shorter for adapters that relied on full text aggregation. - The old `summaryStrategy: "full"` setting no longer changes summary behavior. This is an intentional containment change. - The change does not alter run logs or tool events. It changes only the summary selected for downstream use. > This is a focused security and privacy bug fix. It does not add roadmap scope. ## Model Used - OpenAI Codex on the GPT-5 family. The runtime did not expose the exact model ID or context-window size. Reasoning, tool use, terminal execution, and code editing were enabled. ## 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 and contains no internal task id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant inline documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
233c12f029 |
feat: add kimi-local adapter for Kimi Code CLI (CLI + ACP engines) (#9967)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Local agent adapters (`claude_local`, `gemini_local`, `grok_local`, …) are the integration surface that lets Paperclip run coding CLIs on the host machine > - The Kimi Code CLI (`kimi`, Moonshot AI) has a documented non-interactive mode, `kimi -p --output-format stream-json` with session resume via `kimi -r`, but Paperclip has no built-in adapter for it > - So Kimi users (especially Kimi membership / OAuth subscribers) cannot onboard their CLI to Paperclip agent teams > - This pull request adds a complete built-in `kimi_local` adapter (both execution engines, session management, instructions + skills delivery, thinking-effort control, environment test, UI and CLI modules, docs) following the established `gemini_local`/`grok_local` package pattern > - Kimi Code ships an ACP server (`kimi acp`), so the adapter runs on Paperclip's shared acpx engine by default (streaming transcript with live tool status, like `claude_local`/`gemini_local`) and falls back to a headless CLI lane (`kimi -p --output-format stream-json`) when ACP prerequisites are unavailable > - The benefit is that Kimi Code becomes a first-class Paperclip agent lane: selectable in the UI, resumable across heartbeats, with the same operating context (instruction bundle, skills, effort) and streaming transcript the other local adapters get ## Linked Issues or Issue Description - Supersedes #9880 (same branch; expanded from the CLI-only lane into a complete adapter with the default ACP engine lane, control-plane skill install, and live transcript wiring) - Refs #9879 (adapter request for Kimi Code CLI, filed with this PR) - Refs #163 (original Kimi support request) Duplicate/related prior PRs, per the dedup search (both appear stale: no updates or maintainer review since May 2026, and both target an older Kimi CLI interface; calling them out for reviewer context per CONTRIBUTING.md): - Refs #6276 (`feat: add kimi-local adapter`): targets an older array-based content format (`{type: think}`/`{type: text}` blocks), not the current documented stream-json schema - Refs #5202 (`feat(adapter): add Kimi CLI local adapter with Wire protocol support`): builds on a `--wire` JSON-RPC interface that current Kimi Code CLI (0.27.0) no longer documents; the current documented headless interface is `-p --output-format stream-json` This PR is a fresh implementation against current master and the currently documented/verified Kimi CLI behavior (see Verification). Happy to fold in anything useful from the earlier attempts if a reviewer prefers. ## What Changed - **New adapter package** `packages/adapters/kimi-local` (`@paperclipai/adapter-kimi-local`), modeled on `gemini-local`/`grok-local`: - `src/server/execute.ts`: spawns `kimi -p <prompt> --output-format stream-json` (argv array, no shell), `-m <model>` only when configured, `-r <sessionId>` when the stored session cwd matches the run cwd, automatic fresh-session retry on unrecoverable-session errors, headless-safe env (`CI=1`, `NO_COLOR=1`, `KIMI_CODE_NO_AUTO_UPDATE=1`, `TERM=dumb`; user-configured values win), full remote (ssh/sandbox) execution lane with runtime install via `@moonshot-ai/kimi-code` - **Instruction bundle delivery**: the prompt path directive now names the sibling instruction files (`./HEARTBEAT.md`, `./SOUL.md`, `./TOOLS.md`) alongside the prepended entry file, and local runs pass `--add-dir <instructions-dir>` so Kimi can actually open them (matching `claude_local`). Without this, only the entry file reached Kimi and agents improvised the operating workflow that `HEARTBEAT.md` documents - **Thinking effort**: a configured `effort` is forwarded as the `KIMI_MODEL_THINKING_EFFORT` operational override (Kimi has no per-invocation effort flag). It is only sent for models that advertise `support_efforts` (currently `kimi-code/k3`) to avoid provider rejections, and `medium` maps to `high` since Kimi has no medium tier (`low`/`high`/`max` pass through) - **Skills delivery**: desired Paperclip skills are delivered via Kimi's `--skills-dir` flag from a dedicated per-run directory (a local snapshot, or the synced snapshot on remote targets), so skills load reliably and in isolation. Paperclip never overwrites the shared `$KIMI_CODE_HOME/skills` home, so skills installed by the operator or other agents are left intact. `--skills-dir` is only passed when at least one skill is desired, so unconfigured agents keep Kimi's default skill discovery - **Live run status**: the adapter now forwards each streamed stream-json line to `onEvent` (assistant `content` as an assistant snippet, `tool_calls` as tool-name events), which drives the issue-thread activity indicator (`currentToolName` / `lastAssistantSnippet` / `lastEventAt`). Previously the adapter only wrote the raw run log, so the issue thread showed a stale "no output for N s" line with no tool or reasoning context while Kimi worked. Tool results are omitted so the last meaningful "Using X" / snippet is not overwritten by a generic label - `src/server/parse.ts`: parses the verified Kimi stream-json event shapes (`assistant` text, `assistant.tool_calls` with JSON-string arguments, `tool` results, trailing `meta.session.resume_hint` for session-id capture) plus failure classifiers (`kimi_auth_required`, transient network, unrecoverable session). A signaled exit (null exit code, not a timeout) is now reported as a failure rather than coalesced to success, and the error message names the terminating signal - `src/server/skills.ts`: lists/syncs Paperclip skills for the adapter's skill-management surface - `src/server/test.ts`: environment test covering CLI resolution + `kimi --version`, cwd check, auth detection (OAuth credential dirs, keyed `[providers.*]` in config.toml, or the `KIMI_MODEL_NAME` + `KIMI_MODEL_API_KEY` env pair), and a live hello probe - `src/ui/` (stdout-line parser for transcripts, config builder) and `src/cli/` (stream event formatter) modules - Root metadata: three managed model aliases (`kimi-code/kimi-for-coding`, `kimi-code/kimi-for-coding-highspeed`, `kimi-code/k3`), effort-capable-model metadata (`EFFORT_CAPABLE_MODELS`, effort mapping helpers), `agentConfigurationDoc` - Tests: 101 tests across parse, execute (args building, resume gating, retry, auth error code, timeout, signaled-exit failure, effort forwarding/gating/mapping, `--add-dir` instructions directive, `--skills-dir` gating, `onEvent` runtime-event forwarding), ACP engine (engine resolution, acpx config build, node-version gate), ACP transcript delegation, environment test, UI parse/build-config - **ACP engine lane (default)** (`src/server/acp.ts` + shared `adapter-utils/acpx-engine`): Kimi Code ships an ACP server (`kimi acp`), so `kimi_local` now runs on Paperclip's shared acpx engine by default, matching `claude_local`/`codex_local`/`gemini_local`. The issue-thread transcript streams live (assistant text deltas, tool calls with a `pending`->`completed` status lifecycle) instead of the CLI lane's bursty complete-message output. Registered `kimi_local -> "kimi"` in `ACPX_ADAPTER_AGENT_IDS` and resolved the built-in agent command to `kimi acp`; `execute.ts` dispatches to the ACP executor first with an automatic CLI fallback when ACP prerequisites fail (`engine=acp` requires ACP, `engine=cli` pins the headless lane); `index.ts` falls back to the shared acpx session codec; the UI/CLI delegate `acpx.*` events to the shared acpx transcript parser and event formatter. The headless CLI lane (above) remains as the fallback - **Registration** (one entry each, mirroring existing adapters): server adapter registry + `BUILTIN_ADAPTER_TYPES`, `AGENT_ADAPTER_TYPES` (shared), UI adapter registry + display registry (`Kimi Code`, Moon icon) + capabilities defaults, CLI adapter registry, `Dockerfile` (package copy + `npm install --global @moonshot-ai/kimi-code@latest`), `vitest.config.ts` workspace, `scripts/release-package-manifest.json` - **Behavioral sets** mirroring `gemini_local` (Kimi resumes sessions the same way): `GIT_SENSITIVE_LOCAL_ADAPTER_TYPES`, `SESSIONED_LOCAL_ADAPTERS` (heartbeat + recovery), `REMOTE_MANAGED_ADAPTERS`, ssh/sandbox execution-target allow-lists, `ADAPTER_DEFAULT_RULES_BY_TYPE` (`timeoutSec: 0`, `graceSec: 15`), and `LEGACY_SESSIONED_ADAPTER_TYPES` + `ADAPTER_SESSION_MANAGEMENT` in adapter-utils - **UI touch-points**: New Agent default-model branch, AgentConfigForm command map (`kimi_local: "kimi"`) + model defaults + a Kimi-specific thinking-effort option list (`Low`/`High`/`Max`, reflecting Kimi's tiers rather than borrowing Claude's), OnboardingWizard (command map, model default, `kimi login` / `KIMI_MODEL_NAME + KIMI_MODEL_API_KEY` auth hints, manual-debug command line), InviteLanding enabled adapters - **Control-plane skill install** (`cli/src/commands/client/agent.ts`): `paperclipai agent local-cli` seeded the Paperclip control-plane skills into `~/.codex/skills` and `~/.claude/skills` so Codex/Claude agents auto-discover the API reference every run. Kimi had no equivalent target, so `kimi_local` agents began each session without the control-plane skill and rediscovered routes (e.g. the company-scoped `POST /api/companies/{companyId}/issues`) by trial and error. Added `~/.kimi-code/skills` (honoring `KIMI_CODE_HOME`) as a third install target for parity. Independent of the per-run `--skills-dir` delivery, which only applies to explicitly configured skills. - **Docs**: `docs/adapters/kimi-local.md` (prerequisites, auth options, config fields including `effort`, session resume, instruction bundle, skills delivery, control-plane skill install) + a row in `docs/adapters/overview.md` Out of scope (deliberately): model profiles, built-in agent `allowedAdapterTypes` additions. ## Verification\n\nCurrent-master rebase verification (OpenAI Codex, 2026-08-03): 13 focused files / 231 tests pass; adapter-utils, server, UI, CLI, and Kimi adapter typechecks pass; full repository build and UI token gates pass. The branch is conflict-free against master at head `1249df117c5e12e5771b9a570a6340866450619e`.\n\nAutomated (all from repo root, pnpm 9.15.4, Node 22): - `vitest run packages/adapters/kimi-local`: 89/89 pass (includes coverage for the instruction `--add-dir` directive, effort forwarding/gating/mapping, `--skills-dir` gating, the signaled-exit failure path, and `onEvent` runtime-event forwarding with cross-chunk line buffering) - `vitest run server/src/__tests__/adapter-registry.test.ts server/src/__tests__/adapter-routes.test.ts server/src/services/heartbeat-stop-metadata.test.ts ui/src/adapters/adapter-display-registry.test.ts`: 37/37 pass - `vitest run cli/src/__tests__/skills.test.ts`: 13/13 pass (the control-plane skill install target follows the existing Codex/Claude install path, whose symlink logic is unchanged) - `vitest run packages/shared`: 307/307 pass; `vitest run packages/adapter-utils`: pass except one pre-existing, unrelated failure (`mcp-isolation.integration.test.ts` requires Claude CLI ≥ 2.1.207; host has 2.1.185, fails identically on unmodified master) - `pnpm --filter @paperclipai/adapter-kimi-local typecheck|build`, plus typecheck of `server`, `ui`, `cli`, `adapter-utils`: all clean - `pnpm install --frozen-lockfile`: passes (the PR diff itself contains no lockfile changes, per repo policy; verified against a locally regenerated lockfile) - `node scripts/check-no-git-push.mjs` and `node scripts/check-forbidden-tokens.mjs`: pass - CI note: the `policy` job's release-bootstrap step is expected to stay red until a maintainer bootstraps the first npm publish of `@paperclipai/adapter-kimi-local`; see the CI Note for Maintainers comment. All other contributor-actionable checks are green. Manual end-to-end (real Kimi CLI 0.27.0, OAuth login, dev server on an isolated instance): 1. Server `GET /api/adapters` lists `kimi_local` as builtin with correct capability flags; models endpoint returns the three Kimi models 2. `POST .../adapters/kimi_local/test-environment`: all checks pass, including a live `kimi -p` hello probe 3. Created a `kimi_local` agent and invoked two heartbeats: run 1 spawned `kimi -p ... --output-format stream-json`, Kimi used its `Read` tool, produced the expected answer, and the session id was captured from the `session.resume_hint` meta event; run 2 resumed the **same** Kimi session (`sessionIdBefore == sessionIdAfter`) via `-r` 4. UI: adapter appears in the New Agent dropdown; selecting it shows the Kimi command placeholder, the three models, and the Kimi config fields; the run transcript renders Kimi tool calls via the adapter's stdout parser The instruction-bundle, thinking-effort, and `--skills-dir` changes landed after the manual run above. They are covered by the unit tests listed under Automated, and the Kimi CLI flags they rely on (`--add-dir`, `--skills-dir`, `KIMI_MODEL_THINKING_EFFORT`) were confirmed against the installed Kimi Code CLI 0.27.0 (`kimi --help`, config-file thinking-effort docs). Screenshots (assets branch on the fork, not part of the diff):       ## Risks - Low risk to existing behavior: the change is additive, one new workspace package plus single-entry registrations alongside existing adapters; no existing adapter code paths are modified. - The adapter invokes the locally installed `kimi` CLI; like other local adapters, run behavior depends on the host's Kimi version. The parser is written against the documented/verified 0.27.0 stream-json schema and degrades gracefully (malformed lines are skipped, failures surface as run errors). - `--skills-dir` overrides Kimi's auto-discovery of user and project skills for the run. This is intentional (paperclip-managed agents get a reproducible, isolated skill set), and it is only passed when at least one Paperclip skill is desired, so unconfigured agents keep default discovery. - Thinking effort is only forwarded to models that advertise `support_efforts` (currently `kimi-code/k3`); `EFFORT_CAPABLE_MODELS` must be extended when more Kimi models gain support, otherwise a configured effort is silently ignored for them. - `Dockerfile` now installs `@moonshot-ai/kimi-code@latest` globally alongside the other agent CLIs, so image size increases slightly. - Maintainer action needed for the npm bootstrap gate: the `policy` job's release-bootstrap step fails until the first npm publish of `@paperclipai/adapter-kimi-local` (the gate from #5146 that every new adapter package has passed through). Enrollment with `publishFromCi: true` is required by the manifest validator (dropping the entry, `false`, or `private` are all rejected), so this is intentionally left to a maintainer. Remaining CI lanes are expected to run once it is done. ## Model Used\n\n- **Current-master rebase, conflict adaptation, and registry-parity coverage:** OpenAI, **GPT-5 Codex** (Codex agent; exact serving model ID and context-window size were not exposed to the runtime), with repository, shell, Git, and GitHub tooling. It preserved Hawik’s commit authorship, reconciled ACPX and environment-capability changes, added current registry tests, and ran the verification above.\n- **Adapter implementation and initial review:** Moonshot AI, **Kimi K3 Coding** (latest), via **Kimi Code CLI v0.27.0** (`kimi-code/k3` alias, 1M-token context window, thinking mode, agentic tool use). The CLI agent explored the repo, wrote the adapter implementation (delegated to a coder sub-agent of the same model), ran tests, and drafted the first version of this PR body. A second model-driven review pass (read-only, same model) audited the diff for security/correctness before submission; its findings (shell-quoting hardening, auth-detection false positive, session-compaction registration, test gaps) were fixed and are included. - **Harness-context fixes and review responses:** Anthropic, **Claude Opus 4.8** (`claude-opus-4-8`) via Claude Code. Diagnosed from run logs that Kimi received only the entry instructions file (not the `HEARTBEAT.md`/`SOUL.md`/`TOOLS.md` bundle) and that `effort` was never wired, then implemented the instruction `--add-dir` delivery, `KIMI_MODEL_THINKING_EFFORT` forwarding, and `--skills-dir` skill delivery, added the accompanying tests and docs, and addressed the automated review comments (preserving external skills on remote sync, treating a signaled exit as a failure). Also extended the `paperclipai agent local-cli` installer to seed the control-plane skills into `~/.kimi-code/skills` for Codex/Claude parity, wired `onEvent` runtime events so the issue-thread activity indicator reflects Kimi's tool and reasoning output live, and built the ACP engine lane (`kimi acp` via the shared acpx engine, default) so the transcript streams with live tool status like the other ACP adapters. The Kimi CLI flags, subcommand, and env var relied on here were verified against the installed Kimi Code CLI 0.27.0. - All CLI behaviors claimed here (`-p`, `--output-format stream-json`, `-r` resume, event shapes, `--add-dir`, `--skills-dir`, `KIMI_MODEL_THINKING_EFFORT`) were verified empirically against the installed Kimi CLI, not assumed. ## 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 *(only the release-bootstrap step remains red, pending the maintainer npm publish described in Risks)* - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups *(will address all Greptile comments as they arrive)* - [x] I will address all Greptile and reviewer comments before requesting merge --- ## Maintainer Addendum (2026-08-20) The shared acpx-engine and issue-chat changes (run-summary segmentation, placeholder tool-event coalescing, `ISSUE_CHAT_TRANSCRIPT_MAX_VISIBLE_ENTRIES` 30 → 400, live-reasoning UI) have been **extracted to #11761** so the cross-adapter behavior changes review and revert independently — both commits there preserve @hawikk's authorship. This PR is now the kimi-specific adapter only (60 files, +3,793/−8, essentially pure addition); the only shared-engine touch left is the `kimi acp` command resolution. `publishFromCi` is `true` — the package name is bootstrapped on npm. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Dotta <bippadotta@protonmail.com> Co-authored-by: Devin Foley <devin@paperclip.ing> |
||
|
Devin FoleyandHawik
|
1d0e826767 |
feat(acpx/ui): adapter-declared capabilities for verbose streaming backends (no behavior change by default) (#11761)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Local agent adapters stream their work through the shared acpx engine, which every ACP adapter (`claude_local`, `codex_local`, `gemini_local`, custom ACP) runs on > - Verbose streaming backends break two shared-engine behaviors: the auto-posted run summary concatenates every text delta including the thought stream (a long multi-tool run once auto-posted a ~50k character monologue as an issue comment), and token-by-token tool-argument streaming floods the run log with tens of thousands of placeholder-titled in-progress events per run > - These fixes were developed inside the `kimi_local` adapter PR, where Kimi Code's streaming volume (~16,000 text deltas per run vs ~290 for a comparable Claude run) surfaced both problems > - Changing this behavior for all adapters at once is a fleet-wide risk, and hardcoding adapter identities in shared code does not scale to many adapters (or work at all for externally-shipped plugin adapters) — so the behaviors become invocation-config parameters that an adapter's own acpx config builder sets, with defaults preserving today's behavior byte-for-byte > - The benefit is that the machinery lands fully tested with zero behavior change for existing adapters — pin tests prove it — the engine carries no adapter identities, and any adapter (including `custom_acp` configs for external backends) opts in declaratively ## Linked Issues or Issue Description - Refs #9967 — extracted from the `kimi_local` adapter PR and restructured to be inert by default; the commit preserves the original author's (@hawikk) authorship. **Current behavior** When an acpx-engine run ends without the agent leaving a comment, the auto-posted summary is every streamed text delta concatenated, thought stream included. Backends that stream tool arguments emit tens of thousands of placeholder-titled `in_progress` tool events into the stored run log, pinning the live activity indicator to a generic "tool call". There is no mechanism for an adapter to vary either behavior, and shared code must never branch on adapter identities. **Proposed behavior** Two engine invocation-config parameters, read with behavior-preserving defaults: `summaryStrategy` (`"full"` = existing concatenation, the default; `"lastOutputSegment"` = segment output at tool starts, exclude thought stream, post the last non-empty segment) and `coalescePlaceholderToolUpdates` (`false` = never drop an event, the default; `true` = coalesce placeholder-titled in-progress updates). An adapter opts in from its own acpx config builder — the engine has no per-adapter knowledge, no adapter identity appears anywhere in shared code, and `custom_acp` agent configs can set the same knobs for external verbose backends. **Reason and benefit** Existing adapters are provably unaffected — new pin tests assert the default path's summary and tool-event output byte-for-byte, so any future change that alters behavior for claude/codex/gemini/custom fails the suite. The verbose-backend handling still lands fully tested, activated declaratively by the adapter that needs it (the `kimi_local` adapter PR sets both knobs in its config builder). ## What Changed - `packages/adapter-utils/src/acpx-engine/execute.ts`: the run preparation parses `summaryStrategy` and `coalescePlaceholderToolUpdates` from the invocation config (validated, defaulted); summary accumulation and `emitRuntimeEvent` branch on the prepared values. The default path is the pre-existing code (`textParts.join("")`, no event filtering). `buildAcpxRunSummary` is the exported last-segment strategy. - `packages/adapter-utils/src/acpx-engine/execute.test.ts`: a pin test asserting the default path's exact summary (thought stream included) and full tool-event stream (placeholder-titled in-progress updates present, names restored); opt-in tests for each knob; a `buildAcpxRunSummary` unit test. - `ui/src/adapters/types.ts`: `UIAdapterModule` gains an optional `transcriptPresentation` capability — `maxVisibleEntries` (issue-chat transcript window, default 30) and `liveReasoningView` (`"ticker"` default; `"scrollLog"` renders live reasoning in a scrollable auto-following box with one entry per tool call). - `ui/src/lib/issue-chat-messages.ts` and `ui/src/components/IssueChatThread.tsx`: shared code resolves the hints via `findUIAdapter(adapterType)` with today's defaults as fallback — no adapter identities anywhere. The `scrollLog` rendering component ships here but is unreachable until an adapter declares it. - `ui/src/lib/issue-chat-messages.test.ts`: a capability test registers a synthetic verbose adapter and asserts the wider window; the pre-existing test keeps pinning the default 30-entry window. No adapter declares any of this in this PR — every adapter renders and summarizes exactly as before, and there is no per-adapter data anywhere. The `kimi_local` adapter PR (#9967, stacked on this branch) is the first consumer: it declares `transcriptPresentation` in its own UI module and sets the engine knobs in its own acpx config builder. ## Verification - Engine suite: 130/130 pass (126 existing + 4 new); chat suites (`issue-chat-messages`, `IssueChatThread`, `RunChatSurface`): all pass including the new synthetic-adapter capability test — 237 tests across the touched surfaces - The pin tests are the regression guard: the engine test encodes today's summary text and tool-event sequence for a default-config run, and the existing 30-entry-window test pins the default transcript window, so "nothing changed for Claude/Codex users" is an executable assertion, not a review judgment - `pnpm --filter @paperclipai/adapter-utils --filter @paperclipai/ui typecheck`: clean ## Risks - Low: with no adapter setting the knobs, every code path taken in production is the existing one. The only behavioral surface is additive (an unused strategy and an unused filter), exercised by tests. - The knobs are ordinary invocation-config keys, so a `custom_acp` agent config can also set them — intended: an external verbose backend gets the same handling without code changes. Both knobs only affect that agent's own run summaries and run-log verbosity. ## Model Used - Original implementation authored in #9967 by @hawikk (models documented there: Moonshot AI Kimi K3 Coding via Kimi Code CLI 0.27.0, OpenAI GPT-5 Codex, Anthropic Claude Opus 4.8). The commit preserves that authorship. - Extraction, restructuring into config-declared parameters, pin tests, and verification: Anthropic, **Claude Fable 5** (`claude-fable-5`) via Claude Code, with repository, shell, and Git tooling. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Hawik <davapa@gmail.com> |
||
|
Nicky LeachandPaperclip
|
faab2620ad |
feat(sandbox): add a duplex transport for Daytona behind a default-off kill switch (#11750)
## Thinking Path > - Paperclip is an open source app that manages AI agents for work > - Paperclip runs agents in local and remote sandbox environments > - A sandbox needs a bounded channel for commands and asynchronous input > - Daytona needs a real pseudo-terminal transport for this channel > - The sandbox gateway also needs a mode that handles channel loss safely > - This pull request adds the Daytona transport and gateway mode behind a default-off kill switch > - The benefit is a tested foundation for later transport selection ## Linked Issues or Issue Description **Subsystem affected** Cross-cutting (multiple of the above): sandbox providers, plugin SDK, server settings, and shared types. **Problem or motivation** The merged sandbox protocol has no runtime transport for Daytona. The generated sandbox gateway also has no duplex mode. A later transport-selection change needs both parts and a safe per-run gate. **Proposed solution** Add a Daytona `duplexCommandStream` transport over a raw pseudo-terminal. Add a generated gateway mode named `duplex_v1`. Add the `enableSandboxDuplexBridge` setting with a default value of `false`. Keep transport selection disabled until a later pull request. **Alternatives considered** Keep the protocol unused until the transport-selection change. This would delay provider tests and leave the gateway path without direct coverage. **Roadmap alignment** This change supports the completed Roadmap item for cloud and sandbox agents. It extends the merged sandbox channel foundation in pull request #11738. **Additional context** The Daytona provider remains an untrusted boundary. Deployments must use least-privilege provider credentials and provider-side quota controls. Operators must name an owner for duplex telemetry retention before rollout. ## What Changed - Add the Daytona `duplexCommandStream` capability over a raw pseudo-terminal. - Add a launch wrapper that disables echo and newline translation for NDJSON frames. - Close channels on lease release, destroy, resume of a stopped worker, and worker shutdown. - Declare the capability in the Daytona manifest and set `PLUGIN_VERSION` to `0.1.5`. - Add the worker-to-host notification sink at `ctx.duplexChannel.data` and `ctx.duplexChannel.exit`. - Add the generated sandbox gateway mode `PAPERCLIP_API_BRIDGE_MODE=duplex_v1`. - Add channel-loss results of `409 outcome_indeterminate` and `503 bridge_unavailable`. - Add the per-run setting `enableSandboxDuplexBridge`, with a default value of `false`. - Add unit tests, generated-source codec tests, lifecycle tests, and a credential-gated live Daytona test. ## Verification - Daytona suite: 185 tests pass. - Adapter utilities: 754 tests pass and 4 tests skip. - Plugin SDK: 62 tests pass. - Shared package: 28 tests pass. - Server duplex tests pass. - Shared, plugin SDK, server, and Daytona TypeScript checks pass. - The live Daytona test passes 3 cases when `DAYTONA_API_KEY` is set. - The live Daytona test skips 3 cases without `DAYTONA_API_KEY`. - CI must run the full workspace typecheck, test, and build gates after PR creation. ## Risks - The Daytona control plane and pseudo-terminal remain untrusted boundaries. - The duplex gateway changes behavior only when the mode and per-run setting enable it. - A lost channel fails requests without replay, so callers must handle indeterminate outcomes. - The transport-selection change must require both `duplexCommandStream === true` and `enableSandboxDuplexBridge === true`. - The provider credential and quota limits need operator control before rollout. ## Model Used OpenAI Codex, GPT-5, tool use and code execution. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
d5416fde9a |
fix(runtime): route sandbox git-bundle export through native syncOut (#11749)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Sandbox-managed runtimes move files between a host and an isolated sandbox > - The git-bundle export path reads the full bundle into host memory > - The workspace restore path already uses the provider native `syncOut` transfer > - This pull request uses `syncOut` for bundle export and keeps `readFile` as a fallback > - The change reduces host buffering and exposes the transfer to provider tracing ## Linked Issues or Issue Description **What happened?** The sandbox git-bundle export used `client.readFile` even when the provider supported native `syncOut`. The path buffered the full bundle in host memory and used a chunked base64 command loop. **Expected behavior** The export should use one confined native file transfer when the provider supports `syncOut`. Providers without that capability should keep the existing `readFile` fallback. **Steps to reproduce** 1. Prepare a sandbox-managed runtime with native `syncOut` support. 2. Export the sandbox git bundle. 3. Inspect the sync operations and file reads. 4. Confirm that the bundle uses one native file mapping and that the status file still uses `readFile`. **Paperclip version or commit** Commit `3fc88d14be7133867897e87838276e15aba675fd`. **Deployment mode** Built from source. The change applies to sandbox-managed runtime execution. **Additional context** This PR contains a focused bug fix. No public issue matched this change during the duplicate search. ## What Changed - Route bundle export through `nativeSyncOut` when the provider supports it. - Keep the existing `readFile` fallback for providers without native sync support. - Confine the native bundle mapping with `assertSyncOperationsConfined`. - Cover the native path, fallback path, retry path, and confinement checks with unit tests. ## Verification - `npx vitest run packages/adapter-utils/src/sandbox-managed-runtime.test.ts` — 52 tests passed. - `npx tsc --noEmit` in `packages/adapter-utils` — passed. - The PR CI workflow must pass before merge. ## Risks Low risk. The native path runs only when the provider advertises `syncOut`. The existing `readFile` path remains available as a fallback. Native transfer progress reports only start and finish events. ## Model Used OpenAI GPT-5 Codex. Exact runtime model ID: GPT-5 Codex. Context window: not exposed in this run. Capabilities used: code review, shell tools, GitHub CLI, and Paperclip API coordination. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
8161244284 |
feat(sandbox): add opt-in duplex command-stream foundation (capability, protocol, bounded host route, frame codec) (#11738)
## Thinking Path > - Paperclip provides a control plane for companies that run AI agents. > - Sandboxed agents need a safe execution path for persistent command streams. > - The existing callback transport does not provide a bounded, generic duplex route. > - The host must control capability access, route identity, protocol limits, and close behavior. > - This pull request adds an opt-in duplex command-stream foundation across the sandbox layers. > - The feature stays inert because no current provider declares the capability. ## Linked Issues or Issue Description **Subsystem affected** Cross-cutting (multiple of the above) **Problem or motivation** Sandbox command execution needs a persistent host-to-sandbox stream. The current callback bridge uses a file transport and does not provide this generic route. **Proposed solution** Add a fail-closed provider capability, generic worker protocol messages, a host-owned bounded route, cross-layer service mediation, and a versioned newline-delimited frame codec. **Alternatives considered** Keep the file transport and add feature-specific commands. This does not provide one reusable duplex contract or host-owned route bounds. **Roadmap alignment** This work supports the completed Cloud / Sandbox agents roadmap area and the safe autonomy goal in the product definition. **Additional context** The change passed a two-stage security review. The final code review verdict was approve after fixes for active-stream bounds and service-layer capability mediation. ## What Changed - Add the opt-in `duplexCommandStream` provider capability with fail-closed narrowing. - Add duplex open, write, stop, and close requests and data and exit notifications to the plugin worker protocol. - Add a host-owned route with bounds for chunk size, cumulative bytes, lifetime, protocol errors, pending requests, and pre-bind buffering. - Add close acknowledgement handling with worker retirement when the close remains unconfirmed. - Wire `openDuplexChannel` through the execution target, runtime service, and plugin worker. - Add a versioned frame codec with shared wire-compatibility vectors and split UTF-8 handling. ## Verification - `server/src/__tests__/plugin-worker-manager-duplex.test.ts` passes 18 tests. - `server/src/__tests__/environment-execution-target-duplex.test.ts` passes 11 tests. - `packages/adapter-utils/src/duplex-frame-codec.test.ts` passes 38 tests. - `server/src/__tests__/sandbox-capability-contract.test.ts` passes 15 tests. - Setup-token pseudo-terminal regression tests pass 47 tests. - Server TypeScript check passes. - Continuous integration will run the full required test, typecheck, build, and policy checks. ## Risks - Providers that opt into the capability must implement the complete worker protocol. - Route limit defaults can close a stream when a workload exceeds the configured bounds. - The capability remains disabled for current providers, so current production behavior does not change. ## Model Used OpenAI GPT-5 (`gpt-5`), with tool use and code execution. The model reviewed and prepared this pull request from the supplied implementation and verification record. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
233be4b36c |
feat: parallelize sandbox file-sync behind a provider opt-in capability (#11736)
## Thinking Path > - Paperclip runs AI agents through local and remote execution adapters. > - Sandbox providers move workspace and asset files before and after agent runs. > - Serial file transfers delay startup and teardown when several operations do not depend on each other. > - Providers need an opt-in contract so existing providers keep their serial behavior. > - This pull request adds a bounded scheduler and routes inbound and outbound sync operations through it. > - The benefit is shorter sandbox setup and teardown with stable errors, clear telemetry, and a safe opt-in path. ## Linked Issues or Issue Description **Subsystem affected** Cross-cutting (multiple of the above): packages/shared, packages/adapter-utils, packages/plugins, and server. **Problem or motivation** Sandbox sync processes the workspace, assets, and referenced projects in series. This adds avoidable wait time to agent startup and teardown. **Proposed solution** Add a fail-closed provider capability named concurrentSyncOperations. Use a bounded scheduler with a limit of four operations. Preserve operation order for error reporting. Keep non-opted-in providers on the serial path. **Alternatives considered** Increase the serial transfer speed or add provider-specific schedulers. Those options do not provide one shared contract or stable behavior across providers. **Roadmap alignment** ROADMAP.md lists cloud and sandbox agents as a product area. This change improves sandbox execution without changing the control-plane contract. **Additional context** The Daytona provider opts in. Board trials on this commit showed overlap for inbound sync and outbound restore, with no referenced-project staging failures. ## What Changed - Add the concurrentSyncOperations sandbox capability and fail-closed parsing. - Add a bounded settle-all scheduler with stable input-order errors. - Parallelize inbound workspace, asset, and referenced-project sync operations when the provider opts in. - Parallelize outbound workspace and asset restore operations when the provider opts in. - Surface referenced-project failure text in run logs and server telemetry. - Add Daytona sync spans and the capability declaration. - Preserve in-flight upload scratch tarballs during workspace wipe. - Add unit and regression tests for the scheduler, coordinators, provider behavior, telemetry, and wipe race. ## Verification - Run the adapter-utils and server type checks. - Run the targeted adapter-utils, server, and Daytona test suites. - Run the full automated sweep. - Review six cold Daytona trials, with three serial and three parallel runs. - Confirm that parallel trials show inbound overlap and outbound restore overlap. - Confirm that providers without the capability keep serial behavior. ## Risks - Providers must opt in only when their file operations can run safely at the same time. - A provider that declares the capability incorrectly can expose transfer races. - The scheduler keeps a limit of four to bound resource use. - Providers without the capability keep the prior serial behavior. ## Model Used OpenAI GPT-5 in the Codex runtime. The model used tool calls, code inspection, and GitHub workflow support. The model did not author the implementation commits. ## 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 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
e0b64529b3 |
feat(auth): normalize agent login in the sandbox onto one session table and a capability contract (#11730)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Sandbox agents need a safe login path for each supported adapter > - Codex device login and Claude setup-token login used separate session stores and route logic > - Separate stores made session lookup, expiry, and login capability checks harder to keep consistent > - This pull request unifies both flows on one session table and one capability contract > - The benefit is one company-scoped login model with public session identifiers and shared lifecycle rules ## Linked Issues or Issue Description **Subsystem affected** Cross-cutting (multiple of the above) **Problem or motivation** Codex and Claude sandbox login used separate session stores and different route paths. This split increased the risk of inconsistent company scoping, session lookup, and cleanup. **Proposed solution** Use `adapter_auth_sessions` for both login flows. Use public session identifiers for API access. Select login behavior from projected adapter capability data. Share the route spine, lease arguments, runner lifecycle, and reaper rules. **Alternatives considered** Keep two session tables and add matching fixes to both routes. This keeps duplicate logic and does not provide one capability contract, so this pull request uses shared infrastructure. **Roadmap alignment** This change supports the shipped Cloud / Sandbox agents milestone in `ROADMAP.md`. ## What Changed - Unify Codex device login and Claude setup-token login on `adapter_auth_sessions`. - Return and look up sessions with company-scoped public session identifiers. - Enforce one active session for each company, owner, and adapter. - Share the login route spine, sandbox lease arguments, runner lifecycle, and missing-auth check. - Add a standalone setup-token reaper with adapter-specific row selection. - Add optional login capability projection for adapters and drive route and UI selection from that data. - Rename the provider flag to `supportsLoginPty` and validate its deprecated alias. - Remove the old Claude setup-token session table and add the required migrations. ## Verification - Server typecheck passed with `tsc`. - Database typecheck passed. - UI typecheck passed with `tsc -b`. - Codex login service and route suites passed. - Setup-token session, route, and reaper suites passed. - Adapter session schema, plugin validator, capability projection, UI render, and Daytona suites passed. - GitHub Actions must confirm the complete CI gate after pull request creation. ## Risks - The migrations remove short-lived in-flight login rows during deployment. A login that spans the migration can continue until its provider lease expires. - The Codex credential store remains company-scoped. A cross-owner credential race remains a documented, board-accepted risk. - API clients that use internal session row identifiers no longer work. The API accepts only public session identifiers. ## Model Used Codex, GPT-5, exact runtime model ID not exposed in this handoff, large context window, reasoning, and repository tool use. The implementing engineer produced the code with AI assistance. ## 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 <noreply@paperclip.ing> |
||
|
Devin Foley
|
393da0f67c |
fix(adapter-utils): graft unrelated imported histories instead of failing the run (#11638)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - At run finalize, the host imports the sandbox git history and reconciles it with the local worktree in `integrateImportedGitHead` > - Transported workspaces are depth-1 shallow clones, so the boundary commit reads as parentless inside the sandbox > - A `git commit --amend` there rewrites the boundary commit into a root commit, and the re-imported history no longer connects to the host history > - `git merge-tree` has no common base to merge against, so the sync throws "Failed to merge concurrent remote git histories" and the run fails with its work stranded in the sandbox > - This pull request grafts the imported tree onto the current head as a single commit instead of failing > - The benefit is that a history rewrite inside the sandbox can no longer lose a run's work ## Linked Issues or Issue Description No existing issue found. I searched issues and PRs for "unrelated histories", "Failed to merge concurrent", and "shallow". Depends on #11637 (merged; the graft commit reuses its identity constant). This PR is now rebased onto `master`. **What happened?** An agent run amended a commit inside its sandbox workspace to address review feedback. The sandbox clone is depth-1 shallow, so git treated the boundary commit as parentless and the amend produced a root commit. At finalize, the host-side sync failed with `Failed to merge concurrent remote git histories for <sha>` and the run was marked failed. A follow-up run had to repair the branch by hand: fetch the true parent from origin and rebuild the commit with `git commit-tree`. **Expected behavior** The sync must never strand completed work. When the imported history shares no ancestor with the local one, the imported tree should still land on the current head, with the imported message preserved and the graft recorded. **Steps to reproduce** 1. Start a run whose workspace transport uses the shallow clone path (`withShallowGitWorkspaceClone`, depth 1). 2. Inside the sandbox workspace, run `git commit --amend` on the boundary commit. The result is a parentless root commit. 3. Finish the run. The host-side `integrateImportedGitHead` finds no merge base, `merge-tree` fails, and the run fails. ## What Changed - `git-workspace-sync.ts`: new exported `createUnrelatedHistoryGraftCommit` helper. It reads the imported head's tree and message, and creates one commit on top of the current head with the deterministic sync identity and a trailer that records the graft and both shas. - `integrateImportedGitHead` (both the remote-git-sync version and the SSH copy in `ssh.ts`): when `merge-base` reports no common ancestor, graft instead of throwing. The ref update keeps the same compare-and-swap and concurrent-retry semantics as the merge path. - The graft is gated on `git merge-base` exiting with status 1 — the no-ancestor signal. Operational failures (timeout, missing object, repository error) keep the loud merge failure instead of rewriting the tip. - New regression tests: one builds the exact shallow-amend shape (a root commit rebuilt from the base tree) and asserts the graft lands on the current head with the imported tree, subject, and graft trailer; one integrates a well-formed sha the repository does not hold and asserts the integration still throws with the branch tip unchanged. ## Verification - `pnpm vitest run packages/adapter-utils/src/git-workspace-sync.test.ts` — 19/19 pass (includes the new graft test and the merge-base failure-discrimination test). - `pnpm --filter @paperclipai/adapter-utils typecheck` — clean. - Full `pnpm vitest run packages/adapter-utils`: every file passes except `local-process-sandbox.test.ts`, which fails identically on an untouched `master` checkout on macOS (bubblewrap-dependent, pre-existing, unrelated). ## Risks - Behavioral shift: unrelated imported histories previously failed the integration; now they land as a squash-graft. In this degenerate case there is no base to merge against, so the imported tree is taken wholesale and concurrent local-only tree changes are superseded at the tip. The local commits keep their place in the graft's ancestry, and the trailer records both shas, so nothing is unrecoverable. The old behavior lost the imported work instead, which is the worse failure for an autonomous run. - The graft reuses the imported head's commit message, so branch history still reads naturally after a sandbox rewrite. ## Model Used - Claude Fable 5 (`claude-fable-5`), extended thinking, via Claude Code CLI (tool use for code exploration, test runs, and verification). ## 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 (no doc surface describes this internal sync path) - [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 |
||
|
Devin Foley
|
9ea8143c87 |
fix(adapter-utils): give sync-created merge commits a deterministic git identity (#11637)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Runs execute in transported workspaces; at finalize, the host syncs the sandbox git history back into the local worktree > - When both sides advanced, `integrateImportedGitHead` reconciles them with `git merge-tree` plus `git commit-tree` on the host > - Execution hosts are often containers with no git config and no resolvable hostname, so `commit-tree` fails with "Author identity unknown" > - That one local command failure marks the whole run as failed, even though the run's work succeeded > - This pull request gives sync-created merge commits an explicit, deterministic identity at the call site > - The benefit is that workspace finalize no longer depends on ambient host git configuration ## Linked Issues or Issue Description No existing issue found. I searched issues and PRs for "Author identity unknown", "unrelated histories", and "commit-tree identity". **What happened?** A run finished its work, but workspace finalize failed. The host-side sync ran `git commit-tree <tree> -p <localHead> -p <importedHead> -m "Paperclip remote git sync merge <sha>"`. Git exited with `Author identity unknown ... fatal: unable to auto-detect email address (got 'node@<container-id>.(none)')`. The adapter recorded the whole run as failed, and the host worktree kept the stale head. Any container deployment without a global gitconfig reproduces this; I observed it on a Paperclip Cloud stack. **Expected behavior** Commits that the sync machinery itself creates must not depend on ambient host git configuration. The merge commit is machine-authored, so it should carry a deterministic Paperclip identity. **Steps to reproduce** 1. Run the Paperclip server in a container with no `user.name`/`user.email` git config and a hostname git cannot turn into an email. 2. Let a run's sandbox branch diverge from the host worktree, so both sides advance. 3. Workspace finalize calls `integrateImportedGitHead`. The `git commit-tree` step fails with "Author identity unknown" and the run fails. ## What Changed - `git-workspace-sync.ts`: new exported `GIT_SYNC_COMMIT_IDENTITY_ARGS` (`-c user.name=Paperclip -c user.email=noreply@paperclip.ing`), applied to the `commit-tree` call in `integrateImportedGitHead`. - `ssh.ts`: the SSH-sync copy of `integrateImportedGitHead` applies the same identity args to its `commit-tree` call. - New regression test: builds divergent histories in a repo with no configured identity and asserts the sync merge commit is created with the deterministic identity, correct parents, and merged tree. ## Verification - `pnpm vitest run packages/adapter-utils/src/git-workspace-sync.test.ts` — 18/18 pass. - `pnpm --filter @paperclipai/adapter-utils typecheck` — clean. - Negative proof: with the source fix stashed, the new test fails on the identity assertion. - Full `pnpm vitest run packages/adapter-utils`: every file passes except `local-process-sandbox.test.ts`, which fails identically on an untouched `master` checkout on macOS (bubblewrap-dependent, pre-existing, unrelated). ## Risks Low risk. The change only adds `-c` identity flags to two machine-generated commit invocations. `GIT_AUTHOR_*` / `GIT_COMMITTER_*` environment variables still take precedence over `-c` when an operator sets them, so existing deployments that configure an identity keep their behavior. ## Model Used - Claude Fable 5 (`claude-fable-5`), extended thinking, via Claude Code CLI (tool use for code exploration, test runs, and verification). ## 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 (no doc surface describes this internal sync path) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
Nicky LeachandPaperclip
|
b446ff59bf |
refactor(acpx-engine): coordinator-owned ACP run lifecycle with a typed resource ledger (#11576)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent adapters run agent sessions through the ACPX engine > - The ACPX engine handled one run attempt as a long implicit procedure > - That shape made resource ownership, cleanup order, and failure behavior hard to verify > - This pull request gives the attempt a coordinator, a typed resource ledger, separate run sites, and explicit turn and settlement sequences > - The benefit is clear ownership, one cleanup path, safer session reuse, and testable failure behavior ## Linked Issues or Issue Description **What existing behavior does this improve?** The ACPX engine manages startup, turn execution, session reuse, and cleanup inside one large run procedure. **Current behavior** The run procedure owns several resources through implicit control flow. Cleanup and session reuse behavior depend on lane-specific branches and error paths. **Proposed behavior** The coordinator owns the run attempt. A typed ledger records six resources and their states. Host and sandbox run sites own lane-specific acquisition. Turn and settlement sequences expose typed outcomes. The engine emits allowlisted phase telemetry. **Reason and benefit** Explicit ownership makes cleanup and failure behavior easier to inspect. The fault matrix and characterization tests protect the external result while the refactor reduces hidden control flow. **Breaking changes** None to the public adapter contract. The host warm-save path now closes and relaunches the runtime because a transferred runtime could retain a run-scoped credential. A cold session-handshake failure now closes the created runtime. **Additional context** This pull request contains the ACPX engine lifecycle refactor, its tests, and the lifecycle document. ## What Changed - Add a run coordinator for startup, turn execution, settlement, and result reproduction. - Add a typed resource ledger with open, sealed, and consumed states. - Add host and sandbox run sites for lane-specific resource acquisition. - Replace separate runtime maps with a generic session reuse store. - Split session fingerprint identity from the outer session key. - Add typed turn and settlement sequences with one cleanup owner. - Add a closed allowlist for phase telemetry. - Add characterization tests and a 17-case fault matrix. - Add `doc/acp-run-lifecycle.md`. ## Verification - `npx vitest run packages/adapter-utils/src/acpx-engine/` passes 18 files and 286 tests at the submitted commit. - `pnpm --filter @paperclipai/adapter-utils typecheck` reports 0 errors at the submitted commit. - Run the full pull request checks after GitHub starts CI. - Run Greptile review after the pull request opens. ## Risks - The refactor changes internal control flow across the ACPX engine. - Host warm-save behavior now closes and relaunches the runtime. - Settlement changes the handling of a cold session-handshake failure from a leak to a close. - The characterization baselines and fault matrix reduce the risk of an external behavior change. > Paperclip is the open source app people use to manage AI agents for work > The adapter layer runs agent sessions through the ACPX engine > The engine needs explicit lifecycle ownership for reliable cleanup > This pull request adds coordinator-owned phases and a typed resource ledger > The result makes lifecycle behavior easier to test and review ## Model Used OpenAI GPT-5 Codex. Exact model ID: GPT-5. The model used tool execution, repository inspection, and code review support. The implementation author supplied the submitted code. ## 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 <noreply@paperclip.ing> |
||
|
|
1c366a9059 |
fix(server): reject invalid agent credentials instead of downgrading to the local user actor (#11589)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server authenticates each agent request in `actorMiddleware` before it attributes chat comments > - When an agent bearer token failed verification, the middleware called `next()` with no error and the request continued without an agent actor > - The request then fell back to the local user actor, so the server stored agent replies as user comments > - The task chat UI renders user comments in blue bubbles, so agent messages appeared as blue user bubbles > - This pull request rejects invalid agent credentials with 401 instead of a silent downgrade > - The benefit is that agent messages keep agent attribution, and broken credentials fail loudly with a clear retry message ## Linked Issues or Issue Description **What happened?** A user cancelled an onboarding question card. The agent posted a follow-up reply. The reply appeared in a blue bubble, which the UI reserves for human messages. The agent run held an expired local agent JWT. The auth middleware could not verify the token, called `next()` without an actor, and the request fell back to the local user identity. The server stored the agent comment as a user comment. **Expected behavior** Agent messages always render as agent bubbles. A request with invalid agent credentials must fail with 401 so the adapter can refresh credentials and retry. It must not post content under a human identity. **Steps to reproduce** 1. Start a local Paperclip instance. 2. Give an agent run an expired or malformed agent JWT. 3. Let the agent post an issue comment through the API bridge. 4. Before this change: the comment is stored with the local user identity and renders as a blue bubble. After this change: the request fails with 401 and a message that tells the caller to obtain fresh credentials. ## What Changed - `server/src/middleware/auth.ts`: a bearer token that fails verification now produces a 401 `unauthorized` error instead of a silent fall-through to the anonymous/local-user actor. - The 401 message states the cause: expired token, unverifiable token, empty bearer token, missing agent record, agent record in another company, terminated agent, or agent pending approval. - The API-key path now also rejects an agent record whose company does not match the key. - `packages/adapter-utils/src/execution-target.ts`: the bridge proxy now writes a `comment id: <id>` marker to the run log for each posted issue comment, so misattributed comments can be traced to a run. - `ui/src/components/task-chat/task-chat-adapter.test.ts`: a regression test asserts that a recovered `local-board` comment with a derived agent author renders as an agent bubble, not a user bubble. - `server/src/__tests__/agent-auth-middleware.test.ts` and `packages/adapter-utils/src/execution-target-sandbox.test.ts`: new tests cover each rejection path and the log marker. ## Verification - Run `pnpm vitest run src/__tests__/agent-auth-middleware.test.ts` in `server/` — 14 tests pass. - Run `pnpm vitest run execution-target-sandbox` at the repo root — 44 tests pass. - Run `pnpm vitest run src/components/task-chat/task-chat-adapter.test.ts` in `ui/` — 4 tests pass. - Manual check: post an issue comment with an expired agent JWT; the API returns 401 with a retry message and no comment is stored. ## Risks - Behavioral shift: requests that previously continued as anonymous or local-user actors after a failed agent-token verification now receive 401. Any caller that relied on the silent downgrade must refresh its credentials. This is the intended fix, and the adapters already handle 401 with a credential refresh. - No schema or migration changes. Low risk otherwise. > 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 - Claude (Anthropic), model ID `claude-fable-5`, via Claude Code with extended thinking and tool use (agent harness with shell, file, and git 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 - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
8087661bb8 |
fix: bound workspace Git scans (#11572)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Workspaces let users and agents inspect files that belong to an issue > - Changed-file views use full-tree Git status scans > - Many issue views could start those scans at the same time and make the server unresponsive > - Route-level limits did not protect the process or coalesce work for one repository > - This pull request adds one bounded scheduler for every expensive workspace Git scan > - It also starts browser scans only when the file panel is open and visible > - The benefit is bounded child-process use and responsive health checks during request storms ## Linked Issues or Issue Description **What happened?** Many changed-file requests could start full `git status --porcelain=v1 -z --untracked-files=all` scans at the same time. One production incident produced about 270 direct Git child processes. The Node process stayed alive but stopped answering health requests in time. **Expected behavior** Paperclip must bound expensive Git work across all companies, actors, issues, repositories, and browser tabs. Duplicate requests for one worktree must share work. Excess requests must fail fast with a retryable response. Hidden or closed file panels must not start scans. **Steps to reproduce** 1. Open changed-file views for many issue and actor keys. 2. Send requests for two large workspace roots at the same time. 3. Observe that route-level limiter keys allow many full Git scans to run together. 4. Observe delayed health responses and accumulated Git children. **Paperclip version or commit** Reproduced on master before commit `43ab441f0f`. **Deployment mode** Self-hosted server with local workspace repositories. ## What Changed - Add a process-wide scheduler with configurable concurrency, queue capacity, timeout, and cache TTL. - Add fair admission, a bounded queue, canonical worktree keys, single-flight joins, and bounded result caching. - Add subprocess timeouts, TERM-to-KILL escalation, bounded output, waiter cancellation, and slot cleanup. - Route full-tree status work from file resources, workspace runtime, execution workspaces, and adapter overlay sync through the scheduler. - Return stable retryable `503` and `504` error codes for saturation and timeout. - Add structured logs with safe workspace hashes, durations, queue state, cache use, joins, and terminal outcomes. - Gate UI queries on panel and document visibility. Cancel queries on close, hide, unmount, and workspace change. - Disable focus and reconnect bursts. Keep one explicit refresh action and a retryable unavailable state. - Document the 10-second default freshness tradeoff and all configuration variables. - Add unit, route, UI, adapter, and deterministic 500-request load coverage. ## Verification - `pnpm -r typecheck` - `pnpm build` - `pnpm check:token-gates` - `pnpm --filter @paperclipai/server exec vitest run src/services/workspace-git-operation-scheduler.test.ts src/__tests__/file-resources-git-scan-load.test.ts --reporter=dot` — 16 tests passed. - `pnpm --filter @paperclipai/ui exec vitest run src/components/WorkspaceFileBrowser.test.tsx src/lib/page-visibility.test.ts --reporter=dot` — 38 tests passed. - `pnpm --filter @paperclipai/adapter-utils exec vitest run src/git-workspace-sync.test.ts --reporter=dot` — 16 tests passed. - Existing file-resource, workspace-runtime, and execution-workspace regression selections passed. - Two cleanup safety regressions prove failed scans preserve the worktree before archive and at the final deletion fence. - Before: the incident produced about 270 Git children and health requests timed out. - After: 500 concurrent requests across 500 issue keys, 73 actors, and two roots started two underlying scans. Peak scan concurrency was 2. All 500 requests succeeded. Health p99 was 4.94 ms. The harness found zero unreaped children. - The full local Vitest run passed 4,267 tests. Ten existing fixed-port HTTPS exposure tests could not run because this host already owns Tailnet listeners on ports 42000 and 52000. Clean GitHub CI is the final full-suite result. - Latest-head GitHub CI passed all required test, typecheck, build, canary, e2e, policy, and security gates. - Greptile completed at 5/5 with zero unresolved comments, recommendations, or follow-ups. ## Risks - Changed-file results can be up to 10 seconds old by default. Explicit refresh remains available. - A full queue returns a retryable `503` instead of waiting without a bound. - A scan that exceeds the default 8-second deadline returns a retryable `504` and terminates its process group. - Operators can tune all limits with documented environment variables. Safe defaults protect local and shared servers. > 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, GPT-5 family. The runtime does not expose the exact deployment ID or context-window size. High reasoning, tool use, and code execution were enabled. ## 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 <noreply@paperclip.ing> |
||
|
Jannes StubbemannandClaude Fable 5
|
d77eeb8914 |
fix(sandbox-bridge): allow the agent-hire skill's routes through the callback bridge (#8978)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Managed agents run inside a sandbox and reach the Paperclip server only through the sandbox callback bridge, which forwards a fixed route allowlist (`DEFAULT_SANDBOX_CALLBACK_BRIDGE_ROUTE_ALLOWLIST`) > - The `paperclip-create-agent` skill instructs an agent to call adapter/icon discovery endpoints, compare existing agent configurations, submit a hire request, and link the resulting approval to its source issue > - None of those routes were on the bridge allowlist, so a sandboxed agent following the skill correctly hit `Route not allowed` on every call — including the hire `POST` itself — making hiring impossible from inside a sandbox > - This pull request adds the six routes the skill uses to the bridge allowlist, while keeping direct agent creation (`POST /api/companies/:id/agents`) denied > - The benefit is that hiring works end-to-end for sandboxed agents through the approval-gated `agent-hires` path, without widening the bridge beyond what the skill needs ## Linked Issues or Issue Description No public issue exists; describing the bug in-PR (bug template fields): - **What happened:** A managed agent running in a sandbox followed the `paperclip-create-agent` skill and got `Route not allowed` from the callback bridge on every endpoint the skill documents — adapter discovery (`/llms/agent-configuration.txt`, `/llms/agent-configuration/:adapterType.txt`, `/llms/agent-icons.txt`), config comparison (`GET /api/companies/:id/agent-configurations`), the hire submission (`POST /api/companies/:id/agent-hires`), and approval linking (`POST /api/issues/:id/approvals`). - **Expected behavior:** An agent with hiring permission can complete the hire flow from inside a sandbox; the bridge forwards the skill's routes and the server enforces authorization (`canCreateAgents`). - **Impact:** Hiring by sandboxed agents was fully broken — the failure is in the transport allowlist, not permissions, so no configuration could work around it. Related: #8981 (companion fix making the `paperclip-create-agent` skill available to agents that can hire; supersedes #8823). The two changes serve the same end-to-end hire flow but are independently mergeable — this PR is purely the bridge transport allowlist. Supersedes #8853. ## What Changed - `packages/adapter-utils/src/sandbox-callback-bridge.ts`: add six routes used by the `paperclip-create-agent` skill to `DEFAULT_SANDBOX_CALLBACK_BRIDGE_ROUTE_ALLOWLIST` (three `GET /llms/...` discovery routes, `GET .../agent-configurations`, `POST .../agent-hires`, `POST /api/issues/:id/approvals`), with a comment documenting why direct agent creation stays denied - `packages/adapter-utils/src/sandbox-callback-bridge.test.ts`: assert the six routes are allowed, and add negative cases proving the regexes do not over-match (no `POST .../agents`, no non-`.txt` or arbitrary `/llms` files, no `agent-hires` sub-resources) ## Verification - `npx vitest run packages/adapter-utils/src/sandbox-callback-bridge.test.ts` — 13/13 tests pass locally - `npx tsc --noEmit -p packages/adapter-utils` — clean - Manual: run a managed agent in a sandbox, invoke the `paperclip-create-agent` skill, and confirm the discovery calls, hire `POST`, and approval linking all pass through the bridge; `POST /api/companies/:id/agents` still returns `Route not allowed` ## Risks - Low risk: additive allowlist entries only; anchored regexes with `[^/]+` segments prevent over-matching (covered by tests) - The bridge allowlist bounds surface area but does not replace server-side authorization — the hire `POST` remains approval-gated and permission-checked (`canCreateAgents`) on the server > 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 - Claude (Anthropic) — Fable 5 (`claude-fable-5`), extended thinking, agentic tool use via Claude Code; original diff authored with Claude Opus 4.8 (1M context) ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Nicky LeachandPaperclip
|
c1c46f1e4e |
feat: Claude login on the new-agent page before agent creation (#11347)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Claude local adapter supports subscription login through a sandbox > - The new-agent page must show login before the user creates an agent > - Test results must not expose raw sandbox diagnostics or secret values > - This pull request adds the login UI to both Test lanes and closes the diagnostic boundary > - The branch also adds durable cleanup recovery for failed sandbox teardown > - Reusable sandboxes must retain both their recorded teardown configuration and a valid lifecycle path until destruction succeeds > - The benefit is a usable login flow with fixed public checks, redacted server logs, and recoverable sandbox cleanup ## Linked Issues or Issue Description Related public work: [#9488](https://github.com/paperclipai/paperclip/pull/9488) adds first-class recognition for `CLAUDE_CODE_OAUTH_TOKEN` in headless and remote runs. Related public issue: [#2681](https://github.com/paperclipai/paperclip/issues/2681) requests Claude Code subscription support. This pull request adds the login transport and new-agent UI flow that those changes do not provide. **Subsystem affected:** Claude local adapter, server login probes, sandbox provider setup, cleanup recovery, and the new-agent UI. **Problem or motivation:** The Test lanes did not show the sandbox login panel in all supported cases. Test results also exposed raw probe diagnostics, and JSON escapes could end secret redaction early. **Proposed solution:** Surface the login capability through the bundled provider manifest. Prepare the same probe runtime in the ACP lane. Send diagnostics only to redacted server logs. Keep Test checks on fixed public messages. Normalize login URL hints to allowlisted HTTPS Claude and Anthropic hosts. Consume JSON escapes during redaction. Preserve failed sandbox cleanup state across retries and restarts, and prevent deletion from severing the lifecycle context of a live reusable sandbox. **Alternatives considered:** Keep raw diagnostics in Test checks or trust login URL text from the sandbox. Both choices increase information exposure. Keep separate probe behavior in the ACP lane. That choice would leave the two Test lanes inconsistent. ## What Changed - Surface the sandbox login panel on both Test lanes. - Reconcile the bundled Daytona plugin manifest so `supportsSetupTokenLogin` reaches the UI capability gate. - Prepare the ACP Test lane with the same probe runtime as the CLI Test lane. - Add the `claude_acp_login_probe_unavailable` warning when the ACP probe cannot run. - Send raw sandbox diagnostics only to redacted server logs. - Keep Test checks on fixed public messages in the ACP, managed-config, and CLI paths. - Normalize login URL hints to allowlisted HTTPS Claude and Anthropic hosts. - Redact JSON and escaped-JSON secret values, including escaped quotes and backslashes. - Preserve orphan cleanup records across provider failures, restarts, and unavailable plugins. - Atomically block environment deletion while a live reusable sandbox lease still depends on it. - Verify pending cleanup destroys plugin sandboxes with the provider configuration recorded on the lease, even after the current environment configuration changes. ## Verification - Head under review: `506b7fa2d83c36bfa5fd722ee9d95b0c7431c241`. - Focused environment route/service/runtime coverage passes: 196 tests across 3 files. - `pnpm -r typecheck` passes. - `pnpm build` passes. - The full Vitest run completed with 4,754 passing and 28 failing tests. All 23 source-test failures reproduce unchanged on parent head `58cfe61a33191ce03d965d65085d26064b4888ba`; the other 5 are duplicate executions from stale `server/dist` output. The failures are unrelated macOS path/listener and scheduler-fixture failures, so there is no new bad commit for bisect to localize. - All required CI checks pass for the current head, including build, typecheck/release registry, all server and workspace shards, serialized server suites, canary, and e2e. - A fresh Greptile review for `506b7fa2d83c36bfa5fd722ee9d95b0c7431c241` reports 5/5, “safe to merge,” with no blocking failure remaining. ## Risks - A probe or redaction change could hide useful server diagnostics. - An allowlist change could reject a valid Claude login URL. - Cleanup recovery changes could affect provider teardown ordering. - An environment with a live reusable sandbox can no longer be deleted until the owning issue or execution workspace completes teardown. - The implementation keeps public Test messages fixed and sends detail to redacted server logs. ## Model Used OpenAI GPT-5 via Codex — exact model ID: GPT-5; tool use and code execution enabled; extended reasoning enabled. The implementation author used AI-assisted development. ## 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 documented the result - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation or confirmed no separate documentation change is needed - [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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
3061ce6901 |
feat(sandbox): stream session output by capability, drop three operator flags (#11557)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Sandboxed agents use provider capabilities to select safe execution paths > - Session output still depends on three operator flags that duplicate capability data > - Duplicate flags can drift from the verified sandbox capability snapshot > - This pull request makes the capability snapshot the only streaming decision and removes the obsolete flags > - The benefit is default streaming with a poll fallback when a capability or stream fails ## Linked Issues or Issue Description **What existing behavior does this improve?** ACP sandbox session-output streaming and sandbox execution configuration. **Subsystem affected** Cross-cutting (multiple of the above): server/, packages/shared/, packages/adapter-utils/, and packages/plugins/. **Current behavior** Session-output streaming requires operator flags in the server and Daytona plugin configuration. Saved configurations can retain a removed key. **Proposed behavior** The verified capability snapshot selects streaming. The Daytona plugin uses persistent sessions by default, keeps bypass commands one-shot, and falls back from the log stream to polling. Removed configuration keys become inert. **Reason and benefit** One capability source prevents configuration drift. The fallback keeps output available when capability resolution or log streaming fails. **Breaking changes** The three operator flags no longer control session-output streaming. Existing saved keys load but have no effect. ## What Changed - Remove `useSessions` and `useLogStream` from the Daytona plugin configuration and manifest. - Remove `streamAgentSessionOutput` from server configuration, shared types, and execution-target plumbing. - Select streaming from `persistentProcessSessions` and `independentControlCommands`. - Keep poll fallback on capability resolution failure and stream failure. - Strip removed keys from strict fake-sandbox and catchall plugin configuration. - Update the sandbox capability documentation and focused tests. ## Verification - `tsc --noEmit` passed in `packages/shared`, `packages/adapter-utils`, `server`, and the Daytona plugin. - Daytona `plugin.test.ts` passed 139 tests. - Server capability, configuration, route, and runtime suites passed 160 tests. - `packages/adapter-utils` `execution-target-sandbox.test.ts` passed 44 tests. - The capability matrix covers stream, poll, and resolution-failure paths. - Removed-key tests cover strict fake-sandbox and catchall plugin schemas. ## Risks - A capability snapshot that lacks either required session capability uses polling. - A log stream failure uses polling and can increase request count. - Existing removed configuration keys no longer change behavior. - The isolated-worktree Daytona Vitest run has a pre-existing missing `packages/adapters/droid-local` reference. CI and standard checkouts use the committed configuration. ## Model Used OpenAI Codex, GPT-5, tool use and code review assistance. The exact runtime context window is managed by the Codex platform. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
e71ce9a9d3 |
feat: sandbox provider capability contract with fail-closed effective resolution (#11463)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Paperclip runs work through adapters and sandbox providers > - Providers need a clear contract so the server can use only verified capabilities > - A declared capability must not grant a method that the live worker did not verify > - This pull request adds manifest declarations and fail-closed effective capability resolution > - The benefit is safe provider reuse across execution targets and run lifecycles ## Linked Issues or Issue Description **Subsystem affected** Cross-cutting (multiple of the above) **Problem or motivation** Sandbox providers expose different runtime methods. The server needs one safe capability contract that accounts for provider declarations, worker verification, and narrowing configuration. **Proposed solution** Add strict manifest validation for five sandbox capabilities. Resolve effective capabilities as the subset of verified, declared, and narrowed values. Store the result as a frozen execution-target snapshot. **Alternatives considered** Trusting the manifest alone could grant methods that the worker does not support. Trusting only a fixed built-in list would reject valid third-party providers. The intersection rule keeps the verified runtime ceiling and supports both provider types. **Roadmap alignment** This change supports the ACP run lifecycle track and the sandbox provider contract work in the current roadmap. **Additional context** The legacy `supportsReusableLeases` field remains supported. The nested capability validator rejects unknown keys. Missing or unavailable verification resolves all capabilities to `false`. ## What Changed - Add strict `sandboxCapabilities` manifest validation with legacy reusable-lease compatibility. - Carry declarations through the ready-driver projection. - Add fail-closed effective resolution from verified, declared, and narrowed capabilities. - Add narrowing for provider configuration, Kubernetes Job leases, and Daytona sessions. - Add a frozen read-only capability snapshot to execution targets. - Add focused tests and keep existing characterization baselines covered. - Add and update sandbox provider capability documentation. ## Verification - `npx vitest run packages/shared/src/validators/plugin.test.ts` - `npx vitest run server/src/__tests__/plugin-environment-driver-sandbox-capabilities.test.ts` - `npx vitest run server/src/__tests__/sandbox-capability-contract.test.ts` - `npx vitest run server/src/__tests__/environment-execution-target-capabilities.test.ts` - `npx vitest run packages/adapter-utils/src/acpx-engine/startup-characterization.test.ts packages/adapter-utils/src/acpx-engine/turn-characterization.test.ts packages/adapter-utils/src/acpx-engine/settlement-characterization.test.ts packages/adapter-utils/src/acpx-engine/composed-run-characterization.test.ts` - Package typechecks for shared, server, and adapter-utils pass. - Stage-2 security review suites pass with 28 tests. ## Risks The resolver fails closed when verification is absent or unavailable. Providers that rely on undeclared capabilities may see narrower behavior until they expose verified worker methods. The change does not alter the existing native-sync guard. ## Model Used OpenAI Codex, GPT-5, exact runtime model ID `gpt-5`, tool use and code execution. The implementation author used this model to assist with the change. ## 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 <noreply@paperclip.ing> |
||
|
TonioandClaude Opus 5
|
92047cac46 |
test(adapter-utils): make the next sandbox flake diagnosable (#11483)
`execution-target-sandbox` has failed twice in CI and not once in several hundred local runs. This does not fix it. It makes the next occurrence carry its own evidence, because a third unreproducible failure would teach nothing. The observed signature was an empty stdout with exit code 0 - the child exited cleanly having produced nothing, which is what a lost stdin frame looks like from the test's side. Three mechanisms were checked and ruled out rather than assumed: the helper resolving on `exit` rather than `close` (a 200-iteration probe produced no truncations, and the failure was empty rather than partial); the wrapper reporting exit before stdout drains (it already listens on `close`); and frame writes racing (the stream wrapper's `writeEvent` is synchronous and sequence-numbered). Two candidates remain and the runtime tree separates them. A stdin queue frame still present means the host wrote it and the wrapper never consumed it; a drained queue with no output means it was consumed and the reply was lost on the way back. The report prints that tree, both proxy streams, the exit code, and the elapsed time - the last because the bridge and proxy run on 5s budgets that are generous locally and tight on a runner sharing a box with 19 other lanes. Timeouts are deliberately unchanged. Raising them would probably make the symptom go away, which is the reason not to do it blind. The first revision capped the tree walk one level above the queue frames, so "the queue is empty" and "the walk never looked" printed identically - the distinction the report exists to make. Caught in review. Verifying that the reporter printed something was not enough; it had to print the thing that discriminates, which is now checked by planting a frame and forcing the assertion. adapter-utils typecheck clean; 44 pass, stable across repeated runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Nicky LeachandPaperclip
|
cd501499a2 |
test: add ACPX run lifecycle characterization baselines (#11461)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The adapter runtime starts, turns, settles, and composes ACPX runs > - Recent lifecycle corrections changed several order and cleanup rules > - Those rules need regression coverage before the planned engine refactor > - This pull request adds characterization suites for the corrected behavior > - The benefit is a clear test baseline for the next refactor ## Linked Issues or Issue Description **What existing behavior does this improve?** The ACPX adapter runtime and server heartbeat lifecycle need stable regression coverage for their current corrected behavior. **Subsystem affected** Cross-cutting (multiple of the above): `packages/adapter-utils` and `server` test suites. **Current behavior** The runtime has corrected rules for startup, turns, settlement, composed results, and heartbeat terminalization. The repository lacks a single characterization baseline for these rules. **Proposed behavior** Keep the current lifecycle rules pinned by five test suites. Let the later engine refactor change behavior only when it updates these tests with a clear reason. **Reason and benefit** The suites expose order, cleanup, transport, timeout, retry, result, and lease-release changes during the refactor. They also record one known latent defect as current behavior. **Breaking changes** None. This pull request adds tests only. ## What Changed - Add startup characterization coverage for commands, launch values, session fingerprints, sync order, bridge overlap, and cleanup paths. - Add turn characterization coverage for inputs, events, transports, timeout and cancel behavior, retry rules, errors, and usage. - Add settlement characterization coverage for teardown, adapter sync-back, workspace restore order, native sync, and error policy. - Add composed-run characterization coverage for result forms, finalization sets, and host-lane warm save and warm hit behavior. - Add server coverage that checks run terminalization before environment lease release. ## Verification - Run `npx vitest run packages/adapter-utils/src/acpx-engine/startup-characterization.test.ts packages/adapter-utils/src/acpx-engine/turn-characterization.test.ts packages/adapter-utils/src/acpx-engine/settlement-characterization.test.ts packages/adapter-utils/src/acpx-engine/composed-run-characterization.test.ts packages/adapter-utils/src/acpx-engine/execute.test.ts`. - Run `npx vitest run server/src/__tests__/heartbeat-run-terminalize-before-release.test.ts`. - The adapter-utils run passes 178 tests, and the server run passes 4 tests. - Check `pnpm --filter @paperclipai/adapter-utils typecheck`. - Check `pnpm --filter @paperclipai/server typecheck`. ## Risks Low risk. The change adds test files and does not change production code. One known cold ensure-session cleanup defect remains pinned as current behavior. ## Model Used OpenAI Codex, GPT-5, with tool use and code execution. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
e52b8a343f |
fix: ACP run lifecycle corrections — failure settlement, workspace sync-back, lease cleanup (#11454)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent adapters run ACP sessions and manage runtime, workspace, and lease resources. > - Several failure paths left runtime bridges, staged workspaces, or environment leases active after an error. > - These leaks reduce run reliability and can leave later runs without clean resources. > - This pull request closes the failure paths, applies one teardown policy, and adds regression tests. > - The benefit is consistent failure settlement and safer reuse of agent workspaces and leases. ## Linked Issues or Issue Description **What happened?** ACP runs could leave runtime bridges, staged workspaces, or environment leases active after failures. Claude and Gemini ACP runs did not restore the sandbox workspace on teardown. Lease release stopped when one lease returned an error. **Expected behavior** Each ACP failure must return an error result and settle its resources. Teardown must run each step, release leases independently, and restore the host workspace when the sandbox ends. Pending cleanup leases must receive bounded retry attempts. **Steps to reproduce** 1. Run an ACP session that fails after runtime creation or during turn preparation. 2. Run an ACP session that fails during a warm hit or staged runtime handoff. 3. Run lease cleanup with more than one lease when the first release returns an error. 4. Inspect the result phase, teardown calls, workspace state, and lease metadata. 5. Run the regression suites listed in the Verification section. ## What Changed - Settle every ACP failure after runtime creation with an error result and one sandbox.startup span closure. - Close the ACP runtime and remove warm entries after every pre-turn failure. - Run all teardown steps, record teardown errors, release staging leases in finally, and prevent duplicate teardown. - Dispose staged runtimes after seam failures and remove borrowed staged entries with identity guards. - Add fail-open workspace sync-back teardown for Claude and Gemini ACP adapters. - Isolate lease release errors and add bounded retry sweeps for stranded pending_cleanup leases. - Atomically claim pending_cleanup retries and clamp attempt readers to keep the five-attempt bound. - Default absent provider reusableLeases values to false and align the fake provider with its runtime declaration. - Add regression tests for engine, adapter, server, and shared environment behavior. ## Verification - [x] `npx vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts` — 124 tests passed. - [x] `npx vitest run packages/adapters/codex-local/src/server/acp.test.ts packages/adapters/claude-local/src/server/acp.test.ts packages/adapters/gemini-local/src/server/acp.test.ts` — 61 tests passed. - [x] `npx vitest run server/src/__tests__/environment-runtime.test.ts server/src/__tests__/heartbeat-pending-cleanup-sweep.test.ts server/src/__tests__/reusable-leases-default.test.ts server/src/__tests__/environment-routes.test.ts packages/shared/src/environment-support.test.ts` — passed. - [x] All listed suites ran from the repository root. - [x] GitHub CI completed successfully for `cfc349c9f232711433897915112a1c52c0e462ca`. - [x] Greptile completed with a 5/5 confidence score and no blocking finding. ## Risks The engine changes affect failure settlement and teardown order across ACP runs. The server changes add retry state to existing lease metadata without a schema migration. The adapter changes restore workspaces after sandbox execution. Regression tests cover the changed paths. GitHub CI and Greptile passed for the current head. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. This change fixes runtime reliability and does not duplicate a roadmap feature. ## Model Used OpenAI GPT-5 Codex. The model used tool-based repository inspection, GitHub operations, and code review support. The runtime does not expose a context-window value. ## 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 (for example, `docs/...` or `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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
ed8075b535 |
fix(adapter-utils): order stdin file writes in the sandbox process-session bridge (#11406)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent adapters can run through a sandbox process-session bridge > - The bridge writes streamed standard input to files before the remote process reads them > - Concurrent file writes can make a later chunk visible before an earlier chunk > - The remote process can then parse a tail fragment and wait forever for the missing head > - This pull request serializes host writes and makes an unexpected file gap a loud error > - The benefit is ordered input with a bounded failure path for sandbox ACP sessions ## Linked Issues or Issue Description No public GitHub issue exists for this change. The description below follows `.github/ISSUE_TEMPLATE/bug_report.yml`. **What happened?** A sandbox ACP process-session bridge could stall after its handshake when a run sent a large prompt. The host sent one un-awaited file write for each standard input chunk. A small later chunk could finish before a large earlier chunk. The remote poller then sent the tail bytes first. The agent parser raised an error on the tail fragment, and the head bytes stayed buffered without a newline. **Expected behavior** The bridge must expose standard input files in sequence. The remote poller must report a clear error when an earlier file remains missing beyond the retry budget. **Steps to reproduce** 1. Start an ACP session through a sandbox process-session bridge. 2. Send a prompt that produces multiple standard input file chunks. 3. Delay finalization of an earlier chunk while a later chunk completes. 4. Observe that the remote parser can receive the later chunk first and the session can stop without a clear error. **Paperclip version or commit** The change targets the current `master` branch at the submitted commit. **Deployment mode** The bug affects sandbox execution. **Agent adapter(s) involved** The failure affects the ACP process-session bridge. **Database mode** Not database-related. **Additional context** The fix keeps the existing per-file atomic write behavior. It adds ordering at the host write boundary and a bounded ordering check in the shared wrapper poll tail. ## What Changed - Add a per-session promise chain for host standard input file writes. - Keep a failed write from blocking later chain entries. - Track the next expected sequence number in the shared wrapper poll tail. - Hold later files while an earlier file is missing within the existing retry budget. - Emit a loud error and advance after the retry budget expires. - Add regression tests for host ordering, gap holding, and the loud error path. ## Verification - `npx vitest run packages/adapter-utils/src/execution-target-stdin-race.test.ts` — 9 tests passed. - `npx vitest run packages/adapter-utils/src/execution-target-sandbox.test.ts` — 43 tests passed. - `pnpm --filter @paperclipai/adapter-utils typecheck` — clean. - With the source fix reverted, the 3 new tests fail and the 6 original tests pass. - CI must pass on the pull request before merge. ## Risks Low risk. - The host now serializes writes for each session, which can reduce write parallelism. - A failed write still emits one error and destroys the socket, as before. - The wrapper can emit a loud error after the existing retry budget when a file gap persists. - The change does not alter the atomic per-file write behavior. ## Model Used OpenAI GPT-5, model ID `gpt-5`, with tool use and code execution. The context window and internal reasoning details are not disclosed. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
6f26f2a450 |
fix(adapter-utils): add per-iteration timeout and watchdog to the sandbox callback bridge poll loop (#11341)
## Thinking Path > - Paperclip runs AI agents through adapters and sandboxed execution paths > - The sandbox callback bridge carries file requests between the host and a sandbox > - The poll loop waited forever when a sandbox call stopped responding > - A permanent wait stranded queued requests and hid the run failure > - This pull request adds bounded timeouts, abort handling, recovery backstops, and trace reporting > - The benefit is prompt request failure, safe mutation outcomes, run-level error reporting, and trace visibility ## Linked Issues or Issue Description **What happened?** The sandbox callback bridge could wait forever when a client call stopped responding without a rejection. **Expected behavior** The bridge should fail queued requests and report a run-level error when the sandbox channel stops responding. **Steps to reproduce** 1. Start a sandbox callback bridge. 2. Queue a request. 3. Make the sandbox call stop responding. 4. Observe that the request does not receive a failure response. **Paperclip version or commit** Commit `edc4f71b460c600f97cf44cb486d5cac72ca2db9`. **Deployment mode** Built from source. **Installation method** Built from source with pnpm. **Agent adapter(s) involved** Custom or external sandbox callback bridge. **Database mode** Not database-related. ## What Changed - Add a per-iteration timeout for `listJsonFiles` and `processRequestFile`. - Add a watchdog that fails pending requests when the loop makes no progress. - Abort a hung handler and use a non-retryable 504 backstop when its outcome can be indeterminate. - Retry recovery writes and keep queued requests when a recovery write fails. - Forward the indeterminate-outcome header through the execution target. - Record worker failures through the `sandbox.callbackBridge.workerFailed` trace span. - Add tests for timeout, watchdog, recovery, mutation safety, header forwarding, and fast-request behavior. ## Verification - Run `pnpm exec vitest run packages/adapter-utils/src/sandbox-callback-bridge.test.ts packages/adapter-utils/src/execution-target-sandbox.test.ts`. - Confirm that the PR test, typecheck, build, end-to-end, serialized test, and security checks pass. - Confirm that the current PR head is `edc4f71b460c600f97cf44cb486d5cac72ca2db9`. - Confirm that the PR changes four files: the callback bridge, its tests, the execution target, and its tests. ## Risks The default timeout can fail a slow but valid sandbox call. The defaults remain configurable, and the iteration timeout stays below the sandbox response deadline. A mutation that may have committed returns a non-retryable 504 outcome so the caller does not apply it twice. ## Model Used OpenAI Codex, GPT-5 current runtime, with extended reasoning and tool use. The exact context window is not exposed by the runtime. ## 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 <noreply@paperclip.ing> |
||
|
LeeJandClaude Opus 5
|
a53cc8819b |
fix(claude-local): pipe print prompt via stdin (#9500)
Fixes #2444. Refs #4947. The `claude_local` adapter launched Claude Code as `claude --print - --output-format stream-json --verbose`. Paperclip writes the rendered task prompt to Claude's stdin, but current Claude Code releases can treat the stale `-` positional marker as the prompt itself, so Claude received the literal string `"-"` instead of the issue body. The customer's task ran against no content at all. The fix keeps `--print` mode and stdin delivery, and removes the stale `-`. Adds regression coverage on both sides of the delivery path: a `claude_local` assertion that `--print` is present, `"-"` is absent and the prompt still reaches stdin, and an adapter-utils case proving the sandbox run-log command wrapper preserves stdin while streaming logs. Authored by @elJayAdvisor, whose commit is included unchanged with their authorship. The branch had gone stale and was showing CONFLICTING; the conflict was in `execution-target-sandbox.test.ts`, where their new test was added at the same point as master's `creates the process session directories only in the launch exec` case and git interleaved the two into one hunk. Resolved by taking master's file and re-inserting their test whole, after checking every helper it needs still exists there. Verified: the bug was still live on master at `execute.ts:838`; the regression test genuinely catches it — restoring the stale `-` fails `expect(captured.argv).not.toContain("-")`; `@paperclipai/adapter-claude-local` and `@paperclipai/adapter-utils` typecheck clean; 67 pass across the two test files. All CI gates green; Greptile 5/5. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
scotttongandPaperclip
|
eabecc6f77 |
feat(annotations): include issue document annotations in agent review context (#11332)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Reviewers annotate plans and issue documents with inline comments, and assigned agents act on that feedback > - The server already builds a bounded review context from open plan annotations and includes it in agent wake payloads > - Non-plan issue documents did not get the same treatment: their open annotation threads never reached the agent, and the properties pane did not surface their annotations > - This pull request extends the review-context path and the properties-pane UI to issue documents, at parity with plans > - The benefit is that agent feedback on any issue document reaches the assigned agent, not only feedback on the plan ## Linked Issues or Issue Description **What existing behavior does this improve?** The review-context pipeline that delivers inline annotation feedback to assigned agents, and the properties pane that surfaces those annotations to reviewers. **Subsystem affected** The server review-context path (`server/src/services/plan-review-context.ts`, wake payload assembly in `server/src/services/heartbeat.ts`, `server/src/routes/issues.ts`), shared wake-payload types (`packages/shared`, `packages/adapter-utils`), and the issue properties pane (`ui/src/components/issue-properties/`). **Current behavior** A reviewer can annotate any issue document, not only the plan. The agent wake payload includes open annotation threads for the plan document only. Feedback left on other issue documents is invisible to the assigned agent. In the properties pane, the Artifacts tab also gives no way to see or open a document's annotations. **Proposed behavior** Add `buildDocumentReviewContext` beside the existing plan builder. It collects open annotation threads for all non-plan issue documents, applies the same thread, comment, and character budgets across documents, and reports truncation. Include the result as a new `documentReviewContext` field in agent wake payloads and in the issue wake-context route. Keep the plan context on its legacy builder and field so plan-only wakes stay byte-for-byte compatible. Render the new context in the adapter wake-payload text, and surface annotation counts and the annotation panel for documents in the properties pane's Plans and Artifacts tabs. **Reason and benefit** The floating annotation popover and persistent highlight UI landed earlier; this change completes the loop so agent feedback on any issue document reaches the assigned agent, not only feedback on the plan. **Breaking changes** None. The wake payload gains a new optional `documentReviewContext` field; the existing plan context field and its legacy builder are unchanged, so plan-only wakes stay byte-for-byte compatible. ## What Changed - Add `buildDocumentReviewContext` in `server/src/services/plan-review-context.ts`: bounded review context (shared thread/comment/character budgets, per-document legacy limits) over all non-plan issue documents - Include `documentReviewContext` in agent wake payloads (`server/src/services/heartbeat.ts`) and in the issue wake-context response (`server/src/routes/issues.ts`) - Add shared `DocumentReviewContext` / `DocumentReviewContextDocument` types in `packages/shared` - Normalize and render the new context in adapter wake-payload text (`packages/adapter-utils/src/server-utils.ts`), with tests - Show a `DocumentAnnotationsCountChip` and the annotation panel for documents in the properties pane Plans and Artifacts tabs, with tests - Extend server document-annotations service tests to cover the new context builder ## Verification - Run `npx vitest run packages/adapter-utils/src/server-utils.test.ts server/src/__tests__/document-annotations-service.test.ts` from the repo root — 104 tests pass - Run `TZ=UTC npx vitest run ui/src/components/issue-properties/IssuePropertiesDocumentAnnotations.test.tsx ui/src/components/IssueProperties.test.tsx ui/src/components/IssueDocumentAnnotations.test.tsx ui/src/components/DocumentAnnotationPopover.test.tsx` from the repo root — 75 tests pass (one pre-existing monitor-row case asserts UTC timestamps, so use `TZ=UTC` locally; CI runs in UTC) - `pnpm run typecheck` in `server/` passes - Manual: annotate a non-plan issue document, then wake the assigned agent with a comment — the wake payload lists the open document annotation threads; the Artifacts tab shows the annotation count chip and opens the panel ## Risks - The wake payload gains a new optional `documentReviewContext` field; consumers that ignore unknown fields are unaffected, and the plan context field is unchanged - The context is new input to agent wakes; shared budgets (same limits as the plan context) bound token cost across all documents - Low UI risk: the properties-pane changes reuse the existing annotation components > 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 - Claude (Anthropic), model ID `claude-fable-5` (Claude Fable 5), with extended thinking and agentic tool use (Claude Code harness) ## 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 <noreply@paperclip.ing> |
||
|
|
b7b8fbf688 |
fix(adapter-utils): let explicit PAPERCLIP_API_URL override the derived runtime URL in run env (#10339)
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies
> - Every agent run gets a run-scoped bridge into the Paperclip API
through the injected `PAPERCLIP_API_URL` / `PAPERCLIP_API_KEY` env vars,
built by `buildPaperclipEnv` in
`packages/adapter-utils/src/server-utils.ts`
> - `buildPaperclipEnv` resolves that URL as `PAPERCLIP_RUNTIME_API_URL
?? PAPERCLIP_API_URL ?? http://<listen-host>:<port>`, and the server
always exports `PAPERCLIP_RUNTIME_API_URL` derived from
`authPublicBaseUrl` at boot
> - When `authPublicBaseUrl` points at an address that is not reachable
from inside the runtime container (e.g. a VPN/tailnet-only address used
to keep the web UI off the public internet), every local run receives a
dead API URL (`curl` exit 7) and agents only survive by hand-rolling a
localhost fallback
> - An operator-set `PAPERCLIP_API_URL` is the documented escape hatch —
`docs/deploy/environment-variables.md` states the server "preserves the
value" when set externally and that the run-level var "inherits the
server-level value" — but the run env builder inverts the precedence, so
the override never actually reaches runs
> - This pull request swaps the precedence in `buildPaperclipEnv` so an
explicit `PAPERCLIP_API_URL` wins over the derived runtime URL, aligning
the behavior with the documented contract
> - The benefit is that operators with split-horizon topologies (public
auth URL != container-reachable URL) can point agent runs at a reachable
endpoint with one env var, with zero behavior change for deployments
that do not set it
## Underlying Issue
No pre-existing public issue covers this, so per CONTRIBUTING ("Link
Issues or Describe Them In-PR") here are the `bug_report.yml` fields
inline:
- **What happened:** with `PAPERCLIP_AUTH_PUBLIC_BASE_URL` on a
tailnet-only address and `PAPERCLIP_API_URL=http://localhost:3100`
explicitly set in the server environment, every agent run still received
`PAPERCLIP_API_URL=http://100.x.y.z:3100` (the derived,
container-unreachable URL); `curl` from inside the run exits 7 and
agents can only reach the API by hand-rolling a localhost fallback
- **Expected behavior:** the run env inherits the operator-configured
`PAPERCLIP_API_URL`, as documented in
`docs/deploy/environment-variables.md` ("preserves the value", run-level
var "inherits the server-level value")
- **Steps to reproduce:** (1) set `PAPERCLIP_AUTH_PUBLIC_BASE_URL` to an
address not reachable from inside the server container, (2) set
`PAPERCLIP_API_URL=http://localhost:3100` in the server env, (3) trigger
any agent run and inspect the spawned process env: it carries the
derived URL, not the override
- **Version/commit:** reproduced on the `91e58acb` image (2026-07-19);
the precedence is unchanged on current `master` (`a3b293e`)
- **Deployment mode:** single-host Docker Compose, local adapters
(`claude_local`/`codex_local`), web UI exposed via VPN/tailnet only
## Related PRs (dedup search)
Several in-flight PRs touch the same pain point (runs receiving an
unreachable injected API URL) — linked for reviewer context; none of
them honors the documented explicit override, and the older ones appear
stale:
- #9916 — reworks `PAPERCLIP_RUNTIME_API_URL` derivation and port
preservation (server side); complementary, does not change run-env
precedence
- #8130 — honors a pre-set `PAPERCLIP_RUNTIME_API_URL` (server side); a
complementary escape hatch via the runtime var instead of the documented
`PAPERCLIP_API_URL` override
- #8025 — heuristic: prefer loopback when the runtime bind is loopback
(no activity since Jun 12)
- #5692 — heuristic loopback-safe URL inside `buildPaperclipEnv` (no
activity since May 14)
- #4877 — broader same-host injection rework across 10 files (no
activity since May 2)
- #4794 — always forces loopback for spawned agents (no activity since
Apr 30; would break split-horizon setups where a reachable non-loopback
URL is intended)
This PR intentionally takes the Path-1 route from CONTRIBUTING: the
smallest possible change (swap two lines so the documented operator
override wins) plus regression tests, rather than a new heuristic.
## What Changed
- `packages/adapter-utils/src/server-utils.ts`: `buildPaperclipEnv` now
resolves the injected URL as `PAPERCLIP_API_URL ??
PAPERCLIP_RUNTIME_API_URL ?? http://<listen-host>:<port>` (explicit
override first), with a short comment explaining why
- `packages/adapter-utils/src/server-utils.test.ts`: three new tests
covering the override precedence, the derived-URL fallback, and the
listen-host default (including the `0.0.0.0` to `localhost` mapping)
- `server/src/__tests__/paperclip-env.test.ts`: updated the expectation
that encoded the old runtime-URL-first precedence and added the
symmetric fallback case (runtime URL used when no explicit override is
set)
- No docs changes needed: `docs/deploy/environment-variables.md` already
describes the fixed behavior
## Verification
- `vitest run` on the new `buildPaperclipEnv` tests in
`packages/adapter-utils`: 3/3 pass
- `vitest run` on `server/src/__tests__/paperclip-env.test.ts` after the
expectation update: 5/5 pass (the first CI run correctly flagged the one
test that encoded the old precedence)
- Reproduced and verified on a production deployment (single-host
Docker, `PAPERCLIP_AUTH_PUBLIC_BASE_URL` on a tailnet-only address):
- Before: freshly spawned runs received
`PAPERCLIP_API_URL=http://100.x.y.z:3100` (verified in the spawned
process `/proc/<pid>/environ`); `curl` to it from inside the container
exits 7
- After (with `PAPERCLIP_API_URL=http://localhost:3100` in the compose
environment): a fresh run received `http://localhost:3100`, and `curl
$PAPERCLIP_API_URL/api/agents/me` with the run-scoped key returned HTTP
200; the run finished `succeeded` with usage telemetry recorded
## Risks
- Low. Behavior changes only for deployments that explicitly set
`PAPERCLIP_API_URL`; when unset (the default),
`PAPERCLIP_RUNTIME_API_URL` is used exactly as before
- The sandbox callback bridge (`execution-target.ts`) is intentionally
untouched: remote sandboxes genuinely need the publicly reachable URL,
and its `input.hostApiUrl || PAPERCLIP_RUNTIME_API_URL || ...` chain
still provides it
## Model Used
- Claude Fable 5 (Anthropic, model ID `claude-fable-5`), extended
thinking + agentic tool use via Claude Code, operating over SSH against
the affected deployment
## Checklist
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [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 run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] If this change affects the UI, I have included before/after
screenshots
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Sergio-LPA <204395363+Sergio-LPA@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Jannes StubbemannandClaude Fable 5
|
8a5c0615f9 |
fix(adapter-utils): forward sandbox callback bridge traffic to the local listen origin (#10017)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents can execute in remote sandboxes, where a callback bridge relays in-sandbox Paperclip API calls back to the host server process > - The bridge worker resolves its forward target from PAPERCLIP_RUNTIME_API_URL / PAPERCLIP_API_URL, which now prefer a configured public base URL and therefore mean "the origin browsers and external agents use" > - The bridge worker runs inside the same process that serves the API, so forwarding through the public origin routes an in-process loopback hop through the network edge > - On a deployment whose public origin sits behind a session-gated edge proxy, every forwarded agent API call is rejected at the edge, so agents in sandboxes cannot read their identity, comment, or hire > - This pull request resolves the bridge forward target from the explicit hostApiUrl override or the local listen host and port only, never the public URL exports > - The benefit is that sandbox agent API calls keep working regardless of how the public base URL is configured or gated ## Linked Issues or Issue Description No existing issue. Describing in-PR following the bug report template: **What happened?** On a cloud deployment with a session-gated public edge, setting a public base URL (PAPERCLIP_PUBLIC_URL) caused every in-sandbox agent API call through the sandbox callback bridge to fail with `403 text/plain "Access denied"` from the edge proxy. With PAPERCLIP_BRIDGE_DEBUG enabled, the bridge logs show the forward target is the public origin, and every proxied request (for example `GET /api/agents/me`) returns the edge proxy's 403 instead of reaching the API. **Expected behavior** The bridge worker runs in the same server process that serves the API, so forwarded calls should target the local listen origin and succeed regardless of how the public origin is configured or gated. **Steps to reproduce** 1. Run the server with a public base URL configured, fronted by a proxy that requires a browser session on API routes. 2. Start a sandbox-executed agent run (any adapter using the sandbox callback bridge). 3. Observe every in-sandbox call to the Paperclip API fail with the proxy's 403; with PAPERCLIP_BRIDGE_DEBUG the forward URL is the public origin. **Paperclip version or commit** Current `master`. **Deployment mode** Self-hosted server behind a reverse proxy. **Agent adapter(s) involved** All sandbox-executed adapters (the bridge is adapter-agnostic). ## What Changed - `packages/adapter-utils/src/execution-target.ts`: `startAdapterExecutionTargetPaperclipBridge` now resolves its forward target as `input.hostApiUrl?.trim() || resolveDefaultPaperclipApiUrl()`. It no longer consults `PAPERCLIP_RUNTIME_API_URL` / `PAPERCLIP_API_URL`, which now describe the public origin for browsers and external agents, exactly the wrong target for an in-process loopback hop. `resolveDefaultPaperclipApiUrl()` builds `http://<PAPERCLIP_LISTEN_HOST>:<PAPERCLIP_LISTEN_PORT>` (exported by server boot before any run executes) and maps wildcard listen hosts to the loopback address of the same family (`0.0.0.0` to `127.0.0.1`, `::` to `[::1]`), so the forward target always matches the address family the server is bound to. `input.hostApiUrl` remains the explicit override seam. A comment documents the reasoning. - `packages/adapter-utils/src/execution-target-sandbox.test.ts`: two new tests. One sets both public URL env vars to an unreachable public https origin and asserts the bridge forwards to the local listen origin (fails before this fix with a 502 because the worker targets the public origin). One asserts an explicit `hostApiUrl` input still overrides everything. - The acpx-engine bridge start (`packages/adapter-utils/src/acpx-engine/execute.ts`) passes no `hostApiUrl` and goes through the same resolution site, so it is covered by the same fix. The sandbox-facing env builder in `server-utils.ts` is intentionally untouched; the bridge env overrides `PAPERCLIP_API_URL` inside the sandbox separately. ## Verification - `npx vitest run packages/adapter-utils/src/execution-target-sandbox.test.ts` (28 tests pass; the new local-origin test fails without the fix) - `pnpm --filter @paperclipai/adapter-utils typecheck` (clean) - Full adapter-utils suite run; the only failures are pre-existing environment-dependent tests (bubblewrap and shallow-clone tests on macOS) identical on a clean `master` checkout ## Risks - Low risk. Deployments where the bridge previously worked did so precisely because the forward target already resolved to the local origin (no public URL configured, so the chain fell through to the same `resolveDefaultPaperclipApiUrl()` result). The only behavioral shift is for deployments with a public URL configured, where forwarding through the edge was either wasteful (an unnecessary network round trip) or broken (session-gated edge). The explicit `hostApiUrl` override seam is preserved for callers that need a nonlocal target. ## Model Used - Claude Fable 5 (claude-fable-5), extended thinking, via Claude Code ## 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: Claude Fable 5 <noreply@anthropic.com> |
||
|
Nicky LeachandClaude Opus 4.8
|
04bf7a6ab5 |
feat(observability): instrument stage.sync host steps and home the agent process span (#11301)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - It runs each agent in a remote sandbox and emits OpenTelemetry spans for the sandbox bring-up and the run. > - A real trace showed two gaps. `stage.sync` had about 3 seconds of unattributed host work before its `pack` span. The persistent agent process showed a `sandbox.exec` span that outlived its parent by about 50 seconds. > - The gaps hide real cost and make the trace read as a sequencing bug, so an operator cannot see where startup time goes. > - This pull request wraps the two pre-`pack` host steps in their own spans. It also homes the long-lived process in a run-scoped `sandbox.agentProcess` span. > - The benefit is that startup time is fully attributed and the process reads as a resource that overlaps the turn, not a child that outlives its parent. ## Linked Issues or Issue Description No public issue exists. This is an enhancement to existing telemetry. It is described inline below, following `.github/ISSUE_TEMPLATE/enhancement.yml`. Prior related work: the merged PR #10999 added the run-time wrapper spans and the telemetry data-contract section this PR extends. **What existing behavior does this improve?** The sandbox bring-up and run OpenTelemetry trace. It closes two attribution gaps in that trace. **Subsystem affected** Observability for sandbox execution. The code lives in `packages/adapter-utils`. The span contract lives in `packages/shared/src/telemetry`. **Current behavior** `stage.sync` opens a `pack` span, but the git enumeration and the baseline content-hash walk that run before `pack` have no span, so about 3 seconds read as a gap. On the streamed process-session path the agent process launches fire-and-forget inside the ~2.3 second `bridge.process-session` bring-up step, so its `sandbox.exec` span parents to that step and then runs about 50 seconds. The child dangles past its parent and overlaps `agent.turn`. **Proposed behavior** Wrap the two pre-`pack` host operations in `snapshot.git` and `snapshot.baseline` spans under `stage.sync`. Wrap the streamed launch in a run-scoped `sandbox.agentProcess` span that parents to the live run root (`task.run` at launch). **Reason and benefit** Startup time is fully attributed. The long-lived process reads as a resource that overlaps the sibling `agent.turn`, not a mis-parented child. **Breaking changes** None. The spans are opt-in and export only when an OTLP endpoint is configured. The span seam is a no-op when no runner is injected. No first-party telemetry event changes. ## What Changed - `sandbox-managed-runtime.ts`: add `snapshot.git` and `snapshot.baseline` spans around the git enumeration and the baseline content-hash walk, nested under `stage.sync`, through a shared `runStepSpan` helper that `pack` now also uses. - `execution-target.ts`: wrap the fire-and-forget streamed launch in a run-rooted `sandbox.agentProcess` span, so it parents to the live run root and holds the inner `sandbox.exec`. The `.then`/`.catch` chain became try/catch inside the span callback, with identical frame-ingestion behavior. - `packages/shared/src/telemetry/README.md`: update the span table and the parenting prose. Add `snapshot.git`, `snapshot.baseline`, `pack`, and `sandbox.agentProcess`, and document the intended `sandbox.agentProcess` / `agent.turn` overlap. - Tests: update the executor span-tree test (`childNames` and parent assertions), update the `sandbox-managed-runtime` span-set and nesting tests, and add two `execution-target-sandbox` tests (the launch opens `sandbox.agentProcess`; it parents to the run root, not the bring-up step). ## Verification - Run `npx vitest run` on the three affected test files. Result: 174 tests pass. This includes the updated executor span-tree test and the new `sandbox.agentProcess` open and parenting tests. - Run `tsc --noEmit` in `packages/adapter-utils`. Result: no errors in the changed source or test files. - The full 37-test streamed process-session suite passes unchanged. This confirms the try/catch restructure preserves frame delivery and exit/error behavior. - Pre-existing and unrelated to this PR (present on `master`): `tsc` errors in `execute.ts` / `execute.test.ts` / `remote-spawn-smoke.test.ts` (`onAgentStderr` / `spawnCwd`), and a `check:forbidden-tokens` failure from internal `PAP-###` ids in `ui/src/components/IssueRecoveryActionCard.test.tsx`. This PR does not touch those files, and its own diff is token-clean. ## Risks Low. The change adds instrumentation on the opt-in span path and does not change control flow on the default path. The one production restructure is the streamed launch, which stays fire-and-forget, so bring-up does not block on it. Only the streamed path gains `sandbox.agentProcess`; the legacy poll path launches the process detached and has no host-side long-lived span to home. ## Model Used Anthropic Claude Opus 4.8 (`claude-opus-4-8`), about 200K-token context, agentic tool use through Claude Code. The trace was reviewed through the Honeycomb MCP. The code was written and tested with the model. ## 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: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
e31951a17d |
feat: Claude agent setup-token login in a sandbox (#11286)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Claude agents that run in a remote sandbox need a safe in-product login path > - The existing host login route cannot open a pseudo-terminal inside that sandbox > - The login flow must protect the browser code, the login URL, and the OAuth token at every step > - This pull request adds the parser, the runner, a Daytona pseudo-terminal transport, and a guarded, owner-bound session route behind an injectable transport > - The route stays inert in the default build and fails closed until a sandbox provider binds the live transport > - The benefit is a company-scoped setup-token flow with one-time secret delivery, redaction, and fail-closed transport checks, ready for a later staged production rollout ## Linked Issues or Issue Description **Agent or provider** Claude Code setup-token login for sandbox agents. **Why this adapter is useful** Sandbox agents need a supported way to sign in without host credentials. An authorized owner completes the browser step and receives the token one time. **How the agent is invoked** When a sandbox provider binds the injectable transport, the server starts `claude setup-token` through a sandbox pseudo-terminal, sends the browser code to the matched prompt, and returns the token through the guarded session route. The default build does not bind the transport. In that state the start route fails closed with a fixed no-secret `503`. It does not start a process and it does not hold a sandbox lease. **Additional context** The transport is injectable, so each sandbox provider binds its own pseudo-terminal. This pull request adds the Daytona transport but does not bind it in the production server. A production wiring needs a lease manager, a live pseudo-terminal factory, a durable token store, and its own security review. The route keeps secrets out of logs, activity details, errors, telemetry, and non-owner responses. ## What Changed - Add strict parsers for the setup-token URL, the prompt, and the success token. - Add a login runner that drives the `claude setup-token` command through a pseudo-terminal. - Add the Daytona pseudo-terminal transport and the sandbox plugin wiring. - Add a company-scoped, owner-bound login session service with rate limits, a reaper, cleanup, and one-time token delivery. - Add the guarded session routes at `/agents/:id/setup-token-login-sessions/*` behind an injectable transport. The routes become the live login path only when a provider binds the transport. - Keep the start route fail-closed in the default build. It returns a fixed no-secret `503` and it does not bind `setupTokenLogin`. - Keep the existing host route `POST /agents/:id/claude-login` in place. This pull request does not replace it. - Keep confidential responses behind a fail-closed TLS transport guard with `Cache-Control: no-store`, and extend redaction for the new fields. - Export the parser and the runner from the Claude local server entry, and document the new session routes in the OpenAPI spec. ## Verification - `pnpm --filter @paperclipai/server exec vitest run setup-token-route setup-token-session` - `pnpm --filter @paperclipai/adapter-claude-local exec vitest run` - `pnpm --filter @paperclipai/server run typecheck` - Confirm that the pull request checks pass on GitHub. ## Risks - Low user-facing risk on merge. The default build does not bind the transport, so the production start route stays fail-closed with a `503`. The merge does not change the production login behavior. - When a provider later binds the transport, the flow starts a live sandbox process and holds a short-lived in-memory secret. Cleanup must stop the child before it releases the sandbox lease. - The transport guard fails closed when the deployment does not provide a trusted TLS path. A wrong proxy allowlist can block a valid request. - The production wiring is out of scope. It needs a lease manager, a live pseudo-terminal factory, a durable token store, and its own security review before the server binds `setupTokenLogin`. ## Model Used Anthropic Claude Opus 4.8 assisted the implementation. It used extended reasoning, code execution, repository tool use, and a 200,000-token context window. ## 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 (the OpenAPI spec covers the new session routes; no user-facing documentation needs 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 <noreply@paperclip.ing> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Nicky LeachandPaperclip
|
8f478242f1 |
fix(adapter-utils): close sandbox stdin file race with atomic write and fault-tolerant poller (#11235)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent adapters use execution targets to exchange input and output with sandbox processes > - The sandbox input path can expose partial files, and the poller can delete or stop on invalid input > - These timing windows can lose agent input without a clear error > - This pull request makes host writes atomic and makes the poller retry invalid files before it drops them > - The benefit is reliable sandbox input delivery with visible failure after bounded retries ## Linked Issues or Issue Description Closes #10874 ## What Changed - Decode host input into a temporary file, then rename it onto the final JSON path. - Apply the same atomic write pattern to the filesystem client. - Parse each input file before deletion. - Retry parse failures and drop a file after the bounded retry limit with an error event. - Add regression tests for empty, partial, and permanently malformed input files. ## Verification - `pnpm exec vitest run packages/adapter-utils/src/execution-target-stdin-race.test.ts` passes 5/5 tests. - Related sandbox callback, execution target, and sandbox execution suites pass 71/71 tests. - TypeScript checks pass for the changed files. - The regression suite fails on the old code and passes on this change. ## Risks - Low risk. The change affects sandbox input file handling and adds bounded retry behavior. - A permanently malformed file now creates an error event after the retry limit. > This bug fix does not add a core feature, so a roadmap change is not needed. ## Model Used Codex, OpenAI GPT-5, current agent runtime, large context window, tool use and code review support. ## 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 <noreply@paperclip.ing> |
||
|
scotttong
|
815e49bb7c | feat: make chat-style tasks the default experience (#11101) | ||
|
Nicky LeachandPaperclip
|
d5208d30c1 |
feat(adapter-utils): add host-side pack span to managed-runtime tarball build (#11072)
## Thinking Path > - Paperclip runs AI agents through adapter execution services. > - The adapter runtime records spans for each stage of agent startup. > - The managed runtime packs workspace tarballs before it uploads them. > - These pack operations had no host span, so `stage.sync` omitted pack time. > - This pull request adds one `pack` span around both tarball builds. > - The span nests under the active `stage.sync` step and improves trace detail. ## Linked Issues or Issue Description **What existing behavior does this improve?** The managed runtime workspace sync builds a git-history tarball and a workspace-overlay tarball before upload. **Subsystem affected** The change affects `packages/adapter-utils`, which provides adapter execution and managed runtime support. **Current behavior** The host builds both tarballs without an OpenTelemetry span. The `stage.sync` trace therefore omits the host pack duration. **Proposed behavior** The host wraps both tarball builds in one `pack` span. The executor parents this span under the active startup step. **Reason and benefit** The trace shows the time that the host spends packing workspace data. Operators can use the existing runtime span tree to find sync delays. **Breaking changes** None. The default span runner remains a no-op runner, and the existing control flow remains unchanged. ## What Changed - Add an optional `runtimeSpan` runner to the managed runtime preparation path. - Create one host `pack` span around the two workspace tarball builds. - Parent the `pack` span under the active startup step. - Add unit coverage for span emission and span nesting. ## Verification - Run `tsc --noEmit` for `@paperclipai/adapter-utils`. - Run the `@paperclipai/adapter-utils` Vitest suite. - Confirm the suite reports 448 passed tests and 4 skipped tests. - Confirm the trace test records `pack` under `stage.sync`. ## Risks The change adds optional tracing only. The default no-op runner preserves behavior when tracing is not configured. ## Model Used OpenAI Codex, GPT-5, tool use and code execution. The model reviewed the handoff and opened this pull request. The implementation author supplied the commit. ## 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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
6b7e0814a0 |
feat(acp): stream Daytona sandbox agent output and remove the host output poll (#11049)
## Thinking Path > - Paperclip is the open source app that manages AI agents for work > - Sandbox providers let agents run in remote and isolated environments > - Daytona session commands need a path that sends agent output to the host without host polling > - Host polling adds delay and repeats provider output work > - This pull request adds typed execute.log notifications and a log sink for incremental output > - This pull request adds an optional ACP session stream with final-result replay protection > - The benefit is lower output delay while the default flags keep current behavior unchanged ## Linked Issues or Issue Description **Subsystem affected** Cross-cutting. The change spans the plugin SDK, Daytona provider, adapter utilities, and server execution services. **Problem or motivation** The Daytona ACP bridge polls a host output file while an agent command runs. This adds delay and can repeat work. The host also needs a safe route for provider output chunks. **Proposed solution** Add a typed `execute.log` notification with host-issued invocation correlation. Add an ordered log sink to the environment execute path. Add an optional ACP session-log path that parses newline-delimited JSON frames and removes the host output poll for that path. **Alternatives considered** Keep the output-file poll as the only path. This keeps the current behavior but does not provide timely output. The new path stays behind flags, so the existing path remains the default fallback. **Roadmap alignment** This change supports the shipped Cloud / Sandbox agents milestone in `ROADMAP.md`, including Daytona support. ## What Changed - Add the typed `execute.log` worker-to-host notification and company-scoped host route. - Add ordered `stdout` and `stderr` chunk delivery before the final execute result. - Add the Daytona session log sink and the optional ACP streamed session path. - Add monotonic frame handling so live and final output reach the host once. - Keep `useLogStream` and `streamAgentSessionOutput` off by default. - Add unit and integration coverage for the notification, execution target, runtime, and Daytona paths. ## Verification - Run adapter-utils tests: 445 tests pass locally. - Run server environment tests: 73 tests pass locally. - Run Daytona plugin tests: 131 tests pass locally. - Run TypeScript checks for shared, adapter-utils, and server. - Review the pull request checks after GitHub completes them. - All required GitHub checks pass on the current head. ## Risks The new paths change output delivery only when a feature flag enables them. The final execute result remains available for parsing and fallback. The main risk is a provider stream or frame-order error; the final-result parser limits that risk. ## Model Used OpenAI Codex, GPT-5, tool use and code execution. The runtime did not supply a context-window value. ## 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 No operator documentation change applies because both new flags remain disabled by default. - [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 <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
9ace548fd2 |
feat(observability): rename sandbox provider spans and add run-time wrapper spans (#10999)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Paperclip uses adapter and sandbox code to start agents and run sandbox work > - The current sandbox spans use mixed names and do not group related run-time work > - Mixed names make traces harder to read and compare across providers > - This pull request renames provider spans, adds run-time wrapper spans, and keeps the host allowlist closed > - The benefit is clearer traces with the same sandbox behavior and trust boundary ## Linked Issues or Issue Description **What existing behavior does this improve?** This improves OpenTelemetry span names and grouping for sandbox startup, execution, callback relay, and agent session work. **Subsystem affected** Cross-cutting (multiple of the above): adapter utilities, sandbox providers, shared telemetry documentation, and server instrumentation. **Current behavior** Sandbox provider spans use mixed names. Related run-time operations expose inner `sandbox.exec` spans without a named wrapper span. The host mapper uses a closed allowlist for provider span names. **Proposed behavior** Use descriptive provider-scoped span names. Add wrapper spans for agent session input, agent session output polling, and callback relay. Keep the host mapper allowlist closed and map unknown names to `other`. **Reason and benefit** Clear names make traces easier to read and reduce ambiguity during sandbox operation analysis. Wrapper spans show the full operation while preserving the inner execution spans. **Breaking changes** None. This change updates telemetry span names and grouping only. It does not change sandbox behavior, endpoint behavior, or the host trust boundary. **Additional context** Related prior work: [#10758](https://github.com/paperclipai/paperclip/pull/10758). ## What Changed - Rename Daytona provider sync and session spans with descriptive provider-scoped names. - Add three run-time wrapper spans for agent session input, output polling, and callback relay. - Add a shared span runner that preserves no-op behavior without a real tracer. - Keep the host mapper allowlist closed and map unknown names to `other`. - Update telemetry documentation and span-name tests. ## Verification - Focused adapter-utils span tests pass for startup timing, callback relay, and sandbox execution. - Focused Daytona plugin span tests pass for renamed leaf spans and session open or close spans. - Focused server tests pass for host mapping and instrumentation. - The stacked diff contains one commit on top of `feat/daytona-persistent-session-model`. ## Risks - Span names change for existing telemetry consumers. - The wrapper spans add trace structure but do not change sandbox execution. - The host mapper keeps the existing closed allowlist and `other` bucket. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI GPT-5 (Codex agent); exact deployment revision and context window are not exposed in this run; tool use and code execution enabled. ## 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] 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 <noreply@paperclip.ing> |
