mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
DottaandClaude
3e572475c8
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a person answers an agent's saved question, Paperclip starts an answer run. For a per-turn session, that run starts a new runner and sends `run.attach` to bind the saved conversation to the run > - The runner journals each command before its effect. A restart that kills it inside that window leaves `run.attach` with an unknown outcome. The restored runner settles it as `execution_indeterminate` and never executes it again > - The controller's same-run resume waited on that old command, so every resume failed with "PRP command run.attach failed". The answer never reached the agent. A Retry or a stall followed > - A kill during the attachment's provider launch can also leave a startup fence that no later runner retires, so every later command fails with "provider startup ownership remains unadmitted" > - This pull request attaches again with a new command when nothing after the cut-off attachment started provider work. It also lets the runner retire that orphaned startup fence > - The benefit is that an answer given just before a restart continues on its own ## Linked Issues or Issue Description **What happened?** Local restart-chaos journey wave `w-20261110T054406-s108` on master `2c2186706e`. Command: `--adapters codex --workloads Q1,Q2 --faults K1 --triggers answered --tasks 4 --concurrency 2 --seed 108`. Result: 1 of 4 passed. - t02 (Q1, K1 2.7 s after the answer). Answer run `0b357a1f` resumed after the restart. Each resume failed with `native_session_recovery_failed: PRP command run.attach failed: execution_indeterminate`. Then "Automatic recovery stopped", and the task needed a Retry. - t01 (Q2, K1 1.1 s after the answer). Answer run `82c60af9` failed the same way on three same-run resumes. Its provider state kept `lifecycle: "prepared"` with a `spawned` startup attempt for `run.attach`, and that provider process was gone. Each later drain and suspend failed with "provider startup ownership remains unadmitted". The successor run then refused with `retirement_refused=journal_commands_unsettled(run.attach.failed)`. The accepted Retry created no run (`retry_no_new_run`). The earlier wave `w-20261110T033426-s108` hit the same window at different points. It showed `authority_indeterminate` (Q1) and an ambiguous `turn.start` stall (Q2). See Risks for what this pull request does not cover. **Expected behavior** An answer run that a restart interrupts before it starts provider work resumes by itself. It does not need a Retry. **Steps to reproduce** 1. Run the wave command above on master. 2. Or run `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-codex-transport-cut-attach.test.ts` on master. Both cases fail with the error from the wave. ## What Changed - `runnerd-codex-transport.ts`: the same-run resume waits on the recovered `run.attach` as before. If that wait fails, the new `cutOffRunAttachmentPayload` checks the journal. It queues one new `run.attach` with the same payload, and waits on it, only when all of these are true: - the latest `run.attach` has an unknown outcome (`indeterminate`, or `failed` with `execution_indeterminate`); - it is a cold attachment (no `paperclipNextAuthority`), with no warm transition and no staged attachment; - every later command is recovery maintenance, as the control plane's shared `isRunnerRecoveryMaintenanceCommand` defines it (`session.snapshot`, `session.goal.get`, `runner.drain`, `runner.suspend`, `runner.shutdown`, `turn.stop`); - no committed event shows provider work (`turn.accepted`, `turn.started`, `item.started`, `tool.execution.started`, `semantic_tool.input`, `runtime_request.created` or `mcp_app.tool_input`). Any other failure is thrown unchanged. - `provider_backend.rs` (Runner): `retire_orphaned_startup_attempt` also treats a `prepared` state with a saved thread and a nonzero process generation as restorable. `attach_run` already documents this state as the settled checkpoint that `run.attach` restores. The other fence rules do not change. The fence is retired only before the spawn, or when the recorded process and its group have exited. - Tests: - New real-runnerd test `runnerd-codex-transport-cut-attach.test.ts`. A real runner and the fake Codex server run an asking run, then a cold answer-run attachment. The test kills both processes and rewinds both journals to "`run.attach` journaled, outcome not recorded", then resumes. One case has a startup fence and one does not. - A unit test for `cutOffRunAttachmentPayload`. It covers each maintenance command, and the cases where it must not re-attach (a definite answer, a pending command, a warm or staged attachment, any work command, a turn event). - The Rust orphaned-startup test also covers `prepared`. A new test shows that a `prepared` state with no saved thread keeps its fence. - `docs/durable-recovery.md`: describes both behaviors. ## Verification - Red/green, real runnerd: - On master, `runnerd-codex-transport-cut-attach.test.ts` fails both cases with `PRP command run.attach failed: ... execution_indeterminate`, the error from the wave. - With only the transport change, the case without a fence passes. The fence case fails with `provider startup ownership remains unadmitted`, the t01 error. - With both changes, both cases pass. - Red/green, Rust: `cargo test -p paperclip-runner-core --lib orphaned` fails `restart_retires_a_spawned_startup_whose_provider_process_exited` for `prepared` without the runner change. With it, all 10 tests pass. - Related transport suites, 8 files and 387 tests, pass: `runnerd-codex-transport`, `-attachment-recovery`, `-cold-restore`, `-warm-attachment`, `-release-resume`, `-cut-attach`, `runnerd-restart-continuation` and `runnerd-saved-continuation`. `tsc --noEmit` for the Runner package passes. - Live proof: the same wave command on this branch. Results follow in a comment. **No duplicate provider work.** The new attachment is sent only when the journal proves that no provider work followed the cut-off one. No command after it can start or steer work: only the shared recovery-maintenance commands are allowed, and none of them starts a turn. No turn, item, tool-input or runtime-request event is committed. `run.attach` itself starts no turn. In the Codex and ACPX runners, `attach_run` refuses to attach unless the session is settled. That means no active or ambiguous turn, no pending tool call and no live active goal. The old command is never executed again, because the runner refuses an `indeterminate` command. The new attachment is a new command with a new ID. ## Risks - Low-medium risk. The new path runs only after a same-run resume has already failed on an unknown-outcome attachment. Before this change, that run could not continue. - If an attachment's partial effect had changed provider state, the runner's own settled-session check refuses the new attachment. The run then fails as it does today. - **Runner release:** the Runner change does not change the pack `contractDigest`. It is `sha256:3164357d…` before and after (`scripts/contracts/contract-fingerprints.mjs`). Local runners use the runnerd built with the app, so the fix takes effect with this release. A remote image pack (for example Daytona) that was built before this change keeps the old fence rule until a new image or pack is built and qualified. No release or pin bump is part of this pull request. - **Mixed versions during an upgrade:** - New server with an old runner: the server re-attaches. A run without a startup fence continues. With a fence, the old runner refuses the new attachment, the same failure as today. - Old server with a new runner: the runner retires the fence, but the old server still fails on the unknown-outcome attachment, as today. - Neither mix adds a new failure. Saved state from older builds needs no migration. - Not covered here, and still open: - A `turn.start` with an unknown outcome where the resumed thread has no active turn (wave `w-20261110T033426-s108` t03). The runner deliberately fails closed for that case. - A warm-runner attachment that may already have been delivered (`authority_indeterminate`). - A Retry that waits on `process_identity_missing` for a run that failed before any process started. A separate pull request will cover it. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use (shell, file edits, test 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 - [ ] 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 Refs #20146, #19784, #20148 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>