Commit Graph
1 Commits
Author SHA1 Message Date
Devin FoleyandPaperclip b5bf86ea09 Clear process identity when settled native runs stop (#17374)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Each heartbeat run stores the process identity of its runner in
`heartbeat_runs.process_pid`, `process_group_id` and
`process_started_at`
> - Only cancellations (`release_run_resources`), restart recovery and
heartbeat recovery cleared these fields
> - Failed native runs take the `schedule_retry` path, so they kept a
PID after the run finished, for weeks
> - Liveness checks for old runs then depend on PID-reuse defenses, and
code that reads a set PID as "running" is misled
> - This pull request releases the process identity of settled native
runs when the process is confirmed gone, and backfills old sandbox rows
> - The benefit is that a finished run no longer looks like it owns a
live process, while live, warm and retrying processes keep their
identity

## Linked Issues or Issue Description

Refs #17358. Bug report described in-PR (bug report template).

**What happened?**
On one instance, 16 of 23 old native workspace sync folders belonged to
failed or cancelled runs that finished more than a week earlier and
still had `process_pid` set. The PID is written when the run's process
starts (for remote targets it is the sandbox PID). The status arbiter
clears it only through `release_run_resources`, which only cancellations
emit. Failed native runs take `schedule_retry`, and other terminal paths
(finalization reconciler, restart recovery terminal failures, watchdog
timeouts) do not clear it either.

**Expected behavior**
When a run is terminal and its process is confirmed gone (or no longer
owned), the process identity is cleared. A process that may still be
alive keeps its identity.

**Steps to reproduce**
1. Run a native task whose provider turn fails, so the arbiter decides
`run_failed_partial_evidence_preserved` with a `schedule_retry` effect.
2. Wait for the run to finish and the runner to exit.
3. Read the run row: `status = 'failed'` and `process_pid` /
`process_group_id` / `process_started_at` are still set.

**Paperclip version or commit:** `master` at 75fcb0b3fc.
**Deployment mode:** any.
**Agent adapter(s) involved:** native runtime (`paperclip_runner`);
local and sandbox targets.

## What Changed

- `releaseSettledNativeRunProcess` (in `native-local-process-stop.ts`)
clears `process_pid`, `process_group_id` and `process_started_at` of a
native run under a row lock, only when all of these are true:
- the status is `failed`, `cancelled`, `timed_out` or `interrupted`, and
`finished_at` is set;
- the finalization coordinator is absent, `committed` or
`terminal_failure` (a `retryable_failure` same-run retry keeps the
existing heartbeat recovery path; unfinished finalization keeps its
identity);
- the run has a session checkpoint or provider evidence, so native
session resume never mistakes it for an unused bootstrap;
- local target: every lease is released without failed cleanup, the
latest `native.process_identity_recorded` event names this host and the
same PID/group, and the PID and group probe absent. The release writes
the existing `native.local_process_stopped` receipt in the same
transaction, so `hasNativeLocalProcessStop`,
`readNativeLocalProcessStop`, explicit continuation and acknowledged
Stop keep their proof;
- sandbox target: every lease has a remote termination receipt (sandbox
PIDs are never probed on this host).
- Succeeded runs keep their identity: a warm runner can serve the next
turn, and chat retry matches warm predecessors by PID.
- The heartbeat executor calls the release after lease and scratch
cleanup at a terminal boundary (not when a same-run resume, workspace
finalization or held ownership is pending).
- Background work stays idle (see
`rules/sleep-aware-background-work.mdc`):
- Trigger: the executor settlement calls `settleNativeRunProcess`. If
the release is not ready yet (process still alive, local lease not
released, remote termination receipt missing), the run goes into an
in-memory pending set.
- Startup: the first `reapOrphanedRuns` pass after boot runs
`discoverSettledNativeRunProcesses` once, in pages of 50, over the
durable rows. This reconstructs work after a restart and covers terminal
transitions outside an executor (reconciler, restart recovery, watchdog,
timeouts).
- Retry: later passes call `retryPendingSettledNativeRunProcesses` only
for pending runs. It makes no database call when the set is empty. A run
leaves the set when it is released, becomes ineligible, or reaches 20
attempts.
  - Runs that this process still executes are skipped.
- Stop receipts now also record `processStartedAt`.
`withReleasedNativeRunProcess` restores a released identity from the
latest stop receipt. Retained session cleanup
(`reconcileRetainedNativeSessionCleanup`) and warm transition restart
checks use it. Quiescent runnerd state recovery reads the stop receipt
when the prior run's identity was released. Without a receipt, all three
still treat the identity as unproven.
- Migration `0377_clear_settled_native_run_process_identity` backfills
sandbox runs (without bumping `updated_at`) with the same rules: native,
settled status, `finished_at` older than 1 day, settled coordinator
phase, checkpoint or provider evidence, and at least one lease where
every lease has a valid remote termination receipt. The snapshot is
carried forward as `0377_snapshot.json`.
- Not changed: legacy (non-native) runs. Case handoff release, scratch
cleanup and conversation continuation read the terminal row's local
PID/group as process-tree absence evidence, so clearing them needs
separate work. `release_run_resources` cancellation behavior and
`native_run_finalizations.controller_*` fields (they track the server
controller, not the run process) are unchanged.

### Migration margin

The 1-day margin is defense in depth. The real proof is the remote
termination receipt: the provider confirmed that the sandbox stopped or
was destroyed. One day is far longer than lease release, sandbox stop,
restart recovery takeover and the finalization reconciler sweep, so no
in-flight teardown or same-run recovery can still read these rows. Local
rows are not backfilled in SQL: only the host that recorded the process
can verify that it stopped and write the stop receipt. The runtime sweep
releases them on that host.

## Verification

- `pnpm exec vitest run
src/__tests__/native-settled-process-release.test.ts` (new, 20 tests):
release with a stop receipt and restore of the identity from it;
`cancelled`/`timed_out`/`interrupted`; live process kept, then released
after exit; kept for succeeded, `retryable_failure`,
`workspace_finalizing`, unreleased local lease, another host, no
evidence, unconfirmed sandbox, mixed leases; sandbox release with
receipts; startup discovery paging and skip; pending retry; retry cap;
no database access while nothing is pending; `reapOrphanedRuns`
discovery, then retry after the process exits; executor release for a
failed native run and no release for a succeeded run.
- `native-provider-failure.integration.test.ts`: a real `schedule_retry`
decision (`run_failed_partial_evidence_preserved`) leaves the failed run
with its PID after finalization, and the release then clears it and the
stop receipt reads back.
- `native-session-executor.test.ts`: quiescent recovery and retained
session cleanup accept a released identity with a stop receipt and
reject one without a receipt.
- `packages/db` `client.test.ts` migration rule: the backfill does not
set `updated_at`.
- `native-finalization-migration.test.ts`: migration 0377 clears only
old settled native sandbox rows with confirmed termination (15 fixtures,
including recent, succeeded, running, legacy, unsettled phases, no
evidence, unconfirmed or local leases, no lease).
- Related suites, all passing: `pnpm exec vitest run` on
heartbeat-process-recovery, native-workspace-sync-retention(-config),
native-finalization-{migration,settled-candidates,recovery},
chat-failed-run-retry-boundary, acknowledged-native-stop,
explicit-native-continuation, conversation-continuation,
native-session-resume, native-run-finalizer-telemetry,
native-session-executor, native-safe-replacement,
native-finalization-reconciler-timing, native-restart-recovery,
native-finalization-recovery-state, run-lifecycle,
agent-instruction/directory working copies, heartbeat-case-handoff (1920
tests across 23 files).
- `pnpm run typecheck` in `server/`: passes. `pnpm run check:migrations`
and the snapshot drift test in `packages/db`: pass.
- Mutation checks (each reverted alone; a test failed every time):
executor call, startup discovery, pending retry in `reapOrphanedRuns`,
retry cap, pending-set cleanup on retry, start time in the stop receipt,
retained cleanup receipt fallback, coordinator phase guard, succeeded
exclusion, liveness probe, host check, remote receipt check,
provider-evidence guard, local lease check, quiescent recovery receipt
fallback, migration margin, migration NULL-safe receipt check, migration
phase guard. The NULL-safe check came from a real bug the migration test
found. The warm transition restart call site has no direct test: that
path has no existing fixture. It uses the same tested helper.
- Not run: the full server suite and e2e tests.

## Risks

- Behavior change: more native runs now have a null `process_pid` after
they settle. Every native reader was checked: explicit continuation,
acknowledged Stop and agent stop use the stop receipt for null
identities; cancelled-startup detection also requires no process events,
so a released run is not mistaken for a cancelled startup; native
session resume selection is protected by the evidence guard; chat retry
reads its warm-predecessor PIDs from succeeded runs, which are not
released.
- A recycled local PID keeps a row unreleased (conservative), and
sandbox runs from providers without termination receipts keep their
identity.
- Background cost: one paged discovery at startup. After that, only
pending runs are checked, at most 20 times each. With nothing pending
there is no recurring query.
- A transition outside an executor that happens after startup, with no
executor settlement, is released at the next startup discovery.
- Migration: a data-only `UPDATE` on `heartbeat_runs` with a selective
`WHERE`; it passes the migration safety check. It is safe to re-run.

## Model Used

Claude Opus 5.5 (`claude-opus-5-5`), 1M context, through Claude Code
with tool use.

## 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

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-23 15:09:43 -07:00