mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs start a local `paperclip-runnerd` process. The control plane keeps a durable state root for each session. > - After a restart, recovery gives a run with no provider evidence the `bootstrap_incomplete` disposition. The executor quarantines the old root and starts the bootstrap again, but only when the root proves that no provider work started. > - #19524 made that proof accept a root killed after `run.prepare` and before `session.open` (a hard kill). > - A graceful restart (SIGTERM) leaves different shapes. The drain sends `runner.drain` and `runner.suspend`, and the kill can land just after `session.open` failed or just after it started a fresh session. The proof refused both shapes. The run failed with `runner_state_identity_mismatch: bootstrap_not_proven_incomplete`, although no turn was ever sent. > - This pull request extends the proof to both drain shapes and keeps provider work fail-closed. > - The benefit is that a graceful restart during native bootstrap continues automatically. ## Linked Issues or Issue Description Refs #19524 **What happened?** Restart chaos waves (ACPX Claude, K2 graceful restart about 10 s after the run started) failed with `adapter_failed` and `runner_state_identity_mismatch: bootstrap_not_proven_incomplete`. Startup recovery chose `bootstrap_incomplete`, and the executor proof refused the state root. Two real roots were kept: 1. `session.open` failed (the ACPX sidecar timed out), then `runner.drain` completed and `runner.suspend` completed. There was 1 connection, no committed or acknowledged event, an empty runner outbox, and `nextSourceSeq: 1`. 2. `session.open` completed with a freshly started session (`status: "started"`). The committed events were only `session.started`, `session.capabilities.updated` and `session.goal.snapshot`, and the runner outbox held one `harness.diagnostic` (the sidecar process). No turn command existed, and the sidecar process had exited. The database had not yet recorded the session events, so the classifier saw no provider evidence. **Steps to reproduce** 1. Start an ACPX Claude task on a local runner. 2. Send SIGTERM to the control plane about 10 s after the run starts, before the first turn. 3. Let the control plane restart. **Expected behavior** Recovery starts the bootstrap again, and the task continues without a user action. **Actual behavior** The run fails and the task stops. ## What Changed - `native-bootstrap-proof.ts`, pre-session proof: a `session.open` that is pending or `failed` is now accepted, as are the runner lifecycle commands `runner.drain`, `runner.suspend` and `runner.shutdown` in any state. The controller journal and the runner journal must both agree. Any committed or acknowledged event, any turn command, a completed `session.open`, and any queued runner event still fail closed. - New `runnerdStateIsOpenedSessionBeforeTurn`. It accepts a completed `session.open` only when all of these are true: - the session was freshly started (`status: "started"`, not resumed); - the commands are only `run.prepare`, `session.open` and runner maintenance commands; - every committed event and every event in the runner outbox is a session setup or runner lifecycle event; - `ackedSourceSeq` equals the count of committed events, with no gap; - the runner journal is present; - every recorded provider process ID has exited on this host. - `native-session-executor.ts`: the executor applies the opened-session proof only for a local execution target. Remote runners stay fail-closed for that shape, because their process IDs belong to another host. The remote checkpoint classifier still reads only commands 1 and 2. ## Verification - Red before the fix: "allows a fresh bootstrap after a drain suspended a runner whose session.open failed" and "allows a fresh bootstrap after a drain that landed between session.open and the first turn" both failed with `expected false to be true`. Green after the fix: the `native incomplete-bootstrap evidence` block passes 32/32. - Fail-closed counter-tests: - completed `session.open`; - committed event, acknowledged event, or queued runner event; - turn command, `turn.stop`, or a turn processed by the runner; - `turn.started` or `provider.event`; - resumed session; - compacted events; - no recorded provider process; - missing runner journal; - live provider process; - remote runner. - I ran the new proof read-only against the three real kept roots: the K1 root from #19524, the K2 `session.open` failed root and the K2 opened-session root. All three return `true` for a local runner. The opened-session root returns `false` when the processes are on another host. - `native-session-executor.test.ts` and `native-restart-recovery.test.ts` filtered to bootstrap, restart, remote and drain: 333 passed. `native-runner-restart-recovery.integration.test.ts` and `native-bootstrap-retry.integration.test.ts`: 106 passed. `tsc --noEmit -p server/tsconfig.json` passes. - Live restart chaos replay: see the PR comment. ## Risks - After a `session.open` failure or timeout, the provider sidecar might have created an empty session. No turn command was sent, so no provider work can repeat. - An opened session before any turn is discarded, and a new one is started. The proof requires a fresh `started` session, so no earlier conversation is lost. Resumed sessions stay fail-closed. - The opened-session proof is local only. It requires every recorded provider process to be gone, so a live provider process blocks it. - Out of scope: the Retry offer when the proof fails legitimately (RC-05/06). > 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>