mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
DottaandClaude
6d1eae73c0
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The native runtime resumes a run after a control-plane restart. It launches a Runner and waits for recovery barriers (`run.attach`, a `runner.drain` probe, the provider identity). > - These waits poll with a wall-clock deadline. When the control plane and its Runner are stopped together (SIGSTOP, host sleep), the wall clock passes the deadline before either process can act. > - On thaw, the wait reports a timeout although nothing was slow. Recovery then spends an attempt and a retry delay. > - This pull request gives each startup or recovery admission a clock that does not count stopped time. > - The benefit is that a freeze or host sleep during an admission no longer turns into a false `PRP command ... timed out` failure. ## Linked Issues or Issue Description **What happened?** A restart-chaos wave (local, codex, fault TM2) stopped the server and its Runner tree for 44 s, 1 s after a resumed Runner started. The resume barrier `runner.drain` had a 30 s deadline. On thaw, the server reported `runner_local_connect_failed: PRP command runner.drain timed out` 45 s after the wait began (`provider.session.load` span 45130 ms). Recovery spent attempt 1 and waited 30 s more before attempt 2. **Expected behavior** Time that the waiting process spends stopped does not count as waiting time. A real timeout still fires when the Runner does not answer while both processes run. **Steps to reproduce** 1. Start a Runner command wait with a 30 s deadline. 2. After 1 s, stop the process for 44 s (the test moves the system clock without running timers). 3. Let the Runner complete the command 2 s after thaw. 4. Before this change, the wait rejects with `runner.drain timed out`. After this change, it resolves. Note: in that chaos task, this false timeout was not the reason that the task needed a manual Retry. An earlier SIGKILL of the whole server tree stopped the Codex turn while a foreground shell command (`./bench.sh`) was running. On restore, Codex had no active turn (`provider_turn_lost_on_restore`). Recovery correctly blocked automatic replay (`uncertain_provider_action`), because no durable record proves that the interrupted command had no side effect. The manual Retry then completed the task. This PR does not change that policy. Refs #20675 ## What Changed - `packages/paperclip-runner/src/live/runnerd-codex-transport.ts`: add `RunnerStallLedger`, the clock of one startup or recovery admission. `now()` is wall time minus the stopped time observed since the admission began. Its poll loops and a 1 s ticker share one last-observation time, so each stop is credited once, wherever the process was when it stopped (a poll loop, a checkpoint flush, a spawn). Only the part of a gap beyond 5 s is credited. - `#start` and `#resume` run under one ledger (`#withAdmissionClock`). A nested startup keeps the outer ledger. The ticker stops when the admission ends. - During an admission, `waitForRunnerCommand`, the provider-identity wait, the cold admission deadline, the recovery admission deadline, and the default 30 s command deadline all use this clock. - Waits after admission, including `turn.start` and its event and snapshot waits, keep wall-clock deadlines. Their behavior does not change. - Tests in `runnerd-codex-transport.test.ts`: the chaos sequence (red before, green after), the unchanged wall-clock timeout outside an admission, a barrier that still times out after a freeze, a 70 s freeze between two barriers on one cold deadline, and a slow-but-running process that still reaches its deadline. ## Verification - Red: `cd packages/paperclip-runner && npx vitest run src/live/runnerd-codex-transport.test.ts -t "stopped"` failed before the fix with `runner_local_connect_failed: PRP command runner.drain timed out`. - Green: the same command passes after the fix. - `npx vitest run src/live/runnerd-codex-transport.test.ts`: 301/301 pass, including the existing "shares Pi's absolute cold deadline across recovery barriers and rejects a late ACK" test. - `npx tsc --noEmit -p packages/paperclip-runner/tsconfig.json`: clean. - `pnpm verify` exits 2 (changes outside resolved test graphs). Required CI supplies the broad coverage. - Not run: a new chaos wave. The unit tests reproduce the exact wait and clock sequence from the artifacts. ## Risks - A real event-loop hang longer than 5 s during an admission now extends that admission by the hang minus 5 s. The Runner answer could not be observed during the hang either. Each credited gap still costs 5 s of budget, so a process that is slow on every observation still reaches its deadline. - The ticker runs only while a startup or recovery admission runs (an active operation), every 1 s, does no I/O, and is cleared in `finally`. It does not keep an idle process awake. - A `turn.start` that is cut by a freeze still reports a wall-clock timeout, as before. That case needs its own change, because its event and snapshot waits share one deadline. - No persisted state, launch identity, or saved-session shape changes. Old saved runs resume through the same code. A deploy mid-run only changes how long an in-process admission wait tolerates a stop. No new wait or file operation runs before `turn.start`. > 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 - Claude (Anthropic), model ID `claude-opus-5-5`, run in Claude Code with tool use (shell, file edit) and extended reasoning. ## 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 <noreply@anthropic.com>