mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 06:10:44 +01:00
## Thinking Path - Paperclip runs agent tasks in remote sandboxes (Daytona). With a warm lifecycle, a sandbox outlives its turn so the next turn on the same workspace resumes it instead of provisioning and uploading again. - #20048 makes warm the default on remote sandboxes. Its merge condition: an idle warm sandbox must be an owned record, adopted by its successor or destroyed or suspended once its owner is proven gone, with zero orphans in a Daytona chaos wave. - That wave (warm environment, K1/K2/K4 faults) left sandboxes alive behind dead lease rows. Reading the wave's database shows three warm-specific shapes that nothing ever tears down: 1. **A Retry moved the task to a new workspace.** `retainedIssueWorkspaceForRetry` reuses only local leases, so a remote task always gets a fresh workspace. Reuse matches only the exact workspace, so the old sandbox is unreachable: `released` and stopped (wave t05), or still `retained` and running. 2. **A failed warm lease.** The plugin stopped the sandbox for reuse, but reuse never resumes a `failed` row (wave t12). 3. **A warm sandbox kept running (`retained`).** Its idle timer lived only in the process that kept it. If that task moves on, nothing stops the sandbox, even after a restart. - The shared ownership path already has a kind for exactly this lifecycle: `environment_retained_orphan_sandbox` destroys a stopped, retained sandbox once nothing references it, but it skipped warm leases on purpose. This PR extends that kind instead of adding a new sweep. ## Linked Issues or Issue Description Refs #20048 **What happened?** In Daytona warm wave w-20261108T142906-s9501: - t05: run 8758207a's sandbox was resumed by c99b3a2e and released stopped in workspace 35a678f0. The Retry kept a fresh workspace (8ac860c3) and provisioned a new sandbox, and the old one stayed forever. - t12: run f2bf2f05's lease on sandbox 6d4c91fb was released as `failed` with the sandbox stopped, and the next run on the same workspace provisioned a new one. Both sandboxes were still alive when the wave ended. **Steps to reproduce** 1. Use a sandbox environment with warm reuse (`reuseLease: true`). 2. Complete a turn so the warm lease is kept, retained or released and stopped. 3. Make the next run fail so the Retry keeps a fresh workspace (always the case for a remote task), or so the lease is released as `failed`. 4. The next run provisions a new sandbox; the old one is never resumed or destroyed. **Expected behavior** A warm sandbox that no turn can resume is an owned record. Once its owning run is terminal and nothing references it, the ownership reconciler stops it if it is still running, then destroys it. A sandbox its task can still resume on the same workspace is left for reuse. ## What Changed - `server/src/services/environment-retained-orphan-settlement.ts`: the listing (`retainedOrphanSandboxCondition`) now has two warm branches, each on the newest lease row for its sandbox: - `abandonedWarmSandboxCondition`: a `reuse_by_environment` lease with a confirmed `stopped` receipt for this lease and run, either `failed`, or `released` while the task's bound workspace differs from the lease's. The settle hands it to the cleanup sweep through the existing stopped-cleanup contract, so the old run's successors are never blocked on the destroy. - `supersededRunningWarmSandboxCondition`: a `retained` (still running) warm lease whose task moved workspace. The settle queues a stop-only intent. When the provider confirms the stop, the row becomes `released` with a receipt, and the first branch destroys it on a later pass. - The settle re-checks the same predicate under the row lock. Every existing hold still applies: the run still active, open runs or recovery actions on the issue, pending exports, stop or restore intents, or another live lease on the same sandbox. - `server/src/services/sandbox-stop-and-retain.ts`: new `prepareSupersededWarmSandboxStop`, which prepares the existing stop-only intent from `retained`. The intent carries the recovery marker, so the sweep dispatches its first attempt at once. - `server/src/__tests__/environment-retained-orphan-sandbox.test.ts`, new tests: - a superseded released lease is destroyed; - a failed warm lease is destroyed; - a running superseded lease is stopped, then destroyed; - keep cases: the task can still resume it (released, and retained after a restart), no stop receipt, a newer row on the same sandbox, a run in flight on the task. The positive tests fail on master and pass with this change. **Previous-release state:** the conditions read existing columns only (status, lease policy, workspace binding, stop receipt). Rows the previous release saved are judged as-is. The tests build them in exactly the shape that release writes (no new marker), so old-saved state is handled by the new code without migration. Rows without a confirmed stop are never listed. ## Verification - `cd server && npx vitest run src/__tests__/environment-retained-orphan-sandbox.test.ts`: 29/29 pass. With the two service files reverted to master, the 3 positive tests fail and the other 26 pass. - Neighbouring suites `environment-run-lease-ownership`, `heartbeat-pending-cleanup-sweep`, `ownership-reconciliation-report`, `heartbeat-orphaned-active-lease-sweep`, `environment-pending-cleanup-queue-owner` and `retained-native-session-cleanup-work`: 150/150 pass. - The wave's lease rows were read from the run's kept database copy to confirm the shapes above. - Not yet covered: the same Daytona warm wave, rerun with this fix. #20048 merges only at ≥11/12 with zero orphans. ## Risks - **Destroying a sandbox someone can still use.** Only the newest row for a sandbox is judged. `released` and `retained` rows qualify only when the task's bound workspace differs from the lease's, which is the exact rule reuse matches on. All existing holds still apply, and a stop must be provider-confirmed before any destroy. - **Unsaved work.** Pending native exports and stop or restore intents hold the sandbox. A stopped sandbox is destroyed only after its owning run is terminal and nothing on the task is still running. - **Blocking a successor.** The destroy uses the stopped-cleanup contract (`workspaceRecoveryCleanup` with the stop receipt), which `hasPendingRunCleanup` treats as resource cleanup. This is covered by a test. - **Out of scope.** A `retain_on_failure` lease kept for workspace restore (wave t12, a095e0b5) stays with restore recovery, which owns its own cleanup. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## 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>