mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 05:16:50 +01:00
DottaandPaperclip
db11d899cf
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native agents keep remote processes and conversation history across warm turns. > - A completed turn can leave handoff authority only in controller memory. > - Losing that controller can block a follow-up even while its sandbox survives. > - This pull request verifies durable ownership and reconnects the exact prior process before handing it to the next run. > - Healthy follow-ups stay warm. Recovery preserves saved work without replaying a completed turn. ## Linked Issues or Issue Description **What happened?** A remote warm turn can finish without suspended backup authority. A replacement controller rejects that prior owner even when its exact process and completed checkpoint remain. A single unavailable identity probe can also stop an otherwise valid reattachment. **Expected behavior** Keep healthy follow-ups warm. Recover a completed session after proving that its controller stopped and that the successor holds current authority. Reuse compatible processes; otherwise save and resume the conversation. Retry unavailable identity reads within one shared deadline. **Steps to reproduce** Use `doc/testing-native-remote-continuity.md`. Its Linux cases execute real turns through PostgreSQL, TLS ingress, and the Rust runner. They kill a separate controller process, recover old checkpoint metadata, and change process configuration. The provider is deterministic. Related: #16432 and #16441 fix live warm-owner handoffs. #16341 adds recovery diagnostics. #14695 covers provider process retention. ## What Changed - Preserve warm process reuse between healthy remote turns. - Verify a committed prior result, stopped controller, matching checkpoint and sandbox, and current task/coordinator ownership before recovering an orphaned warm session. - Reconnect through the prior run route, then use the existing authenticated run attachment to change authority. - Persist a process-configuration digest. Close and save recovered sessions when old or changed configuration cannot support reuse. - Retry unavailable state, marker, and process reads within one 30-second deadline. Keep signals and definitive identity failures single-attempt. - Record closed recovery diagnostics, including unreadable or malformed saved state. - Retire the recovery-only ownership check after verified attachment, so later warm turns use their current authority. - Add real-process Linux regressions and refusal controls. - Expose the existing source-only file-recovery form for retained native holds, with the same board authority, evidence, and exact stop-receipt requirements as legacy holds. Recording recovered files does not change task status or replay a run. ## Verification - `pnpm -r typecheck` passed; the server check was repeated for the third-turn fix. - `pnpm build` passed after the final UI change. The UI typecheck, token gates, and map checks also pass. - Four Linux continuity cases passed: warm reuse, controller loss, legacy checkpoint metadata, and changed configuration. Each creates one conversation and starts exactly three turns, including a healthy follow-up after recovery. The fixture uses expired prior leases; the third turn keeps the runner and provider processes. - Greptile caught the retained recovery gate; the third-turn regression now covers it. An initial test omitted the heartbeat’s saved input and left a test process after failure; the fixture was corrected and its disposable container reset before the passing run. - The current focused executor/restart group passes all 749 tests, including the original failure classifications. - Native file-recovery and UI tests pass (52 tests), including blocked tasks and both native and legacy holds. - After merging current master and retaining its equivalent CI expression check, the CI policy tests pass (15 tests) and all four three-turn Linux continuity cases pass again on `dec418ae55ff21b4fb067132613658cce445f181`. - `pnpm test:run` completed its general server group with 20,572 passes, 103 skips, and three failures: the skill-audit child process and two HTTP test timeouts. The same three suites then passed in isolation (89 tests) without code changes. That local command stopped before later groups; the complete final-head CI, including general, serialized, Runner, build, and browser gates, is green. - Greptile reviewed the combined head `dec418ae55ff21b4fb067132613658cce445f181` at 5/5 with no actionable findings; both earlier findings are resolved. All final-head CI gates and the security scan passed. Staging deployment and retained-incident disposition remain deployment acceptance work. ## Risks - Recovery requires exact durable evidence. Live conflicting controllers, replacement sandboxes, Stop/pause, and uncertain effects must remain blocked. - After controller loss, tool-contract or process-configuration changes can restart the provider. Normal healthy follow-ups retain their processes. - The new idle-recovery path has Linux fake-provider evidence. Live Codex/OpenCode and retained quarantined-state recovery still need staging qualification. - No database schema, telemetry, or runner protocol changes. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository inspection, code execution, and browser tools. The session does not expose a more specific model version or 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 (targeted checks and isolated reruns; full-run limitation recorded above) - [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 on `dec418ae55ff21b4fb067132613658cce445f181` - [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>
9.0 KiB
9.0 KiB
Native runner continuity through controller and process loss
Goal and delivery boundary
Ship a verified fix to the selected staging instance and recover the two reported
tasks from their retained work. Normal follow-up messages, server restarts, and
recoverable runner loss must not strand a task behind
native_continuation_requires_reconciliation or
runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate.
Removing an error label, clearing a hold without evidence, or blindly starting
another provider does not meet this goal.
The user authorized implementation, review, merge, staging deployment, and task recovery. The supported release covers the affected native Codex and native OpenCode remote execution paths, plus shared lifecycle regression controls. It preserves existing company, ownership, Stop, pause, approval, budget, and workspace boundaries. Genuine conflicting ownership or an unresolved external effect must remain safe and actionable. This is not a promise that infrastructure can never fail; it is a checkable promise that the specified failures do not lose work or require users to diagnose runner internals.
Warm process reuse is a firm requirement. The user superseded the earlier save-and-suspend choice: healthy follow-up turns must keep the existing provider process. Durable ownership and recovery must coexist with that behavior. The suspend-every-turn candidate is withdrawn from delivery; PR #16451 is draft and has not been merged or deployed.
Acceptance matrix
| Journey or fault | Required result | Evidence |
|---|---|---|
| Completed turn followed immediately by another user message | Next turn starts once with the saved history, workspace, and message | Real assembled-service regression and staging journey |
| Healthy remote warm follow-up | Same runner and provider process, with a verified handoff to the new run | Process-start identity and call-log assertions across real turns |
| Controller restart while a session is idle, including immediately after completion | Durable evidence suffices to resume or safely replace the runner; no process-local ownership dependency strands the task | Hard and graceful restart fault tests |
| Restart during startup or an active turn | Adopt the exact live execution or fence it before one bounded successor; preserve accepted results and pending messages | Real process/transport boundary fault tests |
| Remote process exits or identity cannot initially be read | Distinguish a transient probe failure, a dead execution, and a conflicting owner; reconcile automatically when evidence permits | Linux remote-process tests and persisted recovery timeline |
| Crash during checkpoint, recovery, or workspace copy-back | Resume the persistence obligation without replaying completed provider work; retain the sole recoverable source until saved | Restart/concurrency regression with independently checked file contents |
| Existing saved or quarantined state from the affected release | Recover eligible old state without requiring a newly created task or discarding its history | Compatibility fixture and recovery of both reported tasks |
| Duplicate recovery workers, lost acknowledgements, and newer user messages | At most one successor and one delivery of each message; no replay of a completed external action | Durable ledger and side-effect counter assertions |
| Wrong company/session, a genuinely live competing owner, Stop/pause, revoked access, or unknown external effect | No unsafe adoption, signalling, destruction, or speculative action replay; concrete remaining blocker | Negative controls |
| Deployment | Running staging application and any changed runner artifact match the tested revisions; both tasks have useful results or their genuine task-level disposition | Live revision, run, file, and task inspection |
Use the existing recovery budgets (three provider attempts including the original, 30-second failure retry interval) and control-transition contract (60-second deadline, 15-second reconciliation, resolution or explicit blocker within 90 seconds with healthy dependencies). Provider work duration is not bounded by this timer. Restarts must not reset budgets. Any necessary departure requires explicit evidence and an updated contract, not a silent relaxation of acceptance.
Investigation and implementation slices
- Establish both baseline failure paths from persisted evidence. The remote identity failure still needs its exact failing operation established; do not infer process death from the generic error alone.
- Close the completed-turn ownership/checkpoint gap in its existing lifecycle owner, including compatibility with retained old state. Test prevention and recovery, not only an isolated predicate.
- Close the demonstrated remote recovery gap using existing fencing, finalization, workspace retention, and continuation services. Keep diagnostics precise enough to distinguish probe unavailability from conflicting identity.
- Qualify the changed boundary locally first, then on a representative remote Linux target and staging. Prepare reviewable changes and required repository checks; deploy the verified revision and recover the original tasks.
The first discriminating experiment is a completed native remote turn whose controller is lost before a follow-up. Recreate the old persisted state through the real lifecycle, then attempt the follow-up with an empty process-local owner map. Preserve real persistence, transport, and process boundaries where mocks would hide the failure. Existing recovery E2E and native restart suites are the starting harnesses; past passing evidence does not qualify this change.
Current state
- Branch:
codex/native-runner-recovery, based onafe5b3418d. - PR: https://github.com/paperclipai/paperclip/pull/16451 (draft, unmerged).
- Normal warm turns retain their processes. The previous suspend-every-turn implementation is withdrawn. Earlier suspension tests are historical only.
- The base includes #16432 (idle expiry serialized with admission) and #16441 (exact warm-owner identity independent of configuration changes).
- The revised executor can recover a completed remote session after its controller dies. It checks the committed prior result, frozen scope, saved checkpoint, exact remote identity, and current task/coordinator lease. It reconnects through the prior route before authenticated run attachment.
- A saved process-configuration digest controls reuse after controller loss. Legacy metadata or changed configuration uses verified close and checkpoint followed by saved-history resume. Healthy warm follow-ups stay open.
- Four Linux container/TLS/PostgreSQL cases pass: ordinary warm reuse, actual controller SIGKILL, older checkpoint metadata, and changed environment. They assert one thread creation and exactly three turn starts, including a healthy follow-up after recovery. The hard-restart cases also reject a still-live prior controller and a replacement sandbox. Normal follow-ups use one provider process; recovery may refresh the provider. These are real runner/process tests with a deterministic fake Codex, not live Daytona or live model evidence. The prior lease fixture uses the expired status observed on staging. The third turn retains the runner and provider processes.
- Full workspace typecheck and production build pass on the warm candidate. The server and UI typechecks and production build also pass after the final changes. All 749 focused executor/restart tests pass; the full required test suite is running.
- Greptile found a stale recovery ownership check retained across later warm turns. Recovery admission now retires after verified attachment; the three-turn regression passes. The earlier saved-state diagnostic finding is also fixed. The third-turn fix received 5/5. Fresh review of the final changes is pending.
- The existing source-only file-recovery form now supports retained native holds as well as legacy holds. It preserves board authority, exact stop-receipt validation, task status, and the prohibition on replaying the original run. All 52 UI/native recovery tests, token gates, and map checks pass.
- Fresh read-only staging records confirm both original tasks remain blocked. The Codex failure preceded its coordinator claim. The OpenCode failure followed controller takeover, with an active turn in the saved checkpoint. Its original log lacks the failed probe reason; process death is not established.
- Both retained sandboxes were restored for file inspection. The completed website commit is intact; the other workspace retains a benchmark script and lockfile changes. Backup and task disposition remain in progress. No agent turn, task status, or deployment has been changed.
- The selected tenant has an existing fleet rollout deferred by active work. Do not override it. Remaining: full tests and review, staging qualification, retained-state recovery, and useful disposition of both tasks.
Deferred: unrelated skill-import setup failures, database query optimization, general UI redesign, and broad provider qualification unrelated to the changed shared lifecycle.