mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 22:13:59 +01:00
Devin FoleyandClaude Opus 5.5
6a45ea479e
<!-- entire-trail-link-start --> https://entire.com/gh/paperclipai/-/trails/52/changes/paperclip/52 <!-- entire-trail-link-end --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A legacy agent run keeps a durable controller lease. The server renews the lease every 10 s. The lease lasts 60 s. > - The lease watchdog stops the run when it loses the lease, because another controller can then take the run. > - #17032 moved renewal to a private pool. It also retries a renewal that hits its statement timeout. But a renewal that fails with any other error still stops the run at once. > - Staging still shows this failure after #17032. A legacy run made 186 good renewals over 31 minutes. Then one renewal failed with an error 23.9 s after the last good renewal, and the watchdog stopped the run immediately (`controllerLeaseLossCause=renewal_error`). The lease still had about 36 s left. > - This pull request retries a failed renewal until the lease deadline, the same as a timed-out renewal. > - The benefit is that one dropped database connection no longer cancels a healthy run. The 60 s lease deadline still bounds the retries, so the ownership guarantee does not change. ## Linked Issues or Issue Description Refs #17032 Refs #16084 **What happened?** A legacy agent run on staging was cancelled mid-turn. The run was healthy. The cancellation evidence was `cancellationSource=controller_lease` and `controllerLeaseLossCause=renewal_error`, with `controllerLeaseRenewalCount=186` and `controllerLeaseSinceRenewalMs=23913`. The release already contained #17032. **Expected behavior** One failed lease renewal must not stop a run while the lease is still valid. The watchdog must try the renewal again. It must stop the run only when the lease deadline passes or when the database rejects the renewal. **Steps to reproduce** 1. Start a legacy run so that `watchLegacyControllerLease` watches it. 2. Make one renewal query fail with a connection error, for example terminate its Postgres backend with `pg_terminate_backend`. 3. See that the watchdog aborts the run immediately with `renewal_error`, although the lease has not expired. ## What Changed - `watchLegacyControllerLease` retries a renewal that fails with an error. It waits 1 s between attempts (`LEGACY_CONTROLLER_RENEWAL_RETRY_DELAY_MS`). The existing lease deadline stops the retries. - A retried renewal is safe. The UPDATE is idempotent, and it still requires this controller's boot ID and an unexpired lease. A rejected renewal (no row updated) still stops the run at once with `renewal_rejected`. - If the deadline passes while the attempts still fail with an error, the cause stays `renewal_error`. A statement timeout counts as a stall, so it stays `renewal_deadline`, as before. A good renewal clears the error state. - `deadlineLateMs` is now recorded whenever the deadline ends the lease, also for `renewal_error`. - Tests: - New embedded-Postgres test: it terminates the renewal's own backend while the renewal waits on the run row. The renewal retries on a new connection and keeps the lease. Without the fix, `assertOwned` rejects. - New unit tests: one failed attempt is retried and the lease is kept; a stall after a recovered error is still `renewal_deadline`. - The existing `renewal_error` test now checks that a renewal that always fails is retried and is reported as `renewal_error` at the deadline, with its timings. - `doc/run-log-events.md` describes the retry and the updated `renewal_error` and `deadlineLateMs` meaning. ## Verification - `server/src/services/legacy-controller-lease.test.ts` with embedded Postgres: 23 tests pass. - With the original `legacy-controller-lease.ts`, 4 of these tests fail. One of them is the real-Postgres test that terminates the renewal connection. - `server/src/services/heartbeat/run-completion.test.ts` and the `run-cancellation` tests: 35 tests pass. - A typecheck of the changed files reports 0 errors. CI runs the full typecheck. ## Risks - Low risk. The change is limited to the legacy lease watchdog. - A run with a permanently failing renewal now stops at the lease deadline (at most 60 s after the last good renewal), not at the first error. Another controller cannot take the run before the lease expires in the database, so ownership stays safe. - A non-transient error (for example a code bug in the query) is retried once per second until the deadline. The cause label is still `renewal_error`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, code search, test execution). ## 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>