mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-15 01:52:55 +01:00
ff8ec41e6a1236694d28f463b60c7d136442f9c9
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandClaude Opus 5.5
|
fd58378ee6 |
fix(runner): never dispatch an OpenCode tool call its MCP client abandoned (#19546)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The Paperclip runner runs native OpenCode agents. OpenCode calls Paperclip tools through an MCP bridge inside the OpenCode proxy. The proxy sends each call to runnerd as an `item/tool/call` frame on stdout. > - runnerd reads the proxy stdout only after the controller acknowledges earlier events. A backlog of event frames can build in the proxy. A tool call waits behind that backlog. > - The OpenCode MCP client can time out while the call waits. The agent then gets `MCP error -32001: Request timed out`. But the queued frame still reaches runnerd later, and runnerd still runs the call. > - This is a safety bug. A mutating call (for example, create a gateway token) can succeed after the agent was told that it failed. The agent can then repeat it. > - This pull request makes sure that a call whose MCP client abandoned it is never sent to runnerd while it is still queued in the proxy. > - The benefit: "timed out" now means "did not run" for every call that had not left the proxy. ## Linked Issues or Issue Description **What happened?** In chat-feature evals with native OpenCode, the agent got `MCP error -32001: Request timed out` for a POST tool call. The server still ran the POST and returned 201 about one second later. Other Paperclip tools that took 0.03–0.5 s on the server reached OpenCode 10–22 s later. The delay grew over the run, and then the calls failed with -32001. Sometimes they failed at once. **Expected behavior** If the MCP client abandons a tool call before Paperclip dispatches it, Paperclip must not run it. **Steps to reproduce** 1. Run a native OpenCode agent (`paperclip_runner` adapter, runnerd path) for a long chat turn with many tool calls. 2. Make the controller acknowledge runnerd events slowly, so a backlog builds in the proxy. 3. Let the OpenCode MCP client time out on a mutating call that waits in the backlog. 4. See that the agent gets -32001, but the call still runs on the server. **Where the per-call lag comes from (finding)** - runnerd (`provider_backend.rs`, `poll_current_provider_with_control`) does not read proxy stdout while any event is not acknowledged. It reads at most 128 events per poll. - Each poll that admits events, and each acknowledgement, calls `persist_state`. That function writes the full provider state JSON again (up to 16 MiB, and it grows during the run) and does two fsyncs. So each round trip gets slower during the run. - Pipe writes from Node are asynchronous on POSIX. OpenCode event frames and `item/tool/call` requests therefore wait in one FIFO buffer in the proxy. A tool call waits behind all earlier event frames, and the results come back through the same gated path. - OpenCode starts its MCP timeout when it calls the tool, not when runnerd reads the frame. The -32001 event frame can itself wait in the backlog, so it can look "instant" when it arrives. - Evidence: the delay from runner emit to server receipt stays below 0.5 s. But runnerd's `item.completed` for a call comes 10–22 s before OpenCode's `tool.execution.completed`, and the gap grows during the run. - The fix for the lag is not small. It needs a change to runnerd persistence: do not gate stdout reads on a full acknowledgement, or use an append-only event log instead of a full state rewrite with two fsyncs for each batch. This pull request does not change runnerd. ## What Changed - `mcp-bridge.ts`: abort the call signal when the MCP client drops its HTTP connection before a response. The `notifications/cancelled` path already aborted it. - `opencode-server-driver.ts`: send the bridge abort signal to `dynamicToolHandler` (new optional `signal` field). - `opencode-proxy-output.ts`: add `createBackpressuredFrameWriter`. It gives the stream only what its buffer accepts and keeps the backlog in its own queue. Frames keep their order. If a queued frame's signal aborts, the writer removes the frame and calls `onAbandoned`. - `opencode-app-server-proxy.ts`: write stdout through the new writer. Tool-call requests carry the call signal. An abandoned request fails with "abandoned by the MCP client before it was dispatched; it did not run". Shutdown drains all live frames. - Tests: a new regression test uses the real bridge and the writer with a stalled stdout. It covers a cancel after timeout, a dropped connection, and a live call. There are also unit tests for the writer. ## Verification - `npx vitest run src/drivers/opencode src/cli/opencode` in `packages/paperclip-runner`: 23 files, 226 tests pass. - Red check: I removed the disconnect abort and the queue gating. Then 2 of the 3 tests in `opencode-proxy-abandoned-tool-call.test.ts` failed: the abandoned `item/tool/call` reached stdout. After I restored the code, they pass. - `pnpm verify` exits 2 because the changed proxy files are outside its resolved test graph. The required CI jobs supply broad coverage. - `tsc --noEmit` for the runner package shows no errors in the changed files. One error remains in `src/eval/workflow-harness.ts`. It comes from a workspace package that is not built and is not related to this change. ## Risks - A call that already left the proxy (it is in the kernel pipe buffer or in runnerd's reader) still runs after a client timeout. The window is about 64 KiB of pipe buffer plus Node's 16 KiB high-water mark. To close it fully, runnerd must get a deadline or a cancellation for each call. That is a follow-up. - Frames now wait in a proxy queue instead of the Node stream buffer. Delivery order does not change. The new code drains this queue on shutdown before the last stdout flush. - When a client disconnects, a durable call is now cancelled. Before this change it stayed pending. The client cannot get the result in either case. - No schema, contract, or ACPX profile changes. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used in Claude Code with tool use (shell, file edit, test runs). ## 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 Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
a386a59998 |
Reduce repeated native completion guidance and preserve final replies (#15151)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native agents receive task constraints and completion tools from Paperclip. > - Completion tools already define the procedure for reporting a result. > - Repeated procedure text adds instructions to each full task turn. > - The final reply must still explain a blocker and link a saved document. > - This pull request removes repeated procedure text and keeps these visible outcome requirements explicit. > - A document receipt supplies the exact link, and stricter evals check the persisted reply and browser navigation. ## Linked Issues or Issue Description Refs: #14961. Related: #14948 and #15007. **What happened?** Native task envelopes repeat completion procedure text. A reduced envelope needs explicit final-reply requirements. The `write_document` receipt also lacks a canonical document link. **Expected behavior** Keep the completion tools as the source of procedure details. Require one accepted completion result before the final reply. A blocked reply must explain the reason, owner and unblock action. A document reply must contain a working link to the saved document. **Steps to reproduce** 1. Run the native assigned-skill document case and native blocker case. 2. Inspect the run-attributed provider final and its persisted comment. 3. Check the blocker explanation or open the final reply's document link. ## What Changed - Remove repeated completion procedure text from the native task constraints and backend instructions. - Keep explicit blocker and document-link requirements in full task turns. - Return a company/task-scoped `documentHref` from `write_document`. Preserve the link in the idempotent mutation receipt. - Repeat canonical links for this run's current saved revisions in accepted completion feedback. Give blocked providers final-response guidance for the cause, owner and unblock action. - Keep internal document/comment anchors when Markdown issue links load cached issue details. - Add a manual six-cell comparison suite with strict source, build, default-instruction and budget admission. - Capture eighteen shared runnerd RPC projections and six direct OpenCode HTTP projections across start, resume and continuation phases, using scripted local transports and no provider execution. - Apply v3 checks only to the manual instruction comparison; preserve v2 checks for the existing native completion suite. Check the actual persisted blocker reason and exact saved-document link. Click the rendered document link and check the original content marker in the classic document card or the new document tab. - Forward exact OpenCode finishing calls through the controller. Wait for acceptance, keep accepted feedback and concrete rejection text, and reject malformed responses. Preserve ordinary dynamic-tool response handling. - Settle the completion decision and tool response before mapping a racing idle/error/abort event or handling explicit close/interruption. Reject a concurrent finishing call before controller admission. - Add a provider-free regression through real runnerd, the OpenCode proxy and a fake provider. Reject the first completion, accept the corrected report in the same turn, and propose one result. - Keep all original verdicts unchanged. Treat replay under new checks as separate diagnostics. ## Verification - `pnpm -r typecheck` and `pnpm build` pass locally. - Native document-authority tests pass, including company/run authorization and idempotent replay. - Native runtime-context, backend and measurement tests pass. - Final-answer calibration, protocol scoring, source-admission and catalog tests pass. Wrong reasons, absent links and wrong link targets fail. - `pnpm test:e2e:runner:typecheck` passes. Discovery lists exactly six single-attempt local cells with the declared models. - Exported `prepareNativeInstructionPreflight` then `verifyNativeInstructionPreflight` pass on this clean committed source. They build locally and make zero provider calls. - Corrective live confirmation is incomplete. Source |
||
|
Dotta
|
2e5a24e177 |
feat(runner): add qualified OpenCode runtime (#12588)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Paperclip Runner provides a durable execution boundary for supported providers. > - The current production runtime supports Codex but cannot execute OpenCode sessions. > - OpenCode needs a qualified transport, strict input mapping, and normalized events. > - This pull request adds the OpenCode runtime as one isolated provider unit. > - The benefit is a reviewable provider expansion that does not weaken the existing Codex path. ## Linked Issues or Issue Description **Subsystem affected** Cross-cutting: packages/paperclip-runner and the Codex-local adapter configuration contract. **Problem or motivation** Paperclip Runner has provider-neutral contracts, but the production backend factory cannot start a qualified OpenCode session. This blocks OpenCode from using the durable runner path. **Proposed solution** Add the qualified OpenCode app-server proxy, driver, MCP bridge, backend, fixtures, and factory wiring. Keep existing Codex behavior unchanged. **Alternatives considered** Keeping OpenCode only on the direct adapter path would avoid this runtime work, but it would not provide durable runner recovery or normalized provider events. **Roadmap alignment** ROADMAP.md does not list a conflicting provider-runtime project. This change extends the existing Paperclip Runner architecture. ## What Changed - Added the qualified OpenCode app-server proxy and input queue. - Added collaboration-mode and provider-event normalization. - Added the OpenCode MCP bridge and native session backend. - Added strict fixtures and focused unit coverage. - Added only the package exports and adapter configuration required by this runtime. - Kept deferred SDK, lab, eval, and public package surfaces out of this change. ## Verification - GitHub Actions is the authoritative verification environment for this PR. - Run the package type checks and focused OpenCode tests in CI. - Run repository typecheck, test, build, security, and policy gates through the stack-aware workflow. - Local tests were not run because this checkout is resource constrained. ## Risks - OpenCode protocol changes could affect event normalization or recovery. - The driver fails closed on malformed input and unsupported runtime behavior. - Existing Codex selection remains unchanged unless the stored provider is OpenCode. > 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. ## Model Used OpenAI Codex, GPT-5.6, with repository tools, code execution, and parallel agent review. ## 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 linked existing issues or described the issue in-PR following the relevant issue template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal Paperclip ticket id - [ ] I have run tests locally and they pass — GitHub Actions is authoritative for this resource-constrained checkout - [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 ## Stack - Position: 1 of 4 - Base: master - Next: additional qualified provider runtimes |