mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
DottaandClaude Opus 5.5
156453fe7c
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A deploy replaces the server. The old server stops its running legacy runs at SIGTERM (`server_shutdown_interrupted`). The new server finds runs whose owner died without that drain (`process_lost`, "server may have restarted"). > - Each of these runs got its follow-up from the bounded transient retry. That retry spends the failure budget (2 attempts) and waits 30 to 60 seconds. > - So the first deploy spent one failure retry. A run that was already the last automatic retry got no follow-up at all. The task then rested on a manual Retry. > - A deploy is routine. An interruption by the server is not a failed attempt, and it must never leave a task waiting for a person. > - This pull request moves these runs to their own `server_restart` retry lane. The lane never spends the failure budget, starts the first continuation at once, and has its own bound. > - The benefit is that a task continues by itself through several deploys a day, and a real loop still stops and shows why. ## Linked Issues or Issue Description **What happened?** A zero-downtime deploy replaced a hosted server. In the 11 minutes around the swap, 11 legacy runs ended with `server_shutdown_interrupted` and 7 ended with `process_lost`. At least 7 tasks were left with no follow-up run and no scheduled retry, so each needed a manual Retry. Two of the stranded runs were already automatic retries. **Expected behavior** Each interrupted run continues once, automatically, on the surviving server, with its original wake. **Paperclip version or commit** Core commit7d9973405c. The behavior is the same on masterbf18b348c5. **Steps to reproduce** 1. Start a legacy run with a conversation adapter (for example `codex_local`) as the second automatic retry of its chain (failure budget spent). 2. Send SIGTERM to the server (`drainRunningRunsForShutdown`), or let a new server boot find the run without its owner (`reapOrphanedRuns`). 3. See that no successor run is created. The scheduler logs "Bounded retry exhausted". **Root cause** - `enqueueProcessLossRetry` (`server/src/services/heartbeat/recovery.ts`) calls `scheduleBoundedRetryForRun`. That function put these runs in the `transient_failure` lane, with `maxAttempts = 2` counted from `executionRetryAccounting.failureRetries` (`server/src/services/heartbeat/retries.ts`). A first attempt got a charged retry 30 to 60 seconds later. A run with 2 spent failure retries got `retry_exhausted`. - The orphan reaper (`recoverActiveExecutions`) skipped the retry when `failedRunRetryBudgetSpent(run)` was true. It also skipped a conversation run that had no local child process recorded (for example a run that was dispatching), because `shouldRetry` needed a local child, a preparing stage, or a monitor wake. - The stranded-task sweeper then saw the spent budget and moved the task to `blocked`. ## What Changed - `execution-recovery-attempt.ts`: add the `server_restart` retry lane. It is an uncharged lane. Its counter (`serverRestarts`, `serverRestartWindowStartedAt`) is saved in `executionRetryAccounting`, so it is visible on each run. The bound is 6 continuations in one 60-minute window. A later interruption after the window starts a new window. - New `server-replacement-interruption.ts`: `isServerReplacementInterruption(run)` names a legacy `server_shutdown_interrupted` run, or a `process_lost` run whose owner was another server boot. It reads the new `processLossDiagnostic.controllerReplaced` verdict. For rows saved by an older release, it reads the owner boot id or `runPredatesObserver`. - `retries.ts`: `scheduleBoundedRetryForRun` uses the `server_restart` lane for these runs (`usesServerRestartLane`). The first continuation is due at once; later ones in the window back off like the interrupted-preparation lane. `failedRunRetryBudgetSpent` reports the restart bound for these runs, so the sweeper escalates only when that bound trips. An interrupted preparation keeps its own lane while its chain has failure budget left. - `recovery.ts` (orphan reaper): record `controllerReplaced`. A conversation run that a restart ended is eligible for continuation even without a local child process, and it checks the restart bound, not the failure budget. - `wake-queue/adapters/postgres.ts`: carry the restart counter to successors that the wake queue creates. - No schema change. No change to how runs start (no new waits before a turn). Behavior that did not change: - At most one successor per run: every path still uses the predecessor row lock and the `retry_of_run_id` check in `scheduleBoundedRetryForRun`. Two servers alive at once reuse the same successor. - A non-conversation adapter that reached dispatch stays held for reconciliation, because a replay can repeat its actions. - Native runtime runs keep their own recovery. - A server that began shutdown already starts no queued run (`closeProcessRunAdmission` in `server/src/index.ts`, checked in `heartbeat/queue.ts`). Its queued and newly scheduled runs stay in the database for the new server. ## 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"): - Graceful shutdown, first attempt: master scheduled a charged `transient_failure` retry. Now one immediate `server_restart` continuation. - Graceful shutdown, already the last retry: master scheduled nothing. Now one continuation; the failure count stays 2. - Owner lost at boot (`process_lost`), first attempt and already-retried: same results, with two servers running the boot sweep at the same time. Only one successor. - Old server exited after it committed the interruption and before it scheduled the continuation: the new server's sweep continues the run. - Run saved by an older release (no `controllerReplaced`, spent failure budget): the sweep continues it instead of moving the task to `blocked`. - The restart bound trips after 6 in one window, records `retryReason: server_restart` on the run, and the sweeper escalates. The same count in a closed window continues. - A run that failed by itself (`adapter_failed`) with a spent failure budget still gets no retry. - Each continuation's execution continuation reports the original wake (`trigger.retriedReason = issue_monitor_due`). - Unit tests: `server/src/services/server-replacement-interruption.test.ts`. - Six earlier tests asserted escalation after "three deploy restarts" spent the failure budget. They now spend the restart bound instead, so they still prove escalation when the bound trips. - Existing test `server/src/services/heartbeat/queue.test.ts` covers "starts no run once shutdown closed run admission". - `pnpm verify`: see the PR checks. ## Risks - A run that a restart ends now continues more often (up to 6 times per hour per chain, not 2 in total). A run that itself crashes the server is still stopped by the bound, and the stop is shown on the task. - Tasks that an older release already moved to `blocked` stay blocked. Tasks that are still `in_progress` or `todo` with a stranded run continue on the next sweep. - A `process_lost` row from an older release whose owner was the same boot that later restarted is read as a restart. That case is a lost in-memory handle, which is also not a failure of the agent's work. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (code search, shell, test execution) 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>