Commit Graph
3 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
Devin FoleyandPaperclip f73ace9f1a Clear stale native recovery state so closed work expires (#17359)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs keep a finalization row, and tasks can keep recovery
actions while work is stuck
> - Git cache expiration and native harness backup expiration read these
rows to decide if closed work is still live
> - Commit paths never cleared
`native_run_finalizations.recovery_state`, so committed rows kept
`resuming_session` forever
> - Recovery actions on done or cancelled tasks also stayed active when
the close did not go through the issue routes
> - So workspaces and backups of tasks that closed weeks ago stayed live
and used disk space
> - This pull request clears the stale state at commit, backfills old
rows, resolves stale recovery actions on closed tasks, and lets the two
expiration jobs ignore settled state on closed trees
> - The benefit is that closed work expires on schedule, while live
leases, unsettled phases, unverified owners, and in-flight system
recovery still keep it

## Linked Issues or Issue Description

Refs #17248

**What happened?**

On one instance, Git cache expiration and native harness backup
expiration kept the workspaces and backups of tasks that were done for
more than 7 days. Three kinds of stale state caused this:

1. 51 `native_run_finalizations` rows had `phase = 'committed'` and
`recovery_state = 'resuming_session'`. The commit writes (status
decision commit, attention replay, result persistence, and the finalizer
claim) never cleared `recovery_state`. No reader uses `recovery_state`
on a row that has a result, except the expiration predicates.
2. Rows with `phase = 'terminal_failure'` and `recovery_state =
'blocked'` (or NULL) are permanent states. Nothing clears them, and
continuations are refused for closed tasks.
3. `issue_recovery_actions` with status `active` or `escalated` stayed
open after the source task became done or cancelled. Only the issue
routes cancel them at close. The periodic sweep
`reconcileActiveRecoveryActions` skipped every action unless
`wakePolicy.type` was a bounded owner policy or the owner was the board.
Agent-owned `resume_native_run` actions (which use `wakePolicy.kind`)
and system-owned actions were never resolved.

**Expected behavior**

When a task tree is closed for the configured time, its Git cache and
native backup expire. Only live state keeps them: a held finalization
lease, an unsettled finalization phase, an unverified native workspace
owner, or system recovery that is scheduled, running, or awaiting
evidence.

**Steps to reproduce**

1. Run a native task. Let the runner restart so that restart recovery
sets `recovery_state = 'resuming_session'` on the finalization row.
2. Let the resumed run commit its result and close the task as done.
3. Wait past the expiration window (or run the sweep with a later
clock).
4. See that the finalization row is still `committed` +
`resuming_session`, and that Git cache expiration and native backup
expiration skip the closed workspace.

**Paperclip version or commit**

`master` at 837add0caa

**Deployment mode**

Self-hosted server with native runtime enabled

**Agent adapter(s) involved**

Not adapter-specific (core bug)

## What Changed

- Clear `recovery_state` and `recovery_request_id` in every write that
persists or commits a native result: `recordCoordinatorDecision` in the
status decision committer, the attention replay in
`routePersistedNativeResultAttention`, `completeRun` in both
`NativeRunCoordinatorStore` and `PaperclipControlPlanePort`, and
`claimCoordinator` in the finalizer.
- Add data migration `0376_clear_committed_native_recovery_state.sql`
with its unchanged schema snapshot (the oldest snapshot is pruned, as
the generator does). It clears `recovery_state` and
`recovery_request_id` on rows that are `committed` with no lease owner.
`recovery_history` keeps the audit trail.
- In `reconcileActiveRecoveryActions`, resolve active and escalated
actions on done or cancelled tasks (`source_terminal`) before the wake
policy and owner filter. The existing owner-unverified guard still runs
first. Actions with system recovery in `scheduled`, `running`, or
`awaiting_evidence` are skipped.
- In `isGitWorkspaceExpirationEligible` (closed-tree mode only), a
recovery action blocks only if its cause is
`native_workspace_finalization_owner_unverified` or its system recovery
is in flight. A finalization blocks only if it has a lease owner or its
phase is not `committed` or `terminal_failure`. Idle mode keeps the old
strict checks.
- In native harness backup expiration, a settled finalization
(`committed` or `terminal_failure`, no lease owner) no longer blocks
once the session's tasks are closed for the configured days.

## Verification

- `pnpm exec vitest run` in `server/` on the touched and related files:
`git-workspace-expiration`, `git-workspace-expiration-eligibility`,
`native-harness-backup-expiration`, `native-finalization-migration`,
`native-interaction-bridge`, `native-run-finalizer-telemetry`,
`paperclip-control-plane-port`, `issue-recovery-actions`,
`native-status-arbiter-corpus`: 9 files, 321 tests passed. After the
tests moved to `native-finalization-recovery-state.test.ts`, that file,
`git-workspace-expiration` and `native-run-finalizer-telemetry` passed
again (95 tests).
- `pnpm exec vitest run src/migration-snapshot-drift.test.ts` in
`packages/db`: passed.
- `pnpm exec vitest run
src/__tests__/heartbeat-process-recovery.test.ts`: 408 tests passed.
- `pnpm run typecheck` in `server/`: passed.
- `pnpm run check:migrations` in `packages/db`: passed.
- New tests:
- Result persistence (port and coordinator store), finalizer claim and
status decision commit (new focused file
`native-finalization-recovery-state.test.ts`), and attention replay each
clear `recovery_state` and `recovery_request_id`.
- The migration clears committed, unleased rows. It keeps leased
committed rows, `observed` rows, and `terminal_failure` rows unchanged,
and keeps `recovery_history`.
- The sweep resolves an agent-owned `resume_native_run` action and an
escalated system action on a done task, and keeps an action with
scheduled system recovery. The existing test "retains the exact native
owner hold without changing a terminal task's disposition" still covers
the owner-unverified hold.
- Git expiration eligibility: a closed tree expires with `committed` +
`resuming_session`, `terminal_failure` + `blocked`, an active agent
recovery action, or an escalated system action. It stays with a leased
finalization, an `observed` finalization, an owner-unverified hold,
in-flight system recovery, an open tree, and in idle mode.
- A full Git expiration of a closed tree with stale native state, then a
reopen to the recorded commit. The existing "preserves scheduled runs,
unresolved recovery, and live previews" test now uses an `observed` and
a leased finalization as the unresolved recovery.
- Native backup expiration expires `committed` + `resuming_session` and
`terminal_failure` + `blocked`, and keeps an `observed` or leased
finalization.
- Mutation checks. I reverted each fix on its own, saw the matching test
fail, and restored it:
- Status decision commit, finalizer claim, port `completeRun`, store
`completeRun`, attention replay: each new clearing test failed.
  - Migration (`WHERE false`): the backfill test failed.
- Sweep order (wake policy filter moved back in front) and the in-flight
system recovery guard: the sweep test failed.
- Git eligibility: the closed-tree action relaxation, the finalization
relaxation, the owner-unverified guard, and idle strictness each made
the matching eligibility tests fail.
- Backup expiration (`recovery_state IS NOT NULL` restored): both new
expiry cases failed.
- Not run: the full server suite and the e2e suites. CI runs them.

## Risks

- The migration is a data-only `UPDATE` on `native_run_finalizations`.
It touches only committed rows with no lease owner and a non-null
`recovery_state`. The table is not on the known-large list, and the
migration safety check passes. `recovery_history` is not changed.
- Clearing `recovery_state` at commit and claim: the readers of this
column need `phase = 'observed'` (continuation contract), `result_id IS
NULL` (recovery service, failed-run retry), or running runs (heartbeat
recovery). A row that has a result does not match any of them. The
restart recovery summary counts will drop because they no longer count
committed rows.
- The sweep now resolves more actions on closed tasks. A done or
cancelled task cannot continue, so these actions cannot act. Unverified
owner holds and in-flight system recovery are kept.
- Expiration now removes more closed workspaces and backups. Restore
points and Git cache restoration still allow a reopen to rebuild the
workspace.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.

## Model Used

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

## 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 14:06:17 -07:00
DottaandDev Agent 25cf079ec5 feat(runner): add Codex-native application integration (#12591)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The runner package is useful only when the application can start,
observe, and recover a native Codex run safely.
> - Existing direct adapters must keep their current execution and
finalization paths.
> - The application boundary therefore needs additive persistence,
authorization, coordination, and recovery behind an explicit
experimental adapter.
> - This pull request adds that Codex-only boundary without activating
generalized providers, remote environments, or the later task/SDK
surfaces.

## Linked Issues or Issue Description

**Subsystem affected**

Shared contracts, database persistence, adapter utilities, server
native-runtime services, and the experimental Paperclip Runner adapter.

**Problem or motivation**

The already-landed runner package has a qualified Codex path, but the
application needs durable native-run state, guarded runtime selection,
authenticated coordination, tool security, finalization, and recovery
before the experimental adapter can be exercised safely.

**Proposed solution**

Add a Codex-only `paperclip_runner` application path behind the existing
default-off native-runner setting. Bind native state and coordination to
company/run identity, preserve persisted-run recovery, and leave every
direct adapter on its existing legacy execution path.

**Alternatives considered**

The earlier stack boundary introduced a generalized executor and
remote-environment lifecycle here. That made this PR depend on
implementations in higher PRs and changed reusable sandbox behavior
globally. Those pieces are now deferred together to #12592.

**Roadmap alignment**

ROADMAP.md does not list a conflicting native-runner integration
project. This change adds the application boundary for the existing
Runner architecture.

## What Changed

- Added native run/result/finalization/provider-trace persistence,
shared validators, and idempotent migration/replay coverage.
- Added guarded Codex-only runtime selection, authenticated PRP
coordination, recovery, finalization, and interaction services.
- Added run/company-bound tool-gateway authorization, credential
redaction, SSRF protections, and replay-safe behavior.
- Added the explicit `paperclip_runner` adapter behind the default-off
rollout setting.
- Preserved legacy answered-question wake projection and direct-adapter
execution/finalization paths.
- Hardened cancellation so only owned in-memory child processes are
signaled; persisted recycled PIDs/process groups are never trusted.
- Retained the narrow Claude ACPX isolated-context security follow-up
discovered after #12590.
- Deferred the generalized executor, provider ingress, remote lifecycle,
SDK/lab/eval work, release-process changes, and lockfile.

## Verification

- Changed-file delta against `master`: 133 files.
- GitHub Actions is the authoritative verification environment for this
PR.
- Full CI, security, and Greptile review will run on this lowest
unmerged stack PR.
- Local tests/build/typecheck were not run because this checkout is
resource constrained.
- Static diff/reference checks pass, and `pnpm-lock.yaml` is unchanged.

## Risks

- This touches central heartbeat and agent-route code, so legacy
compatibility is the primary risk.
- Runtime selection remains Codex-only and explicit; direct Codex,
Claude, OpenCode, process, HTTP, and plugin adapters remain on their
existing paths.
- Fresh native starts fail closed while the rollout flag is off;
persisted native records remain readable and recoverable.
- Cancellation, company/run binding, tool calls, status decisions, and
completion writes are guarded or replay-safe.

> For core feature work, check [ROADMAP.md](ROADMAP.md) first and
discuss it in #dev before opening the PR. Feature PRs that overlap with
planned core work may need to be redirected.

## Model Used

OpenAI Codex, GPT-5.6, with repository tools, code execution, and
parallel agent review.

## 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 linked existing issues or described the issue in-PR
following the relevant issue template
- [x] I have not referenced internal or instance-local Paperclip issues
or links
- [x] My branch name describes the change and contains no internal
Paperclip ticket id
- [ ] I have run tests locally and they pass — GitHub Actions is
authoritative for this resource-constrained checkout
- [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 risks above
- [ ] All Paperclip CI and security gates are green
- [ ] Greptile is 5/5 with no open actionable findings
- [x] I will address all Greptile and reviewer comments before merge

## Stack

- Position: 3 of 5 overall; lowest of 3 currently unmerged
- Base: `master`
- Previous:
[#12590](https://github.com/paperclipai/paperclip/pull/12590), qualified
Claude ACPX runtime — merged
- Next: [#12592](https://github.com/paperclipai/paperclip/pull/12592),
generalized Codex executor, task experience, and developer SDKs

---------

Co-authored-by: Dev Agent <dev@paperclip.ing>
2026-08-31 14:38:38 -05:00