mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-20 06:02:18 +02:00
76164b53cb
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The heartbeat runner executes legacy agent runs. A controller lease watchdog keeps durable ownership of each legacy run, and it aborts the run when it loses the lease. > - #16084 records a lease loss as a `controller_lease` cancellation when the adapter returns or throws. It does not cover a lease loss during setup, before the adapter starts. > - In setup, the abort reason goes to the setup catch. The catch records an opaque `setup_failed` and sends the raw `LegacyControllerLeaseLostError` to Sentry with no cause label. > - Issue continuation recovery treats `setup_failed` as non-retryable. The task moves to `blocked`, but no provider work started. > - This pull request records a lease loss before dispatch as `controller_lease_lost`, with the same `controller_lease` evidence. Recovery retries it as transient infrastructure, and so does the accepted-interaction (plan approval) continuation retry. > - The benefit is that a host lease loss during setup no longer strands the task, and the next Sentry event shows which renewal failure caused the loss. ## Linked Issues or Issue Description Refs #16084 Refs #16355 Refs #16359 **What happened?** A staging server reported `LegacyControllerLeaseLostError: Legacy controller lease lost` from the heartbeat. The run failed during setup with `setup_failed`. Continuation recovery then said "non-retryable failure on this issue's continuation run (`setup_failed`)" and moved the task to `blocked`. A scanner had to move the task back to `in_progress` by hand. **Expected behavior** A lease loss before adapter dispatch is a host infrastructure stop. The run records the `controller_lease` cause, like the adapter-phase path. The run keeps `providerWorkStarted: false`. Recovery retries the continuation with its bounded transient budget, and it does not block the task. **Steps to reproduce** 1. Start a legacy run that has a controller lease. 2. Before the adapter dispatch, make the lease renewal fail: the update matches no row, the query errors, or the lease deadline passes. 3. `controllerLease.assertOwned()` throws the abort reason in workspace preparation or before dispatch. 4. Before this change: the run is `failed`/`setup_failed`, `resultJson` has no `cancellation`, and continuation recovery blocks the task. After this change: the run is `failed`/`controller_lease_lost` with `cancellation.source: "controller_lease"` and a closed cause, and continuation recovery classifies it as `transient_infra`. **Additional context** #16355 handles a different path: a restart orphan that the next boot reaps as `process_lost`. This change handles a live controller that loses its own lease during setup. This change does not find the root cause of the lease loss. The closed cause label in the next Sentry event gives that fact. ## What Changed - `server/src/services/heartbeat/run-completion.ts`: `failRunSetup` receives the execution abort signal. When the abort reason is a `LegacyControllerLeaseLostError`, it writes error code `controller_lease_lost`, adds the `controller_lease` cancellation evidence, and keeps the existing bootstrap recovery facts. The agent goes back to idle, not `error`, because the stop is a host fault. This applies only before provider dispatch: `failRunSetup` also receives `providerDispatchStarted`, and a setup-path error after a legacy adapter or native dispatch began keeps `setup_failed`, so started work is never retried. - `server/src/services/legacy-controller-lease.ts`: add the `CONTROLLER_LEASE_LOST_FAILURE_CODE` constant. - Merged with #16359 (lease-loss timings): the setup failure keeps its `resultJson.controllerLeaseLoss` timings and adds this PR's `controller_lease` cancellation from the abort signal. #16359's setup test now aborts the signal first, as `assertOwned()` does, and expects `controller_lease_lost`. - `server/src/services/heartbeat.ts`: pass the execution abort signal and `providerDispatchStarted` (`legacyAdapterEntered || nativeDispatchStarted`) to `failRunSetup`. - `server/src/services/recovery/service.ts`: add `controller_lease_lost` to the transient-infrastructure continuation codes (3 bounded attempts with backoff). - `server/src/services/heartbeat/retries.ts`: add `controller_lease_lost` to the accepted-interaction continuation retry allowlist, next to `process_lost`. Without it, a plan-approval continuation that lost its lease before dispatch was escalated and blocked before generic recovery could see it. - `server/src/services/heartbeat/run-lifecycle.ts`: add `controller_lease_lost` to the pre-adapter setup codes, so the comment policy does not queue a missing-comment retry. - `doc/run-log-events.md`: document the new error code. - Tests: setup lease loss when the abort reason is thrown, and when a setup step throws a different error after the abort. A unit test shows a lease loss after dispatch keeps `setup_failed`. A classifier test shows `controller_lease_lost` is transient and `setup_failed` stays non-retryable. Two assembled heartbeat tests (`heartbeat-process-recovery.test.ts`) lose the real lease during workspace preparation: one for a plain issue continuation (one `transient_failure` retry) and one for an accepted plan-approval interaction continuation (one `interaction_continuation_infra_retry` retry). Both check the saved run, no adapter dispatch, and the issue still `in_progress`. ## Verification - `cd server && npx vitest run src/services/heartbeat/run-completion.test.ts`: 25 passed (embedded Postgres), including the new post-dispatch test. - `cd server && npx vitest run src/services/run-cancellation.test.ts src/services/recovery/provider-failure-classification.test.ts`: passed on the first head (29 with run-completion; 25 classifier). - `cd server && npx vitest run src/services/legacy-controller-lease.test.ts`: 13 passed (needs `pnpm --filter @paperclipai/plugin-sdk... build` first). - After merging master (head `2148b27`): `run-completion`, `legacy-controller-lease`, `run-cancellation`, `provider-failure-classification` and `run-failure-report` suites, 310 passed; `heartbeat-process-recovery.test.ts -t "lease|dispatch"`, 31 passed. - Head `c52825c`: `retries`, `run-completion`, `heartbeat-retry-scheduling`, `legacy-controller-lease` and `provider-failure-classification` suites, 134 passed; `heartbeat-process-recovery.test.ts -t "lease|dispatch|interaction"`, 41 passed. - Negative controls: with the lease-loss branch in `run-completion.ts` disabled, the plain-continuation assembled test fails (`setup_failed` instead of `controller_lease_lost`). With the `retries.ts` allowlist change reverted, the accepted-interaction assembled test fails because no retry is scheduled. - Not run locally: full `tsc` and `pnpm build`. The sandbox has a 4 GB memory limit and the OOM killer stops `tsc`. CI is the typecheck and build gate. ## Risks - Low risk. The change applies only when the execution abort reason is a `LegacyControllerLeaseLostError` in the setup catch before dispatch. Other setup failures keep their current codes. - The retry uses the existing continuation recovery budgets and admission checks (agent state, budget, pause). The interaction-continuation retry keeps its existing 2-attempt budget. The retry is a new run. The revoked controller cannot dispatch, because its renewal compare-and-set fails. - A run that the reaper settles first keeps the reaper outcome. `setRunStatusIfRunning` makes the first terminal write win. - Sentry still gets this unexpected stop, with the closed `controllerLeaseLossCause` label. If the lease loss is frequent, the event volume does not change. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, test runs). ## 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 - [ ] I have run tests locally and they pass (targeted suites pass; typecheck/build not possible in this sandbox, see Verification) - [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: Devin Foley <139239+devinfoley@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>