mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
DottaandClaude Opus 5.5
876a2e4857
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a server restart, a task must continue automatically and promptly when that is safe > - A user clicks Retry, and the server is killed before the new run starts a process > - Recovery leaves that run `running` until the orphan reap's five-minute stale-age threshold passes > - The user sees nothing happen for about five minutes > - This pull request recovers that run when its controller lease from the earlier boot expires > - The benefit is that the automatic retry starts about one lease period after the kill, not five minutes later ## Linked Issues or Issue Description **What happened?** A Retry admitted a legacy run. The server was killed before the run recorded a process. The run kept a controller lease from the killed boot and `executionStage: preparing`. After the restart, live recovery skipped the run until `updatedAt` was five minutes old (`server/src/services/heartbeat/recovery.ts`, `staleThresholdMs ?? 5 * 60_000`). Restart-chaos measured 5 m 01 s from run creation to `process_lost` and the automatic retry, twice (4 m 20 s after the server was ready). **Expected behavior** Recovery should claim the run as soon as the earlier boot's lease expires. The lease is the only liveness proof for a legacy controller, and no process was ever recorded. **Paperclip version or commit** `master` at `71313207eb`. **Steps to reproduce** Click Retry on an interrupted task, then SIGKILL the server within about one second. Observe the retried run stay `running` for about five minutes. ## What Changed - `recoverActiveExecutions` skips the stale-age gate for a legacy `running` run when all of these are true: its controller lease is expired (checked by the existing live-lease gate), the lease belongs to an earlier boot (`controllerBootId !== legacyControllerBootId`), the run is still `preparing` (no adapter dispatch), and the run has no recorded `processPid` or `processGroupId`. - The stage and process fields committed with the revocation are checked again. If the old controller dispatched before its lease expired, the run keeps the age guard. - The existing revocation (`revokeExpiredLegacyControllerWithState`) still requires the expired boot ID, so a concurrent renewal wins. The existing `restartInterruptedBeforeDispatch` rule still decides the single process-loss retry. - `doc/execution-semantics.md` describes the rule in section 10. - Two regressions in `server/src/__tests__/live-recovery-work.test.ts` (embedded Postgres, real residual-recovery workers). ## Verification - Red before the fix: `recovers a fresh run from a prior boot at lease expiry ...` failed with `expected 'running' to be 'failed'`. - Green after the fix: `pnpm exec vitest run src/__tests__/live-recovery-work.test.ts -t "stale-age|fresh foreign controller|suppressed live runs"` passes 4/4. The run fails `process_lost` with `executionRecovery.providerWorkStarted: false` within seconds of lease expiry, and exactly one retry runs to `succeeded`. - Race case: a DB trigger makes the row committed with the revocation say `dispatching` while the scan saw `preparing`. The run stays `running`, and no retry is created. - Guard cases: a run with an expired lease of the current boot, a run from an earlier boot with a recorded process, and a run from an earlier boot that was `dispatching` (for example a remote HTTP adapter) all stay `running` after four minutes. - Related suites: `heartbeat-process-recovery`, `heartbeat-observed-workspace-preservation`, `native-startup-cleanup-recovery`, `services/heartbeat/recovery.test.ts`, full `live-recovery-work`. - Live K8 probe (restart-chaos supervisor, local instance, ACPX Claude agent): the first run fails with a provider error, the probe clicks Retry, and the server is SIGKILLed 100 ms later, before the Retry run records a process. The probe then measures how long the Retry run stays `running`. | Build | Retry run created to `process_lost` | Server ready to `process_lost` | Automatic retry run | | --- | --- | --- | --- | | Base (`71313207eb`) | 300.1 s | 293.5 s | started at 5 m 00 s | | This PR | 60.1 s | 54.5 s | started 0.1 s after `process_lost` | The claim now happens when the killed boot's 60-second controller lease expires. In this probe the restart took about 6 s, so the claim came about 54 s after ready. In the restart-chaos waves, kill to ready took about 41 s, so the claim would come about 20 s after ready. - A chaos wave with the `retry` trigger (`w-20261106T195715-s2727`, branch #19513 merged locally) missed the trigger: no Retry was offered because recovery completed the task automatically (claude t01 PASS). The probe above makes the Retry-then-kill timing deterministic. ## Risks - Low risk. The change only removes an age wait. Ownership is still proved by the expired lease of another boot. A rolling deploy or hot restart whose earlier instance is still alive keeps renewing its lease, so the run is not touched. - Runs with a recorded process, and dispatched runs, keep the five-minute guard. A process on another host or a remote request is not inferred dead. - Native runs are not changed. Their ownership is in `native_run_finalizations` and startup native recovery. > 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 Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## 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>