mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
Devin FoleyandPaperclip
f73ace9f1a
## 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>