Files
PaperClipAI/server/src/services/run-process-owner.ts
T
DottaandClaude Opus 5.5 7c508d3e42 fix(recovery): collapse the two lost-process reapers onto one stamp and the shared owner prover (#20004)
## 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>
2026-11-08 03:50:34 -06:00

35 lines
1.7 KiB
TypeScript

import type { heartbeatRuns } from "@paperclipai/db";
import { proveOwnerGone, type LocalOwnerStamp, type OwnerProof, type ProcessProbe } from "./owner-proof.js";
import { runProcessOwnerStamp } from "./native-runtime/native-owner-stamps.js";
/** The process identity a heartbeat run records for its adapter child
* (`persistHeartbeatRunProcessMetadata`). */
export type RunProcessIdentity = Pick<typeof heartbeatRuns.$inferSelect,
"processPid" | "processGroupId" | "processStartedAt" | "processHostHash">;
/**
* The owner stamp the lost-process reapers judge a run's adapter child by.
*
* It is the shared `runProcessOwnerStamp` (native-owner-stamps.ts) with the
* one-release local proof for rows saved before `process_host_hash` (#19871,
* #19883; single-host servers only). A row that recorded only a process group
* is judged by that group: its leader's PID is the group id. The active
* execution reaper, the orphan sweep and hot-restart adoption all read it, so
* they reach one verdict for one row.
*
* Returns null when the run recorded no process.
*/
export function lostProcessOwnerStamp(run: RunProcessIdentity, scope: string): LocalOwnerStamp | null {
const pid = run.processPid ?? run.processGroupId ?? null;
if (pid === null) return null;
return runProcessOwnerStamp({ ...run, processPid: pid }, scope, { legacyFallback: true });
}
/** Proves whether a run's adapter child is gone. Null when no process was
* recorded. `unprovable` never releases the run: its operator action is Stop. */
export async function proveRunProcessGone(run: RunProcessIdentity, scope: string,
probe?: ProcessProbe): Promise<OwnerProof | null> {
const stamp = lostProcessOwnerStamp(run, scope);
return stamp ? proveOwnerGone(stamp, probe) : null;
}