mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-12 18:06:11 +02:00
codex/codex-runner-default
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
4611d6e95a |
fix(restore): retry sandbox copy-back before holding the task for repair (#15920)
## 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> |
||
|
|
f1c44b7b56 |
Preserve remote work when required workspace restore fails (#15479)
Persist exact source-retention obligations before run finalization and protect them across cancellation, restart and task changes. Require board-authorized repair evidence without replaying old work or changing current task ownership, state or locks. Hide an unavailable retry action and document operator recovery and retention costs. Validated with 674 scoped regressions, 224 combined integration tests, full typecheck/build, independent safety reviews and green CI with Greptile 5/5. No historical file recovery is claimed. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
dd9983b894 |
fix(adapter-utils): release restore locks when a process crashes (#14869)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent runs restore workspace files and collect instruction-file changes. > - Writers to the same target directory must wait for each other. > - The current lock records a PID, which a new process can reuse after a crash. > - A reused PID can keep an orphaned lock alive and make each later run fail. > - This pull request makes a SQLite file lock decide ownership. The OS releases it when the process exits. > - Later runs can proceed after a crash, and concurrent live writers remain protected. ## Linked Issues or Issue Description Refs #10914. This addresses crash recovery. It does not cancel a stalled operation in a process that is still alive. Related work: #9667, #14787, and #12187. The earlier attempt in #9667 assumes one live server per lock root. This implementation uses an OS-backed lock to support concurrent writers without treating a different process token or an old timestamp as proof of a dead owner. It retains the private lock root and bounded timeout diagnostics from the merged changes. After a process dies while holding a restore lock, a replacement process can reuse its PID. The existing `process.kill(pid, 0)` check then reports a live owner forever. Later runs can complete their model turn but fail during file collection or restore. ## What Changed - Hold a SQLite `BEGIN IMMEDIATE` transaction for each directory write. Use the existing built-in `node:sqlite` dependency. - Keep each lock database on a stable inode. Keep PID and time metadata only for diagnostics. - Retain the 30-second asynchronous wait and existing timeout error code and diagnostic fields. - Fail closed when an old directory lock exists. Document a stopped-writer upgrade and rollback procedure. - Add real child-process tests for crashes, PID reuse, live owners, and connection cleanup. Cover callback failures, independent targets, stable inodes, invalid lock files, and ambiguous legacy records. ## Verification - Before the fix, the crash/PID-reuse test and the live-owner test both failed. Both pass with this change. - `pnpm exec vitest run packages/adapter-utils/src/directory-merge-lock.test.ts packages/adapter-utils/src/workspace-restore-merge.test.ts`: 56 tests passed. - Restore and agent-file working-copy integration tests: 118 tests passed before the additional connection-cleanup test. - `pnpm -r typecheck`: passed. - `pnpm build`: passed. - Full GitHub CI: all checks passed, including Linux workspace tests, server test shards, build, typecheck, and browser tests. - Greptile: 5/5, with no review threads or unresolved comments. - `pnpm test:run`: started locally, then stopped with SIGINT (exit 130) after full CI passed. The local serial run did not complete and is not counted as a full local pass. The completed CI shards provide the full-suite result. ## Risks - **Upgrade and rollback require a drain.** Stop every old writer that shares an instance root before switching protocols. Old and new versions must not write concurrently. - Existing legacy `.lock/` directories remain blocking. After all writers stop, preserve run evidence and move those directories to an operator scratch directory. The new code does not infer that they are abandoned from PID or age. - Never delete or replace a `.lock.sqlite` file while writers can run. These small files remain after release. - The shared filesystem must support reliable SQLite locking. Broken network-filesystem locking is unsupported. - This change prevents new orphaned ownership. It does not recover file changes lost during earlier failed collections, or interrupt a live operation that stalls. - No application database migration or new native dependency is required. See `doc/workspace-restore-locks.md` for the procedure. ## Model Used OpenAI Codex based on GPT-6, with code execution and repository tools. The exact model variant and context window are not exposed in this session. ## 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 (focused regression and integration suites; see the full-suite note above) - [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 - [x] All Paperclip CI gates are green - [x] 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> |