mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
DottaandClaude Opus 5.5
fff70fe49e
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs execute through `paperclip-runnerd`. When the control plane restarts, recovery starts a new runnerd process, and that process restores the provider (for example the Codex app server) from durable state. > - Each provider launch writes a durable startup fence before it spawns the process. Admission clears the fence. The fence stops a second runner from launching a provider while an unadmitted one may still run. > - A restart chaos wave sent SIGTERM and then SIGKILL to the control plane in quick succession. The second kill landed after the restored Codex provider was spawned and before it was admitted. > - The fence stayed in state with phase `spawned`. Every later runner refused to restore. The recovery probe's `runner.drain` failed with "provider startup ownership remains unadmitted", and the task needed a manual Retry. > - This pull request lets a new runner retire that fence when the recorded provider process and its process group have provably exited. It keeps every other fence closed. > - The benefit is that a double restart during provider relaunch continues automatically. ## Linked Issues or Issue Description **What happened?** A Codex run was active. A graceful restart (K2) suspended it, and recovery started a new runnerd that began restoring the provider. That runnerd spawned the Codex app server (pid recorded, phase `spawned`). A hard restart (K1) 9 s later killed it before admission. At the next boot, recovery chose `resume_dead_runner` and queued its `runner.drain` probe. The new runnerd failed the probe with `command_execution_failed: provider startup ownership remains unadmitted`. The run ended `native_session_recovery_failed`, and the task needed a Retry. The kept provider state had `lifecycle: "turn_active"` and a `startupAttempt` with `phase: "spawned"`, `trigger: "restore"`, `processId` = `processGroupId`, and no recorded exit. That process no longer existed. **Steps to reproduce** 1. Start a Codex task on a local runner. 2. Restart the control plane gracefully during the turn. 3. Kill the control plane again while recovery relaunches the provider, before the provider is admitted. **Expected behavior** Recovery relaunches the provider and continues the run. **Actual behavior** Every later runner refuses to restore, and the run fails. ## What Changed - `provider_backend.rs`: after a runner loads durable provider state, `retire_orphaned_startup_attempt` clears the startup fence only when all of these hold: - the attempt is in phase `spawned` (neither admitted nor failed); - the session is still restorable (`session_open`, `turn_active` or `provider_exited`); - signal 0 returns `ESRCH` for both the recorded process and its process group. It records a `provider_startup_retired` diagnostic event with the attempt, persists the state, and lets the normal restore path relaunch the provider. - Intent-phase fences (OpenCode). Three OpenCode runs after a K6 crash loop, and other runs on master, stopped at phase `intent` with no recorded process. Under load, the runner spends seconds verifying launch artifacts after it records intent. An intent alone could not show whether a process was started. - A new intent now carries `beforeSpawn: true`. - A new `SpawnStarting` observation clears the mark immediately before every provider spawn site in `start_with_tools_observed`. That clear is fsynced, but it emits no extra diagnostic event. - An intent still marked `beforeSpawn` (in a restorable session) proves that no process was started, so the fence is retired. - An intent that reached the spawn, or one written before this mark existed, stays fenced. - A runner that predates the field rejects the state as unknown (`deny_unknown_fields`), which fails closed. - `validate()` only allows the mark on an intent or on a spawn-stage failure. - The fence stays closed for: - an intent that reached the spawn, or one without the before-spawn mark; - a failed startup (initialization or spawn); - a session that recovery closed; - a process or group that may still run; - a permission error or an invalid id. - The existing test `failed_authenticated_identity_commit_keeps_the_durable_startup_fence` now uses a live process group instead of the fixed pid 123. That pid might not exist on a CI host, so the test would no longer prove a running provider keeps the fence. ## Verification - Red before the fix: the new integration test `restart_relaunches_a_provider_whose_runner_died_before_admitting_it` (fake Codex app server; turn active; a `spawned` restore fence with an exited process group; then the recovery `runner.drain` probe) failed with `provider startup ownership remains unadmitted`, the incident error. After the fix it passes. It shows one more `thread/resume`, the retired diagnostic, and a cleared fence. - Unit tests (unix-only module): retirement for each restorable lifecycle, and for an intent still before the spawn. A `SpawnStarting` outside its intent is rejected. The fence stays closed for: - an intent that reached the spawn; - an intent without the before-spawn mark; - a real child that survives its killed leader (process-group probe, no stubs); - a failed spawn; - a failed admission; - a closed session; - a live provider; - a surviving process group. - `cargo test -p paperclip-runner-core --test codex_provider`: 119 passed. The startup event counts are unchanged, because the spawn-start mark emits no event. The existing fence tests (`failed_provider_startup_is_persistently_fenced_before_another_process_can_resume`, `durable_recovery_closes_a_provider_that_reopens_a_settled_turn`) still pass. - `cargo test -p paperclip-runner-core --lib`: 570 passed. `cargo check --workspace --tests` passes. - `cargo fmt --check` passes. ## Risks - Process ids can be reused. A reused id only makes a gone process look alive, so the fence stays closed. Retirement needs `ESRCH` for both the pid and the group. - A provider process that moved to a new process group is outside this check. Before admission the runner has sent it no turn, so it cannot repeat provider work. - Intent fences written by older runners, including the kept incident states, have no before-spawn mark. They stay fenced and still need the Retry. New launches carry the mark. - A runner-side change takes effect on local runners after the runner build. Daytona needs a new image. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude Code with tool use (shell, file edits, test execution). Default context window. Extended reasoning was available. ## 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 Opus 5.5 <noreply@anthropic.com>