mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 16:27:18 +02:00
938a441944
## 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 sends
runtime MCP servers to Claude, and Claude can start subagents (the Task
tool) that call those MCP tools.
> - #16360 set `MCP_TOOL_TIMEOUT` for Claude. That bound works: in a
local test with the same Claude binary, a direct MCP call and a subagent
MCP call to a server that never answers both ended at the bound.
> - A hosted run still hit the 4h sandbox timeout on a release that has
#16360. Its run log shows a subagent that started four MCP calls. ACP
reported the four calls as `pending`, and they never got another update.
About 40 other MCP calls in the same minutes finished in 3–25 seconds.
So the four calls most likely never reached the MCP server, and
`MCP_TOOL_TIMEOUT` did not apply to them.
> - The subagent's Task call got an `in_progress` heartbeat every 30
seconds for the full 4 hours. The turn looked active, so no existing
bound stopped it.
> - This pull request adds a stalled-tool watchdog to the ACPX engine.
It ends the turn when a pending tool call gets no update for 30 minutes
and no other tool call finishes in that time.
> - The benefit is that a stalled run fails in about 30 minutes with a
clear error code, not after 4 hours with a general timeout.
## Linked Issues or Issue Description
- Refs #16360 (MCP client deadline; this PR covers the stall that the
deadline cannot reach)
- Refs #15962 (stop-time diagnostics that showed the pending tools and
their age)
**What happened?**
A hosted `claude_local` run gave lookups to two subagents. One subagent
started four MCP tool calls. ACP reported each call as `pending`. No
call got a result, an update, or an error. The other subagent finished
in 3 minutes. The stalled subagent's Task call got a heartbeat every 30
seconds, so the last-event age at stop was only 28 seconds. The oldest
pending tool was 3h58m old. The sandbox execution timeout ended the run
after 4 hours.
**Expected behavior**
A tool call that never gets a result ends the turn after a bounded time.
The run fails with an error that names the stall.
**Steps to reproduce**
1. Start an ACP `claude_local` run that uses a subagent to call runtime
MCP tools.
2. Make one of the subagent's tool calls stay `pending` with no further
ACP update (in production this happened without a known trigger).
3. See that the run continues until the adapter execution timeout,
because the Task heartbeats keep the turn active.
**Paperclip version or commit**
`master` at 4415665.
## What Changed
- Add `ACPX_STALLED_TOOL_TIMEOUT_MS` (30 minutes),
`ACPX_STALLED_TOOL_MARGIN_MS` (15 minutes), the check interval, and the
`acpx_tool_stalled` error code in `acpx-engine/constants.ts`.
- Add `resolveStalledToolTimeoutMs()`. Claude turns use 30 minutes, or
`MCP_TOOL_TIMEOUT` / `BASH_MAX_TIMEOUT_MS` plus 15 minutes if that is
longer. Other agents are not watched. The adapter config field
`stalledToolTimeoutSec` overrides the bound for any agent, and 0
disables it. The bound is not part of the session fingerprint.
- The engine records the time of the last update for each tool call, and
the time a tool call last reached a final status.
- During a turn, the engine checks the pending tool calls every 60
seconds. If a pending call has had no update for the bound, and no tool
call reached a final status in that time, the engine:
- records the stop-time activity (the same fields as a timeout or an
operator stop),
- writes a `[paperclip]` stderr line with the closed tool kind and the
silent time,
- cancels the turn, and starts the existing fail-fast cancel deadline
that the duplex-loss path uses. The deadline helper is now shared by
both paths.
- Finalization reports the stall as a failure: `errorCode:
"acpx_tool_stalled"`, `resultJson.status: "failed"`,
`acpStalledToolKind` (one closed ACP `ToolKind` label or `unknown`), and
`acpStalledToolSilentMs`. A timeout or a duplex loss still takes
priority. A turn that completed anyway keeps its success.
- The server Sentry diagnostics allowlist the two new fields. It drops
the kind if it is not one closed label.
- Document the watchdog in `doc/observability.md` and the
`stalledToolTimeoutSec` field in the `claude_local` config notes.
## Verification
- `npx vitest run src/acpx-engine` in `packages/adapter-utils`: 21
files, 422 tests pass.
- New engine tests:
- A silent pending call next to a Task call that keeps getting
heartbeats ends the turn with `acpx_tool_stalled`, the closed kind, and
the silent time. No tool title or ID appears in the diagnostics.
- An agent that ignores the cancel: the fail-fast deadline closes the
stream and the run fails with `acpx_tool_stalled`.
- A long tool that keeps getting updates is not cancelled.
- A queued call that waits behind slow calls that keep finishing is not
cancelled.
- `stalledToolTimeoutSec: 0` disables the watchdog, and only the
wall-clock timeout ends the turn.
- `resolveStalledToolTimeoutMs()` defaults, env bounds, other agents,
and config override.
- Mutation checks: with the watchdog disabled, the two stall tests time
out (the same hang as production). Without the "no tool finished" guard,
the queued-call test fails.
- `npx vitest run
src/services/__tests__/run-failure-diagnostics.test.ts` in `server`: 115
tests pass.
- `npx tsc --noEmit -p packages/adapter-utils`: passes.
## Risks
- A legitimate tool call that gets no ACP update for 30 minutes while no
other tool call finishes now fails the run. Claude's own tool bounds
(MCP 15 minutes, Bash 10 minutes by default) end healthy calls before
that. If an operator raises `MCP_TOOL_TIMEOUT` or `BASH_MAX_TIMEOUT_MS`
in the env, the bound moves up with it. Operators can also set
`stalledToolTimeoutSec` or disable the watchdog with 0.
- A long sync subagent stays safe while its Task call gets heartbeats.
Only a call that is itself silent can trip the watchdog.
- The run fails instead of continuing. The failure uses the normal
failed-run recovery, and it reports to Sentry under its own error code,
not under the generic timeout.
- This PR does not find why the four calls never got a result. Claude
runs with `--permission-mode default`, so each MCP call makes an ACP
permission round trip, and the ACP stream goes through the remote
process-session relay. A lost request or response on that path is the
most likely cause. The new diagnostics show how often the stall happens
and for which tool kind.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`) through Claude Code, with tool use
and code execution. It read the run log and the ACP agent and client
code, 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 <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@paperclipai/adapter-utils
Shared utilities for Paperclip adapters: process spawning, environment injection, sandbox/SSH transport, workspace sync, and the round-trip helpers that move code between the local execution-workspace cwd and wherever the agent actually runs.
For the adapter-author guide see
docs/adapters/creating-an-adapter.md
and the in-repo notes at packages/adapters/AUTHORING.md.
Sandbox bridge deadlines
Command-managed bridge control operations (queue reads, input delivery, setup, and cleanup) have a host-enforced deadline of at most 30 seconds per shell command. A shorter configured timeout still applies. These operations cannot inherit the agent run's hours-long lifetime or wait forever on a provider that ignores its timeout. Long-lived agent session commands keep their own limits.
A control timeout reports a transport failure. It does not prove that the remote command stopped and does not replay an uncertain write. The process session closes failed input delivery and uses its existing shutdown path; normal execution settlement must still verify termination.
No-remote-git contract
The local execution-workspace cwd is the only persistence boundary across runs. No adapter may depend on a git remote for cross-run state.
Adapters that run the agent on a different host should use the SSH round-trip
helpers in src/ssh.ts:
prepareWorkspaceForSshExecution({ spec, localDir, remoteDir })— bundles the local cwd (tracked files, dirty edits, untracked additions, and the git history needed to reconstruct it) toremoteDirbefore the run starts. Runs with nogit remoteconfigured.restoreWorkspaceFromSshExecution({ spec, localDir, remoteDir, ... })— syncs the remote cwd back intolocalDirafter the run, including any new commits the agent created. Also runs with nogit remoteconfigured.
prepareRemoteManagedRuntime in
src/remote-managed-runtime.ts wraps both
calls for adapters that want a per-run remote workspace and an automatic
restoreWorkspace() finally hook.
The invariant is pinned by the no-remote-git contract case in
src/ssh-fixture.test.ts, which asserts that a
remote-only commit propagates to the local worktree through the
prepare → restore round-trip with no git remote configured at any point. Do
not regress that test.