mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 06:10:44 +01:00
Devin FoleyandPaperclip
ab82ac0191
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - With remote (sandbox) execution, every turn ends by saving the whole working folder, so the next turn can continue on any sandbox. > - When a run is cut off mid-turn and its task continues in a new workspace, preparation attaches the dead run's saved workspace under `.paperclip-recovered/<operationId>`, so the agent can recover unfinished work. > - Nothing ever removes that attachment. Every later save archives and uploads it again, and a new sandbox restores it again. > - The attachment is a full workspace, `.git` included, so every save for that task is roughly twice as big. The extra size lasts until the task ends. > - This pull request removes an attachment after a run that was shown it succeeds, and keeps a retired record so it is never attached or shown again. > - The benefit is that affected tasks go back to normal save sizes after their first successful turn, without changing how recovered work reaches the agent. ## Linked Issues or Issue Description **What existing behavior does this improve?** Recovery attachments in remote workspaces (`.paperclip-recovered/<operationId>`). **Subsystem affected** Server: remote workspace preparation and recovery attachments. **Current behavior** `retainRemoteRecoveryAttachments` keeps every recorded attachment on every later preparation, and no code path removes one. On one hosted instance, 10 active workspaces carried 18 attachments. One task's saves went from 746 MB to 1,472 MB at a single save and stayed there. The largest grew from 709 MB to 3.3 GB. A 3.3 GB workspace needs most of a 10 GB sandbox disk during each save. When a task moves again, the new attachment contains the older one, so the copies nest. **Proposed behavior** Each live attachment records the run it was shown to. When that run has succeeded, the next preparation removes the attachment from the sandbox, after checking its exact provenance. It keeps a `retired` record, so: - a sandbox restored from an older save is cleaned up too; - the attachment is never attached or shown to an agent again. **Reason and benefit** A run that saw the attachment and succeeded had a full turn to use it. Removing it then returns the task's saves to normal size and frees sandbox disk. **Breaking changes** None. Records written before this change have no `presentedToRunId`. They are stamped on their next preparation and retired after the following successful run. Related: none found. ## What Changed - `remote-workspace-recovery-attachments.ts`: - New `retiredRecoveryAttachments`. It returns attachments already retired, and attachments shown to a different run that has since succeeded. - `retainRemoteRecoveryAttachments` removes those attachments from the sandbox and keeps a `retired: true` record for each. - `attachPreservedRemoteWorkspaceSnapshots` skips retired operation IDs. - `workspace-recovery-snapshot.ts`: `attachPreservedWorkspaceSnapshots` also skips retired operation IDs. - `remote-workspace-transport.ts`: new `removeRemoteRecoveryAttachment`. - It reuses the attachment probe's checks: a real directory under the workspace root, and an exact `provenance.json`. - It makes read-only directories writable before removing them. - It removes `.paperclip-recovered` once it is empty. - Anything else at that path fails the preparation instead of being deleted. - `remote-workspaces.ts`: computes the retired set before attaching. It writes `presentedToRunId` on each live record, and keeps retired records unchanged. - `heartbeat/workspace-preparation.ts`: never shows retired records to an agent. - `doc/execution-semantics.md`: describes retirement. ## Verification - `pnpm --filter @paperclipai/server typecheck`: exit 0. - New tests: - `remote-workspace-transport.test.ts`: removal against real sandbox commands. It removes an attachment with exact provenance, including a read-only directory, and leaves sibling attachments. It refuses a foreign proof or a symlinked directory without deleting anything. It reports a missing attachment, and removes the empty parent folder. - `workspace-recovery-preparation.test.ts`, new `retired_followup` case. The run shown the attachment succeeds. The next preparation, on a fresh sandbox restored from that run's save, removes the attachment and records it as retired. A later preparation after a failed run still neither attaches nor shows it. - `ordinary_followup` (the agent deleted the attachment itself) now also ends with a retired record instead of an empty list. - A retired operation ID is not attached again, even to an empty destination. - 13 recovery and remote-workspace suites: 836 passed. With the source change reverted, the 3 new or changed preparation cases and the 5 new transport cases fail. ## Risks - **An agent can lose access to recovered work after one successful turn.** That happens if a successful run never looked at the attachment. The source run's saved snapshot in storage is not touched, so an operator can still recover it. - **Retired records stay in workspace metadata.** There is one small record per attachment, and each later preparation sends one sandbox probe per record. - **Nested attachments are unchanged.** If no turn succeeds between two moves, the newer attachment still contains the older one. That older copy may hold work nobody has recovered yet, so this change does not drop it. - Low risk overall: the change is limited to remote workspace preparation and is fail-closed on provenance. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, test runs, read-only database queries). AI-assisted investigation, change and tests, with human direction. ## 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>