mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
DottaandClaude Opus 5.5
a5fe5deebe
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote agent runs execute in provider sandboxes (for example Daytona) that Paperclip creates and must clean up > - Before each provider create, Paperclip saves a durable creation intent (`sandbox_creation_operations`). Recovery uses it to find and destroy a sandbox whose lease was never committed > - Recovery waited 30 minutes after creation before it checked an intent, because a live server could still be waiting for the provider > - After a restart, the creating server is gone, but the intent still waited the full 30 minutes. A short-lived or redeployed instance never reclaimed those sandboxes > - The teardown that follows has the same problem: a teardown claim fences other sweeps for 15 minutes, and a restart during the claim kept the sandbox alive until the fence expired > - This pull request saves the server process identity with each intent and each teardown claim, and ends the wait when same-host proof shows that process exited > - The benefit is that a restart during sandbox acquire or teardown no longer leaves paid provider sandboxes behind ## Linked Issues or Issue Description **What happened?** A control-plane restart during Daytona sandbox acquire left the created sandbox without an `environment_leases` row. In one restart chaos wave, 6 creation intents stayed `pending` with `next_attempt_at` = creation + 30 minutes. The wave ended (and its environment was deleted) before that time, so 4 sandboxes stayed `stopped` in Daytona. Only Daytona's own auto-delete would remove them. Two of the runs did not exist in `heartbeat_runs` because they were non-run acquires; their intents still had the run id. **Expected behavior** After a restart, each sandbox that this instance and company created, with no committed lease and no live creator, is found by its exact creation name and labels and destroyed through the plugin, after the existing ownership checks. Sandboxes from another instance, another host, or a live server stay untouched. Nothing acts org-wide. **Steps to reproduce** 1. Start a Daytona-backed run. 2. Kill the server (SIGKILL) after the creation intent commits and before the lease commits. 3. Restart the server. On master, the intent stays `pending` for 30 minutes and the sandbox stays in Daytona. 4. The regression tests in `server/src/__tests__/sandbox-creation-operations.test.ts` reproduce this with the controlled provider fixture. Refs #18357 (added the durable creation intent this change completes). **Paperclip version or commit** master07a548073a(live proof at8d525dba7d+ this change, content-matched to the runner image) ## What Changed - New `server/src/services/controller-process-identity.ts`: records the server boot id, PID, a PID-space hash and the process start time. The PID-space hash covers the hostname, the kernel boot id and the PID namespace on Linux (the hostname and boot time elsewhere), so containers that share a hostname but not a PID namespace never judge each other. It proves that a recorded process exited only in the same PID space: the PID is gone, or the PID is live with a definitely different start time. Another PID space, an unreadable identity, an unidentified record, or this server never counts as exited. - `sandbox-creation-operations.ts`: each creation intent stores the creating process and its settle deadline. Each recovery pass first moves intents whose creator exited to "due now". If the early lookup finds no sandbox before the settle deadline, the intent stays `pending` and retries. It does not become manual reconciliation, because the provider can still materialize an aborted request. - `heartbeat/recovery.ts`: each pending-cleanup teardown claim stores the claiming process, bound to its claim attempt id. Before the sweep reads due leases, it ends the 15-minute fence of a claim whose claimer exited. A newer claim from another path (workspace repair, export retry) that kept an older record has a different attempt id, so it is never released. - Both recovery passes page through candidates with a stable id cursor. Live, foreign or unidentified owners therefore cannot hide an abandoned record behind one page. - This covers every `createOwnedSandbox` caller, including non-task probe and qualification acquires. Their intents have no issue and a generated run id, which is why their run ids are missing from `heartbeat_runs`. - Recovery uses only the existing paths: exact `paperclip-create-<operation id>` name and label inspection in the plugin, `pending_cleanup` lease, and plugin teardown. No org-wide listing or deletion was added. ## Verification - Red, then green (controlled provider fixture, real embedded PostgreSQL): - `sandbox-creation-operations.test.ts` → "after the creating server exits mid-acquire". Before the fix, the exited-creator case left no cleanup lease. After the fix, the next sweep creates the `pending_cleanup` lease and the teardown destroys the sandbox. Counter-tests: a creation owned by this live server, another live process, another host, or a committed lease is not inspected or destroyed. An early "not found" keeps the intent pending, and a later sweep still finds a late allocation. - `heartbeat-pending-cleanup-sweep.test.ts` → "a teardown claim whose server stopped mid-attempt". Before the fix, a lost claim blocked the teardown. After the fix, it retries now. Counter-tests: claims from this live server, another host, or without an identity keep their fence. - Review regressions: "keeps the fence of a newer claim that kept an earlier attempt's server record" and "pages past more fenced claims than one sweep page" fail on the previous commit and pass now. "pages past more unprovable creations than one page" guards the creation-side cursor; the old unordered page happened to include the row, so it was not red. - After the review changes, `sandbox-creation-operations`, `heartbeat-pending-cleanup-sweep`, `live-recovery-work` and `environment-cleanup-work` pass (155 tests), and `tsc --noEmit` for `server` passes. - Related suites pass: sandbox-creation-operations (28), environment-cleanup-work, task-creation-recovery, native-startup-cleanup-recovery, sandbox-orphan-cleanup-spool, plugin-warm-tasks, idle-completed-history (187 total), and the pending-cleanup sweep suite. `tsc --noEmit` for `server` passes. - Live Daytona restart chaos wave `w-20261107T001609-s11` (codex, A1, K2+K4, concurrency 2, RC-06 masked, image `sha256:2767e91c…`, first fix only): 4/4 tasks pass, all restart-crossing, R6 pass for every task, `orphanSandboxes: []`. The server log and database show two creation intents killed by K4 being reconciled right after restart and destroyed through Paperclip. A third reclaimed sandbox lost its teardown claim to the next K4 kill in the same second, so its 15-minute fence blocked the environment delete. The second commit fixes that. - Live Daytona wave `w-20261107T012048-s11`, same plan, both commits: **4/4 pass, all restart-crossing, R6 pass for every task, `orphanSandboxes: []`, no held leases.** The environment delete through Paperclip succeeded with `remainingSandboxes: []`. One restart-time sweep destroyed one sandbox through the plugin. ## Risks - Same-host proof relies on PID liveness and process start time, as native controller takeover already does. A false "exited" result is limited to recovery running early. The plugin still checks exact name and labels before any delete. An early not-found result stays pending. - Multi-host deployments sharing one database keep the old deadlines, because a different PID space is never judged. A redeploy into a new host or PID namespace also keeps the old deadlines (fail closed). - The live waves ran before the review commits. The review commits only narrow when a record is judged, and they add paging. - Intents and claims written before this change have no process identity, so they keep the old deadlines. - No schema change. The new fields live in existing JSON metadata. > 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`), Anthropic, via Claude Code with tool use (shell, file editing, test execution). Extended reasoning enabled. ## 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>