mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 06:10:44 +01:00
DottaandClaude Opus 5.5
738058553f
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A chat follow-up on a remote native task runs as a continuation: it starts from its predecessor's saved provider session and, for a warm session, copies the predecessor's runner process identity onto the new run > - When the continuation fails before it asks for a runner of its own, its due same-run retry sees a recorded process identity and reattaches to "this run's" runner. That runner never existed, so reattachment always fails with `native_remote_recovery_lease_mismatch` and the task is blocked > - On staging, 34 OpenCode runs on 31 tasks failed this way in one evening. Each first attempt failed while staging its provider pack, recorded no runner start, and kept its own sandbox lease active > - This pull request proves that shape from durable state, retires the inherited identity, and resumes the run the way its failed attempt ran, in its own retained sandbox > - The benefit is that these retries recover automatically, without a second sandbox and without the unsaved-work hold, while every run that may hold its own provider work keeps verified reattachment ## Linked Issues or Issue Description Refs #19839, Refs #19399, Refs #19755, Refs #19779, Refs #19827 **What happened?** A remote OpenCode chat continuation failed during provider-pack staging. The run had no runner start event, no process identity event, no controller checkpoint, and no provider event of its own. Its saved session (no active turn, no pending request) and its process identity were inherited from the previous run. Its failure detail recorded `providerEventsExist: false`, and its own sandbox lease stayed active. The due same-run retry chose `reattach_remote_runner` because a process identity was recorded. Reattachment failed with `native_remote_recovery_lease_mismatch`, and the run ended in `native_continuation_requires_reconciliation`. **Steps to reproduce** 1. Continue a warm remote native session so the new run inherits the predecessor's runner process identity and saved session. 2. Fail the new run with a retryable error before it requests a runner (for example during provider-pack staging). 3. Let the same-run retry become due. Recovery requests `reattach_remote_runner`, and reattachment fails. **Expected behavior** The retry resumes the run the way its failed attempt ran, in the same sandbox, and continues the inherited provider session. A run with any provider work, runner start, or controller checkpoint of its own keeps verified reattachment. ## What Changed - New `server/src/services/native-runtime/native-inherited-runner-retry.ts`: - `retireInheritedRemoteRunnerIdentity` locks the issue, the coordinator, then the run (the same order as same-run resumption). It retires the recorded process identity only when all of these hold: the coordinator is a result-less, unowned `exact_checkpoint_resume` retry with `providerEventsExist: false`; the run has no runner start, process identity, adapter invocation, tool start, runner event, controller checkpoint, provider event or provider session of its own; the saved session has no active turn and no pending runtime request; and a settled predecessor of the same native session recorded the exact same process identity (PID, group, start time). - It records `nativeInheritedRunnerRetirement` on the run profile, with the run's own sandbox location when that lease is still active, and an `inherited_runner_never_launched` lifecycle event. - `readInheritedRunnerRetainedSandbox` returns that sandbox only while it still matches the run's saved remote location. - `heartbeat/recovery.ts`: a due remote retry calls the retirement first. It returns `retired`, `not_applicable` or `transient`. A retired run (including one a concurrent sweep already retired) goes to the ordinary same-run resume (`claimNativeSessionResumptions`), with every existing admission gate. A lock timeout or database error is `transient`: the sweep defers the run and checks again, and never falls back to reattachment. Any other run keeps verified reattachment. - `heartbeat/workspace-preparation.ts`: an ordinary same-run resume with a retired inherited runner acquires its retained sandbox through the existing exact-lease path (`reattachRemoteLease`), which validates and resumes that lease and never acquires a replacement. It acquires as an ordinary resume does only when the exact-lease failure proves the sandbox is unusable (`stage=environment_acquire|environment_resume`, `field=lease|lease_status|lease_expired|provider_lease_id`). Any other failure, such as a scope mismatch on a live lease, is kept and acquires nothing. - New `server/src/services/retained-sandbox-absent-error.ts`: a sandbox provider's resume reply with no provider lease and `expired: true` (the reply Daytona and other providers send when the sandbox is gone) now throws `RetainedSandboxAbsentError`. Every other mismatched reply keeps the existing error. The exact-lease reattachment in `environment-run-orchestrator.ts` maps it to `native_remote_recovery_lease_mismatch: stage=environment_resume field=lease_expired`, so the same-run resume above acquires as an ordinary resume when the provider has removed the retained sandbox. A transient resume failure is not treated as absence and never acquires another sandbox. - `native-restart-recovery.ts` exports the existing `hasNativeExecutionStartEvidence` so both paths use one definition of "execution reached a runner". - `doc/execution-semantics.md` documents the rule. ## Previous-release state - No schema change. The new profile field is additive. - A run saved by an older release without `providerEventsExist`, without a native session ID, or without a matching predecessor keeps verified reattachment. That path is unchanged. When it loses its runner, it still requests the user's Retry once (#19399). - A run that an older release left with its own runner start, controller checkpoint or provider event keeps verified reattachment, because its outcome may be ambiguous. - A profile without the new marker never changes acquisition. ## Verification - `npx vitest run server/src/services/heartbeat/recovery.test.ts -t "never launched its own runner|keeps verified reattachment"` (embedded PostgreSQL): - Staging shape (inherited session and process identity, no runner start of its own, saved remote workspace, active own lease, recorded PID). Before the fix: fails, because recovery requested reattachment. After the fix: passes. The PID fields are cleared, the marker names the predecessor and the retained lease, `executeRun` is called once with a plain `nativeLeaseOwner` claim, the run still has exactly one (active) lease, and no process is signalled. - Negatives, passing before and after: a provider turn of its own, a runner start of its own, a controller checkpoint of its own, no matching predecessor, a pending runtime request in the saved session, and a failure detail without `providerEventsExist`. Each keeps verified reattachment and gets no marker. - `npx vitest run server/src/services/heartbeat/workspace-preparation.test.ts -t "retained sandbox|retired inherited"`: - A retired inherited runner acquires its retained sandbox with `reattachRemoteLease`. Before: fails. After: passes. - When the retained sandbox is gone, it acquires as an ordinary resume. Before: fails. After: passes. - Without the marker, acquisition is unchanged (passes before and after). - A `nativeWorkspaceSync` reference also resumes its retained sandbox. A live lease's `lease_scope` failure is kept and acquires nothing. - `recovery.test.ts` also covers a second retirement call (returns `retired`, one event) and a held issue lock (defers; no reattachment, no dispatch, PID kept). - `environment-runtime.test.ts`: on exact resume, only a provider reply with no lease and `expired: true` (with or without a workspace sentinel mismatch) is `RetainedSandboxAbsentError`; a different lease or a reply without `expired` keeps the existing error. `environment-run-orchestrator.test.ts`: confirmed absence becomes `stage=environment_resume field=lease_expired` and acquires nothing. - Affected suites pass locally: `recovery.test.ts`, `workspace-preparation.test.ts`, `heartbeat-native-remote-failure-retention.test.ts`, `workspace-recovery-retry-copyback.test.ts`, `native-runner-restart-recovery.integration.test.ts`. `pnpm --filter @paperclipai/server typecheck` passes. ## Risks - **No second executor.** The rule applies only when this run never requested a runner, so no runner of this run exists. The predecessor must be settled, and its identity is matched exactly. The resume then takes the normal same-run claim, which holds the issue lock and admission gates. - **No repeated side effects.** The run has no provider event, turn or tool start of its own. A saved session with a pending runtime request or an active turn keeps verified reattachment. The resumed provider session goes through the existing provider recovery, which refuses a pending side-effecting call with a permanent, named code. - **Bounded.** The retirement takes effect once per run, because afterwards the run has no recorded identity. A contended retirement waits for the next sweep. Further failures use the existing three-attempt provider budget. - I did not run a live Daytona sandbox, or a full preparation through a real sandbox plugin, through this path. The orchestrator's exact-lease checks are covered by its own unit tests. The evidence is recovery and preparation tests on a real database with the staging run shape. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, run in Claude Code with tool use (shell, file editing, read-only browser access to staging). ## 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 - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>