mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
DottaandClaude Opus 5.5
612cc7e3e6
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A legacy run holds a durable controller lease while it prepares. The server renews the lease every 10 seconds, and it stops the run if no renewal succeeds within the 60 second lease. > - Each run renewed in its own transaction on a private pool of two connections. Each renewal used six round trips (begin, statement timeout, transaction id, notify, update, commit) and woke live-run recovery for that run. > - When many runs prepare at the same time on a loaded database, the renewals queue behind each other. The queue grows with the number of runs, and every run loses its lease at about the same time. > - A healthy server must not fail its own runs because it has many of them. > - This pull request renews all leases of one server process through one shared queue. Requests that arrive while a renewal is in flight share the next statement. > - The benefit is that the renewal cost no longer grows with the number of live runs, so a burst cannot starve the lease. ## Linked Issues or Issue Description **What happened?** On a staging server that did not restart, 22 runs failed with `controller_lease_lost` in about 13 minutes. Every run was a legacy run in the `preparing` stage with `providerWorkStarted: false`. Every loss had cause `renewal_deadline`. The renewal had been pending 28 to 48 seconds (`renewalPendingMs`). The deadline callback ran on time (`deadlineLateMs` 3 to 861 ms), so the event loop was not blocked. The successful renewals before the loss had each taken 20 to 30 seconds, because the deadline fired only 25 to 48 seconds after the last renewal (`sinceRenewalMs`). About 30 runs were live in the process. The first wave started when a user retried about 14 blocked tasks at once. The automatic retries of the first wave then failed the same way, which made the second wave. 12 of 16 affected tasks became blocked. **Steps to reproduce** 1. Start about 30 legacy runs in one server process. 2. Make each renewal transaction slow (for example, a loaded database). 3. Wait one lease period. **Expected behavior** The leases stay owned. The renewal cost does not grow with the number of runs. **Actual behavior** All runs fail with `controller_lease_lost` (`renewal_deadline`). ## What Changed - `server/src/services/legacy-controller-lease.ts`: All controllers of one process renew through one queue per database. One flush is in flight at a time. Requests that arrive during a flush share the next flush, so a renewal waits behind at most one other statement. A flush is one transaction on the private lease pool: `statement_timeout` and one statement that renews all requested rows. - The statement locks the requested rows with `for update skip locked`. A row that another transaction holds is reported busy, and only that run retries after 250 ms. It does not stall the other runs in the batch. A row that is not this boot's live legacy lease is reported rejected, as before. - Renewal no longer emits the `active-execution-recovery` notification. A renewal only moves the deadline later. Recovery already has a deadline at the earlier expiry and rereads the moved deadline then. This removes one recovery pass, `pg_current_xact_id` and `pg_notify` per renewal. - The lease contract does not change: 60 second lease, 10 second renewal, boot-id fence, expired leases cannot renew, the local deadline is measured from the attempt start, and the loss causes and timings are the same. - `doc/run-log-events.md`: Documents the shared renewal queue. Upgrade story: no persisted shape changes. The row columns, lease length and boot-id fence are the same. A lease held by the previous release has a different boot id, so the new batch rejects it and leaves it untouched. A test covers a previous-release lease in the same batch. ## Verification - Red on `origin/master`, green with this change: - `keeps thirty preparing controllers owned when every renewal takes four seconds on a two-connection pool`: before, all 30 controllers lost the lease with `renewal_deadline` (the staging shape). After, none lose it. - `renews fifteen concurrent controllers in one statement` (real Postgres): before, 15 update statements. After, 1. - `renews through one private statement without waking live-run recovery`: before, the renewal woke `active-execution-recovery`. After, it does not. - `live-recovery-work.test.ts` `renews without waking recovery and re-reads the moved controller deadline at the old one`: before, 2 recovery passes per renewal. After, none, and recovery still moves to the new deadline at the old one. - Safety cases: a row lock on one run does not delay another run; a previous-release lease in the same batch stays untouched; a terminated lease connection is retried; existing deadline, retry, rejection and revocation tests pass. - `npx vitest run src/services/legacy-controller-lease.test.ts src/__tests__/live-recovery-work.test.ts`: 88 passed. - `pnpm verify`: see the CI checks on this head. ## Risks - Medium-low. All renewals of a process now share one statement. A failed flush fails every request in it; each watcher retries independently, as it did before. - A busy row is retried every 250 ms until the lease deadline. Before, the renewal waited on the row lock for up to the statement timeout. The loss behavior is the same. - Recovery wakes at the old lease expiry instead of after each renewal. It finds the live lease and moves to the new expiry. - Not in scope: the downstream workspace recovery that blocked tasks after a lost lease (`legacy_execution_requires_reconciliation`, `source_unavailable`). > 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 and code 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>