mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
Devin FoleyandPaperclip
dfe7df0015
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs sync their workspace with a remote sandbox through a
per-run folder under `<instance>/native-workspace-sync/<run>/`
> - The folder holds a descriptor, a manifest, and workspace and git
seed archives, and a seed can be more than a gigabyte
> - Only a few finalization commit paths delete the folder, so many runs
leave it on disk forever
> - Instance disks fill up with folders that no code will read again
> - This pull request adds a bounded, deadline-driven retention worker
that removes a folder only when no reader can still need it
> - The benefit is that instance disks stop growing with stale sync
folders, and recovery and export paths keep their files
## Linked Issues or Issue Description
**What happened?**
`cleanupNativeWorkspaceSync(runId)` is the only code that deletes
`native-workspace-sync/<run>/`. It runs only when finalization commits
through specific paths. Runs that end in `terminal_failure`, retained
recovery, reconstruction, superseded runs, commits through other paths,
and server crashes leave the folder forever. On one instance, 22 folders
older than 7 days held 6.8 GB. Single seed archives were up to 1.4 GB.
**Expected behavior**
A run's sync folder is removed after a retention window, once the run
and its finalization are settled and no recovery or export path can
still read it. The shared `generations` seed cache stays.
**Steps to reproduce**
1. Run a native agent with a remote sandbox and let the run end in
`terminal_failure` (or crash the server during finalization).
2. Wait more than 7 days.
3. Look in `<instance>/native-workspace-sync/`. The run's folder and its
seed archives are still there.
**Paperclip version or commit**
`master` at 837add0caa
**Deployment mode**
Any deployment that runs native agents with remote sandboxes.
**Agent adapter(s) involved**
Native runtime with remote sandbox workspace sync (for example Codex on
Daytona).
## What Changed
- Added
`server/src/services/native-runtime/native-workspace-sync-retention.ts`.
It removes `native-workspace-sync/<name>/` when `<name>` is a UUID, the
entry is a real directory, its newest file is older than the window, and
one of these holds:
- no `heartbeat_runs` row has that id (orphan);
- the run has no valid `nativeWorkspaceSync` reference and is settled:
terminal status, `finished_at` before the window, no live recorded
process (see below), no live controller lease, no
`nativeWorkspaceFinalizationOwner` receipt;
- the run is settled as above, its `native_run_finalizations` row (if
any) has no `lease_owner` and no `next_attempt_at` and is `committed`
(any recovery state) or `terminal_failure` with no or `blocked` recovery
state, no active/escalated export-cause or owner-unverified recovery
action names it, no lease carries `nativeWorkspaceExportResume` or
`pendingCleanupInFlight`, retained recovery cannot still use it
(referenced lease destroyed, or no scheduled/running/awaiting_evidence
system recovery and no queued/recovering workspace recovery), and
historical recovery adoption (`adoptHistoricalWorkspaceRecoveries`) has
no pending candidate for it.
- Each folder is decided in one transaction that holds
`pg_try_advisory_xact_lock` on the run's finalization ownership key. The
rule is evaluated while the lock is held. A held lock skips the folder.
The folder is renamed to `.expired-<uuid>` in that transaction and
deleted after commit. The next sweep removes leftover `.expired-*`
folders. `generations`, non-UUID names, symlinks and files are never
touched. Each tick tries at most 50 leftovers, then checks at most 200
folders and removes at most 50. Leftovers and folders each continue from
their own cursor, so undeletable leftovers do not block run folders. A
leftover that cannot be deleted schedules the 15-minute retry. Deadline
discovery reads run state 200 IDs at a time and stops when asked to. It
supports dry run and `shouldContinue`.
- A recorded `process_pid` / `process_group_id` blocks removal only
while it can still be the run's own process. This reuses the rule from
`conversation-continuation.ts`: PIDs are ignored when every lease of the
run is remote (they are sandbox PIDs), and a local PID or process group
blocks only while it exists on this host and its start time matches
`process_started_at`. An unreadable start time keeps the folder. A
recycled PID does not.
- Separate finding, not changed here: native runs do not clear
`process_pid` when they finish. It is cleared only by the status
arbiter's `release_run_resources` effect (cancellations and commits) and
by the recovery paths. A failed run gets a `schedule_retry` effect
instead, so its PID stays set. For remote targets the stored value is
the sandbox PID. Other readers already treat these PIDs as liveness
hints only, so this PR does not change that write path.
- Exported `nativeWorkspaceFinalizationLockKey` from
`native-workspace-finalization-ownership.ts` and used it in
`withNativeWorkspaceFinalizationOwnership` and the retention worker, so
both use the same key derivation.
- Exported `nativeWorkspaceSyncStateRoot()` from
`native-workspace-sync.ts`; `descriptorDirectory` now uses it.
- Registered the worker on the shared delivery-work coordinator
(`DELIVERY_QUEUES.nativeWorkspaceSyncRetention`) in
`server/src/index.ts`, next to the git restore point worker. It follows
`rules/sleep-aware-background-work.mdc`: no interval. It runs at startup
and at real deadlines: the earliest settled run's finish (or an orphan's
newest file) plus the window; a recheck one window later for folders it
kept; and a 15-minute retry after a failure or a held lock. With no
folders, `nextRunAt` is `null`. The run folder is now created through
one helper in `native-workspace-sync.ts` that notifies process-local
listeners; the worker wakes on that signal only while it has no
deadline, because any scheduled deadline is never later than a new
folder's own deadline. `index.ts` unsubscribes the listener on shutdown.
It pauses passive maintenance during idle drain.
- Added `PAPERCLIP_NATIVE_WORKSPACE_SYNC_RETENTION_ENABLED` (default on
unless `false`), `PAPERCLIP_NATIVE_WORKSPACE_SYNC_RETENTION_DRY_RUN`
(default off) and `PAPERCLIP_NATIVE_WORKSPACE_SYNC_RETENTION_DAYS`
(default `7`, minimum `1`) in `server/src/config.ts`. Documented them in
`doc/DEVELOPING.md` and added a "Native workspace sync folders" section
to `doc/recoverable-disk-cleanup.md`.
## Verification
- `pnpm exec vitest run
src/__tests__/native-workspace-sync-retention.test.ts
src/__tests__/native-workspace-sync-retention-config.test.ts` in
`server/`: 62 tests pass. They cover:
- deletes a settled committed run (also with a left-over
`resuming_session` recovery state), a blocked `terminal_failure` with no
recovery owner, an orphan, and an unreferenced settled run;
- keeps runs whose local process or process group is still alive, or
whose live process start time cannot be read; keeps a running run with a
PID; deletes terminal runs whose recorded local process exited, whose
PID was recycled, or whose PID came from a destroyed sandbox;
- keeps recent files, recently finished runs, unsettled runs, a live
controller lease, an owner receipt, `observed` and `retryable_failure`
phases, a live `lease_owner`, a scheduled `next_attempt_at`, and each
non-blocked recovery state on `terminal_failure`;
- keeps runs with an open export-cause or owner-unverified recovery
action, `nativeWorkspaceExportResume` or `pendingCleanupInFlight` lease
metadata, retained recovery in flight on a lease that was not destroyed,
or pending historical recovery adoption; deletes when the retained lease
was destroyed;
- never touches `generations`, non-UUID names, symlinks or files; dry
run deletes nothing; leftover `.expired-*` folders are removed; an
advisory lock held by another transaction makes it skip; ticks are
bounded and resume from a cursor;
- `nextDeadlineAt` returns `null` with no folders, now with leftovers,
the settled finish plus the window, the orphan mtime plus the window,
and one window later for kept folders; the registered worker goes quiet
with no folders, retries after a held lock or an undeletable leftover,
and the real delivery-work coordinator wakes again when the real
producer (`writeDescriptor`) creates a run folder; leftover removal is
bounded and undeletable leftovers do not block run folders.
- Mutation checks: I removed each guard one at a time (advisory lock,
owner receipt, process pid, finished_at window, lease owner, next
attempt, phase, recovery state, export recovery, owner-unverified,
export resume, retained recovery, backfill pending,
symlink/real-directory check, dry run, and both mtime checks together,
plus the folder-creation wake, the process liveness rule (restoring the
old `process_pid IS NULL` rule, the sandbox exemption, the process group
check, the recycled-PID check, the unreadable start time), the leftover
batch limit, the leftover cursor and the leftover retry delay). Each
mutation made at least one test fail. I restored the code after each
one. Removing only the first mtime check does not fail a test, because
the second check inside the transaction still keeps the folder.
- `pnpm exec vitest run` on related suites in `server/`:
`native-workspace-sync.test.ts`,
`native-workspace-sync-history.test.ts`,
`native-workspace-finalizer.test.ts`,
`native-workspace-finalizer-failure.test.ts`,
`native-harness-backup-expiration*.test.ts`,
`git-restore-point-ledger*.test.ts`,
`delivery-work-coordinator.test.ts`, `chat-delivery-work.test.ts`. All
pass.
- `pnpm run typecheck` in `server/` passes.
- Greptile review: earlier rounds; all findings fixed with tests; final
score 5/5 with no open threads.
- Not run: the full server test suite and an end-to-end run against a
real instance. I did not measure disk savings on a live instance.
## Risks
- Deleting a folder that a reader still needs would make that path fall
back or fail. Same-run resume and export run under the finalization
ownership lock or before the run settles, and the worker checks the rule
while it holds the same lock. Retained workspace recovery already
handles a missing folder (ENOENT) by snapshotting the sandbox. The rule
keeps the folder for every in-flight or reopenable recovery state listed
above.
- A committed finalization row with a stale recovery state and no lease
owner is treated as settled. Nothing reads the recovery state of
committed rows.
- The worker is on by default. Operators can set
`PAPERCLIP_NATIVE_WORKSPACE_SYNC_RETENTION_DRY_RUN=true` first to see
candidates in the log, or turn it off with
`PAPERCLIP_NATIVE_WORKSPACE_SYNC_RETENTION_ENABLED=false`.
- The first sweep on a large instance does more filesystem work. It is
bounded to 200 folders checked and 50 removed per tick.
- The folder-creation wake is process-local. A folder created by another
server process sharing the same instance directory is picked up at the
next deadline or restart.
- A stale `process_pid` no longer blocks removal. If a run's local
process is still alive with its recorded start time, the folder stays.
Live work is also still guarded by the run status, controller lease,
finalization lease and ownership lock checks.
- No schema migration.
> 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`), 1M context, through Claude Code
with tool use (shell, file edits, test runs).
## 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
- [ ] I will address all Greptile and reviewer comments before
requesting merge
---------
Co-authored-by: Paperclip <noreply@paperclip.ing>