Commit Graph
3 Commits
Author SHA1 Message Date
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
DottaandPaperclip e200104727 feat: review connection actions from tasks (#13063)
Bring governed connection reviews into task history and composer approvals. Share resolution with Connections, add scoped remembered permissions, and resume agents through durable outcome receipts.

Keep cards compact, collapse raw results, isolate untrusted provider output, bound continuation payloads, and reconcile missed live events. Add Storybook coverage, browser journeys, and service regression tests.

Verification: all PR CI gates passed, Greptile 5/5, security scans passed, five connection-review browser journeys passed, and real native Codex approval/continuation was verified against the local MCP fixture.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-09-08 19:37:13 -05: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