mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
DottaandClaude
914e3ae14f
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Claude agents run through the native Runner and the ACPX sidecar > - A saved question or a board Stop cancels the active turn with `turn.cancel` > - The Runner waited 30 s for the sidecar's reply to that cancel, and the Runner serves controller commands one at a time > - When that reply was lost, the controller's shutdown commands timed out first, so the run failed and the task needed a human Retry > - This pull request bounds that wait, keeps the transport open, and lets the provider's own turn terminal settle the turn > - The benefit is that a lost cancel reply on Claude settles the same way it already does on Codex (#20336) ## Linked Issues or Issue Description Refs #20336 (Codex equivalent: settle a saved handoff after verified shutdown). Related, no overlap: #20477, #20482, #20483. **What happened?** The restart-chaos harness dropped the ACPX sidecar's reply to the Runner's `turn.cancel` (fault PV2, target `interrupt_ack`). A Claude task that saved a question (Q1) failed with "The waiting state is saved; confirming shutdown of the previous execution is still pending". Its next attempt failed `native_session_cleanup_quarantined`. A Claude task stopped by the board (S1) ended `blocked`. Retry was accepted but started no run. Cause, in `packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_session.rs` (`interrupt_turn`, master line ~800): the cancel used the ordinary sidecar request with the 30 s bound from `acpx_provider_backend.rs:539`. While it waited: - `turn.stop` and `runner.suspend` stayed queued. The controller's 15 s governed drain plus its 10 s close grace ran out first (`runner did not durably suspend before checkpoint`, suspend command `pending`). - At 30 s, `fail_closed` killed the sidecar and dropped the provider's buffered turn terminal. The poll path then recorded `turn.failed` (`acpx_provider_transport_failed`) and `provider_exit_unconfirmed` (`acpx_provider_backend.rs:2716-2738`). `turn.stop` returned `already_settled`, and `runner.suspend` refused with `ACPX provider lifetime cleanup is not yet proven` (`acpx_provider_backend.rs:2613-2621`). Codex passes the same fault because its Runner does not block on the interrupt reply. **Expected behavior** A lost cancel reply must not fail the run. The saved question must settle as `succeeded`. A board Stop must end the run as cancelled with a proven suspension, so the next turn can start. **Steps to reproduce** `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --adapters claude --workloads Q1,S1 --faults PV2 --provider-fault-message interrupt_ack --concurrency 1 --tasks 4` Failing wave on master: `w-20261110T060120-s207` (t04, t05). **Deployment mode** Native Runner with the Claude ACPX profile, local. Hosted sandboxes run the same Runner code. ## What Changed - `AcpxSidecarTransport::request_turn_cancel`: sends `turn.cancel` and waits only to a caller deadline. On a missing reply it keeps the transport usable and retains the request ID, so a late reply is skipped in both the response and the event-poll paths. A rejected reply and transport errors keep their existing fail-closed result. - `AcpxProviderSession::interrupt_turn`: waits at most 5 s (or the shorter configured request bound) for the cancel reply. On a missing reply it retires the runtime from warm reuse and returns. The provider's turn terminal still arrives through polling, and `turn.stop` still reaps a provider that never stops. - `AcpxProviderSession::deliver_tool_result`: a board Stop can land while a semantic tool runs. The session's own `turn.cancel` then retires the call in the sidecar before its result arrives, and the turn terminal may still be buffered. The sidecar rejects the result as stale (`ACPX_TOOL_CALL_STALE`). When this session sent `turn.cancel` for that turn, the Runner now keeps the durable result, like a result for an already closed turn. Every other rejection still fails closed. (`AcpxSidecarTransport::request_tool_resolve` reports whether a rejection was the stale-call case.) - A repeated interruption while a cancel is unanswered sends nothing and returns at once (review finding). - `fake-acpx-sidecar`: new `--cancel-reply lost|late` flag. After that cancel, `tool.resolve` is rejected as stale. The provider ends the turn, and the reply is dropped or written just before the next request. - Three regression tests (below) and a short section in `packages/paperclip-runner/docs/durable-recovery.md`. No TypeScript, server, schema or protocol change. The normal start path is unchanged; nothing waits before turn start. ## Verification Red/green (red runs used the master versions of `acpx_provider_session.rs` and `acpx_sidecar_transport.rs`): - `cargo test -p paperclip-runner-core --test acpx_warm_attachment a_lost_cancellation_reply_still_settles_the_turn_and_suspends_the_runner` - Red: after 30.4 s, `ACPX turn interrupt failed: ACPX sidecar request timed out at turn.cancel` (the live error). - Green: `turn.interrupt` returns in about 5 s, then `turn.interrupted` and a `cancelled` `run.terminal` (no `turn.failed`), `turn.stop`, and `runner.suspend` completes. - `cargo test -p paperclip-runner-core --test acpx_provider_turns lost_cancellation_reply_keeps_the_turn_terminal_and_the_transport` - Red: `interrupt_turn` fails at the request bound. - Green, for a lost reply and for a late reply: the terminal is polled, warm reuse is refused, and the next request (`session.close`) succeeds after the late reply arrives. A late reply causes no second settle and no second prompt. - `cargo test -p paperclip-runner-core --test acpx_provider_turns a_tool_result_for_a_call_retired_by_our_cancellation_does_not_fail_the_transport` - Red (guard disabled): `ACPX sidecar command tool.resolve was rejected (retryable=false, classification=provider_tool_call_retired)`, the same error as live wave task t03 below. - Green: the result is kept, the pending tool clears, and the turn terminal is polled after it. - `cargo test -p paperclip-runner-core` (whole crate): all pass. Existing `turns-tool-resolution-rejected` coverage still fails closed when no cancel was sent. `cargo fmt --check` passes. - Pack contract digest: unchanged (`sha256:3164357d…` before and after, from `scripts/contracts/contract-fingerprints.mjs`). - Live wave 1, `w-20261110T072706-s211` (commit `d91bff1e9f`, the first fix only), `--adapters claude --workloads Q1,S1 --faults PV2 --provider-fault-message interrupt_ack --tasks 4`: 3 of 4 pass. Both Q1 tasks pass (the asking run `succeeded`, no failed run, answer in the brief). One S1 passes. S1 task t03 failed: Stop landed during a `write_task_document` call and hit the stale tool-result error above. The second commit fixes that case. - Live wave 2, `w-20261110T082702-s212` on head `76dd57515d`, same flags: every task passes R1–R6 and U1. Both Q1 tasks pass with the fault applied (asking run `succeeded`, no failed run). S1 t01 passes with the fault applied (Stop took effect, task `done`, no Retry). S1 t03 fails only `provider_fault_applied` (`applied=0`): the agent finished before the Stop window, so nothing was injected and the task proves nothing either way. - Live wave 3, `w-20261110T091057-s213` on head `76dd57515d`, same flags: the same result. t01 (S1), t02 and t04 (Q1) pass with the fault applied; t01's Stop took effect (`status=cancelled`), task `done`, no Retry. t03 again fails only `provider_fault_applied` (`applied=0`, the run finished before the Stop window); all its R1–R6 and U checks pass. - Across waves 2 and 3 on the final head: 6 of 6 tasks with the fault applied pass (4 Q1, 2 S1). No product rule failed. The two misses are harness timing (no injection), not product failures. ## Risks - A healthy cancel reply that takes longer than 5 s now retires the warm runtime. The next turn starts a fresh provider process for that session. The observed healthy reply time is about 1 s. - If the provider never stops and the reply is lost, the turn stays active until `turn.stop` terminates the provider and proves the lifetime fence. If that proof fails, the existing settlement-pending and quarantine states are unchanged; this PR adds no success path without proof. - Runner binary change: hosted Daytona images need a normal image rebuild to pick up the fix. The pack `contractDigest` does not change, so existing image packs stay reusable. ## Model Used - Anthropic Claude, model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file edit, 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>