mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 14:19:00 +01:00
DottaandClaude Opus 5.5
e015c2ef21
## Thinking Path - #20145 prepares the next Claude provider generation at park, and #20190 restarts the ACPX provider when a turn settles. They move the sidecar respawn off the follow-up's critical path. - When that turn-end restart fails, the runner "fails closed": `restart_session_after_turn` saves the session `closed` and drops it. Within the run, the next turn fails exactly like a failed restart at turn start. That's intended. - Across runs it breaks the conversation: - the next run's `run.attach` refuses a `closed` session ("run.attach requires the same settled ACPX provider profile and session"); - it reconnects for about 10 s; - the controller then records a continuity break and starts a fresh Claude conversation (`run.startup.reuse provider_session: fresh/created`). - A local A/B (`ttft-baseline --adapters claude --samples 3 --phases coldwarm`, on a host with load average about 17–27) confirmed it: | |d40a7494(before #20145/#20190) |a3a25ce6(after) | |---|---|---| | Follow-ups that resumed the conversation | 3 of 3 (`verified_reuse`, resume 4.4–8.2 s) | 1 of 3 | | Follow-ups that started a fresh conversation | 0 of 3 | 2 of 3 (bootstrap 16–17 s) | The kept instance shows each lost follow-up's runner received provider state `lifecycle: "closed"`, with `run.attach` refused. - A failed park preparation already recovers safely: it waits for the reaped provider's lifetime fence, then saves `suspended`. The turn-end restart should do the same. ## Linked Issues or Issue Description Refs #20190, #20145 **What happened?** When the post-turn ACPX provider restart failed, the next Claude run could not attach. It spent about 10 s and then started a new conversation, losing the task's conversation context. **Steps to reproduce** 1. Run a Claude task on a busy host so the turn-end provider restart fails (the fixture `reject-open` fault reproduces it). 2. Post a follow-up comment (a new run). 3. The follow-up's `run.attach` is refused, then `run.startup.reuse` reports `provider_session: fresh/created`. **Expected behavior** The follow-up resumes the same Claude conversation. Within the failing run, later turns still fail closed. ## What Changed - `acpx_park_preparation.rs`: - A failed `restart_session_after_turn` now calls the shared `recover_failed_generation_restart`, renamed from `recover_failed_park_preparation`. - That recovery drops the session, marks it `prepared` with the provider exit unconfirmed, waits for the provider-lifetime fence, then saves `suspended`. - If the fence or the save fails, the state stays exit-unconfirmed, which is the safe direction. - `acpx_provider_backend.rs`: - Later turns of the failing run still fail closed: the first with "ACPX turn start failed closed: <cause>", then "ACPX session is not open". That's the same as a failed restart at turn start. - The restriction is saved in the ACPX state as `turnEndRestartFailedRun` (serialized only while set), so it outlasts a Runner process restart inside the same run. Same-run restore doesn't reopen the session while it's set. - Only a `run.attach` from a different run clears it; a same-run attach keeps it. State validation requires it to match the descriptor's run. - The cause is handed to the durable Runner once (`take_turn_end_restart_failure`), which records it as a redacted `turn-end provider restart failed: <cause>` diagnostic in `runner-state.json`. That gives the under-load cause a first recorded sample. - Tests (`tests/support/acpx_turn_end_restart.rs`): - New: `the_next_run_resumes_the_conversation_after_a_failed_turn_end_restart`, red on master (the state is `closed`). It covers the same Runner process with a warm attach, and a new Runner process with a cold attach. The conversation id is preserved, and the next run completes a turn. - New: `a_runner_restart_in_the_same_run_keeps_the_failed_turn_end_restart_closed`, red without the saved field. A new Runner process with the same run id fails `turn.start` without spawning a provider; then a new run cold-attaches and resumes the same conversation. - The existing `a_failed_turn_end_restart_fails_the_next_turn_like_a_failed_turn_start_restart` now also asserts that the turn-end failure saves `suspended` (the turn-start failure stays `closed`). Everything else about the run's failing turns is unchanged. ## Verification - `cargo test -p paperclip-runner-core`: - `--test acpx_park_preparation`: 21 passed. - `--lib acpx`: 93 passed. - `acpx_park_readiness`: 8 passed. - `acpx_warm_attachment`: 4 passed. - `acpx_provider_suspend`: 5 passed. - `acpx_provider_session`: 12 passed. - `acpx_provider_turns`: 29 passed. - `native_provider_backend`: 47 passed. - `durable_recovery`: 3 passed. - Live A/B on this branch (`ttft-baseline --adapters claude --samples 3 --phases coldwarm`): follow-ups resumed 3/3 (1/3 before), follow-up first activity p50 6.2 s (about 20–23 s before). - `cargo clippy --all-targets`: no findings on changed lines. ## Risks - **Late-callback guarantee:** unchanged. The failed restart already reaped the old generation, and the next generation starts only via `run.attach` or a later reopen. That's the same resume path a failed park preparation uses. - **Fence wait:** the failure path now waits for the reaped provider's lifetime fence before saving, matching park recovery. If the fence never clears, the state stays exit-unconfirmed and fails closed. - **Rollback:** `AcpxDurableState` denies unknown fields. The new field is written only between a failed turn-end restart and the next run's attach. A rollback inside that window makes the older Runner refuse the saved state, which still fails closed, as master's `closed` state does today. - **Scope:** the cause of restart failures under load isn't changed here. This makes their failure mode safe. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## 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>