mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
DottaandClaude Opus 5.5
1674902b4f
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run saves its workspace in a `workspace_finalize` operation after the agent gives a result > - A crash loop can kill the server after it records this operation as `running` and before it finishes > - The next server resumes finalization and records a new attempt, which succeeds > - Nothing settles the first attempt, so it stays `running` forever and looks like live work > - This pull request settles the old attempt when a later attempt for the same run finishes, under the proof that the old controller is gone > - The benefit is that no workspace operation stays `running` after its owner is dead ## Linked Issues or Issue Description Refs #19666, #19759, #19721, #19717 **What happened?** A control-plane restart chaos test sent SIGKILL to the server while a native run finalized its workspace. The server crash-looped. One boot recorded a `workspace_finalize` operation as `running` and then died. A later boot resumed finalization. Its new operation (with `resumedFromOperationId` set to the old one) succeeded, and the run succeeded. The old operation stayed `running` at rest. **Expected behavior** When a later finalization attempt for the same run finishes, and the owner of the earlier attempt is proven gone, the earlier attempt gets a terminal status with a reason. A live owner keeps its attempt. **Steps to reproduce** 1. Start a native run that finalizes a workspace. 2. Kill the server with SIGKILL after it inserts the `workspace_finalize` row, and before it updates the row. 3. Restart the server and let it finalize the run again. 4. See that the first `workspace_finalize` row stays `running`. **Paperclip version or commit** `ef83d566ca` (also on `27170dca49`). ## What Changed - Root cause: `attemptNativeWorkspaceFinalization` in `server/src/services/native-runtime/native-workspace-finalizer.ts` reads the previous attempt only to set `resumedFromOperationId`. It records the new attempt but never settles the previous `running` attempt. The early "already succeeded" return path also left it alone. Only the recorder that started a row can settle it (`workspace-operations.ts` `recordOperation`), and a killed process never does. - Add `settleSupersededNativeWorkspaceFinalizations` in `native-workspace-finalization-supersession.ts`. It sets earlier `running` native finalize attempts for the run to `failed`. It writes `metadata.supersededFinalization` (reason `superseded_workspace_finalization`, superseding operation id and status), an stderr note, a terminal-workspace wake (registered before the write), and a `workspace_operation.reconciled` activity entry. - The proof is the run's workspace-finalization ownership, not age. The caller holds the advisory lock and the durable owner receipt. That claim requires the previous controller to have released it or to be proven stopped (same-host process proof, platform proof, or a completed copyback barrier). The function also skips an attempt that this process is still running, and attempts with independent obligations (`uncertain`, `saveWork`). It only touches rows that `native_workspace_finalizer` recorded and that are older than the superseding attempt. - The finalizer calls it after a new attempt reaches a terminal status, and on the "already succeeded" path. A settlement error is logged and does not change the finalization result. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-workspace-finalization-supersession.test.ts src/services/native-runtime/native-workspace-finalizer.test.ts` on embedded Postgres: 72 passed. - New tests in `native-workspace-finalization-supersession.test.ts` (a separate file, so `native-workspace-finalizer.test.ts` stays under 1,000 lines): - A `running` finalize from a dead controller plus a resumed attempt that succeeds: the old row becomes `failed` with the supersession reason, no `running` rows remain, and one activity entry exists. - The resumed attempt fails: the old row is still settled, and `supersedingStatus` is `failed`. - A `running` finalize older than an already succeeded attempt: the old row is settled on the next sweep. - While another controller holds ownership, the `running` row stays `running`. - A `running` finalize recorded by another service stays `running`. - Attempts with `saveWork` or `uncertain: true` stay `running`. - Red/green: with the settle call disabled, the three settle tests fail (row stays `running`). With the fix, all pass. - `npx tsc --noEmit -p server` passes. ## Risks - Low. The change only writes rows that are already proven abandoned, and only under the finalization ownership that already fences copyback. - A settled attempt now reads as `failed`. Readers pick the latest attempt by creation time, and the superseding attempt is always later, so the run's finalization outcome does not change. - Existing leaked rows on runs that never finalize again stay `running`. This PR does not add a backfill. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edit) and code execution. ## 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>