mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A run that hits a file-storage outage gets a bounded storage wait: a retry that starts again after storage is back > - The run error says "Paperclip will retry automatically when storage is back" > - On staging, a server restart reattached a remote native runner while its turn was still running. A storage outage then failed the reattachment > - Restart recovery kept the workspace under a no-replay hold, so the run took the code `native_continuation_requires_reconciliation`. Only a `storage_unavailable` run gets a storage wait, so no retry was scheduled. The run still showed the storage retry promise, and the task stayed blocked for about 15 hours > - This pull request makes the storage text follow the run's final code and the real storage wait outcome, and reads old rows truthfully > - The benefit is that every storage message is true: it promises a retry only when one is scheduled, and it offers a working Retry when nothing retries the run ## Linked Issues or Issue Description **What happened?** A native run on a remote sandbox was reattached after a server restart. Its provider turn was still in flight. File storage failed during the reattachment. Restart recovery settled the run as `native_continuation_requires_reconciliation`, kept its files, and did not replay the unfinished turn (this is the existing policy for a turn with an unknown outcome). The run error said "Paperclip will retry automatically when storage is back". No storage wait was scheduled, so nothing retried. The task stayed blocked until an operator clicked Retry. **Root cause** `server/src/services/heartbeat/run-completion.ts` chose the message and the error code separately. The message used `isStorageUnavailableError(err)`. The code used a chain where `nativeTerminalFailureCode` wins over `storage_unavailable`. The storage wait is scheduled only when the code is `storage_unavailable` (and no native terminal failure is recorded). So the message promised a wait that the code made impossible. The same false promise stayed when the storage wait was refused (`not_scheduled`) or its write failed. **Expected behavior** A storage failure promises an automatic retry only when the storage wait is scheduled. Otherwise it offers Retry. Rows saved by earlier releases read the same way after deploy, with no migration. **Steps to reproduce** Fail a native run with a storage-unavailable error after its native coordinator is settled as `terminal_failure` / `native_continuation_requires_reconciliation`. See the new regression in `run-completion.test.ts`. ## What Changed - `storage-unavailable.ts`: add the held messages (outage and rejected credentials). They say Paperclip is not retrying automatically and offer Retry or a message. Add `storageUnavailableHeldMessage()` and `presentStorageFailureMessage()`. - `run-completion.ts` (`failRun`): compute the error code first. Use the storage retry promise only when the code is `storage_unavailable`. Under any other code (for example a native recovery hold), use the held message. No storage wait is created for such a run, because it could only be refused. - `run-completion.ts` (`failRun` and `failRunSetup`): replace `markStorageWaitExhausted` with `settleStorageWaitMessage`. It keeps the promise only when a retry is scheduled. It writes the exhausted text when the budget is spent, and the held text when the wait was refused, its write failed, or a case handoff took over. - `run-completion.ts`: when the promise is withdrawn, the failure's `error` event (written before the retry decision) gets the same final text, so the transcript does not keep the promise after reload. - `run-completion.ts` (`failRunSetup`): a lost controller lease keeps its own code and text, not the storage promise. - `run-lifecycle.ts`: `decorateHeartbeatRunRuntimeStatus` presents a saved storage retry promise under a code other than `storage_unavailable` with the held text. This covers run detail and issue run presence. `listRuns` in `run-state.ts` applies the same projection, so the company run list and the Inbox failure read agree. Rows from earlier releases read truthfully with no migration. - `doc/run-log-events.md`: document which runs get a storage wait and what each message means. Previous-release story (upgrade-safe run state): this change adds no persisted shape. Rows saved by the previous release carry forward. A row with the storage promise under another code reads with the held text. A `storage_unavailable` row keeps its text. The native hold policy is unchanged: an unfinished turn with an unknown outcome is not replayed automatically, and the existing Retry continues it. ## Verification - Red before the fix (assertion failures, not setup errors): the new `run-completion.test.ts` cases received "Paperclip will retry automatically when storage is back" under `native_continuation_requires_reconciliation`, and the `run-lifecycle.test.ts` cases read the saved promise unchanged. - Green after the fix: - `cd server && npx vitest run src/services/heartbeat/run-completion.test.ts src/services/heartbeat/run-lifecycle.test.ts -t storage` (13 passed) - `cd server && npx vitest run src/services/heartbeat/run-completion.test.ts src/services/heartbeat/run-lifecycle.test.ts src/storage/storage-unavailable.test.ts` (213 passed) - `cd server && npx tsc --noEmit -p tsconfig.json` (clean) - Existing storage cases still pass: a scheduled storage wait keeps its promise and suppresses rapid recovery; an exhausted wait writes the exhausted text. - `pnpm verify` exited 2 (a doc is a global CI input), so required CI supplies the broad coverage. - The full `run-completion.test.ts` file passed every test locally. Its `afterAll` hook timed out under machine load (load average about 17), also with the new tests skipped. ## Risks - Low risk. The change affects only the text of failed runs and one read projection. It does not change retry scheduling, recovery ownership, or replay policy. - The read projection matches this repository's own two promise constants under a code other than `storage_unavailable`. A future change to those constants must keep the projection in sync. - Error events already written by earlier releases keep their original text in the run log. Live events already published before a withdrawn promise are not resent. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, browser reads for GET-only diagnostics). ## 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 - [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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>