mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-12 18:06:11 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Legacy sandbox runs copy the agent's workspace files back to the host when the run settles. > - When this copy-back fails one time, the server keeps the sandbox and puts a board-owned repair hold on the task. > - The hold blocks new messages. Only an operator API runbook can clear it. The board shows that runbook text, so users do not know what to do. > - Many copy-back failures are transient: a dropped transfer, a provider error, or a merge lock that another restore holds. > - This pull request retries those failures automatically while the run still has its merge baseline and before any host file changes. > - The benefit is that most failures repair themselves, and the remaining holds explain the situation in plain language. ## Linked Issues or Issue Description Refs #14590 (related: no-replay recovery holds) **What happened?** A legacy sandbox run finished its work, but one workspace copy-back attempt failed. The task then showed "Recovery needed" with operator-only steps ("verify its exact stop receipt … record workspaceRepairEvidence"). Messages from the user stayed saved and were not delivered. **What did you expect?** Paperclip retries a transient copy-back failure automatically. If a repair is still necessary, the task says clearly what happened and who must act. **Steps to reproduce** 1. Run an agent in a legacy sandbox environment. 2. Make the workspace copy-back fail one time, for example with a dropped transfer or a held merge lock. 3. Open the task. It is blocked behind a repair hold, and new messages wait. **Additional context** Separately, distinct live events can render toasts with the same title and body (for example, two runs of the same agent that both succeed). The toast list showed both, and the desktop app forwards each one as a notification. ## What Changed - `packages/adapter-utils/src/sandbox-managed-runtime.ts`: retry the workspace copy-back up to three times in total, with a 2 s and then a 5 s backoff. A retry occurs only when the failure is `restore_failed` or `restore_lock_timeout` and no host workspace file was written. The merge now marks the first host write in `beforeApply`, which runs under the merge lock before any write. - Unsafe archives, permission failures, and failures after the merge started writing are never retried. Each retry uses a new temp directory and a new imported Git ref. - New optional `workspaceRestoreRetryDelaysMs` input, so tests can shorten the backoff. Nested repository restores receive the same value. - `ui/src/components/ExecutionBlockerNotice.tsx`: for a workspace repair hold, the notice now says what happened in plain language. The stored operator runbook text moves under "Technical details". - `ui/src/context/ToastContext.tsx`: also dedupe toasts on their visible tone, title, and body inside the existing 3.5 s window. - `doc/workspace-restore-locks.md`: document the automatic retry. ## Verification - `pnpm --filter @paperclipai/adapter-utils exec vitest run src/sandbox-managed-runtime.test.ts src/workspace-restore` (218 tests pass). New tests cover: - a retry after a dropped transfer, - a retry after a merge-lock timeout, - a stop after three attempts, - no retry for an unsafe archive, a permission failure, or a failure after the merge started writing. - The existing step-diagnostic test now uses failures that persist across retries. It still expects one diagnostic per failed step. - `pnpm --filter @paperclipai/ui exec vitest run src/context/ToastContext.test.tsx src/components/ExecutionBlockerNotice.test.tsx src/context/LiveUpdatesProvider` (120 tests pass). - `tsc --noEmit` for adapter-utils passes. `node scripts/check-token-gates.mjs` is clean. - Not fully run: the full adapter-utils suite has two failures in `github-launcher*.test.ts`. They read the managed GitHub credentials of the local sandbox environment and do not touch restore code. UI `tsc -b` reports five errors in `PluginOrganizationSwitcher.tsx`, which is not changed here, because the plugin SDK is not built in a fresh install. ## Risks - A copy-back that fails before the merge can take about 7 s longer before it reports a failure. Lock timeouts can take up to about 97 s in total (three 30 s lock waits plus backoff). Settlement already waits for the copy-back. - The retry logic depends on `beforeApply` running before the first host write in `mergeDirectoryWithBaseline`. That is true today, and a comment marks it. - A Git bundle import before the merge can leave unreferenced Git objects after a retry. They are harmless, and the existing ref cleanup still runs on each attempt. - Content-based toast dedupe can hide a second toast with identical text inside 3.5 s. That is the intended behavior. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`, Anthropic) in the Claude Agent SDK, with tool use (shell, file edit, code search) and a subagent for code exploration. ## 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: Paperclip Agent <agent@paperclip.ing> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
109 lines
6.5 KiB
Markdown
109 lines
6.5 KiB
Markdown
# Workspace restore locks
|
|
|
|
Workspace restore and agent-file collection serialize writes to each canonical
|
|
target directory. Their lock files live in
|
|
`<instance root>/locks/directory-merge`, outside the writable target. All writers
|
|
must use the same instance root and a filesystem with reliable SQLite file
|
|
locking. Network filesystems that do not provide that locking are not supported.
|
|
|
|
Each target has a permanent `<hash>.lock.sqlite` file. An open SQLite
|
|
`BEGIN IMMEDIATE` transaction holds its reserved file lock for the entire write
|
|
operation. Contenders retry without blocking the Node event loop, up to the
|
|
existing 30-second limit. Closing the connection releases the lock; the operating
|
|
system also releases it when the process exits or crashes. Independent targets
|
|
use different files and can proceed concurrently.
|
|
|
|
This uses the same built-in `node:sqlite` dependency as workspace manifests.
|
|
See [SQLite file locking](https://www.sqlite.org/lockingv3.html) for the reserved
|
|
lock contract. No application database or schema migration is involved.
|
|
|
|
The adjacent `<hash>.lock.owner.json` records a PID and creation time only for
|
|
bounded timeout diagnostics. Missing, stale, or incorrect metadata cannot grant
|
|
or retain ownership. PID reuse, PID namespaces, and wall-clock changes do not
|
|
decide whether a writer owns the lock.
|
|
|
|
**Never delete, replace, or move a `.lock.sqlite` file while an instance can
|
|
write to it.** Its stable inode is part of the locking contract. Replacing it
|
|
could let two writers lock different files for the same target. Files remain
|
|
after release, including for targets that no longer exist. Their diagnostic
|
|
owner sidecars are normally removed after release.
|
|
|
|
## Upgrade from directory locks
|
|
|
|
Older versions created `<hash>.lock/owner.json` and checked only whether the
|
|
recorded PID existed. A server restart could reuse that PID and leave an orphaned
|
|
lock permanently protected. Those records cannot establish a process lifetime or
|
|
PID namespace, so the new implementation never guesses that a legacy holder is
|
|
dead. An existing legacy directory continues to block admission.
|
|
|
|
Do not run old and new lock protocols concurrently against the same instance
|
|
root. An old process does not participate in the SQLite lock protocol.
|
|
|
|
1. Drain and stop **all** old server and worker processes that can write to the
|
|
instance root, including processes on other hosts or in other containers.
|
|
2. Preserve any unfinished-run evidence needed for recovery. After all writers
|
|
are stopped, move leftover legacy `<hash>.lock/` directories to an operator
|
|
scratch directory outside the lock root. Do not remove `.lock.sqlite` files.
|
|
3. Start all writers on the new version. Verify that a run completes both the
|
|
provider turn and file collection/restore. A successful model response alone
|
|
does not prove that its local file changes were saved.
|
|
|
|
The same drain requirement applies to rollback. Existing permanent SQLite files
|
|
can remain on disk; older versions ignore them. Do not infer that a legacy lock
|
|
is safe to remove from its age, an absent PID, or a successful task response.
|
|
|
|
This change prevents new orphaned ownership. It cannot recover file changes
|
|
that an earlier failed collection discarded.
|
|
|
|
## Required remote restore failures
|
|
|
|
The copy-back first repairs itself. While the run still holds its merge
|
|
baseline, a workspace copy-back that fails before writing any host file is
|
|
retried up to three times in total, with a short backoff. This covers dropped
|
|
transfers, provider errors, and a merge lock held by another restore. An unsafe
|
|
archive, a permission failure, or any failure after the merge started writing
|
|
host files is never retried. Only a copy-back that still fails reaches the
|
|
repair hold below. Adapter environment Tests restore into a throwaway temp
|
|
workspace, so they opt out of these retries and report a failure at once.
|
|
|
|
When a legacy sandbox run cannot copy required workspace files back, the server
|
|
keeps the original allocation for repair. It writes the failed run, the exact
|
|
lease retention intent, and a board-owned repair hold in one transaction. The
|
|
hold survives reassignment and conversation resets and coexists with newer
|
|
recovery incidents; it never supersedes them.
|
|
Cleanup stops that allocation and verifies its provider receipt. It does not
|
|
use ordinary ephemeral destruction or return the source to warm reuse. Stop
|
|
failures use the existing cleanup retry and backoff policy.
|
|
|
|
The retained source uses `retain_on_failure`. A stopped source can still incur
|
|
provider storage charges. It stays available until an operator recovers or
|
|
explicitly discards it; elapsed time never proves that its files were copied.
|
|
The task's `GET /api/issues/:id/recovery-actions` response lists these repair holds
|
|
alongside any newer active incident, including the original lease IDs. Inspect
|
|
those exact leases through `GET /api/environment-leases/:leaseId`,
|
|
confirm that cleanup stopped them, recover the missing files, and record
|
|
`workspaceRepairEvidence` through the existing recovery-action resolution API.
|
|
Send `POST /api/issues/:id/recovery-actions/resolve` with the exact recovery action
|
|
ID, `outcome: "restored"`, the task's **unchanged**
|
|
`sourceIssueStatus`, and `executionReconciliation` for the recorded source run.
|
|
Only a board operator can record this repair. It preserves the current task
|
|
owner, conversation generation, status, and execution locks. It does not queue a
|
|
continuation or replay the stopped run; after repair, any new work still follows
|
|
the normal admission rules.
|
|
The board currently shows repair guidance and an Inspect run link, not a repair
|
|
form. Recording the decision is an operator API/runbook workflow; the recovery
|
|
API response contains the exact action and lease IDs. Provider access may require
|
|
an instance administrator or Cloud support. This change does not add a repair UI.
|
|
Then use the provider's operator console to remove that exact retained
|
|
allocation when its files are no longer needed. Verify its immutable provider
|
|
allocation ID against the recorded lease before removal. The ordinary reusable
|
|
sandbox cleanup does not select `retain_on_failure` sources; do not use
|
|
environment deletion as proof that this allocation was removed. Keep the
|
|
environment and its provider credentials until recovery is complete. Recording a
|
|
repair decision does not automatically destroy it.
|
|
|
|
A new message, Retry, or `/new` cannot clear this workspace repair hold. Ordinary
|
|
local lock-timeout retries and runs without a required remote copy-back failure
|
|
keep their existing policy. This change preserves future failures; it cannot
|
|
recover files from an allocation that was already destroyed.
|