mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 00:26:28 +01:00
DottaandClaude
dc9971bdb8
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run holds an executor lease on its `native_run_finalizations` row. #20474 lets the same owner take back its own lease when the lease lapsed only because the host slept. > - That take-back is safe only if every other actor that acts on a lapsed lease records its action under the coordinator row lock first. The plan `doc/plans/2026-11-09-explicit-native-lease-claim-tokens.md` lists the actors that do not. Gap 2 is another server's orphan sweep. > - The sweep reads the lease as lapsed. Later it writes the ownership-unverified marker on the run, without the coordinator lock and without a new check of the lease. If the owner wakes and takes its lease back between the read and the write, a healthy run gets the marker. > - The board then shows "automatic recovery is blocked" on a run that works. After the next host sleep, the owner cannot take its lease back, so the turn is interrupted and the user must click Retry. The owner's rule is that a task interrupted by sleep must resume by itself. > - This pull request makes the sweep write its marker under the coordinator row lock, and only while no executor lease on the run is live. > - The benefit is that exactly one side wins: the sweep marks a run whose owner did not come back, and it leaves alone a run whose owner took its lease back. ## Linked Issues or Issue Description Refs #20474 **What happened?** Two servers share one database. Server A's host sleeps, so the executor lease of its native run lapses. Server B's orphan sweep reads the lapsed lease. Server A wakes and takes its lease back (#20474). Server B then writes `native_execution_ownership_unverified` on the run that A still owns. **What did you expect?** Server B writes the marker only if the lease is still lapsed when it writes. One side wins. **Steps to reproduce** `server/src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts` holds the owner's take-back open, runs the real `heartbeatService(db).reapOrphanedRuns()` until its write waits on a lock, then commits the take-back. Before this change the run ends with `errorCode: native_execution_ownership_unverified` and the owner's next take-back fails with `native_session_lease_lost`. ## What Changed - `setRunStatusFromLive` (`server/src/services/heartbeat/run-lifecycle.ts`) has a new option `onlyWhileNativeLeaseLapsed`. It locks the coordinator row first. This is the same order that `lockNativeExecutionLeaseForReacquire` uses. The UPDATE then requires that no executor lease on the run is live. - `markNativeOwnershipUnverified` (`server/src/services/heartbeat/recovery.ts`) takes `leaseLapsed`. The orphan sweep in `recoverActiveExecutions` sets it when it read the lease as lapsed or unset. Other callers do not change. They keep the same call shape. - A new regression test, red before the fix: `server/src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts`. It also checks that the sweep still marks a lapsed run whose owner did not come back, and that the late owner stays fenced out. - `doc/plans/2026-11-09-explicit-native-lease-claim-tokens.md`: gap 2 is marked closed without the epoch design. ## Verification - `cd server && pnpm exec vitest run src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts` — red on `origin/master` (the run keeps the marker), green with this change. - `cd server && pnpm exec vitest run src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts src/services/heartbeat/recovery.test.ts src/services/native-runtime/native-execution-lease-reacquire.test.ts src/__tests__/heartbeat-status-preserving-telemetry.test.ts` — 129 passed. - `cd server && pnpm exec tsc --noEmit -p tsconfig.json` — clean. - Restart-chaos time-fault wave on this branch: see the wave comment below. ## Risks - Low risk. Only the orphan sweep's marker write changes, and only for a run whose lease it read as lapsed. The sweep now takes the coordinator row lock before the run row. The owner's take-back uses the same order, so the two cannot deadlock. - If the owner took the lease back, the sweep now writes nothing and appends no event. The next sweep sees a live lease and defers, as before. - The other gaps in the plan (workspace finalization, shared-workspace holder detection, board copyback confirmation) do not change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via 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 - [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 <noreply@anthropic.com>