Commit Graph
2 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>
2026-11-06 23:29:26 +00:00
DottaandPaperclip 66e8fab944 fix: complete native chat responses and improve API discovery (#18364)
## Thinking Path

> - Paperclip must perform work and report its outcome accurately.
> - A chat response and its delegated or operator-owned work are
separate stages.
> - Native chat contracts instead required literal completion of the
user's whole request.
> - Agents kept searching or waiting even when a truthful handoff should
finish the response.
> - OpenCode also has separate acknowledgement backlog and
empty-response failure paths.
> - This PR aligns chat completion with its current stage and makes
OpenCode completion truthful.

## Linked Issues or Issue Description

Refs #18098 for retained OpenCode evidence and #18377 for separate eval
setup, oracle and reporting repairs.

**What happened?** Real pinned OpenCode 1.18.34 reproduced an MCP
timeout behind 800 text fragments. Pilot
[41222196525](https://github.com/paperclipai/paperclip/actions/runs/41222196525)
separately exposed a fabricated successful reply after a reasoning-only
response. Later restart attempts timed out without an answer. Their
native criterion literally required restarting the instance, although
the chat instructions and authority required an operator handoff.
General authority guidance alone did not resolve it.

**Expected behavior:** A standing chat completes its current authorized
response. It performs and verifies permitted actions, verifies delegated
handoffs, or explains the specific unavailable action and operator path.
The worker's task remains separate. Ordinary execution tasks must still
complete their requested work. Empty provider responses must not become
invented success.

**Version and reproduction:** Actual pinned OpenCode and native Rust
Runner, with retained full-stack AWS attempts. Product baseline master
`51ea624344`. This PR adds no historical catalog to master.

## What Changed

- Build standing-chat completion contracts from the server-owned
conversation identity. Scope each current request to the response stage;
preserve request text, criterion IDs, order, human-response scope and
source revisions. Require verified authorized actions and truthful
handoffs. Ordinary task contracts stay strict. Existing admitted inputs
retain their saved contract during recovery; new contract content uses
the existing revision/hash mechanism.
- Guide prospective defaults to their owning resource and preserve
existing work unless backfill is requested. Guide unavailable
board/operator operations to the supported handoff. These are general
chat instructions with no eval identities or expected answers.
- Batch adjacent assistant text for at most 50 ms and 4,000 UTF-8 bytes.
Preserve text, metadata and order; flush before non-text frames and
shutdown.
- Give an empty OpenCode response one correction within its existing
turn, model, reasoning, context and deadline. Accept actual completed
answer parts with incomplete headers. Preserve actual active tool work,
bound idle evidence grace and fail truthfully if correction stays empty.
- Reject empty OpenCode terminals in Rust without actual prose or an
accepted semantic result. Never invent successful replies or evidence.
Refresh generated project-discovery contracts.
- Preserve accurate public API-discovery descriptions for existing
project defaults and managed instance restart operations, including
their field scope and authority limits.

## Verification

**Qualified: [pilot
41256592937](https://github.com/paperclipai/paperclip/actions/runs/41256592937)
passes 3/3, with all 35 independent checks passing.**

| Case | Checks | Full case duration | Evidence and cleanup |
| --- | --- | --- | --- |
| Instance restart | 9/9 | 140.1 s | complete |
| Project defaults | 11/11 | 204.2 s | complete |
| Project work | 15/15 | 204.0 s | complete |

All three are original-attempt results from source
`c51e079f7f2364305b131ffd549e263ff78c5335`, containing the exact product
files from `13c2b61f31d873a475ea0c3c026b19926ff42301`. Actual saved
conversation contracts were inspected, as were ordinary native worker
contracts where present. Each protected archive includes the actual API
state and final PNG; cleanup completes. Original queries,
`openrouter/deepseek/deepseek-v4.1-flash-1017`, finite case clocks,
concurrency one and zero eval retries are preserved. Earlier passes do
not transfer into this qualification.

Both current heads pass all checks: product **62** (56 success, one
neutral, five skipped), catalog **56** (54 success, one neutral, one
skipped). Both have exact-head Greptile **5/5**, zero unresolved review
threads and clean mergeability. The three trusted private-evidence
protocol files remain byte-identical to the dispatch workflow. Raw
evidence stays protected.

- **2,044 API discovery tests pass.** Red controls showed that “restart
instance” selected unrelated drain/settings operations and “save project
defaults” selected repository replacement. Accurate public summaries now
rank the existing operations correctly; route registration preserves
descriptions. No endpoint, permission or field behavior changes.
- **112 real-database, authenticated HTTP, native Runner and OpenAPI
consumer tests pass locally**, using disposable task-owned Docker
PostgreSQL. It was removed afterward; shared IPC was untouched. These
cover persisted contract selection, admitted native input and the model
envelope together. The isolated CI suites also pass.
- **77 focused contract/input/context tests pass.** Red controls catch
the wrong conversation predicate; canonical identity requires both agent
and user IDs, including the actual heartbeat projection. Ordinary tasks,
agent-only issues and planner tasks remain strict. Coalesced requests
preserve criterion identities, source hashes and human-response scope.
- **144 OpenCode driver tests pass**; native core **441 pass, one
ignored**; packaging **56 pass, six skipped**. Server typecheck and
build pass.
- Real pinned OpenCode/native Runner: a delayed 25-second
acknowledgement completes within the original 30-second MCP deadline.
The text-fragment control changes 817 notifications / 54.1 seconds /
timeout to 45 / 19.0 seconds / accepted finish. Separate fast,
six-minute and real authenticated API probes complete 32 tool calls plus
finish. These do not reproduce every remaining historical MCP timeout,
and no common cause is claimed for all such failures.

Retained failures remain attributed to their original source. Earlier
runs exposed fabricated empty-response success, prospective-default
backfill, a literal whole-task chat criterion, fixture readiness and
oracle/reporting defects. The immediately preceding
[41250502608](https://github.com/paperclipai/paperclip/actions/runs/41250502608),
product `c0bf249807` / source `7b1de24ce1`, is **1/3**: work 15/15;
restart 4/9 with the repaired contract actually admitted but no answer;
defaults timed out before settlement, then exceeded the unchanged 256
MiB original-evidence allowance. Defaults domain behavior on that
attempt remains inconclusive. #18377 retains the full attempt ledger,
including missing archives and pre-paid cancellations. The fresh 3/3
establishes these selected outcomes; it does not establish full-catalog
reliability.

## Risks

The shared contract change applies to standing native chats across
providers. It changes model instructions and completion scope, while
permission, approval and review gates remain enforced by their existing
owners. Incorrect model claims remain possible; the product evals
independently require actual mutations, native work and delivery. The
correction adds at most one provider request within the existing
deadline; text batching can delay adjacent text by 50 ms. Three
qualified attempts do not establish full-catalog reliability.

## Model Used

OpenAI GPT-6 through Codex, with repository inspection, tool use, code
execution and live eval tooling. Exact hosted model revision and
context-window size are not exposed.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` 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

- [x] Final same-source three-cell pilot passes 3/3 with required
evidence and cleanup

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-31 19:52:42 -05:00