From 3935e300dde652e2556b7f4f33b53ad8336f0f52 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 14 Oct 2026 19:01:42 -0700 Subject: [PATCH] fix(acpx): bound Claude MCP tool calls when runtime MCP servers attach (#16360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Hosted `claude_local` runs use the ACPX engine. Paperclip gives each run its runtime MCP servers (for example the `paperclip-assigned` tool gateway) as ACP `mcpServers` entries. > - Claude's default MCP tool timeout is far longer than a run. ACP `mcpServers` entries have no per-server timeout field. So the agent waits on an MCP call until the call answers. > - Hosted error reports show runs that end with "Run exceeded the adapter execution timeout (sandbox)". The stop-time diagnostics from #15962 show the problem: the agent went silent about 50 seconds into the run, three tool calls of ACP kind `other` stayed pending, and nothing ended them until the 4h sandbox backstop. > - The gateway bounds the remote provider call (10s default, 60s max). But other steps on the gateway `tools/call` path have no deadline: DB queries, row locks and pool acquisition, the credential steps before the provider timer starts, and the stdio child exit wait. One stuck step holds the agent turn for hours. > - This pull request gives the agent's MCP client a deadline: `MCP_TOOL_TIMEOUT` for ACP Claude runs that receive runtime MCP servers. > - The benefit is that a stalled gateway call becomes an MCP error that the model can see. The turn continues and does not spend the full 4h execution budget. ## Linked Issues or Issue Description - Refs #15962 (diagnostics that identified the silent turn with pending tools) - Refs #15448, #15456, #15850 (related gateway timeout and connection-health work; this PR does not overlap with them) **What happened?** A hosted `claude_local` run sent parallel MCP tool calls. No call answered. The run stayed silent for about 4 hours. Then the sandbox execution timeout ended it with "Run exceeded the adapter execution timeout (sandbox)". **Expected behavior** A stalled MCP call ends with an error after a bounded time. The agent sees the error and continues. **Steps to reproduce** 1. Start an ACP `claude_local` run that has runtime MCP servers. 2. Make the gateway `tools/call` request stall (for example, hold a lock that the request needs). 3. See that the run stays silent until the adapter execution timeout. **Paperclip version or commit** `master` at 976cdc7. ## What Changed - Add `ACPX_CLAUDE_MCP_TOOL_TIMEOUT_MS` (15 minutes) in `acpx-engine/constants.ts`. The value has wide headroom over the gateway's longest legitimate wait (60s provider calls, 2 minutes of approval preparation, and the 10-minute connection cap proposed in #15850). - The ACPX engine sets `MCP_TOOL_TIMEOUT` in the agent env for Claude when the run has runtime MCP servers. If the adapter `env` already sets `MCP_TOOL_TIMEOUT`, the engine keeps that value. Other agents and runs without MCP servers do not change. - Add an engine test for the bound, the no-server case, the user override, and a non-Claude agent. - Document the client-side deadline in `doc/MCP-RUNTIME-OPERATIONS.md`. ## Verification - `npx vitest run src/acpx-engine/` in `packages/adapter-utils`: 21 files, 423 tests pass. - The new test fails without the env assignment (`expected undefined to be '900000'`), and passes with it. - `npx tsc --noEmit -p packages/adapter-utils`: passes. - The env reaches Claude through the same per-session env path as `ANTHROPIC_MODEL`. `claude-agent-acp` merges this env into the Claude SDK env. ## Risks - A legitimate MCP call that takes longer than 15 minutes now fails. No gateway path is designed to wait that long. Operators can set `MCP_TOOL_TIMEOUT` in the adapter `env` to change the value. - The value also applies to other MCP servers in the same Claude session, for example servers in user settings. The same override is available. - Warm sessions get the env when the agent process starts again. The env is not part of the session fingerprint, so this change does not reset resumable sessions. - This PR does not fix the server-side step that stalls. It bounds the effect on the run. Server-side deadlines on the gateway `tools/call` path (DB lock and statement timeouts, a request-level deadline, SIGKILL escalation for stdio children) are possible follow-ups. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) through Claude Code, with tool use and code execution. It read the repository, traced the gateway path, edited the code, and ran the tests. ## 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: devinfoley Co-authored-by: Claude Opus 5.5 --- doc/MCP-RUNTIME-OPERATIONS.md | 12 +++++++++++ .../src/acpx-engine/constants.ts | 10 ++++++++++ .../src/acpx-engine/execute.test.ts | 20 ++++++++++++++++++- .../adapter-utils/src/acpx-engine/execute.ts | 6 ++++++ 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/doc/MCP-RUNTIME-OPERATIONS.md b/doc/MCP-RUNTIME-OPERATIONS.md index 9a05b0157a6..011c5c6fbbc 100644 --- a/doc/MCP-RUNTIME-OPERATIONS.md +++ b/doc/MCP-RUNTIME-OPERATIONS.md @@ -36,6 +36,18 @@ delivery path. Managed gateway discovery excludes historical native assignments. Their stored rows remain available for existing runs. Explicit company gateways continue to use their configured scopes. +## Client-side tool-call deadline + +The gateway bounds each provider call, but the agent's MCP client also needs a +bound. Without one, a gateway request that never answers holds the agent turn +until the adapter execution timeout ends the run. Claude's default MCP tool +timeout is far longer than a run, and ACP `mcpServers` entries have no +per-server timeout field. So when an ACP Claude run receives runtime MCP +servers, the ACPX engine sets `MCP_TOOL_TIMEOUT` to 15 minutes. A call that +passes this deadline returns an MCP error to the model, and the turn +continues. An `MCP_TOOL_TIMEOUT` value in the adapter `env` configuration +takes priority. + ## Metrics The board runtime health API summarizes one-hour event windows plus current durable slot state: diff --git a/packages/adapter-utils/src/acpx-engine/constants.ts b/packages/adapter-utils/src/acpx-engine/constants.ts index 1c37dc958cd..a3330c29a2d 100644 --- a/packages/adapter-utils/src/acpx-engine/constants.ts +++ b/packages/adapter-utils/src/acpx-engine/constants.ts @@ -26,6 +26,16 @@ export const ACPX_HANDSHAKE_TRANSPORT_POLL_MS = 250; // much smaller than the whole-adapter execution timeout. export const ACPX_DUPLEX_LOSS_CANCEL_DEADLINE_MS = 30_000; +// The per-call deadline Claude applies to MCP tool calls (`MCP_TOOL_TIMEOUT`) +// when Paperclip attaches runtime MCP servers. Claude's own default is far +// longer than a run, so a gateway call that never answers holds the turn until +// the 4h sandbox backstop ends the run. The gateway bounds each provider call +// at 60s and approval preparation at 2 minutes; this leaves wide headroom over +// both. +// On expiry Claude returns an MCP error to the model and the turn continues. +// A user-configured `MCP_TOOL_TIMEOUT` still wins. +export const ACPX_CLAUDE_MCP_TOOL_TIMEOUT_MS = 15 * 60 * 1000; + // The closed ACP `ToolKind` enum. Run diagnostics may report these labels for // pending tool calls; tool titles, names, and inputs never leave the run. // Any other reported kind is collapsed to "unknown". diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index 095996c1f83..2e604a9719f 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -37,7 +37,7 @@ import { summarizeAcpxTurnUsage, type AcpxEngineExecutorOptions, } from "./execute.js"; -import { ACPX_HANDSHAKE_TIMEOUT_MS } from "./constants.js"; +import { ACPX_CLAUDE_MCP_TOOL_TIMEOUT_MS, ACPX_HANDSHAKE_TIMEOUT_MS } from "./constants.js"; import { sessionCodec } from "./session-codec.js"; import { runChildProcess } from "../server-utils.js"; import { createPromptContextFixture } from "../test-fixtures/prompt-context.js"; @@ -2986,6 +2986,24 @@ describe("shared ACPX engine runtime behavior", () => { expect(third.runtimeOptions[0]?.mcpServers).toEqual([]); }); + it("bounds Claude MCP tool calls only when runtime MCP servers are attached", async () => { + const server = { name: "paperclip-assigned", url: "https://example.test/mcp/gateways/g", connectionId: "assignment:x", token: "t" }; + const runtimeMcp = { getServers: () => [server] }; + const envOf = (run: { meta: Array> }) => run.meta[0]?.env as Record; + + const bounded = await runExecutor({ agent: "claude" }, { runtimeMcp }); + expect(envOf(bounded).MCP_TOOL_TIMEOUT).toBe(String(ACPX_CLAUDE_MCP_TOOL_TIMEOUT_MS)); + + const withoutServers = await runExecutor({ agent: "claude" }); + expect(envOf(withoutServers).MCP_TOOL_TIMEOUT).toBeUndefined(); + + const userConfigured = await runExecutor({ agent: "claude", env: { MCP_TOOL_TIMEOUT: "120000" } }, { runtimeMcp }); + expect(envOf(userConfigured).MCP_TOOL_TIMEOUT).toBe("120000"); + + const codex = await runExecutor({ agent: "codex" }, { runtimeMcp }); + expect(envOf(codex).MCP_TOOL_TIMEOUT).toBeUndefined(); + }); + it("injects runtime MCP servers and fingerprints their identity without persisting bearer tokens", async () => { const root = await makeTempRoot(); const baseConfig = { diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index 01ae1020b8d..e052d04ace0 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -106,6 +106,7 @@ import { } from "acpx/runtime"; import { ACP_TOOL_KINDS, + ACPX_CLAUDE_MCP_TOOL_TIMEOUT_MS, ACPX_DUPLEX_LOSS_CANCEL_DEADLINE_MS, ACPX_HANDSHAKE_TIMEOUT_MS, ACPX_HANDSHAKE_TRANSPORT_POLL_MS, @@ -2047,6 +2048,11 @@ async function buildRuntime(input: { if (requestedModel && acpxAgent === "claude" && !env.ANTHROPIC_MODEL) { env.ANTHROPIC_MODEL = requestedModel; } + // ACP `mcpServers` entries carry no per-server timeout, so bound Claude's + // MCP tool calls through its env setting instead. + if (acpxAgent === "claude" && mcpServers.length > 0 && !env.MCP_TOOL_TIMEOUT) { + env.MCP_TOOL_TIMEOUT = String(ACPX_CLAUDE_MCP_TOOL_TIMEOUT_MS); + } // Gemini CLI reads GEMINI_MODEL when creating each ACP session. Its ACP // implementation supports set_model, but not set_config_option; configure // the selected model before startup without changing the transport.