From c02fd8d427320d9e1cb1ba199581f9ea5f86ebb3 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 8 Nov 2026 14:13:05 -0600 Subject: [PATCH] docs(plans): salvage-then-retire design for kept restore-source and done-task warm sandboxes (#20087) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path - Paperclip keeps some remote sandboxes after their run: warm ones for reuse, and failed runs' stopped sandboxes as workspace-restore sources. - In the warm Daytona chaos waves for #20048, the real leaks were restore sources that nothing retires once a user Retry closes their recovery actions. Separately, a done task's suspended warm sandbox is only destroyed when its environment is deleted. - Retiring them early needs a provider-I/O salvage step: compare against the durable save, export anything unsaved to the issue, then destroy. That spans recovery, storage, attachments and teardown. The coordinator asked for a design note so it can be its own reviewed task instead of a rushed patch. ## Linked Issues or Issue Description Refs #20048, #20085, #20086 **What happened?** Closed restore sources and done-task warm sandboxes stay suspended until their environment is deleted, costing storage. No path compares them with the last durable save or exports their unsaved files before retirement. **Steps to reproduce** 1. A run fails; its sandbox is kept as a `retain_on_failure` restore source. 2. The user clicks Retry; a fresh run succeeds and the task is done. 3. The restore-source sandbox is never retired until the environment is deleted. **Expected behavior** After a clean compare, or a confirmed export of anything unsaved to the issue, the sandbox is destroyed. If salvage fails, it stays suspended and is retried. ## What Changed - `doc/plans/2026-11-08-restore-source-salvage-and-warm-retirement.md`: a design for one owned kind (`environment_kept_sandbox_retirement`) on the shared reconciler. It covers listing rules for both cases (24 h default retention for done-task warm sandboxes), a three-stage salvage state machine (compare, export, retire) with a provider-I/O step and receipt table, failure handling that never destroys without proof, and the test plan. ## Verification Docs only; no code or tests changed. ## Risks None at runtime. Additive documentation. ## 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 --- ...tore-source-salvage-and-warm-retirement.md | 195 ++++++++++++++++++ 1 file changed, 195 insertions(+) create mode 100644 doc/plans/2026-11-08-restore-source-salvage-and-warm-retirement.md diff --git a/doc/plans/2026-11-08-restore-source-salvage-and-warm-retirement.md b/doc/plans/2026-11-08-restore-source-salvage-and-warm-retirement.md new file mode 100644 index 00000000000..f0eea8c0c5a --- /dev/null +++ b/doc/plans/2026-11-08-restore-source-salvage-and-warm-retirement.md @@ -0,0 +1,195 @@ +# Retiring kept sandboxes: salvage, then destroy + +Status: proposed design, for its own reviewed task. Not implemented. + +## Problem + +Paperclip keeps a remote sandbox after its run in two cases: + +- **Warm reuse** (`reuse_by_environment`): the next turn on the same workspace + resumes the sandbox. +- **Workspace-restore source** (`retain_on_failure`, lease and run carry + `workspaceRestoreRecovery`): after a failed run, the stopped sandbox may hold + the only copy of files that were never saved back. + +Some of these sandboxes are never retired: + +| Case | Why it stays | What ends it today | +|---|---|---| +| Restore source whose recovery actions were closed by a user Retry (`explicit_user_continuation`), where a later run on the task succeeded | Restore recovery only destroys after a verified repair with disposition `destroy` (`scheduleRecoveredWorkspaceCleanup`), and only for open or exhausted actions (`workspaceRecoverySourceEligible`). The retained-orphan kind treats `workspaceRestoreRecovery` as permanently held. | Environment deletion (#20085) | +| Warm sandbox, suspended, whose task is done or cancelled but still bound to the workspace | The idle timer lives in the server process. After a restart, Daytona's own auto-stop suspends it. Nothing destroys it while the task stays bound. | Environment deletion | + +Both are suspended (stopped) and owned, so they leak storage cost, not compute. +In the warm Daytona chaos wave w-20261108T170423-s9501, all four real +"orphans" were closed restore sources. + +Decision so far (2026-11-08): grade a stopped, owned restore source as +retained, not as a leak (#20086). Environment deletion destroys both kinds with +consent (#20085). This note covers retiring them earlier without losing work +(R2: no unsaved work is lost). + +## Rule + +One owned kind on the shared ownership reconciler, +`environment_kept_sandbox_retirement`, lists a kept sandbox when: + +- **Restore source:** + - every recovery action for the source run is closed (resolved or cancelled, + including by explicit user continuation); + - a later run on the same task has succeeded; + - no native workspace export or preservation obligation is pending. +- **Warm sandbox:** + - the task is `done` or `cancelled`; + - the sandbox has been idle (`releasedAt`, or the last turn's end) longer + than the retention window: default 24 h, configurable per instance; + - its newest lease row is `released` or `retained`. + A new turn on the task inside the window adopts the sandbox as it does today. + +Its owner is the lease's run (`runOwner`), which is terminal. Most holds from +`environment-retained-orphan-settlement.ts` still apply: shared provider +resource, stop or restore intents in flight, pending cleanup claim, open runs +on the task. + +Two existing guards are deliberately replaced for this kind, and only for it: + +- The unconditional hold on `workspaceRestoreRecovery` lease metadata + (`alwaysHeldSql`, `heldBy`). For a restore source it is replaced by the + closed-actions and later-success conditions above, plus the salvage receipt. +- The `resourceDisposition === "destroy"` requirement in + `scheduleRecoveredWorkspaceCleanup`. Restore sources (`retain_on_failure`) + and warm leases (`keep_running`) never record `destroy`, so retirement uses + the salvage receipt instead. + +**Adoption versus retirement.** Reuse eligibility today is status-only +(`reusableLeaseCanBeResumed` accepts any `released` or `retained` lease), so a +metadata stamp cannot stop a new run from resuming a sandbox mid-salvage. At +claim time, the settle moves the lease to a non-resumable state with a +compare-and-swap on its status (the stop-only `pending_cleanup` claim shape). +A run admitted after that provisions a fresh sandbox. A run admitted before the +claim adopts the sandbox, and the claim's compare-and-swap then fails. + +## Settle: salvage, then retire + +Reconciler settles never call providers; they hand provider work to a sweep +with a durable claim. Retirement does the same, as a three-stage state machine +recorded on the lease (`keptSandboxRetirement`), each stage idempotent and +retried with backoff. + +1. **Compare.** Claim the lease (`pendingCleanupInFlight`-style attempt id with + a controller stamp), then resume the stopped sandbox read-only: the same + path `recoverRetainedWorkspace` uses (`resumeWorkspaceRecoveryLease`), under + `withNativeWorkspaceFinalizationOwnership`. + - **Remote workspace reference:** capture the read-only manifest digest and + compare it with the committed `executionWorkspaceSnapshots.manifestDigest` + of the reference's snapshot. (`sha256` hashes the encrypted archive bytes and + can never match a manifest digest.) + - **Legacy native descriptor:** compare `captureDirectorySnapshotOffThread` + + `directorySnapshotSha256` with the descriptor's `finalHostSha256`, else its + `baselineSha256`. + - **Warm sandbox after a successful turn:** the run already recorded + `finalHostSha256` when it synced back, so a match needs no capture of new + content. + - **Equal:** record `compare: clean` and go to stage 3. **Unequal or + unknown:** go to stage 2. + - **Every exit after a resume attempt re-stops the sandbox**, clean or not, + in a `finally`. `recoverRetainedWorkspace` does the same with + `retryPendingSandboxTeardown` and `settleStopOnlyCleanup`. If the stop + cannot be confirmed, a durable stop obligation stays on the lease. + Preparing the resume clears `remoteExecutionTermination`, so stage 3 uses + the new stop receipt from this re-stop, not the old one. +2. **Export.** Capture the filtered workspace: extract `quarantineRetainedRoot`'s + capture, `omitDeniedWorkspaceFiles` and `putFile` steps from + `workspace-recovery-fallback.ts` into an exported helper. + - Store the archive under a deterministic object key derived from the lease + and the attempt. + - **Write the receipt with the attachment, in one transaction:** the asset, + the issue attachment and `artifact` work product, the comment, and a + `workspaceOperations` receipt (`kept_sandbox_salvage`, archive sha256), + as `quarantineRetainedRoot` already does. `createAttachment` always + inserts and assets are unique on `(company_id, object_key)`, so a retry + first looks up an existing receipt or asset by object key and reuses it. + It never creates a second artifact or comment. + - The re-stop happens in the single resume window's `finally` (see Fencing), + after the export, not between compare and export. + - **Exposure.** The filter (`omitDeniedWorkspaceFiles`) only denies known + secret names and directories; a credential in an ordinary file would be + included. The export is therefore restricted to the issue's existing + readers, stored with a bounded retention, and run through the run-log + secret redaction on file contents. If the design review prefers, the + first version can instead record "unsaved files exist" on the issue and + wait for a user's decision before attaching the archive. + - **First version:** export the whole filtered workspace. A file-level diff + needs new code on top of the manifest and `selectDirectorySnapshot`, and + can follow. +3. **Retire.** With `compare: clean` or a verified export receipt, hand the + lease to the pending-cleanup sweep for destruction. Use the same lease update + as `scheduleRecoveredWorkspaceCleanup` (`pending_cleanup`, policy + `ephemeral`, `workspaceRestoreRecovery` removed, stop receipt kept as + `workspaceRecoveryCleanup.stopReceipt`, so successors are never blocked), + with the salvage receipt in place of `hasVerifiedWorkspaceRepair`. + +**Failure keeps the sandbox.** If the resume, capture, upload or attachment +fails, the lease stays suspended, and the stage retries with backoff up to a +bounded retention window. Past the window it is handed to the board as an +issue recovery action ("salvage failed; the sandbox is kept"). It is never +destroyed without a clean compare or a confirmed export. + +## Fencing (required before implementation) + +- **One claim status that the sweep skips.** The retirement claim sets the lease + to `pending_cleanup` and records `keptSandboxRetirement` with `stage` set to + `compare` or `export` and an attempt id. The pending-cleanup sweep must treat + a lease with an in-flight `keptSandboxRetirement` stage as held, exactly as + it treats `workspaceRecoveryCleanup` or an in-flight cleanup claim. Only + stage 3 clears the marker and queues the destroy. Otherwise the sweep could + destroy the sandbox while its files are still being saved. +- **Admission claims before it resumes.** Run admission must win a + compare-and-swap on the reusable lease's status before it calls the + provider's resume. A resume that loses the claim provisions a fresh sandbox + and never touches this one. This closes two overlaps: a turn that already + selected the sandbox, and a provider resume in flight when retirement claims. + Today `environmentResumeLease` runs before `acquireLease` updates the old row, + so this ordering change is part of the work. Test: a turn racing a retirement + claim leaves exactly one owner of the sandbox, and the loser never calls the + provider for it. +- **One resume lifetime.** Compare and export run inside a single resumed + window. The re-stop `finally` wraps both stages together, so the sandbox is + running for the capture and stopped on every exit (clean, dirty or failed). + +## Provider I/O steps and their receipts + +| Step | Provider call | Durable receipt before the next step | +|---|---|---| +| Claim | none | attempt id plus controller stamp on the lease | +| Resume read-only | `environmentResumeLease` | none needed; the step is re-done on retry | +| Capture | sandbox exec/read | manifest digest on `keptSandboxRetirement` | +| Upload | storage `putFile` | object key plus sha256 | +| Attach and comment | none (database) | `issueWorkProducts` row, comment id, `workspaceOperations` receipt | +| Re-stop | `environmentStopLease` | stop receipt (`remoteExecutionTermination.state = stopped`) | +| Destroy | `environmentDestroyLease` (pending-cleanup sweep) | `expired` with a destroyed receipt | + +## Tests + +- Old-saved/new-resumed: restore sources and warm leases written by the + current release, in their exact row shape. +- Clean compare → destroyed without export. Dirty compare → one artifact work + product and one comment, then destroyed. Export failure → kept and retried, + never destroyed. +- Holds: open run, open recovery action, pending export, another lease on the + sandbox, an operator owner-gone attestation. +- Concurrency: two reconciler processes settle once (one work product). +- Crash between the attachment write and the receipt: the retry reuses the + artifact by object key and records the receipt once. +- Every exit after resume re-stops the sandbox (clean compare, capture + failure, upload failure); an unconfirmed stop leaves a durable obligation. +- A turn admitted at the claim boundary: before the claim it adopts the + sandbox and the claim fails its compare-and-swap; after the claim it gets a + fresh sandbox. +- Warm: a done task within the window is adopted by a new turn; past the window + it is retired; a still-running task's warm sandbox is never listed. + +## Out of scope + +- Hard-deleted issues: the lease's `issue_id` is set null on delete, so no rule + can prove the task moved. The issue-delete path should hand its kept leases + to this teardown. That is a separate follow-up.