mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 19:28:46 +01:00
Devin FoleyandPaperclip
b0c4e34526
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native Runner sessions keep one conversation across task turns. > - A follow-up turn resumes from the newest earlier run that holds a provider checkpoint. > - A follow-up can fail during admission, before provider work, and still hold the checkpoint that it inherited. > - A later Retry then resumes from that failed run. Controller recovery finds no controller checkpoint there and fails. > - This pull request makes resume source selection skip a failed follow-up that proves that it never started provider work. > - The benefit is that a board Retry resumes from the last committed owner, and the task can continue. ## Linked Issues or Issue Description Refs #18846, which lets a follow-up start after a recovered per-turn owner. Refs #18854 (same-run retry of failures before provider work) and #18718 (controller projection refresh). **What happened?** Run 1 was a per-turn remote sandbox run. Recovery committed its result, but it saved no Runner suspension receipt. Run 2 was a follow-up that was admitted before #18846. Admission failed with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate`, before any provider work. Run 2 has these properties: - Status `failed`, and the result has `executionRecovery: { kind: "bootstrap", providerWorkStarted: false }`. - The coordinator is `terminal_failure`, with no result, no lease owner, and no controller process. - No controller checkpoint rows and no controller recovery pointer. - A `sessionCheckpoint` bound to run 2 with a null cursor. The provider session in it is copied from run 1. A board Retry then started run 3. Resume source selection treats any checkpoint as a progress barrier, so it selected run 2. Controller recovery then failed with `native_controller_recovery_unsupported:metadata_format`, because run 2 has no controller recovery pointer. The task stayed blocked. Every later Retry selects the newest failed follow-up and fails the same way. **Expected behavior** A Retry must resume from the last committed owner (run 1) when the newer follow-up provably never started provider work. If the follow-up might have started provider work, or holds its own controller evidence, it must stay the source. **Steps to reproduce** 1. Commit a per-turn remote owner through recovery, without a suspension receipt. 2. Admit a follow-up, and make its admission fail with `prior_owner_terminal_state_indeterminate`. 3. Settle that follow-up as a failed bootstrap: `providerWorkStarted: false`, `terminal_failure`, and no result. 4. Admit a Retry. It selects the failed follow-up and fails with `native_controller_recovery_unsupported:metadata_format`. The new regression test runs these steps against PostgreSQL. **Paperclip version or commit** Reproduced on `909d86b59a`. **Deployment mode** Native Runner with a per-turn lifecycle on the built-in remote sandbox transport. Source selection is shared by all native providers. ## What Changed - `findNativeSessionResumeRun` skips a newest barrier that is an unstarted inherited follow-up and selects again without it. The selector repeats this inside the existing 32-run window. - A run is skipped only when all of these are true: - It is a native run with status `failed` and a `bootstrap` receipt with `providerWorkStarted: false`. - Its coordinator is `terminal_failure`, with no result, lease owner, or controller process. - It has no process identity, provider event, controller checkpoint row, run result row, or controller recovery pointer. - Its checkpoint is bound to this run and session, and it has no cursor. - After the skips, every skipped run must name the same backend, driver, and provider session as the selected source. Otherwise the selector returns the original barrier. - Runtime selection and controller recovery both call this selector. Both now use the committed owner. Admission then reaches the #18846 path, which retires the old authority and starts a new Runner. - Add four cases to `native-recovered-owner-followup.test.ts`, with one negative control for each guard. - Document the rule in `doc/execution-semantics.md` and the test in `doc/testing-native-remote-continuity.md`. ### Why source selection - Source selection made the wrong choice. The failed follow-up has no provider progress. Its checkpoint is a copy, and the provider never saw its turn. - One shared selector feeds runtime selection (checkpoint rebind, previous turn, runner instance) and controller recovery. A fix in only one of them would let them disagree about the source. - Controller recovery keeps its rule: it never searches backward when the selected run is incomplete. The selector skips a run only with positive proof, so an incomplete run with any evidence of work still stops recovery. - A fix in the executor could not help. The executor sees only the source that the selector gives it. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-recovered-owner-followup.test.ts`: 22 tests pass. - Before the change, the two new retry tests failed with `native_controller_recovery_unsupported:metadata_format`. - After the change, the retry selects the committed owner and is admitted. It is also admitted after two failed follow-ups. - Eight controls keep the failed follow-up as the source: process identity, provider event, controller checkpoint row, a receipt with `providerWorkStarted: true`, a coordinator with a controller process, a run result row, a different provider session, and a non-null cursor. - I disabled each guard in turn. Each change made its matching control fail. - These suites pass: native session resume, resume projection, controller recovery (unit and integration), native startup retry, runtime selection, execution workspace service, native status corpus, task workspace binding, and all 1,088 native executor tests. The container and Runner binary cases skip locally. - Server typecheck, `node scripts/typecheck-low-memory.mjs`, `pnpm check:module-boundaries`, and `git diff --check` pass. - I did not run the full `pnpm test:run`, `pnpm build`, or the opt-in Linux Runner container cases locally. CI runs them. - The tests start at the selector and executor admission. They do not exercise the Retry route, the task retry queue, or a live sandbox provider. ## Risks - A wrong skip could resume from an older checkpoint. Each guard requires positive evidence of no provider work. A skipped run must also name the same provider session as the selected source. If any check fails, the selector returns the original barrier. - Only `failed` runs qualify. Cancelled, interrupted, and timed-out runs keep their existing handling. - The skip adds one proof query per candidate. It runs only when the newest barrier is a failed native run whose checkpoint is bound to itself and has no cursor. - The rebind now uses the owner's tool-contract fingerprint. If the tool contract changed since the owner ran, the Retry starts a new provider session. The provider history in this failure mode is already unavailable. - No migration or configuration change is required. ## Model Used - Anthropic Claude Opus 5.5 (model ID `claude-opus-5-5`) in Claude Code, with extended thinking, repository tools, shell and code execution, and local test runs. The runtime does not expose the context window size. ## 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 - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>