mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
DottaandClaude
1402e92d1f
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run holds an execution lease on its `native_run_finalizations` row. The lease lasts 20 minutes, and the owner renews it every 5 minutes. > - When a laptop sleeps for longer than the lease, the wall clock and the database clock move past `lease_expires_at`. The server's monotonic timers barely move. > - After the wake, the next renewal finds its own lease expired. It interrupts a healthy turn with `native_session_lease_lost`, and the user must click Retry. Nobody else took the lease. > - Several background consumers act on an expired lease without changing its owner. So expiry alone cannot simply be ignored for the owner. > - This pull request makes takeover explicit. The same owner can take back its own lapsed lease only when no other actor has recorded anything on the row or the run. > - The benefit is that a run interrupted only by host sleep continues with no Retry. A real takeover still fences the late owner out. ## Linked Issues or Issue Description **What happened?** The restart-chaos time fault TM2 stops the server and runner tree and ages every database time by one hour, as a stand-in for a laptop that slept. After the thaw, the next 5-minute renewal of the native execution lease found the lease expired. It interrupted the turn (`turn.interrupted` at 300 s, `native_session_lease_lost`). The failure settlement was then refused with the same code, and the UI showed "Couldn't finish this attempt" with Retry. The owner, attempt, boot and pid were unchanged. Evidence: wave `w-20261110T051535-s304` t01 (TM2, 1 h) failed R3. t02 in the same wave (TM1, a 113 s server-only freeze with no clock jump) passed, so the failure follows the clock jump and not the pause. **Is this a harness artifact?** No. TM2 ages the stored rows, and a real sleep moves the clock forward. Both produce the same comparison: a stored `lease_expires_at` is earlier than `clock_timestamp()` after the wake. The server's renewal timer and local deadline use the monotonic clock. On macOS and Linux that clock does not count suspended time, so they do not notice the sleep in either case. The first database check after the wake is the renewal predicate `lease_expires_at > clock_timestamp()`, and it fails in both cases. **Expected behavior** A run whose lease lapsed only because time jumped, and that no other actor claimed, keeps running and finishes without Retry. **Steps to reproduce** `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --adapters codex --workloads B5 --faults TM2 --time-age-hours 1 --concurrency 1 --tasks 2 --seed 304` with the harness from #20458. Refs #20458, #20500 ## What Changed - `native-execution-lease.ts`: add `nativeExecutionLeaseHeldCondition` and `lockNativeExecutionLeaseForReacquire`. A lease is held when it is live. For a local run, it is also held when it lapsed and is untouched. "Untouched" means all of these are true: - the claimed generation matches; - the phase is `observed` and no result is accepted; - `recovery_state`, `recovery_request_id`, `failure_code` and `next_attempt_at` are null; - `controller_host_hash` is not the board's attested-stopped stamp; - the run is still `running`, without the ownership-unverified error code and without a `nativeStartupCleanup` marker. - The caller's WHERE already matches the owner token, attempt and controller boot/pid/start. - The lock helper locks the coordinator row. If the lease lapsed, it also takes a share lock on the run row. The predicate then reads the latest committed run, and an in-flight run write cannot slip past it. - Renewal (`startNativeSessionExecutionLeaseRenewal` / `renewNativeSessionExecutionLease`) takes back a lapsed lease when the executor passes its claimed generation. Remote targets never do. A deadlock (40P01) on that rare path retries at the next tick, as a statement timeout already does. - `native-session-executor.ts`: the executor passes its generation to renewal. Failure settlement and the final release use the same lock and condition. After a result is accepted, a lapsed lease can only be relinquished (set to null), never renewed, because workspace finalization may act on it. - New `native-execution-lease-reacquire.test.ts` (embedded Postgres, 24 cases). - `doc/plans/2026-11-09-explicit-native-lease-claim-tokens.md`: step 2, which adds explicit claim tokens for the consumers this PR does not yet move. Consumer inventory (readers of `native_run_finalizations.lease_expires_at` that can act on a lapsed lease): - Restart recovery and takeover (`claimNativeRestartRecoveries`): claims first. It writes `recovery_state`, `lease_owner` or `controller_*` under the coordinator lock. - Finalizer and cleanup claims: claim first (`lease_owner`). - Failed-run owner release, environment settlement, observed-workspace preservation, retained-session cleanup: require a terminal run status, `committed`, or a recovery state. The rule refuses all of these. - Finalization reconciler, live recovery: require `retryable_failure`, a field the rule checks. - Board controller-stopped confirmation: writes `controller_host_hash`, which the rule checks. - Startup cleanup promotion: writes the run's `nativeStartupCleanup` marker, which the rule checks. It also applies only to runner-task leases. - Another server's orphan sweep: writes the run's ownership-unverified code. The rule checks it when the marker lands first. The reverse order is deferred (see Risks). - Workspace finalization copyback: acts on a lapsed post-result lease without a coordinator write. Post-result leases are therefore never renewed (deferred to step 2). - Accepted-result and token lease settlement, checkpoints, cancellation delivery, admission and the other readers: act on live leases only, or are read-only. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-execution-lease-reacquire.test.ts` passes (24/24) on this branch. With the source files from `origin/master`, 5 fail, including "lets the same owner take back a lease that lapsed untouched" and "renews from the shared timer after the sleep instead of interrupting the turn". - (a) The owner takes back its lapsed lease when no other actor claimed it. Also covered: the shared 5-minute timer, rows saved by the previous release, and the post-result relinquish. - (b) 14 cases where a consumer claimed first fence the owner out and leave the row unchanged. A release over a recorded recovery claim is refused. A different boot (a deploy mid-run), a different generation, a missing generation and a remote target are refused. - (c) Races have exactly one winner: a claimant and the owner lock the coordinator in each order, an in-flight run status write races the owner, and 8 simultaneous starts each have one winner. - Existing suites pass: `native-execution-lease.test.ts`, `native-session-executor.test.ts`, `native-runner-restart-recovery`, `native-runner-restart-lease-renewal`, `native-runner-restart-preparation` and `native-runner-restart-checkpoint-cost` integration tests. - Live proof: `B5 × TM2 --time-age-hours 1 --tasks 2 --seed 304` with `--server-from` this branch. Wave `w-20261110T083444-s304` passed 2/2, with no Retry and no `native_session_lease_lost`. - Both runs ran past the 300 s renewal after the one-hour aging, at the exact point where `origin/master` fails. t01 ran 310 s and t02 ran 319 s. Each was a single run that succeeded. - The harness used the aging exclusion from #20500 (merged): process start identity is not aged. Without that exclusion, TM2 also moves `controller_process_started_at`. The live owner then fails its identity check, which a real sleep never causes. An earlier wave on the unpatched harness, `w-20261110T072106-s304`, failed 0/2 for that reason only. ## Risks - Previous release: no persisted shape changes and no migration. Rows saved by the previous release have the same columns, and a null `controller_host_hash` is accepted. A deploy that lands mid-run cannot take back the old server's lease, because the boot and pid differ. - On the normal path, a live lease takes the same single coordinator lock as before. The run share lock is taken only after a lapse, and nothing is added before `turn.start`. - Deferred to step 2 (see the plan): - The orphan sweep of another server process can read the row before the owner takes the lease back and write after. That run would carry an ownership-unverified error while it is live. - Shared-workspace holder detection reads a silent run with lapsed leases as gone. - Post-result leases are not renewed. - Remote leases are unchanged. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used in Claude Code with tool use (shell, file editing, sub-agent code audit). ## 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 - [ ] 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>