mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 06:10:44 +01:00
DottaandClaude Opus 5.5
7c508d3e42
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When the control plane restarts, it must decide whether each running agent's process is gone before it releases the run > - Two reapers made that decision for legacy runs, with different rules and different outcomes. One also waited 5 minutes even when it could already prove the process was dead (RC-27) > - The durable ownership plan moves every such decision onto one prover, `proveOwnerGone`, and never releases work by age alone > - This pull request collapses the two reapers onto one owner stamp and the shared prover, removes the age wait for a proven-dead process, and never releases a process it cannot prove gone > - The benefit is that a lost process ends one way, at once, with its normal retry, and a process on another host is never released by mistake ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2 sections A and C), #19838 (inventory: "One fact, many proofs" and the age-only list), #19871, #19883, #19953, #19997. **What happened?** - `recoverActiveExecutions` (heartbeat) and `terminalizeOrphanedRunningRun` (recovery service) both judged a lost legacy child. They used different liveness rules (EPERM counted as alive in one, dead in the other) and gave different outcomes: `failed/process_lost` with a retry in one, `interrupted/orphaned_running_run` with no retry in the other. - The active-execution reaper kept a 5-minute age guard even when the child was provably dead. - Hot-restart adoption judged the child with raw PID checks. **Expected behavior** One prover decides. A child that is proven gone is settled at once, through one path with one outcome. An unprovable child (another PID space, a sandbox, or a pre-#19871 row on an ephemeral server) is never released, and Stop is its operator action. ## What Changed - New `server/src/services/run-process-owner.ts`: `lostProcessOwnerStamp` and `proveRunProcessGone`. They wrap the shared `runProcessOwnerStamp` (#19997) with the one-release legacy fallback (#19883) and the group-leader case. - `heartbeat/recovery.ts`: - The active-execution reaper judges a legacy child with the shared prover. A child proven gone skips the age guard; master's exemption for a proven-gone controller (#19953) is kept. - An unprovable child gets the detached marker, whose message says to stop the run. - The legacy host fallback is recorded as `LEGACY_HOST_FALLBACK_EVENT`. - `heartbeat/recovery.ts` (hot-restart adoption): a run is marked lost only on a `gone` proof. An unprovable child stays adopted and held. - `recovery/service.ts`: the stale-lock sweep no longer judges a legacy child. It hands the run to `settleLostRunProcess`, which is the active-execution reaper wired in `heartbeat.ts`, and then reads the outcome back. Native runs keep the sweep's own backstop, now judged with the shared prover. Salvaged from the stopped Phase 2 recovery agent's uncommitted work, which had no tests. I completed and corrected it: the salvaged adoption marked unprovable children lost, which risked duplicate execution. ## Verification - `recovery-stale-issue-lock-sweep.test.ts` (51 pass). Three new tests fail on `master` and pass here: - "hands a legacy run whose child is proven gone to the one lost-process reaper"; - "settles a legacy run whose child is proven gone without waiting out the age guard"; - "never releases a legacy run whose child is unprovable, and names Stop as the way out". Updated tests: - The PID-space test now expects `failed` for the one legacy outcome. - The explicit-retry race now injects `settleLostRunProcess`. - The two tests of the sweep's own terminal write use native runs, which keep that path. - `heartbeat-process-recovery.test.ts`: 418 of 419 pass. The one failure, "redacts opaque environment-bound credentials from Sentry diagnostics", also fails on unchanged `master`. - `heartbeat/recovery.test.ts` (97), `ownership-reconciliation-report.test.ts` (24), `native-remote-retry-admission.test.ts` (92) and `heartbeat-provider-limit-wait.test.ts` (4) pass. - The server package typecheck passes. ## Risks - Behavior change: a legacy run whose child died, when the orphan sweep finds it, now ends `failed/process_lost` with the reaper's retry policy, instead of `interrupted/orphaned_running_run`. - A pre-#19871 row on an ephemeral server, or a child in another PID space, now stays `running` with a detached marker until Stop. It is no longer released by age. - Not changed: a dispatched legacy run that recorded no PID and has no controller stamp keeps its lease-expiry plus age fence. Rows written since #19953 are judged by the controller proof. Older rows, and remote handoff, keep the fence. A different host is never proof, so removing the fence would strand runs after an ephemeral replacement. ## 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>