mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
DottaandClaude Opus 5.5
424e2c6488
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - An upgrade replaces the server. The old and the new server serve together for several minutes. A legacy run proves that its owner is alive by renewing a 60-second lease on its `heartbeat_runs` row. > - #21019 added an uncharged `server_restart` retry lane for runs that a restart or replacement ended. On the first upgrade that shipped it, no run used that lane. The new server also failed many runs as `process_lost` while the old server that owned them was still serving. > - Each of these runs then spent one failure retry. A run that was already the last automatic retry was held for manual reconciliation, so the task needed a person. > - An upgrade is routine. It must not cost a task its work or a retry, and it must not leave a task waiting for a manual Retry. > - This pull request sends a run that a replacement stopped during preparation to the `server_restart` lane. It also stops the reaper from failing a run while the server process that owns it is proven alive. > - The benefit is that a task continues by itself through an upgrade, also when the database is slow during the swap. ## Linked Issues or Issue Description Refs #21019, #21042 **What happened?** A hosted staging server was replaced by an automatic upgrade. The new server started at 06:18:46Z, became ready at 06:26:17Z, and the old server received SIGTERM at 06:27:34Z. Between 06:23Z and 06:26Z the new server failed 44 runs as `process_lost` ("Process lost -- server may have restarted; retrying once", `processLossDiagnostic.controllerReplaced: true`). The old server still owned these runs and was still serving. Every reaped run was a Runner run in `executionStage: preparing` with no process recorded. Each successor was scheduled with `scheduledRetryReason: transient_failure` and `executionRetryAccounting.failureRetries: 1`. No run on the server has `retryReason: server_restart` or `executionRetryAccounting.serverRestarts`. The run evidence also shows why the leases lapsed. The old server's lease renewals waited 19 to 50 seconds on the database (`controllerLeaseLoss.renewalPendingMs`) while its timers fired on time (`deadlineLateMs` 0 to 17 ms). #21042 fixes that renewal stall. This pull request fixes how the reaper and the retry scheduler treat these runs. **Expected behavior** - A server never fails a run as lost while the server process that owns it is alive and serving. - A run that a replacement really ended continues once, automatically, without spending the failure budget. **Steps to reproduce** 1. Seed a legacy `paperclip_runner` run in `executionStage: preparing`, owned by another server boot, with a lapsed run lease. 2. Run `reapOrphanedRuns()` on the new server. 3. See that the successor is `transient_failure` with `failureRetries: 1`. When the chain already spent its failure budget, see that no successor is created and the run is held for reconciliation. 4. Give the owner a live controller instance lease (the old server is still serving) and run `reapOrphanedRuns()` again: the run is failed as `process_lost` anyway. **Root cause** - `usesServerRestartLane` (`server/src/services/heartbeat/retries.ts`) left every run with the bootstrap evidence `providerWorkStarted: false` to the `interrupted_preparation` lane. But `scheduleBoundedRetryForRun` gives that lane only to an `interrupted` run (or a native preparation). The reaper writes the same evidence on a `failed`/`process_lost` run that it stops during preparation. That run fell through to the charged `transient_failure` lane. The #21019 tests used runs in `dispatching`, so they did not catch it. - `legacyExecutionNeedsReconciliation` held every run whose failure budget was spent before it checked the bootstrap evidence. A run that a replacement stopped before dispatch did no provider work, but it was held for reconciliation, and the task waited for a person. - The orphan reaper (`recoverActiveExecutions`, `server/src/services/heartbeat/recovery.ts`) treated a lapsed run lease as proof that the owner was gone. It checked the shared owner prover only for "gone", never for "alive". During the swap the old server's renewals were slow, so its leases lapsed while it was alive. **Paperclip version or commit** Staging Core `51d77648bc` (first build with #21019). The behavior is the same on master `148d2b629c`. ## What Changed - `retries.ts`: `usesServerRestartLane(run, preparationLaneTakesRun?)` keeps the `interrupted_preparation` exception only when that lane really takes the run. `scheduleBoundedRetryForRun` passes its own verdict. Without one, only an `interrupted` run qualifies. A `process_lost` run that a replacement stopped during preparation now continues in the `server_restart` lane. - `legacy-execution-recovery.ts`: a replaced preparation (`isReplacedPreparation`) does not need reconciliation, also when its failure budget is spent. The `server_restart` bound limits it. - `recovery.ts` (orphan reaper): - When a run's lease lapsed but the shared owner prover proves its owner process alive (for example, the owner's controller instance lease is live), the reaper leaves the run with that owner. The owner stops and settles the run itself (`controller_lease_lost`, uncharged `interrupted_preparation` lane). The reaper recovers the run once the owner is gone or stopped. It also recovers the run when the owner has not settled it within `LEGACY_LIVE_OWNER_SETTLE_GRACE_MS` (5 minutes) after the lease lapsed. - A setup that a replacement stopped before dispatch uses the `server_restart` bound for its retry decision, whatever the adapter. - `legacy-controller-lease.ts`: add `proveLegacyController(run)`, which returns the shared prover's verdict. `legacyControllerProvenGone` uses it, and the reaper computes the proof once per run. - `server-replacement-interruption.ts`: add `isReplacedPreparation(run)`: bootstrap evidence `providerWorkStarted: false`, a recorded controller process stamp, and a server-replacement interruption. A row saved by an older release without the stamp keeps the charged lane (the existing documented rule). - `legacy-execution-recovery.ts` (`settleInterruptedNativeBootstrap`): a reconciliation hold that an older release saved on a replaced preparation is settled once its owner is proven gone, even when the failure budget is spent. The run continues in the `server_restart` lane. - `legacy-controller-lease.ts`, `recovery/ownership-registry.ts`: the `legacy_interrupted_preparation_hold` kind judges a finished run's hold by its recorded controller process (`finishedLegacyControllerOwnerStamp`). Revocation by the orphan reaper swaps in a random boot id, so before this change a reaped preparation's hold had owner "none" and was never settled. - `recovery/service.ts`, `heartbeat.ts`: the stranded-task sweep reports exhaustion from the scheduler's own `retry_exhausted` result, so a spent `interrupted_preparation` lane blocks the task even when the synchronous budget check reads an unspent restart bound. - `doc/execution-semantics.md`: document these rules. - No schema change. Nothing changes on the path that starts a run or a turn (no new waits and no file work). ## Verification - Red on `origin/master`, green on this branch (`server/src/__tests__/heartbeat-process-recovery.test.ts`, block "runs ended by a server restart or replacement"): - "continues a first attempt / a run that was already the last automatic retry that a replaced server left preparing, in the restart lane". This is the production shape: a `paperclip_runner` run in `preparing`, two servers sweep at the same time. Master: `transient_failure` with `failureRetries: 1`, and no successor for the spent chain. Now: one `server_restart` continuation, due at once, with the failure count unchanged. - "leaves a preparing/dispatching run whose lease lapsed with its owner, then recovers it once the owner stops". The owner's controller instance lease is live and two servers sweep. Master: `process_lost` at once. Now: the run stays `running` with its owner. After the owner stops, it becomes `process_lost` and a `server_restart` continuation is scheduled. - "recovers the run when its live owner has not settled it within the grace". - `interrupted-preparation-hold-settle.test.ts`: "releases an older release's hold on a preparation a server replacement stopped with its failure budget spent" for an unrevoked and a reaper-revoked lease (red without the settlement and owner-stamp changes) and "blocks the task when a replaced preparation's lane is spent even though the restart bound is not" (red without the sweep change). - Related suites: heartbeat process recovery, retry scheduling, orphaned active lease sweep, recovery deadline (bootstrap and ordinary bootstrap), legacy continuation authority, undispatched execution wait, agent status recovery, legacy workspace restore recovery, legacy execution recovery, legacy controller lease, server replacement interruption, heartbeat recovery, provider failure classification, reconciliation run read, and wake-queue policy. - `tsc --noEmit` for `server`. ## Risks - A run whose owner is alive but no longer drives it now stays `running` for up to 5 minutes after its lease lapsed, and then the reaper recovers it. The owner's own sweep still recovers a run that the owner itself lost the handle for (this boot is never deferred). - The guard trusts the shared owner prover. That prover reads "alive" only from a live or grace controller instance lease, or from a live PID in the same PID space. A server whose instance lease cannot renew for 60 seconds stops itself, so a stuck owner cannot hold runs forever. - More runs continue in the uncharged lanes. The `server_restart` bound (6 in 60 minutes) still stops a loop, and the stop is shown on the task. - Rows saved by older releases: a `process_lost` row with the bootstrap evidence and an old-boot owner now continues in the `server_restart` lane instead of being held. These runs did no provider work. > 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, with tool use (code search, shell, test execution, read-only log and API inspection) in 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>