mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 21:01:54 +01:00
Devin FoleyandPaperclip
c7390bef20
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run can accept its result before the workspace copy-back finishes. A background worker retries a failed copy-back from the retained sandbox without another provider turn. > - Until that retry succeeds, the run stays `running` and keeps its task's execution lock. User answers and wakes for that task stay queued. > - The worker checked one entry at a time (`batchSize: 1`, no concurrent checks). A slow sandbox export held every other task's due retry for the whole export. > - Work without a saved retry time used a deadline of `0`. Copy-backs that a restart interrupted always sorted ahead of retries that were due hours earlier. > - On a busy server that restarts often, scheduled retries never got to the front of the queue. Their tasks stayed stuck. > - This pull request runs up to four checks at once, one per task, and orders pending work by when it became pending. > - The benefit is that one slow export cannot stall unrelated tasks, and an old retry runs before newer work. ## Linked Issues or Issue Description Refs #17734 (added this worker), #18421 (the same concurrency pattern for native reply recovery) and #19063 (open; records why finalization failed a run). This pull request does not change their behavior. **What happened?** On a hosted instance, sandbox exports were slow for a few hours. A harness-state save timed out after about 5.5 minutes in 27 runs, and worker copy-back attempts took 2 to 34 minutes each. Eleven native runs then waited in `retryable_failure` (or `workspace_finalizing` with an expired lease) for more than three hours. Each one had exactly one copy-back attempt and no retry. The worker's operation receipts show its attempts ran strictly one after another. After each restart, the worker first took the copy-backs that the restart interrupted. They have no `nextAttemptAt`, so their deadline was `0`. The server restarted every 30 to 40 minutes, so the overdue retries never ran. One stuck run held a task whose user had answered a question card. The answer's wake stayed queued for the same three hours. **Expected behavior** A slow copy-back for one task does not delay due copy-backs for other tasks. Due work runs oldest first. New work without a saved retry time does not go ahead of older due retries. **Steps to reproduce** 1. Create two native runs with accepted results on different tasks. Put both in `retryable_failure` with an overdue `nextAttemptAt`. The first is older. 2. Make the copy-back for the older run block. 3. Start the worker. On `master` the second task's copy-back never runs while the first one blocks. The new worker test runs these steps against PostgreSQL. **Paperclip version or commit** `master` at `69dea878c0b`. **Deployment mode** Any server with native runs that use remote sandbox workspaces. ## What Changed - `server/src/services/native-runtime/native-workspace-finalization-work.ts`: - Remove `batchSize: 1`. Add `concurrentChecks: { limit: 4 }` keyed by `issue:<company>:<issue>`. Runs that share a task keep their copy-backs serialized. Ownership is still checked per run by the existing advisory lock and owner receipt. - Work without a saved retry time uses the coordinator's `updatedAt` as its deadline, not `0`. The value is clamped to now, because the database clock can be slightly ahead of the process clock. Saved retry times and lease expiry behave as before. - `doc/native-workspace-finalization-recovery.md`: one paragraph on worker concurrency and ordering. ## Verification - Failing first: "retries another task while an older copy-back is still running". On `master` the second task's copy-back never ran while the older one blocked (the test timed out after 15 s). Now the second task finalizes while the first is still blocked. After release, the first one also finalizes. - Failing first: "orders work without a saved retry behind older overdue retries". On `master` the interrupted copy-back had deadline `0` and sorted first. Now it sorts after the older overdue retry, and its deadline is not in the future. - `cd server && npx vitest run src/__tests__/native-workspace-finalization-work.test.ts src/__tests__/scoped-deadline-work.test.ts src/__tests__/server-startup-feedback-export.test.ts`: 3 files, 159 passed. After rebasing onto `69dea878c0b`, the first two files: 38 passed. - `cd server && npx tsc --noEmit -p tsconfig.json`: pass. - Not verified: the worker on a live instance under slow exports. The diagnosis comes from that instance's operation receipts and run events. ## Risks - Up to four copy-backs can now run at once on one server. Each one uses its own dedicated database connection and sandbox transfer. Live runs already finalize in parallel, so this is within the existing load pattern. - The batch size is now the default (25). A pass ends when its slowest check ends, but the other lanes keep processing the rest of the batch. - `updatedAt` changes on any coordinator write. A pending entry with new writes moves later in the order, but it stays due. - No schema migration. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) through Claude Code, with tool use (shell, file edits, test runs, read-only database and log queries). The runtime does not expose the context window or reasoning configuration. ## 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>