mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
d221501fb5c0b83385ce021b23d9d1f91fdf4219
413
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandClaude Opus 5.5
|
4c04c20c9e |
fix(recovery): wake a parent whose refused completion waited on dependencies that later clear (#20803)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A parent task that waits on blocker tasks resumes through the `issue_blockers_resolved` wake > - A periodic dependency sweep sends that wake when a blocked task's dependencies become ready and no wake covers the current blocked cycle > - The sweep skips a task that its own agent blocked, because that agent waits on something outside the blocker graph > - A native completion that Paperclip refuses for unresolved dependencies is also recorded as a block by the agent's own run (`durable_dependency_blocker_bound`). The sweep treated it as a self-block. It woke the task only if a blocker later completed or finalized on its workspace > - When readiness cleared another way, nothing woke the task. It stayed `blocked` with no next action > - This pull request makes the sweep treat that refusal as a dependency wait, and count a "Recover saved files" finalize without a workspace as the blocker settling > - The benefit is that a blocked task is not a silent dead end: it resumes once, automatically, when its dependencies clear ## Linked Issues or Issue Description Refs #20728 **What happened?** A parent task had three done child tasks that block it. One child was still listed as "saving its files", because a "Recover saved files" finalize had no workspace. The parent's agent tried to finish. Paperclip refused the completion because dependencies were unresolved and set the parent `blocked` (`durable_dependency_blocker_bound`). After #20728 was deployed, the parent's dependencies were ready. But the task stayed `blocked` with no wake, no Retry, and no next action. A person had to post a comment to wake it. **Expected behavior** When the last blocker of a blocked task clears, by status or by finalize settlement, Paperclip wakes the assignee once. This also applies to tasks already blocked before a deploy that changes readiness. **Steps to reproduce** 1. Block a task on a done blocker whose workspace finalize is still pending. 2. Let the assignee agent's native run report the task done. Paperclip sets the task `blocked` with `durable_dependency_blocker_bound`. 3. Make the blocker settle without a later blocker completion or a finalize on its workspace (for example, a recovery finalize with no workspace, or a deploy that changes the readiness rule). 4. Run the dependency sweep. Before this fix it reports `selfBlockedSkipped: 1` and sends no wake. ## What Changed - `server/src/services/recovery/service.ts` (`wasBlockedByAssigneeAgent`): a native block with reason `durable_dependency_blocker_bound` is not a self-block. The dependency owns its wake. Other native blocks and agent blocks keep the self-block rule. - The same function counts a later succeeded `workspace_finalize` with `owningService: workspace_recovery` and no workspace as the blocker settling, which matches the readiness rule from #20728. - `server/src/__tests__/heartbeat-issue-liveness-escalation.test.ts`: regression tests for the refused-completion shape (one wake, no second wake), the recovery finalize without a workspace, and a safety case for a different native block reason. - `server/src/services/workspace-recovery-dependency-finalize.test.ts`: a characterization test that a real "Recover saved files" signals the dependency sweep for the company. This path already existed; the test keeps it. - `doc/execution-semantics.md`: documents when the dependency sweep wakes after the last blocker clears. ## Verification - Red (unfixed `service.ts`): `npx vitest run src/__tests__/heartbeat-issue-liveness-escalation.test.ts -t "refused for unresolved|still holds a native block|recovery finalize without a workspace"` fails 2 of 3 tests with `selfBlockedSkipped` 1 instead of 0. - Green (fixed): `src/__tests__/heartbeat-issue-liveness-escalation.test.ts` passes 44 of 44. - Related suites pass: `src/__tests__/issue-dependency-wakeups-routes.test.ts`, `src/__tests__/heartbeat-dependency-scheduling.test.ts`, `src/services/workspace-recovery-dependency-finalize.test.ts`, `src/__tests__/issue-recovery-dependencies.test.ts` (43 tests). Server `tsc --noEmit` passes. - `pnpm verify` exits 2 because a doc change is a global CI input. Required CI supplies broad coverage. - Staging evidence (read only): the affected parent's activity shows `native-status-committer` set it `blocked` with `durable_dependency_blocker_bound`, and no later blocker completion or finalize. ## Risks - Previous-release story: no persisted shape changes. Tasks already `blocked` with `durable_dependency_blocker_bound` and ready dependencies are picked up by the existing dependency sweep after deploy (it visits every blocked task at startup and on recovery signals). No migration. - Bounded: the wake uses the cycle-aware ready-state key, so one blocked cycle gets one wake. The sweep keeps its live-run, queued-wake, pending-interaction, and pause-hold checks. - No loop: if the woken agent blocks the task again while its dependencies are ready, the reason is not `durable_dependency_blocker_bound`, so the self-block rule applies. - The recovery finalize rule only applies to a finalize on the blocker task itself with `owningService: workspace_recovery` and no workspace. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, code search, test execution, read-only staging API inspection). ## 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> |
||
|
DottaandClaude Opus 5.5
|
f125b823f4 |
fix(recovery): release a parent's Retry once a done child's recovered files are saved (#20728)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A parent task waits for its done child tasks to save their workspace files before it runs again (the workspace-finalize barrier) > - "Recover saved files" saves the retained work of a done child and records a successful `workspace_finalize` > - That finalize row had no `execution_workspace_id`. The barrier reads finalize rows by the blocker's workspace, so it never saw the row > - The parent's saved Retry then waited forever with "Saving its files" on a child that had already saved its files > - This pull request lets the barrier accept the recovery's own finalize for the recovered task's finished operations of the same run. It does not make the recovery finalize a workspace-wide save > - The benefit is that recoveries work by themselves: the parent's dependency wait releases and its saved Retry starts on the next dependency pass, with no migration ## Linked Issues or Issue Description **What happened?** A parent task had three done children. One legacy child shared the parent's execution workspace. The operator pressed "Recover saved files" on that child. The recovery saved the files and marked the old run succeeded. The operator then pressed Retry on the parent. The server accepted the Retry, but the parent stayed on "Retry is waiting for a finished task this one depends on to save its files" for more than 15 minutes. `blockerAttention.pendingFinalizeBlockerIssueIds` named the recovered child. The child had no active recovery action and no execution blocker. The workspace was clean (`dirty: false`, `writerRunId: null`). The child's workspace operations showed the cause. The latest operation on the workspace was a succeeded `workspace_recovery`. The recovery's succeeded `workspace_finalize` existed, but with `execution_workspace_id = NULL`. Earlier finalize attempts of that run had failed. `pendingFinalizeBlockerCondition` only looks at operations on the blocker's workspace, so it found no successful finalize after the recovery. **Expected behavior** When "Recover saved files" saves a done child's work, the child must stop holding its dependents. The parent's saved Retry must start without another operator action. **Steps to reproduce** 1. Let a child task finish as `done` with a retained remote workspace whose finalize failed. 2. Run "Recover saved files" on the child, so recovery saves the remote workspace snapshot. 3. Block a parent task on the child. The parent can share the child's workspace. 4. Read the parent's dependency readiness. Before this fix it reports the child in `pendingFinalizeBlockerIssueIds` forever. **Paperclip version or commit** Seen on staging at `a46b9ac7`. Reproduced on `master` at `e039be9202`. ## What Changed - `server/src/services/issues.ts` (`pendingFinalizeBlockerCondition`): the blocker task's latest workspace operation is settled when a succeeded recovery finalize (`owningService: workspace_recovery`) from the same company, task, and run started at or after that operation finished. The operation must be finished (`succeeded`, `failed`, or `skipped`). The recovery finalize can have no workspace (all rows written so far) or this workspace. - The rule is scoped to the blocker task and its run. It does not change the workspace-wide `finalized_at`, so other tasks that share the workspace keep their own barrier. - `server/src/services/workspace-recovery-dependency-finalize.test.ts`: new regression tests that run the real remote "Recover saved files" path, make the child `done`, and block a parent that shares the child's workspace. - `doc/execution-semantics.md`: documents how the recovery finalize settles the dependency barrier. ## Verification - Red (unfixed `issues.ts`): `npx vitest run src/services/workspace-recovery-dependency-finalize.test.ts` fails 3 of 4 tests. The parent stays not ready with the recovered child in `pendingFinalizeBlockerIssueIds`. - Green (fixed): the same file passes 4 of 4 tests: - a real recovery releases the parent in `getDependencyReadiness` and in the issue list `blockerAttention`; - snapshot-fallback shape: the preservation receipt has no workspace and the child's latest operation is the run's failed finalize. The parent is released; - safety: a sibling task that started work on the shared workspace while the recovery waited for teardown still holds the parent; - safety: child work that starts after the recovery still holds the parent. - Focused suites pass: `src/__tests__/issues-service.test.ts`, `src/services/workspace-recovery-admission.test.ts`, `src/services/task-retry.test.ts`, `src/services/saved-bootstrap-continuation.test.ts`, `src/services/remote-workspace-recovery-receipt-fallback.test.ts`, `src/__tests__/issue-dependency-wakeups-routes.test.ts` (414 tests with the new file). - `pnpm verify` exits 2 (a doc change is a global CI input), so required CI supplies broad coverage. Server `tsc --noEmit` passes. ## Risks - Previous-release story: no persisted shape changes. Recovery finalize rows already saved (with no workspace) are read by the new rule. Affected parents become dependency-ready on the next read, with no migration or backfill. The deferred-receipt and dependency backstop passes then start the saved Retry. - The barrier keeps its protection. Unfinished operations, operations from another run, operations that start after the recovery finalize, and other tasks on a shared workspace still wait for their own save. - No schema change and no write-path change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, code search, test execution, read-only staging API inspection). ## 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> |
||
|
DottaandClaude
|
1ac8b6a711 |
fix(workspaces): stop re-saving a retained sandbox whose disk is still full (#20758)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a native run on a remote sandbox, Paperclip saves the sandbox workspace back to durable storage. When that save fails, saved-work recovery saves the files again from the retained sandbox, with no provider turn. > - A staging census of the last 48 hours (`doc/plans/2026-11-11-staging-failures-vs-chaos-coverage.md`, class F) found saves that failed because the sandbox disk was full. > - #20678 already ends finalization at the first full-disk copy-back and hands the work to saved-work recovery. Recovery then deletes inactive tool caches that it can prove safe, and saves again. > - When that save still found the disk full, recovery recorded a transient `export_interrupted` failure with cause `provider_error`. It then saved again on its automatic schedule (up to 5 retries in 30 minutes). Nothing frees space in a stopped, retained sandbox between those attempts, so each attempt failed the same way. The task showed "Recovering saved work…" and no Retry while this went on. > - This pull request ends the recovery generation at the first such failure with a named cause, `sandbox_disk_full`, and hands it to the board with a Retry. > - The benefit is that the task does not spend 30 minutes on saves that cannot succeed, the user sees why, and one Retry saves the files and starts one run. ## Linked Issues or Issue Description Refs #20678, #20695 (named sandbox disk-full failures for copy-back and restore) **What happened?** A native run on a remote sandbox finished, and its workspace copy-back hit a full sandbox disk. Finalization handed the task to saved-work recovery (#20678). Recovery's own save also hit the full disk. Recovery classified this as `export_interrupted` / `retry_transient` with cause `provider_error`, so it scheduled more automatic saves of the same sandbox. Each one failed the same way. The task showed "Recovering saved work…" with no Retry until the 30-minute window ran out. **Expected behavior** A save that still finds the disk full after the safe cache cleanup is not repeated automatically. The recovery ends at once with the cause `sandbox_disk_full`, and the board gets a Retry. The workspace keeps its previous saved version, and the run's unsaved files stay in the retained sandbox. One Retry saves the files once there is room and starts exactly one run. **Steps to reproduce** 1. Give a native run an accepted result whose copy-back failed (`native_workspace_sync_out_retry_exhausted`). 2. Make every save from its retained sandbox fail with `remote_workspace_sandbox_disk_full`, with no inactive tool cache to delete. 3. Run saved-work recovery. Before this change, it records `export_interrupted` with cause `provider_error` and schedules another save. ## What Changed - `workspace-recovery.ts`: a save that fails with `remote_workspace_sandbox_disk_full` after a confirmed cache cleanup (or with nothing to clean up) now fails the generation with `export_interrupted`, retry class `user_action_required`, and cause `{ label: "sandbox_disk_full", code: "ENOSPC" }`. That class is not retried automatically, and it is not a permanent-source class, so recovery never falls back to the last save and never drops the run's unsaved files. The next sweep hands the hold to the board. - A cleanup whose outcome is unknown (lost acknowledgement, interrupted journal) keeps the existing scheduled retry, because its journal resumes on the next attempt and can still free space. - `retained-provider-cache.ts`: `reclaimInactiveNativeProviderCache` returns `{ changed, confirmed }` instead of a boolean, so the caller can tell "nothing to delete" from "outcome unknown". It has one caller. - `system-recovery-state.ts`: the board handoff text for this cause says that the sandbox disk is full, that Paperclip deletes only inactive tool caches it can prove safe, and that Retry saves the files again. - New `workspace-recovery-disk-full.test.ts` covers the whole path: one failed attempt, no further automatic saves, the board handoff text, the previous saved version unchanged, the agent's files still in the sandbox, no unfinished in-sandbox capture attempt, a Retry offered in the task presentation and blocker, `POST /issues/:id/retry` accepted (202), the files saved after Retry, and exactly one run started (also with a replayed dispatch). - `doc/execution-semantics.md`: documents the behavior. ## Verification - Red on master: `cd server && npx vitest run src/services/workspace-recovery-disk-full.test.ts` fails. The recovery state is `scheduled` with cause `provider_error` instead of `exhausted` with cause `sandbox_disk_full`. - Green on this branch: the same command passes. - Related suites pass: `workspace-recovery-retry-copyback.test.ts` (79 tests, including the 8 cache-cleanup resume tests that need the scheduled retry after an unconfirmed cleanup), `system-recovery-state.test.ts`, `workspace-recovery-capture-policy.test.ts`, `workspace-recovery-fallback.test.ts`, `workspace-recovery-stop-proof.test.ts`, `native-run-finalizer-telemetry.test.ts` (167 tests in all). - `npx tsc --noEmit -p server/tsconfig.json` is clean. - `pnpm verify`: exit 2 (broader verification required), because `doc/execution-semantics.md` is a global CI input. The focused checks above pass. Required CI supplies the broad coverage. ## Risks - Low. The change touches only the saved-work recovery path when its save fails with the exact full-disk message. - Upgrade-safe: the failure code (`export_interrupted`), the retry class (`user_action_required`) and the cause label all pass the earlier release's recovery schema. An earlier release that reads this state shows its generic "Retry to try again" text. - A full-disk failure no longer gets automatic retries in its 30-minute window. If space became free by itself in that window, the user must now press Retry. Nothing in a stopped, retained sandbox frees space by itself, so this is expected to be rare. - No Runner (Rust) change, so no new sandbox image is needed. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file editing, test execution) and extended reasoning. ## 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 <noreply@anthropic.com> |
||
|
DottaandClaude
|
ea25af759e |
fix(native): name a resume that finds no saved Runner state, so Retry starts a fresh session (#20757)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run can continue the provider session of the previous run on the same task. In a remote sandbox, the Runner keeps the session state inside the sandbox, and the server saves a harness backup after each run. > - On staging, five runs in 48 hours failed with `native_session_recovery_failed: Error: PRP provider resume state is unavailable` (class P in `doc/plans/2026-11-11-staging-failures-vs-chaos-coverage.md`). > - In each case we read, the previous run had an accepted result, but its `harness_state.backup.persist` failed. Its per-turn sandbox was then destroyed. The next run started in a new sandbox with no Runner state for the session. > - The Runner transport threw an untyped error. The runtime could not tell it apart from a transport drop. A governed-wait or same-session checkpoint ended as a generic `native_session_interrupted` with an "ambiguous state" hold that said not to open a replacement session. > - This pull request types that refusal and lets the driver classify it as `provider_state_incompatible` when nothing is in flight. The existing recovery policy then decides. > - The benefit is that the run ends with the permanent named code `native_provider_state_incompatible`. Retry, or the next message, continues in a fresh provider session in the same workspace. No provider turn or tool call can be repeated, because the follow-up never started a turn. ## Linked Issues or Issue Description Refs #20712, #20723 (the same classification for a moved remote workspace root). Refs #20691 (fixes the `provider_identity_incomplete` part of class P). **What happened?** 1. Run A on a chat task finished. Its result was accepted. Its harness backup failed to save (`harness_state.backup.persist` failed, `native.post_result_failure`). 2. Run B started for the next chat message in a new sandbox. It rebound run A's checkpoint with `providerRecoveryPolicy: allow_replacement_after_governed_wait`. 3. The Runner found no control-plane state and no archived authority epoch. It threw `PRP provider resume state is unavailable` as a plain `Error`. 4. The runtime refused a replacement for the governed-wait policy and failed run B with the generic `native_session_recovery_failed`. The task stayed blocked with an "ambiguous state" hold. **Expected behavior** The run ends with a permanent named code and a working Retry, as it does for other deterministic refusals. **Steps to reproduce** 1. Create a Runner transport that resumes a saved provider session from an empty state directory. 2. Request `thread/read`. On `master` it rejects with a plain `Error`. 3. Recover a saved Codex, OpenCode or ACPX session through the driver when the transport fails that way. On `master` the result has no `failureClass`. **Paperclip version or commit** `master` at `9574065a28`. ## What Changed - `packages/paperclip-runner/src/contracts/harness-driver.ts`: add `NativeRunnerResumeStateUnavailableError` (same message as before) and `isRunnerResumeStateUnavailable`. - `packages/paperclip-runner/src/live/runnerd-codex-transport.ts`: throw the typed error at the three branches where the control-plane state is absent. The branch with no PRP identity and the quarantine branch are unchanged. - `packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts`: classify the typed error as `provider_state_incompatible` when the saved session has no turn or runtime request in flight. Extract the in-flight test that the moved-root check already used, without changing it. - `doc/execution-semantics.md`: describe the new classification (additive). - Tests: a transport test (local and external Runner state), driver tests for Codex, OpenCode and ACPX Claude, in-flight cases that stay unclassified, and a server case that shows the blocked task offers Retry and that one click starts exactly one fresh-session run. ## Verification - `cd packages/paperclip-runner && npx vitest run src/backends/native-backend-factory.test.ts -t "no saved state"`: 5 pass. With the driver change reverted, the 3 "reports … as provider state incompatible" cases fail (no `failureClass`). - `cd packages/paperclip-runner && npx vitest run src/live/runnerd-codex-transport-cold-restore.test.ts`: 5 pass. On `master` the error is a plain `Error`. - `cd packages/paperclip-runner && npx vitest run src/backends/native-backend-factory.test.ts src/drivers/codex/codex-app-server-driver.recovery.test.ts src/live/runnerd-codex-transport-cold-restore.test.ts src/native-session-runtime.test.ts`: 410 pass. - `cd server && npx vitest run src/services/workspace-recovery-retry-copyback.test.ts -t "does not continue automatically"`: 5 pass, including the new case. It checks `canRetry: true`, an accepted Retry, exactly one queued successor, and `forceFreshSession: true`. - `cd packages/paperclip-runner && npx tsc --noEmit -p .`: clean. - `pnpm verify`: exit 2 (broader verification required) because `doc/execution-semantics.md` is a global CI input. The focused checks above pass. Required CI supplies the broad coverage. ## Risks - Low. The change only adds a class to a failure that already failed. A checkpoint with `allow_replacement_after_resume_failure` already replaced the session for this failure, and still does. - For `same_session_only` and `allow_replacement_after_governed_wait`, the run now ends with `native_provider_state_incompatible` instead of a retryable `native_session_interrupted`. Same-run retries stop at once. They could not succeed, because the state is absent. - A session with a turn or runtime request in flight keeps the old unclassified behavior. This PR does not change it. - The server code path is unchanged. The Rust Runner is unchanged, so no new runner image is needed. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code, with tool use (shell, file edit, test execution) and read-only staging API reads. ## 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 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
1143abaf92 |
fix(recovery): give due exhausted holds their own fair lane in the recovery worker (#20725)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The workspace recovery worker saves the files of failed runs. It also re-arms exhausted holds under the rules of #20295, #20569, #20641 and #20648 > - After #20680 and #20661 were deployed to staging, the worker still re-armed none of 29 exhausted holds in 36 minutes. One hold's id sorts in the first percent of all candidates, so the restart starvation that #20680 fixed was not the only cause > - The worker skipped every obligation while any unfinished save was selectable. Discovery is instance-wide. A save that another server owns stays unfinished on every run, so obligations never ran. The obligation pass also put due holds behind hundreds of settled holds whose saved input waits, at two per run. A completed pass never revisited a hold that became due by time alone > - This pull request gives due exhausted holds their own lane, and it shares the worker's slots fairly between unfinished saves, due holds and the obligation pass > - The benefit is that every due hold is reached within a few runs of the worker, whatever its other load. The holds save their files and free their tasks without a person ## Linked Issues or Issue Description Refs #20680 Refs #20661 Refs #20648 Refs #20641 Refs #20569 Refs #20295 **What happened?** On staging, 36 minutes after a deploy that included #20680 and #20661, the workspace recovery worker had touched none of the 29 exhausted holds of the company (22 capture-policy holds and 7 stop-proof or fallback holds). They had no `stopProofRecovery` or `capturePolicyRecovery` marker, no re-arm activity, and `updatedAt` from Nov 2 to Nov 10. The worker did run manual "Recover saved files" requests: one stop-proof hold saved its files 57 seconds after the request. So the worker was alive and the stop proof was available. The holds were not reached. **What did you expect to happen?** The worker reaches each exhausted hold whose re-arm or fallback is due, re-arms it, and saves the files, within a bounded time. **Steps to reproduce** 1. Create a scheduled save whose run's finalization lock another database session holds (another server owns the run). 2. Create an exhausted stop-proof hold and an exhausted capture-policy hold, in the evidence shape that the Nov 4 and Nov 10 releases saved. 3. Start the real workspace recovery worker. 4. The worker retries the busy save on every run and never re-arms either hold. ## What Changed - `server/src/services/workspace-recovery.ts`: `reconcileRetainedWorkspaces` draws candidates from lanes. Each lane is a page that its own cursor advances: unfinished saves; for the worker, exhausted holds whose re-arm or fallback is due; and the obligation pass. Before, the obligation pass ran only when no unfinished save was selectable (the gate at `const unfinished = rows.length > 0; if (!rows.length) { ... }`). - Each lane with work takes one free slot before any lane takes a second. The lane that goes first rotates on each run, so with one free slot (a long local export holds the other) each lane still gets it in one run of three. Actions of a run that one lane took join that run's job without another slot. - The due lane wraps at its tail. A hold that becomes due by time alone (a lease expiry or a re-arm backoff) is reached after the obligation pass completed with no committed change. - `retainedWorkspaceCandidates` takes a `dueOnly` filter (due re-arm or fallback of an exhausted hold). The obligation filter uses the same condition. One `now` is used for the whole query. - The startup sweep (no worker sweep state) also shares its slots between unfinished saves and its general pass. - `doc/execution-semantics.md`: document the lanes and the fairness rule. - New `workspace-recovery-passive-fairness.test.ts` with four real-Postgres regressions. ## Verification - Red on `origin/master` ( |
||
|
DottaandClaude
|
08ea18085e |
fix(workspaces): restore a saved workspace into one copy of sandbox disk (#20695)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A task on a remote (Daytona) sandbox continues from its saved workspace. Before the agent starts, the server uploads the saved archive into the sandbox and a sandbox script extracts it. > - The extraction wrote a full second copy of the workspace beside the uploaded archive. A 7.5 GB workspace on a fresh sandbox then needed about 15 GB, and the restore failed with `stage=extract code=ENOSPC disk=full`. > - The failure was classified as retryable, so every automatic retry and every Retry repeated it on a same-size sandbox. The task could not continue. > - On a retained sandbox it is worse: the old root, the archive, and the new tree all share the disk. > - This pull request makes the restore need about one copy of the workspace. The sandbox removes only data that control-plane storage already holds durably, and it never removes a root whose save the server did not prove. > - The benefit is that a workspace that fits once on the sandbox disk restores. A disk that is too small for one copy now gets a named failure that does not loop. ## Linked Issues or Issue Description **What happened?** A saved remote workspace of about 7.5 GB could not be restored on a new Daytona sandbox. The run failed with `remote_workspace_restore_failed: step=sandbox_restore; remote_workspace_operation_failed: operation=restore exit=1; remote_workspace_restore_failed: stage=extract code=ENOSPC disk=full exit=1`. The run marked the failure as retryable, and each retry failed the same way. **Steps to reproduce** 1. Save a remote workspace whose archive is larger than half of the free sandbox disk. 2. Start the next run on another sandbox, so that the server restores the saved archive. 3. The upload succeeds. The extraction writes the second copy and fails with ENOSPC. **Expected behavior** A workspace that fits once on the sandbox disk restores. A disk that cannot hold one copy gives a specific failure that is not retried automatically, and Retry works after the sandbox has more space. **What the disk holds during a restore (from the code)** - The uploaded archive (`<parent>/.paperclip-runtime/workspace-snapshots/<operation>/workspace.pcws`, about the workspace size, because the archive is not compressed). - The extracted tree (`<root>.restoring-*`, the workspace size). - On a retained sandbox only: the old root, which stayed until after the new root was published. - After a restore that died: its partial tree and its archive, which no later attempt removed. Refs #20678 (the save side of the same full-disk class). ## What Changed - `workspaceArchive` restore has a `consumeArchive` mode. It creates the directories and symlinks first, then writes the file bodies from the last to the first. After it verifies each file against the manifest hash, it truncates the archive behind that file. The archive and the tree together need the workspace size plus the largest file. The default mode, which the server uses for local restores, does not change. - The sandbox restore script uses that mode on a private rename of the uploaded archive. A later upload to the same path cannot change the file that is being consumed. The script removes the consumed file when it stops. - The helper's own staging is now inside the root-specific `<root>.restoring-*` folder, not in a shared `.workspace-restore-*` sibling. - While it holds the root's restore lock, and after the epoch fence, the script removes leftovers of an earlier attempt on the same root that died: `<root>.restoring-*` folders, `.consuming` archives in its own staging, and archive staging folders that a `restore-target.json` binds to the same root. Each restore writes that binding after the reclaim step. - When the server proved that a retained root is its own earlier, fully saved allocation (`replaceRetiredRoot` and a Paperclip receipt, as before), the script removes the root's files before the extraction, not after the publish. The receipt and `.paperclip-runtime` stay, so an interrupted restore proves the same root again and continues. - A full disk at any restore stage reports `reason=sandbox_disk_full`. The existing named-refusal rule makes it non-retryable, so it goes to the board recovery action and does not loop. - `doc/execution-semantics.md` describes the disk need, the reclaim, the early removal, and the new reason. ## Verification - `cd server && npx vitest run src/services/remote-workspace-transport.test.ts src/services/remote-workspace-snapshot-archive.test.ts` (287 passed). - `cd server && npx vitest run src/services/remote-workspaces.integration.test.ts src/services/remote-workspace-lifecycle.test.ts src/services/remote-workspace-archive-worker.test.ts src/services/remote-workspace-snapshot-archive-environments.test.ts` (122 passed, 16 skipped). - `cd server && npx tsc --noEmit -p tsconfig.json` (pass). - The new tests run the real sandbox scripts against a folder with a byte capacity. A `--require` preload makes each file write fail with ENOSPC when the folder would exceed the capacity. - Red on `origin/master`, green with this change: - Fresh sandbox, room for the archive plus one file: master fails with the exact production error `stage=extract code=ENOSPC disk=full exit=1`. The fix restores every byte and leaves only the restored files. - Retained root that the server proved, room for the old root, the archive, and one file: master fails with ENOSPC. The fix restores and keeps the runner state. - Too small for one copy: the failure is `stage=extract reason=sandbox_disk_full`, with no partial tree or archive left. A retry with more space then restores. - Both sandbox processes are killed mid-extraction (a restart): a retry with the same identity and a retry with a new identity and epoch both reclaim the leftovers and restore into the same capacity. - The command dies right after it removed a proven root's files: the next restore proves the root again from its kept receipt, restores, and keeps the runner state. - Integration test through `prepareRemoteWorkspace`: a full disk gives `retryable: false` with `reason=sandbox_disk_full`. On master the same case is `retryable: true`. - Guards (pass on master and with the fix): a retained root without the server proof, or without a Paperclip receipt, is refused with `reason=occupied_root`, and none of its files are removed. - Previous-release story: the archive format and the saved state do not change. A partial `<root>.restoring-*` tree from the previous release is reclaimed (test). Archive staging from the previous release has no binding, so it is kept (test). The previous release's shared `.workspace-restore-*` siblings cannot be bound to a root, so they are kept. ## Risks - The consumed archive cannot be read again. Each restore uploads a new copy from control-plane storage, and the early-return path for an already restored root does not read the archive. Every file is still checked against the manifest, and the manifest is checked against the saved digest. - The reclaim step removes data only under the root's restore lock and after the epoch fence. It removes only names that a restore of the same root creates. Staging that names another root, or no root, is kept. - The early removal of a retained root depends on the existing server proof (`isRetiredOwnAllocation`) and the receipt check. If a restore fails after the removal, the saved version in storage is the source of truth, and the next restore proves the same root again. - Files in a lower overlay layer of a resumed sandbox image do not free space when they are removed. Such a sandbox can still be full. - A workspace that is larger than the free disk after the upload, plus its largest file, still fails. It now fails with the named, non-retryable `reason=sandbox_disk_full`. - No change to `packages/paperclip-runner`, the database schema, or the API. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude Code with tool use (shell, file edits, test runs). ## 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 <noreply@anthropic.com> |
||
|
DottaandClaude
|
4baff83225 |
fix(chat): never batch messages of two chat conversations into one wake (#20697)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Chat channels let a linked user talk to an agent from GitHub, Slack and other platforms. Each external thread is a chat conversation. Many conversations can share one task. > - A reviewed chat run attests exactly one conversation. #17054 keeps each conversation in its own wake batch, but it compares only the conversation that a wake records. > - The work-intent outbox saves unanswered user messages as a plain comment wake. That wake records no conversation, even when its messages are chat messages. A wake with no recorded conversation matched every conversation, so a message from another conversation merged into it. > - The promoted run held messages of two conversations. The binding check correctly refused it (`reviewed_chat_execution_binding_not_authorized`, `binding_denied`), and the task needed Retry. > - This pull request compares the conversations of the queued messages themselves. Wakes of different conversations never merge. Each conversation gets its own run, in order. > - The benefit is that replies in several threads of one task each get an attested run, without a failed run or a Retry. ## Linked Issues or Issue Description Refs #17054 (the earlier per-conversation batching fix). Refs #20689 (a different shape of the same error). **What happened?** On staging, a GitHub pull-request review task had two runs fail with `setup_failed` / `reviewed_chat_execution_binding_not_authorized` (`reviewedChatBinding.denialReason: binding_denied`). Each failed run held the same two older saved messages plus one new message. The two runs recorded two different `chatConversationId` values. The work-intent outbox had saved the older messages as plain deferred wakes with no conversation. A new message from each conversation then merged into one of these saved wakes. **Steps to reproduce** 1. Bind two chat conversations to one `in_review` task. 2. Let the outbox save an unanswered message of conversation B as a deferred wake while a run is active. 3. As the linked user, send a message in conversation A while the run is still active. 4. See that the message of A merges into the saved wake of B. The promoted run fails the binding check. **Expected behavior** The saved message of B runs as its own wake. The message of A runs after it, in its own attested run. ## What Changed - New `server/src/modules/wake-queue/adapters/chat-conversation-batch.ts`: reads the inbound chat links (`chat_message_links`, direction `inbound`) of queued messages and finds messages of a conversation other than the expected one. - Deferred-wake lookup (`findExistingDeferredWake`): when the incoming wake has a conversation, a saved wake qualifies only if none of its queued messages is an inbound message of another conversation. This is one more predicate in the same query (no new round trip). It uses the existing `chat_message_links_company_comment_idx` index. - Coalescing into the active run (`coalesceIntoActiveExecutionRun`): returns no merge when the merged batch would hold messages of a conversation other than the one it records. The wake is then saved behind the run, which is the existing fallback. - Release drain (`findNextDeferredWake`), upgrade repair: a saved chat wake that older code already merged across two conversations is split. The other conversation's messages move to a plain saved wake (the outbox shape), ordered just before it. The original wake keeps its conversation and its own messages. The drain then reads the queue again. - `doc/execution-semantics.md`: describe the comparison and the upgrade repair. - New test `chat-wake-conversation-batch.test.ts` (embedded Postgres, real admission and release code, real `authorizeChatConversationForBoundRun`). ## Verification - Red on `origin/master` (test file only): `pnpm --dir server exec vitest run src/modules/wake-queue/adapters/chat-wake-conversation-batch.test.ts` gives 4 failed, 6 passed. The 4 failures are the bug: the new message merges into the other conversation's saved wake (`[[earlier, reply]]`), two conversations cross-merge, a message coalesces into a queued run of another conversation, and the old merged shape stays merged. The 6 passing cases pin unchanged batching. - Green with the fix: 10 of 10 pass. Each separated wake passes `authorizeChatConversationForBoundRun` for its own conversation. The old merged shape fails it with `paperclip_runner_chat_attachment_binding_denied` before the split. - Batching that stays the same (pinned, pass before and after): comment wakes with no conversation merge; messages of one conversation merge; an outbox-saved wake of the same conversation still absorbs the reply; a chat message and plain comments merge in both orders; same-conversation messages still coalesce into a queued run; two recorded conversations stay apart. - Related suites pass locally: `src/modules/wake-queue`, `src/modules/run-dispatch`, `chat-run-milestone-work`, `durable-chat-wakeup`, `heartbeat-comment-wake-batching`, `heartbeat-reviewed-chat-binding.integration`, `slack-conversation-lifecycle`, `queued-comment-consumption`, `issue-queued-comments-routes` (23 files, 1037 tests). `heartbeat-process-recovery`: see the comment below. - `tsc --noEmit -p server/tsconfig.json` passes. ## Risks - Low risk. No schema change and no migration. - Behavior change only for chat messages: a message no longer joins a saved wake or a run that holds another conversation's inbound messages. It gets its own wake behind them, and it runs after them. Before this change, that batch always failed the binding check. - Board comments have no inbound chat link, so they belong to no conversation. Wakes without a conversation, wakes of one conversation, and mixed board-comment and chat wakes batch exactly as before. - Ordering: the deferred queue still promotes one wake per release, oldest first. The second conversation waits for the issue execution lock. Single-assignee and atomic checkout are unchanged. - Upgrade: a saved wake that older code merged across conversations is split at promotion. No message is dropped or delivered twice: each message id moves to exactly one wake. A run that older code already promoted in the merged shape is not changed; it fails the binding check as before and keeps its existing Retry path. - No wait is added before a turn starts. Admission adds one predicate to an existing query. Coalescing adds one indexed read only when the merged batch has a chat conversation. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), in Claude Code with tool use (shell, file edits, GitHub CLI). ## 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 <noreply@anthropic.com> |
||
|
DottaandClaude
|
a46b9ac724 |
fix(native): wait out an ACP usage limit's reported reset, then Retry the task (#20692)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs (Paperclip Runner with an ACP harness, such as Claude)
can stop because the provider account runs out of its usage allowance
> - The ACP harness reports this as a "terminal limit failure".
Paperclip blocked the task behind a board hold that called the session
state ambiguous, and the only way forward was a manual Retry
> - The provider also says when the limit resets, for example "resets
Nov 12, 6pm (UTC)" for a weekly limit. Paperclip read only clock-only
resets ("resets 2am"), so the agent-level wait fell back to one hour.
Each later wake for the agent then hit the limit again and blocked
another task
> - This pull request reads calendar resets, says on the hold when the
limit resets, and requests the task's Retry itself at that time when the
reset is reported and bounded
> - The benefit is that a usage limit no longer needs a person to click
Retry, and the agent's other tasks wait instead of failing one by one
## Linked Issues or Issue Description
**What happened?**
A native ACP run failed with `native_provider_usage_limit` ("ACP agent
reported a terminal limit failure. You've hit your weekly limit · resets
Nov 12, 6pm (UTC)"). The task went to `blocked` with a board-owned hold.
The hold said "Inspect the original provider failure and explicitly
resolve the ambiguous session state". Nothing resumed the task after the
reset. The agent-level `provider_limit_wait` recorded "no reset time was
reported, so Paperclip waits one hour", so every hour another wake
dispatched, hit the limit, and blocked another task. In one 48-hour
window on a staging instance, this blocked 7 tasks for one agent.
**What did you expect?**
When the provider reports a bounded reset, the task waits and continues
after the reset, like the existing `provider_limit_wait` path. When the
reset is unknown, the task fails with a clear message and a working
Retry.
**Steps to reproduce**
1. Run a native ACP agent until its provider reports a weekly or session
limit with a reset time.
2. Look at the task: it is blocked with an "ambiguous session state"
hold.
3. Wait past the reported reset: the task stays blocked until a person
clicks Retry.
Refs #20666 (other recovery work in the same staging census; not changed
here)
## What Changed
- `provider-quota-reset.ts`: parse calendar resets such as "resets Nov
12, 6pm (UTC)", "resets Jul 17th at 4:30 PM (America/Chicago)" and
"Resets on August 2, 9am (UTC)". A past date rolls into next year. An
impossible date (Nov 31) is refused. Clock-only parsing is unchanged.
This also fixes the agent-level `provider_limit_wait` reset and the
legacy `provider_quota` reset for these messages.
- New `native-runtime/provider-limit-resume.ts`:
- `readNativeUsageLimitReset` reads the reset only from the run's own
durable events. The last turn event must be a provider-observed
`turn.failed` with a `rate_limit` or `quota_exhausted` provider failure,
and the reset must be at most eight days out.
- `requestProviderLimitResumeRetry` asks for the task's Retry
(`requestTaskRetry`, `initiatedBy: "provider_limit_reset"`) on behalf of
the user who requested the run, or the run's accountable user for
automatic wakes. It is idempotent per run and records its outcome on the
hold. At most three automatic Retries follow one another without a
successful run.
- `registerProviderLimitResumeWork` is a scoped deadline owner (shared
work scheduler, no polling). Each active usage-limit hold is due at its
reset. Startup rebuilds it from the holds, and the executor notifies it
in the hold's transaction.
- `native-session-executor.ts`: for a terminal usage limit, the hold and
run failure detail say when the limit resets and whether Paperclip
retries then, instead of "ambiguous session state". The evidence records
`providerLimitResetAt`.
- `task-retry.ts`: accept `initiatedBy: "provider_limit_reset"`. Its
activity is recorded as system actor `provider-limit-reset` on behalf of
the user, like `restart_recovery`.
- `doc/execution-semantics.md`: document the wait and its limits.
Previous-release story: holds saved by the previous release carry
forward. At startup the owner reads each active
`native_provider_usage_limit` hold. It rewrites the hold's message with
the reset when the reset is known and still ahead, and it requests the
Retry at the reset. Holds whose reset is unknown, more than a day in the
past, or that have no accountable user keep their Retry and are recorded
as not requested, so they are not read again.
## Verification
- `npx vitest run server/src/services/provider-quota-reset.test.ts` — 4
tests. Three fail on `origin/master` code (calendar resets return
`null`), all pass with this change.
- `npx vitest run
server/src/services/native-runtime/native-provider-capacity.integration.test.ts`
— 5 tests on embedded Postgres:
- the existing usage-terminal test (both replay modes) still blocks with
a Retry, and no automatic Retry is requested without a reported reset;
- "waits for a reported usage-limit reset and then requests the task's
Retry once": the hold says when the limit resets, an old-release hold is
rewritten, nothing starts before the reset, one Retry receipt is created
for the accountable user at the reset, and a second pass does nothing.
Fails on `origin/master` executor code;
- "starts the Retry from the hold's own deadline": a real delivery
coordinator rebuilds the deadline from the hold and calls Retry dispatch
once, after the reset;
- "leaves a visible Retry when no user is accountable for the run".
Fails on `origin/master` executor code.
- Focused regression suites pass (642 tests): `task-retry`,
`quarantined-task-retry`, `heartbeat-native-restart-suspension`,
`native-startup-retry.integration`, `provider-failure-classification`,
`heartbeat-provider-limit-wait`, `deferred-execution-receipt-work`,
`chat-run-publications`, `native-runner-restart-recovery.integration`,
`native-remote-retry-admission`.
- `pnpm --filter @paperclipai/server exec tsc --noEmit` passes.
- `pnpm verify` selected a broad run (exit 2, because the docs file is a
global CI input). CI supplies the broad coverage.
## Risks
- The automatic Retry uses the accountable user's authority for
automatic (system) wakes. It is the same Retry the hold already offered
that user, and dispatch still checks Retry authority, stop proof, saved
work and action reconciliation. It is never broader than a click.
- A provider that keeps reporting resets that do not restore capacity
gets at most three automatic Retries in a row; then the Retry stays
visible.
- The deadline owner reads all active usage-limit holds at startup
(limit 500 rows). The number of such holds is small.
- Not changed: the agent-level wait still holds new wakes for at most
one day, as before.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with
tool use (shell, file edits, read-only staging API reads). Extended
reasoning enabled.
## 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 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
69c69e4c20 |
fix(recovery): finish the passive recovery pass before restarting it (#20680)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The workspace recovery worker saves the files of failed runs. One passive pass of the worker visits holds with obligations: saved input that waits, cleanup, and due re-arms and fallbacks of exhausted holds > - #17741 restarts that pass after each committed workspace-recovery change, so new work is found quickly > - On staging, changes commit continuously (stop receipts, new holds, recovery progress). Every pass restarted from the first candidates, which are settled holds whose saved input waits behind another hold, and it never reached later candidates > - After #20295, #20569 and #20661 were deployed, no due re-arm ran: five stop-proof holds and about twenty capture-policy holds kept their exhausted state, with no marker and no activity > - This pull request restarts the pass only after it reaches its tail, and it continues an unfinished pass on the next run > - The benefit is that every candidate is visited on a busy instance, so the exhausted holds re-arm, save their files and free their tasks without an administrator ## Linked Issues or Issue Description Refs #17741 Refs #20295 Refs #20569 Refs #20661 **What happened?** Exhausted workspace holds were not re-armed on staging, even after the fixes that add their re-arm were deployed. Their recovery actions stayed unchanged for days (`updatedAt` on Nov 5 or Nov 10). No `stopProofRecovery` or `capturePolicyRecovery` marker and no `issue.workspace_recovery_rearmed` activity was recorded. The worker did process new restore holds, which are selected by its unfinished pass. **What did you expect to happen?** The worker's passive pass reaches every hold with a due re-arm or fallback, re-arms it, and saves the files. **Steps to reproduce** 1. Create several settled holds with saved user input that waits, with action ids that sort before an exhausted stop-proof hold. 2. Start the real workspace recovery worker. 3. Commit a workspace-recovery notification every 50 ms, as stop receipts and new holds do on a busy instance. 4. The worker never re-arms the exhausted hold. Without the notifications, it re-arms the hold. ## What Changed - `workspace-recovery-work.ts`: a committed change marks the passive pass for restart (`restart`). It no longer resets the cursor. While a restart is pending, an unfinished pass that made progress continues on the next run. - `workspace-recovery.ts`: when the passive cursor passes the last candidate and a restart is pending, the same run starts a fresh pass from the first candidate, so a new change is not delayed by another run interval. Without a pending restart the pass records `complete` and does not wrap, so paused input does not keep the worker active. - `doc/execution-semantics.md`: document that a pass is never restarted midway. - `workspace-recovery-stop-proof.test.ts`: add a real-worker regression in the staging shape (six earlier settled holds with waiting user input, one exhausted stop-proof hold, continuous notifications). ## Verification - Red: on master, the new test "re-arms the hold on a busy instance whose passive sweep keeps selecting earlier settled holds" fails (`stopProofRecovery` stays undefined). Control: the same test without the notifications passes on master. This shows that the restarts cause the starvation. - Green: with the fix, the test passes. These suites pass: `workspace-recovery-stop-proof` (12), `execution-control-work` (27), which includes "visits a newly committed earlier Retry during a busy recovered-source pass, then becomes quiet", plus `workspace-recovery-capture-policy`, `workspace-recovery-preparation-dispatch`, `workspace-recovery-warm-handoff`, `workspace-recovery-preparation` and `server-startup-feedback-export` (279). The timing-sensitive `heartbeat-observed-workspace-preservation` suite passed 6 of 6 runs (an earlier revision that restarted only on the next run made it flaky). - Staging evidence (read-only GET requests): the 28 exhausted holds of the company have no re-arm marker and no fallback check. Many blocked tasks with settled holds have deferred wakes. ## Risks - A change committed during a pass is now picked up at the next pass, not at once. Unfinished physical work still uses its own pass first, so new recoveries are not delayed. Saved input in a passive obligation can wait up to one full pass. - Previous-release story: no persisted state changes. Holds saved by earlier releases are selected on the first full pass after deploy, with no migration. - Quiet behavior stays: a completed pass with no new change keeps the worker idle. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, code editing, browser GET reads) in Claude Code. ## 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> |
||
|
DottaandClaude
|
06af074452 |
fix(native): deliver the retry after a failed native turn, once per chain (#20681)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The native runtime decides a task's next status after each run, through the status arbiter and the status-decision committer > - When a native turn fails without a classified provider error, the arbiter keeps the task's status and asks for one retry (`schedule_retry`) > - The native-status wake dispatcher refused that retry wake, because its provenance check accepted only `enqueue_continuation` effects > - So the task stayed `in_progress` with no live run, no queued wake and no visible cause, until a person noticed it > - This pull request lets the dispatcher accept the committed retry effect, keeps the retry on its chat route, and limits the automatic continuation to one per chain > - The benefit is that a failed turn always ends with either queued work or a blocked task with a working Retry ## Linked Issues or Issue Description **What happened?** A native run that ends with a failed turn and no classified provider error leaves its task stranded. Examples: an OpenCode server that the sandbox killed with SIGKILL during a turn (`opencode_server_exited`), or an OpenCode turn that ended without an answer (`opencode_empty_response`). The run's accepted result is finalized as `run_failed_partial_evidence_preserved`. The committer queues a `schedule_retry` wake. The dispatcher then skips the wake with `chat_control_recovery_proof_unresolved`. The task stays `in_progress` with no run, no wake and no blocker. **Steps to reproduce** 1. Run a native task. Make the provider turn fail with `opencode_server_exited` or `opencode_empty_response` and an accepted `needs_review` result. 2. Finalize the run. 3. Run the native-status wake dispatcher. 4. See that the retry wake and its dispatch receipt are `skipped`, and that no run starts. **Expected behavior** The task gets its automatic continuation. If that continuation fails the same way, the task is blocked with a clear board blocker and a Retry that starts a new run. **Paperclip version or commit** `origin/master` at |
||
|
DottaandClaude
|
6e524d8a69 |
fix(workspaces): stop retrying a copy-back that hit a full sandbox disk (#20678)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a native run, Paperclip copies the agent's sandbox workspace back to durable storage. This is finalization. > - The copy-back builds an archive inside the sandbox. When the sandbox disk is full, the copy-back fails with `remote_workspace_sandbox_disk_full`. > - Finalization treated that failure as transient. It retried three times. Each attempt took about 15 minutes for a large workspace, and nothing freed space between attempts. > - Saved-work recovery already reclaims the retained sandbox's inactive provider cache when a save hits a full disk. It runs only after finalization ends. > - This pull request ends finalization on the first full-disk copy-back failure, with the accepted result kept, so saved-work recovery starts at once. > - The benefit is that the task gets its files saved sooner, and the user sees what happened instead of "failed after 3 attempts". ## Linked Issues or Issue Description Refs #19619 (related: records why the read-only capture fallback failed; this PR does not change that path) **What happened?** On staging, a native run finished its turn and its result was accepted. The workspace copy-back then failed three times with `remote_workspace_sandbox_disk_full` (about 7.5 GB of workspace, about 6 GB free). The three attempts spanned about one hour across deploys. Finalization then ended as `native_workspace_sync_out_retry_exhausted`. Saved-work recovery reclaimed the inactive provider cache (about 3 GB freed) and saved the files. **Expected behavior** A full sandbox disk is recognised on the first failure. Paperclip does not spend the finalization budget on attempts that cannot succeed. The agent's result is kept. The user sees that the disk is full and that Paperclip saves the files again without another provider turn. **Steps to reproduce** 1. Run a native task on a remote sandbox whose workspace is larger than the free sandbox space. 2. Let the provider turn finish. 3. Observe three copy-back attempts that each fail with `remote_workspace_sandbox_disk_full`. ## What Changed - `recordRetryableFailure` treats a workspace failure whose cause starts with `remote_workspace_sandbox_disk_full:` as an exhausted budget on the first attempt. The code stays `native_workspace_sync_out_retry_exhausted`, so the existing export hold, saved-work recovery, and export Retry apply unchanged. - The run error for that case says that the sandbox disk is full and that Paperclip saves the files again without repeating the agent's work. It no longer says "after 3 attempts". - Other copy-back failures keep their bounded retry. - `captureRemoteWorkspaceArchive` rethrows the read-only fallback's own failure when that fallback only lost its connection (`transport_closed`, `provider_unavailable`, `provider_rate_limited`, `sandbox_state_transition`). That failure is not disk-full, so finalization keeps its bounded retry for it. Other fallback failures keep the disk-full code. - `remoteWorkspaceFailureCause` records the server-built read-only capture failure, so a failed save names where the fallback stopped. - The transport exports `REMOTE_WORKSPACE_SANDBOX_DISK_FULL_CODE`. The finalizer and tests use it. - `doc/execution-semantics.md` describes the new rule. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-run-finalizer-telemetry.test.ts src/services/remote-workspace-transport.test.ts` (195 pass). - New test "hands a full sandbox disk to saved-work recovery on the first copy-back failure" fails on `origin/master` (the failure stays `retryable_failure`) and passes with this change. It completes an accepted result first and checks that the result stays linked and that no agent wakeup is created. - New transport test "keeps a full disk only when the read-only fallback did not just lose its connection": the two dropped-connection cases fail on `origin/master` and pass with this change. - New test "keeps other copy-back failures on their bounded retry" passes before and after. - `cd server && npx tsc --noEmit` passes. `pnpm verify` selected 2504 test files because the finalizer is widely imported; its first selected batch passed, and required CI supplies the full run. ## Risks - Low risk. The terminal state is the same state that three failed attempts produce today. Only the timing changes. - A sandbox whose disk frees space by itself between attempts now waits for saved-work recovery instead of the next finalizer attempt. Saved-work recovery repeats the save with its own bounded attempts, so this does not lose work. - Upgrade safety: a run saved by an older release in `retryable_failure` with this cause ends on its next full-disk failure. No migration is needed. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, file edits) in Claude Code. Extended reasoning was not configured. ## 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 <noreply@anthropic.com> |
||
|
DottaandClaude
|
cec903c6f0 |
fix(native): retire a finished remote owner whose gone Runner left a delivery unacknowledged (#20677)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native follow-up run on a remote sandbox first proves that the previous per-turn owner finished. Then it retires that owner's controller journal and starts a new Runner. > - The proof requires a settled journal. A command that the Runner did not acknowledge holds the owner. > - On staging, a per-turn owner committed its result and its Daytona sandbox was destroyed after the turn (`paperclip_runner_destroy_after_turn`, cleanup `success`). Its journal still held a `run.attach` or a `semantic_tool.result` that the Runner never acknowledged. > - The next run failed with `prior_owner_terminal_state_indeterminate; retirement_refused=journal_commands_unsettled(run.attach.pending)` (or `semantic_tool.result.pending`). A Retry or a new message gets the same journal, so it fails the same way. The task stays blocked. > - Retirement already excuses the Runner's own lifecycle commands when the prior sandbox lease ended on another sandbox. This pull request applies the same rule to deliveries that only that gone Runner could receive. > - The benefit is that these follow-ups start by themselves. Commands that carry user intent still hold the owner. ## Linked Issues or Issue Description **What happened?** A census of staging failures (48 h, Nov 9–10) found 6 failed runs with `prior_owner_terminal_state_indeterminate` and `retirement_refused=journal_commands_unsettled(run.attach.pending | semantic_tool.result.pending)`. The last one was at 18:56 UTC on Nov 10. In two examples that I checked: - The previous run succeeded. Its result was committed and its coordinator lease was released. - Its lease policy was `ephemeral`. The lease is `expired`, with `failureReason=paperclip_runner_destroy_after_turn` and `cleanupStatus=success`. - The follow-up got a new sandbox (a different provider lease) and refused in about 11 s. It started no Runner. The task became `blocked`, and the next action was "Send a message to continue". **Expected behavior** When the record proves that the previous Runner is gone, an undelivered attachment, tool answer or read cannot run. The follow-up retires the finished owner and starts a new Runner. Retirement replays no command. **Steps to reproduce** 1. Finish a per-turn remote native run whose controller journal keeps a `run.attach` or `semantic_tool.result` command in `pending`. 2. Let the sandbox lease of that run end (`expired` or `released`) on its own sandbox. 3. Start a follow-up run that gets a new sandbox. 4. Observe `retirement_refused=journal_commands_unsettled(run.attach.pending)`. **Paperclip version or commit** Staging builds of Nov 10. The refusal still reproduces on `master` at `48c6e996da` (the new tests fail there). ## What Changed - `native-session-executor.ts` (`readSettledRunnerdProjection`): when the prior run's sandbox leases ended on another sandbox (`remoteRunnerGone`, the existing proof), unfinished `run.attach`, `semantic_tool.result`, `session.snapshot` and `session.goal.get` commands no longer hold the retirement. These join the lifecycle commands that the rule already excused. - In the same case, a journaled `semantic_tool.result` answers its `semantic_tool.input`. The controller journals the answer after the tool ran. Only its delivery was outstanding. - No other proof changes. The prior run must still be terminal, with a committed result, a released coordinator lease, no recovery state, a stopped or fenced controller, a delivered `run.terminal`, and no live work after it. Quarantine copies must pass the same checks. - `doc/execution-semantics.md`: describe the rule. ## Verification - `server/src/services/native-runtime/native-recovered-owner-followup.test.ts` (real Postgres): - New: "retires a finished remote owner with an unacknowledged pending-attach / pending-tool-result when its sandbox lease ended on another sandbox". It fails on `master` with the exact staging errors `journal_commands_unsettled(run.attach.pending)` and `journal_commands_unsettled(semantic_tool.result.pending)`. It passes with this change, and the log shows the retirement. - Held cases stay held: a lease that is still retained, a lease on this run's sandbox, no lease record, and a tool input with no journaled answer (`journal_tool_input_unanswered`). - New: "keeps a stopped local owner held by an unacknowledged run.attach". A local Runner is not gone by record. - Whole file: 95 passed. - Neighbouring suites pass: `explicit-native-continuation.test.ts` and `native-controller-recovery.integration.test.ts` (460 tests). - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` exits 2: a global CI input changed (`doc/execution-semantics.md`). Required CI supplies the broad coverage. - Live proof: not done. The redeploy chaos scenario (K15) is not on `master` yet. Tests only. ## Risks - The rule applies only when every sandbox lease of the prior run is `released` or `expired`, has a release time, and names a different provider lease than this run's. A reusable sandbox that this run resumed keeps the existing hold. - A command that carries user intent (`turn.start`, `turn.steer`, `request.resolve`, `interaction.receipt`, `external_provider.operation`, `session.goal.set`/`clear`, `session.close`, `run.cancel` and others) still holds the owner. - Retirement archives the old journal under `continuity-breaks` and replays nothing, as before. - No persisted shape changes. Old saved journals are read by the same rule. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use 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 <noreply@anthropic.com> |
||
|
DottaandClaude
|
c14f2fe45d |
fix(native): read the scheduled-resume evidence the resume reconciler claimed (#20656)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - #20645 lets a local same-run scheduled resume send the one same-conversation continuation for a turn lost on restore. It applies only when durable records prove three facts: the previous runner stopped, the attempt was admitted within the budget, and it is the same checkpointed conversation. > - It read the third proof from the coordinator's saved `failure_detail` while the phase was `retryable_failure`. > - In production, the resume reconciler (`claimNativeSessionResumptions`) claims the run first. It moves the coordinator to `observed` and clears `failure_detail`. So the executor never saw that evidence, and #20645 refused every real scheduled resume. > - The restart-chaos L7 wave showed this live. Codex B5 TM2 hit the exact path (`runner.drain` timeout, scheduled resume, `provider_turn_lost_on_restore`), sent no continuation, and needed Retry. > - This pull request makes the reconciler record that evidence durably when it claims the run, and makes the executor read it from there. > - The benefit is that #20645 works on the real path. The proof rule is unchanged. ## Linked Issues or Issue Description Refs #20645 **What happened?** Wave `w-20261110T212307-s9107` (L7, seed 9107, local, master plus #20645), task codex B5 TM2: - `native_session_recovery_failed: runner_local_connect_failed: PRP command runner.drain timed out`; - `retryable_failure` with `recoveryMode: "exact_checkpoint_resume"`, `recoverable`, `providerSessionEstablished` and `checkpointExists` all set; - `native.local_process_stopped`; - attempt 2 started; - `turn.failed` with `provider_turn_lost_on_restore` for the same turn ID that the checkpoint was running. No continuation was sent. The run failed with `native_provider_terminal_failed` and was preserved with `uncertain_provider_action`. **Expected behavior** The proven scheduled resume sends the single continuation. **Steps to reproduce** The new integration test below. The live path is timing-dependent (the L7 command in #20645). ## What Changed - `native-finalization-reconciler.ts`: when it claims a same-run resume, the reconciler appends one `same_run_resume` entry to `recovery_history`. The append is bounded like other history writes, and it runs in the same update that clears the failure. The entry copies `recoveryMode`, `recoverable`, `providerSessionEstablished`, `checkpointExists`, `successorRunId` and `replacementDenied` from the saved failure, with the controller boot and provider attempt. - `scheduled-resume-continuation.ts`: the evidence for the third proof now comes from either of two durable records: - the saved failure, when the executor claims a `retryable_failure` coordinator directly (unchanged); or - a reconciler claim: an `observed` coordinator whose last history entry is a `same_run_resume` entry naming the executor's own controller boot. The two other proofs are unchanged: the stop receipt, and an admitted attempt from 1 to 3. - `native-session-executor.ts`: passes the coordinator's last history entry and the controller boot to the decision. - `doc/execution-semantics.md`: describes the recorded evidence. Readers of `recovery_history` that look at the last entry do so only for restart claims, Stop recovery and shutdown suspension. Restart recovery's previous-claim lookup skips non-claim dispositions, so a `same_run_resume` entry does not change those paths. **Still refused** (tested): - every #20645 case; - the last history entry is not the resume claim, or there is no history; - the resume claim was recorded by another boot; - the claimed evidence is a bootstrap retry, has no checkpoint, or records a replacement; - a reconciler-claimed resume without a stop receipt. ## Verification - `native-scheduled-resume-continuation.integration.test.ts` now also runs through the real reconciler claim (`claimNativeSessionResumptions`) before `executePaperclipNativeSession`. On master the reconciler-claimed allowed case fails (`resumeInterruptedTurn` is false). With this change it is true, and the claim admits attempt 2. A reconciler-claimed resume without a stop receipt still passes false. - `scheduled-resume-continuation.test.ts`: decision cases for the reconciler-claim entry. - `native-session-resumption.test.ts` passes. 54 tests pass across the three files. - `cd server && npx tsc --noEmit` passes. - Live: not yet. The failing wave above is the reproduction. A rerun is timing-dependent. ## Risks - One more `recovery_history` entry per scheduled resume, inside the existing 20-entry bound. - The proof rule itself is unchanged. This only reads the same evidence from the record where it now survives. - No schema change. A run claimed by an earlier build has no `same_run_resume` entry, so it keeps the existing failure: refused, with Retry offered as today. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test runs). Extended reasoning. ## 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 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
48c6e996da |
fix(native): continue a task whose quarantined completed turn's holds settle (#20662)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a remote run starts, workspace preparation attaches the verified saved work of earlier runs of the same task. > - Preparation failed with `remote_workspace_recovery_receipt_invalid` when any earlier run had a recovery receipt that it could not verify. > - A refusal cannot change the saved state, so every Retry failed in the same way, and the task stayed blocked. Recoveries must fall back automatically. > - This pull request skips an unverifiable source. It attaches nothing unverified and starts from the workspace's newest verified save. > - The benefit is that Retry and continuations start again, while strict receipt checks still decide what is attached, and no saved work is changed. ## Linked Issues or Issue Description Refs #20631 **What happened?** On staging, after the deploy that included #20631, a user pressed Retry on a task whose previous run failed with `native_controller_recovery_missing_evidence:runner_not_settled`. The new run failed in setup in under a minute with `setup_failed` / `remote_workspace_recovery_receipt_invalid` (`executionRecovery: bootstrap`, `providerWorkStarted: false`). The task stayed blocked. Another Retry would fail the same way. #20631 accepted one shape of this refusal (a source that saved only the snapshot its own restore loaded). This task had a different shape. **Expected behavior** A Retry starts. A source whose receipt cannot be verified is not attached, and its saved state stays intact for workspace recovery. **Steps to reproduce** 1. Let a remote native run fail before provider work and leave a recovery receipt or restore hold that does not verify (for example a receipt on an unsaved reference, a receipt from another epoch, or a saved reference without a receipt). 2. Press Retry on the task. 3. See the new run fail in setup with `remote_workspace_recovery_receipt_invalid`. **Paperclip version or commit** Staging deploy that includes `e3bc4ca798`. The gap remains on `master` at `a3b34a9d88`. ## What Changed - `attachPreservedRemoteWorkspaceSnapshots` no longer throws for an unverifiable source. Each of the five refusal shapes now skips the source and logs `remote_workspace_recovery_receipt_invalid` with a reason: `receipt_without_reference`, `reference_unreadable`, `receipt_on_unsaved_reference`, `receipt_unverified`, and `receipt_snapshot_missing` (was `remote_workspace_recovery_snapshot_missing`). - `retainRemoteRecoveryAttachments` no longer throws for a retained recovered copy that it cannot verify. It does not publish that copy again. The files stay in the workspace. - The writer check (`remote_workspace_recovery_writer_changed`), the workspace access check, and snapshot read and digest failures still refuse. - `doc/execution-semantics.md` describes the fallback rule. ## Verification - Red on unfixed code: `remote-workspace-recovery-receipt-fallback.test.ts` (moved out of `workspace-recovery-preparation.test.ts` to keep that file under 1,000 lines) "starts a Retry from the newest verified save when a predecessor has %s" (6 shapes: `receipt_without_reference`, `unreadable_reference_with_hold`, `receipt_on_prepared_reference`, `superseded_snapshot_with_hold`, `receipt_from_other_epoch`, `receipt_for_other_run`). All 6 failed with `remote_workspace_recovery_receipt_invalid`. The source is a native run that failed with `runner_not_settled`, and the writer is a Retry (`retryOfRunId`, `taskRetryRequestId`) that follows the real ancestry query. - Green: the same 6 tests pass. They also assert that nothing is attached, and that the source run row, its workspace operations, and its snapshot row are unchanged. - Updated safety tests: the canonical-attachment fault matrix now expects no attachment for receipt faults, and still expects a refusal for a missing or corrupt archive and for a wrong company or task. The retained-history test expects an unverifiable retained copy to be left out of the published list, while verified copies stay published. - `npx vitest run src/services/remote-workspace-recovery-receipt-fallback.test.ts src/services/workspace-recovery-preparation.test.ts`: 91 passed. - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` returns exit 2, because a global doc input changed (`doc/execution-semantics.md`). Required CI supplies the broad coverage. - Not verified live on Daytona. I could not read the staging rows for this task: the browser had no free tab. The tests cover every refusal shape of this function, so the exact staging shape is one of them. ## Risks - Previous-release state: the rule reads the same rows as before (run reference, result hold, workspace operations, recovery actions, snapshots). Rows that earlier releases saved carry forward. No migration. A blocked task recovers on its next Retry or wake. - A Retry can now start without files from an unverifiable source. Those files are not lost: the source's snapshot, restore hold, receipts and recovery action stay unchanged, and workspace recovery still owns them. The skip is logged with a named reason. - A retained copy that fails verification is dropped from the published list. Its directory stays in the workspace and is no longer retired automatically. - A missing or corrupt snapshot archive still refuses. A permanently lost archive therefore still blocks attach. This PR does not change that. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). Extended reasoning enabled. ## 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> |
||
|
DottaandClaude Opus 5.5
|
8f6ca25976 |
fix(workspaces): skip unverifiable recovery sources instead of failing Retry (#20666)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a remote run starts, workspace preparation attaches the verified saved work of earlier runs of the same task. > - Preparation failed with `remote_workspace_recovery_receipt_invalid` when any earlier run had a recovery receipt that it could not verify. > - A refusal cannot change the saved state, so every Retry failed in the same way, and the task stayed blocked. Recoveries must fall back automatically. > - This pull request skips an unverifiable source. It attaches nothing unverified and starts from the workspace's newest verified save. > - The benefit is that Retry and continuations start again, while strict receipt checks still decide what is attached, and no saved work is changed. ## Linked Issues or Issue Description Refs #20631 **What happened?** On staging, after the deploy that included #20631, a user pressed Retry on a task whose previous run failed with `native_controller_recovery_missing_evidence:runner_not_settled`. The new run failed in setup in under a minute with `setup_failed` / `remote_workspace_recovery_receipt_invalid` (`executionRecovery: bootstrap`, `providerWorkStarted: false`). The task stayed blocked. Another Retry would fail the same way. #20631 accepted one shape of this refusal (a source that saved only the snapshot its own restore loaded). This task had a different shape. **Expected behavior** A Retry starts. A source whose receipt cannot be verified is not attached, and its saved state stays intact for workspace recovery. **Steps to reproduce** 1. Let a remote native run fail before provider work and leave a recovery receipt or restore hold that does not verify (for example a receipt on an unsaved reference, a receipt from another epoch, or a saved reference without a receipt). 2. Press Retry on the task. 3. See the new run fail in setup with `remote_workspace_recovery_receipt_invalid`. **Paperclip version or commit** Staging deploy that includes `e3bc4ca798`. The gap remains on `master` at `a3b34a9d88`. ## What Changed - `attachPreservedRemoteWorkspaceSnapshots` no longer throws for an unverifiable source. Each of the five refusal shapes now skips the source and logs `remote_workspace_recovery_receipt_invalid` with a reason: `receipt_without_reference`, `reference_unreadable`, `receipt_on_unsaved_reference`, `receipt_unverified`, and `receipt_snapshot_missing` (was `remote_workspace_recovery_snapshot_missing`). - `retainRemoteRecoveryAttachments` no longer throws for a retained recovered copy that it cannot verify. It does not publish that copy again. The files stay in the workspace. - The writer check (`remote_workspace_recovery_writer_changed`), the workspace access check, and snapshot read and digest failures still refuse. - `doc/execution-semantics.md` describes the fallback rule. ## Verification - Red on unfixed code: `remote-workspace-recovery-receipt-fallback.test.ts` (moved out of `workspace-recovery-preparation.test.ts` to keep that file under 1,000 lines) "starts a Retry from the newest verified save when a predecessor has %s" (6 shapes: `receipt_without_reference`, `unreadable_reference_with_hold`, `receipt_on_prepared_reference`, `superseded_snapshot_with_hold`, `receipt_from_other_epoch`, `receipt_for_other_run`). All 6 failed with `remote_workspace_recovery_receipt_invalid`. The source is a native run that failed with `runner_not_settled`, and the writer is a Retry (`retryOfRunId`, `taskRetryRequestId`) that follows the real ancestry query. - Green: the same 6 tests pass. They also assert that nothing is attached, and that the source run row, its workspace operations, and its snapshot row are unchanged. - Updated safety tests: the canonical-attachment fault matrix now expects no attachment for receipt faults, and still expects a refusal for a missing or corrupt archive and for a wrong company or task. The retained-history test expects an unverifiable retained copy to be left out of the published list, while verified copies stay published. - `npx vitest run src/services/remote-workspace-recovery-receipt-fallback.test.ts src/services/workspace-recovery-preparation.test.ts`: 91 passed. - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` returns exit 2, because a global doc input changed (`doc/execution-semantics.md`). Required CI supplies the broad coverage. - Not verified live on Daytona. I could not read the staging rows for this task: the browser had no free tab. The tests cover every refusal shape of this function, so the exact staging shape is one of them. ## Risks - Previous-release state: the rule reads the same rows as before (run reference, result hold, workspace operations, recovery actions, snapshots). Rows that earlier releases saved carry forward. No migration. A blocked task recovers on its next Retry or wake. - A Retry can now start without files from an unverifiable source. Those files are not lost: the source's snapshot, restore hold, receipts and recovery action stay unchanged, and workspace recovery still owns them. The skip is logged with a named reason. - A retained copy that fails verification is dropped from the published list. Its directory stays in the workspace and is no longer retired automatically. - A missing or corrupt snapshot archive still refuses. A permanently lost archive therefore still blocks attach. This PR does not change that. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). Extended reasoning enabled. ## 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> |
||
|
DottaandClaude Opus 5.5
|
686893636d |
fix(recovery): release a lapsed owner of a finished coordinator so stop-proof holds re-arm (#20661)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Workspace recovery saves the files of a run whose copy-back failed, and it holds the shared workspace until the save is done > - #20295 re-arms a save that was exhausted with `agent_stop_unconfirmed`, so the run's stop is proven again and the files are saved without a person > - On staging, four holds of this shape were never re-armed. Their runs ended six days ago, their sandbox leases are released with a provider stop receipt, and the servers that ran them were replaced many times. Their tasks stay blocked, and so do the parent tasks that wait for them > - The finished coordinator of each run still names a lease owner whose lease lapsed. Discovery treats that owner as "waiting for the abandoned-owner settlement", but that settlement handles only observed (unfinished) coordinators. Nothing ever releases the owner, so the worker never selects the hold, the re-arm refuses it, and the attempt's stop check refuses it too > - This pull request makes a lapsed owner of a finished coordinator count as stopped. The re-arm releases it in the same transaction, and then the new attempt proves the sandbox stop and saves the files as #20295 designed > - The benefit is that these holds settle on the worker's next pass after deploy, without a migration and without an administrator ## Linked Issues or Issue Description Refs #20295 **What happened?** An exhausted workspace save (`native_workspace_sync_out_retry_exhausted`, last failure `agent_stop_unconfirmed`) stayed blocked for six days. The task showed "Paperclip tries again automatically", but no re-arm was ever recorded (no `stopProofRecovery` marker, no `issue.workspace_recovery_rearmed` activity), and the shared workspace stayed held by the failed run. **What did you expect to happen?** The recovery worker re-arms the save once the run is provably over, saves the dirty files, and frees the workspace so the task and its parent can continue. **Steps to reproduce** 1. Exhaust a workspace save with `agent_stop_unconfirmed` (one attempt, window spent). 2. Leave a lease owner on the run's `terminal_failure` coordinator with an expired lease (or no expiry), as a finalizer pass of a replaced server does. 3. Run the workspace recovery worker. `nextWorkspaceRecoveryAt` returns null, discovery does not select the hold, and `rearmExhaustedStopProof` refuses it. ## What Changed - `workspace-recovery-stop-proof.ts`: add `lapsedFinishedCoordinatorOwner` and `releaseLapsedCoordinatorOwner`. A lease owner on a finished coordinator (committed, terminal failure, applied) whose lease lapsed, or that has no expiry, does not block `sourceRunProvablyOver`. The stop-proof re-arm clears that owner with a compare-and-swap in its locked transaction, and records `releasedLapsedOwner` in the marker and the activity. - `workspace-recovery-capture-policy.ts`: the capture-policy re-arm uses the same rule, because it shares the same check and discovery clause. - `workspace-recovery-fallback-discovery.ts`: the discovery clause still waits for the abandoned-owner settlement for a stale owner of an unfinished coordinator, but it no longer excludes a hold whose finished coordinator has a lapsed owner. A live lease still defers the re-arm to its expiry. - `system-recovery-state.ts`: add `FINISHED_COORDINATOR_PHASES`. - `doc/execution-semantics.md`: document the rule. - Tests: two red-then-green regressions in the staging shape (an expired finalizer lease, and an owner without an expiry saved by an older release), one strictness case (a live lease on a finished coordinator keeps the hold and is not released), and the existing stale-owner case now uses an observed coordinator, which still waits for the abandoned-owner settlement. ## Verification - Red before the fix: `pnpm --filter @paperclipai/server exec vitest run src/services/workspace-recovery-stop-proof.test.ts` failed the two new staging-shape cases (`nextWorkspaceRecoveryAt` returned null: the worker was never woken for the hold). - Green after the fix: `workspace-recovery-stop-proof.test.ts` and `workspace-recovery-capture-policy.test.ts` pass (15 tests). The staging-shape cases assert that the hold is re-armed, the save completes, the dirty files are saved, the workspace hold is free, and the lapsed owner is released. - `pnpm verify`: exit 2 (broader verification required, because `doc/execution-semantics.md` is a global CI input). Focused suites pass: `workspace-recovery-stop-proof`, `workspace-recovery-capture-policy`, `workspace-recovery-fallback`, `workspace-recovery-retry-copyback` (113 tests). Required CI supplies the broad coverage. - Staging evidence (read-only GET requests): the run is `failed`, finished six days ago, `controllerLeaseExpiresAt` is null, its only lease is `released` with `cleanupStatus: success`, a `stopped` termination receipt and a stop-and-retain receipt. The action's evidence has no re-arm marker, and four holds of the same shape were not touched after #20295 was deployed. ## Risks - Previous-release story: rows saved by earlier releases carry the lapsed owner and no marker. They are selected and re-armed on the worker's first pass after deploy. No migration. - Strictness stays where an owner could be live: a live lease on any coordinator defers the re-arm to its expiry, and a stale owner of an unfinished (observed) coordinator still waits for the abandoned-owner settlement, which proves its controller stopped. The new attempt still requires a provider stop receipt for the allocation before it reads files. - Clearing a lapsed owner matches the existing token-lease contract: an expired finalizer or cleanup lease is free for the next claim, and a holder's later write fails its token compare-and-swap. - Low risk otherwise: the change is limited to the two exhausted-hold re-arms and their discovery clause. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, code editing, browser GET reads) in Claude Code. ## 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> |
||
|
DottaandClaude Opus 5.5
|
4f33bb3578 |
fix(workspaces): release the Blocked status of a sibling-settled no-replay hold (#20648)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a remote workspace save fails, recovery actions hold the task until the run's files are saved > - #20641 lets a resolved no-replay hold settle when a sibling action of the same run saved the files > - Review found that the settlement did not release the task's Blocked status, and that a settled hold with a historical source stayed selectable > - A new test with the waiting user comment also showed that the worker skipped the second hold of the same run in every pass, so it never settled > - #20641 was merged before these fixes were pushed > - This pull request completes #20641 with those fixes > - The benefit is that the task leaves Blocked and continues without a person ## Linked Issues or Issue Description Refs #20641, Refs #20569 **What happened?** #20641 merged at its first head. The review follow-ups for it were pushed later and are not on master: - A settled no-replay hold that owned the task's Blocked status (`nativeFailureBlock`) left the task `blocked`. - A hold settled by a sibling stayed selectable through `workspaceRecoverySourceEligible` when it named a `historicalWorkspaceSource`. - With saved user input waiting, the export hold stayed selectable and came first in each worker pass. `reconcileRetainedWorkspaces` skipped the other hold of the same run, so the no-replay hold never settled. **Expected behavior** After the save, the no-replay hold settles in the same or next pass. The task returns to `todo` when that hold owns its Blocked status. The settled hold is never selected again. **Steps to reproduce** 1. Use the staging-shape test in `workspace-recovery-capture-policy.test.ts`, with a waiting "try again" comment. 2. Run the worker until nothing is due. 3. On master, the worker never goes idle and the no-replay hold stays `resolved`/`blocked`. ## What Changed - `workspace-recovery-fallback.ts`: new shared `holdOwnsBlockedStatus`, used by `continueFromLastSavedWorkspace` and `settleSiblingRecoveredHold`. A settled no-replay hold that owns the Blocked status returns the task to `todo`. A newer decision's Blocked status is kept. An export hold's status stays with its own result finalization. - `workspace-recovery-source.ts`: `workspaceRecoverySourceEligible` excludes a resolved hold that carries `siblingWorkspaceRecovery`. - `workspace-recovery.ts`: `reconcileRetainedWorkspaces` runs several holds of one run one after another in that run's job, instead of skipping the later ones. - `workspace-recovery-capture-policy.test.ts`: the staging test now has the waiting comment, and a variant with `nativeFailureBlock` and `historicalWorkspaceSource`. It checks `todo`, no re-selection, and no repeat handoff in a later pass. - `doc/execution-semantics.md`: documents these rules. ## Verification - Red, with each rule disabled: the status stayed `blocked`; the settled hold was selected again; the worker never went idle (starvation). - Green: `npx vitest run --root server` on 14 workspace recovery suites (including `workspace-recovery-capture-policy`, `-fallback`, `-admission`, `-source`, `execution-blocker-read`, `preserved-native-response-wait.integration`): all passed. - `npx tsc --noEmit -p server/tsconfig.json`: clean. ## Risks - Several holds of one run now run in sequence in one job. Concurrency per run is unchanged (one job per run). - A no-replay hold releases the Blocked status only when it still owns it. Otherwise the status is kept. - Previous-release story: no new persisted state. Holds saved by earlier releases are handled on the first worker pass after deploy. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use 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> |
||
|
DottaandClaude
|
805531cc71 |
fix(native): never leave a cleanup-gated task without Retry or an owner (#20639)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs close their provider session at the end of each attempt.
When close cannot prove that the runner stopped, the runner keeps a
cleanup gate (`native_session_cleanup_quarantined`) in server memory
> - #20572 hid Retry for every gated run and said "Paperclip finishes
the cleanup automatically". Only a remote environment release settles
the gate. A local gate that needs an operator never settles, and a
server restart loses it
> - A restart-chaos wave on master found two tasks that rested blocked
with no Retry, no scheduled retry and no recovery owner. A person could
not continue them
> - This pull request makes the run's own recovery retire the gate when
the host proves the old processes stopped. It also offers Retry whenever
nothing settles the gate, and it makes that Retry work
> - The benefit is that a cleanup-gated task either continues by itself
or shows a Retry that starts. It never rests without both
## Linked Issues or Issue Description
Refs #20572, #20578
**What happened?**
Restart-chaos wave `w-20261110T185144-s9107` (local, claude, fault TM2:
the server is paused with SIGSTOP for about 33 s). Tasks t04 and t05
completed their provider turn. The runner checkpoint owner lapsed while
the server was paused (`native_runner_checkpoint_owner_lost`). The
session close then failed with `native_session_close_unrecoverable:
runner did not durably suspend before checkpoint`. The provider had
drained and no tool call was pending. The runner kept an
operator-required cleanup gate. Thirty seconds later the run's own
same-run recovery verified that the runner process and group had stopped
(`native.local_process_stopped`). The gate still refused the recovery,
and the run ended `failed` with `native_session_cleanup_quarantined`.
The task showed `canRetry: false` and a cleanup wait until the deadline.
Nothing owned that wait.
**Expected behavior**
When the old runner and provider processes are proven stopped, the run's
own recovery continues. When nothing settles the gate, the task offers a
Retry that starts.
**Steps to reproduce**
1. Run restart-chaos L7: `--env local --adapters codex,claude
--workloads A1,B5,C1 --faults TM1,TM2,TM3 --time-age-hours 1
--concurrency 3 --tasks 6 --seed 9107`.
2. Observe a claude TM2 task rest `blocked` with `canRetry=false`,
`retryWait=true` and no scheduled retry.
**Paperclip version or commit**
master at
|
||
|
DottaandClaude
|
37c50f70c2 |
fix(native): continue a turn lost on restore in a proven same-run resume (#20645)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run keeps its provider conversation when its local runner dies. When the restored runner finds that the active turn was lost, it reports `provider_turn_lost_on_restore` and does not invent a result. > - After a control-plane restart, the admitted restart attempt may send one continuation in that same conversation (#15297). The agent then reconciles unfinished work. The task is not resent. > - A runner can also be lost without a server restart, for example when `runner.drain` times out. The run then resumes itself after the retry delay from its exact checkpoint. That path never allowed the continuation, because the executor passed `resumeInterruptedTurn` only for a `resume_dead_runner` restart claim. > - So the lost turn failed with `native_provider_terminal_failed`, automatic replacement was refused (`uncertain_provider_action`), and the user had to click Retry. > - This pull request allows the same single continuation for that resume, but only when durable records prove the same three facts that a restart claim proves. > - The benefit is that a runner lost mid-turn continues in the same conversation with no Retry. No case that is refused today becomes allowed without that proof. ## Linked Issues or Issue Description Refs #15297, #20625 **What happened?** Restart-chaos wave L7 (seed 9107, local), task codex B5 TM2, run on master after #20625. A TM3 restart resumed the run (attempt 1). During the TM2 freeze the TS runtime lost its runner: `native_session_recovery_failed: runner_local_connect_failed: PRP command runner.drain timed out`. The executor saved `retryable_failure` with `recoveryMode: "exact_checkpoint_resume"`. After the delay, recovery verified the runner process stopped (`native.local_process_stopped`). The same run then resumed as attempt 2 (`native.process_start_requested`, `providerAttempt: 2`). The restored runner reported `turn.failed` with `provider_turn_lost_on_restore` (`recoverable: true`). No continuation was sent. The run failed with `native_provider_terminal_failed`, recovery stopped with `uncertain_provider_action` / `preserve_without_replay_v1`, and the task needed Retry. **Expected behavior** The run sends the one same-conversation continuation, exactly as it does after a restart claim, and finishes without Retry. **Steps to reproduce** `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --adapters codex,claude --workloads A1,B5,C1 --faults TM1,TM2,TM3 --time-age-hours 1 --concurrency 3 --tasks 6 --seed 9107` (timing-dependent: the drain timeout must land during an active turn). ## What Changed - New `server/src/services/native-runtime/scheduled-resume-continuation.ts`. It decides whether a same-run scheduled resume may continue a lost turn. Each of the three proofs comes from a durable record: 1. **The previous runner is stopped.** The latest process record of the run in `heartbeat_run_events` (`native.process_start_requested`, `native.process_identity_recorded`, `native.local_process_stopped`) must be `native.local_process_stopped`. Its `processPid` must equal its `processGroupId`, and neither the pid nor the group may be alive now. This is the existing `readNativeLocalProcessStop` check, read in the claim transaction before this attempt records its own `native.process_start_requested`. 2. **The attempt was admitted under the bounded budget.** The executor's claim moves `native_run_finalizations` from phase `retryable_failure` to `observed`. It charges a provider attempt, and the claim refuses any attempt above 3 (`native_session_retry_exhausted`). The decision also requires `1 <= attempt <= NATIVE_PROVIDER_RECOVERY_ATTEMPT_LIMIT`. 3. **It is the same provider conversation.** The saved `failure_detail` must show `recoveryMode: "exact_checkpoint_resume"`, `recoverable: true`, `providerSessionEstablished: true` and `checkpointExists: true`, with no `successorRunId` or `replacementDenied`. At run time the existing runtime guards still apply: the backend must recover the checkpointed session (`recovered`, not a continuity-break replacement), and the lost turn must equal the checkpoint's `activeTurnId` and the snapshot's interrupted turn. The provider kinds stay codex, acpx and opencode, with no compaction, goal control, held result or pending runtime request. - `native-session-executor.ts`: computes that decision in the coordinator claim. It passes `resumeInterruptedTurn` when either the restart claim is `resume_dead_runner` (unchanged) or the decision allows it. - `doc/execution-semantics.md`: describes the rule and the refused cases. **Cases that stay refused** (each has a test in `scheduled-resume-continuation.test.ts`): - a restart-recovery claim (it keeps its own existing rule); - a remote execution target; - a coordinator that is not `retryable_failure`, including a fresh run; - `recoveryMode` `bootstrap_retry` or `ambiguous_state`, or no saved failure detail; - a failure that is not recoverable; - no established provider session; - no checkpoint; - a recorded replacement (`successorRunId` or `replacementDenied`); - an attempt outside 1..3; - no stop receipt, a stop receipt superseded by a newer launch, a stopped process that is still alive, or a run that never had a process. The runtime-side refusals are unchanged and already covered by `native-session-runtime.test.ts`: real provider errors, historical error-null failures, and no restart permission. ## Verification - `cd server && npx vitest run src/services/native-runtime/scheduled-resume-continuation.test.ts`: 22 tests, covering the decision table and the stop receipt in embedded Postgres. - `cd server && npx tsc --noEmit` passes. - Live: not proven. An L7 wave on this branch (seed 9107) did not reproduce the drain-timeout path, so this pull request has unit and database tests only. In that wave the same cell (codex B5 TM2) needed Retry for a different reason: TM2 aged its restart-recovery claim lease before dispatch, and budget admission refused with `Native recovery no longer owns provider dispatch`. This pull request does not change that path. Four other tasks passed. Claude C1 TM2 rested blocked without a Retry button, which is a separate cleanup-quarantine issue. ## Risks - This allows one more path to send the same-conversation continuation. It is limited to local runs. It needs the same proofs as a restart claim, and it uses the attempt the resume already charged. It does not change the generation cap or `restartResumeKeepsAttempt`. - A rolling deploy reads `failure_detail` and stop events written by earlier releases. Those fields and events already have this shape, so an older saved failure is judged by the same rule. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test runs). Extended reasoning. ## 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 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
4398a723c0 |
fix(native): settle tool callbacks and holds that a server restart orphaned (#20628)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs send provider tool calls to the server as durable
semantic inputs. The server runs the tool and saves the result in the
controller journal
> - Close must prove that every admitted tool call has a saved result.
Otherwise it reports `native_session_settlement_incomplete` and the
runner holds a cleanup gate (`native_session_cleanup_quarantined`)
> - A server restart in the middle of a tool call leaves its input
without a result. The new controller asks the tool authority for a
receipt. Tools without a receipt (assigned app tools,
`connections_search`) return nothing, so the input stays pending forever
> - Runs that already failed this way kept no-replay holds for a turn
that had completed. The task stayed blocked with no Retry and no
continuation
> - Restarts are normal. Work that a restart interrupts must settle by
itself, not wait for a person
> - This pull request settles such a callback as interrupted, with an
unknown outcome, when its owner is provably gone, and settles the holds
that earlier releases left once their controller is gone. It never runs
the tool or the turn again
> - The benefit is that close drains after a restart, and tasks that
earlier releases blocked this way continue on their own
## Linked Issues or Issue Description
Refs #20572 (that PR fixed the presentation of this quarantine. Its
Risks section names this runner drain rule as a separate follow-up.)
Refs #20641 (it settles a resolved no-replay hold when a sibling action
saved the run's files. This PR covers a different case: the hold's own
recovery saved the files, and its run's turn completed. Both use the
same recovery receipt, `completedWorkspaceRecoveryCondition`.)
**What happened?**
A remote native run started an assigned app tool call. The server
restarted 3 seconds later. The new server adopted the runner, and the
provider turn completed about 9 minutes later. The tool call never got a
saved result. Close reported `native_session_settlement_incomplete` with
`runnerSuspended: true`, `providerDrained: false` and one pending
callback. The session quarantined, and the same-run recovery was refused
at admission. A second run showed the same shape with a
`connections_search` callback after a later restart.
The first run then had two resolved-but-blocked holds with
`preserve_without_replay_v1` and an unknown action outcome: the
cleanup-quarantine hold, and a continuation hold whose workspace
recovery saved the files. The task view said "Saved work recovered. Send
a message to continue." with no Retry and no continuation. The task
waits on a child task and stays blocked.
**Expected behavior**
After a restart, a tool call without a saved result settles
automatically. A durable receipt is replayed. Without a receipt, the
agent is told that the outcome is unknown. Close proves the drain, and
no cleanup gate is left. Holds that earlier releases left for a
completed turn settle without a person, and the task continues when its
own wait ends.
**Steps to reproduce**
1. Start a native run and call an assigned app tool.
2. Restart the server while the tool call runs.
3. Let the adopted turn complete, then close the session. Close reports
`native_session_settlement_incomplete` and quarantines. The task stays
blocked by the no-replay holds.
**Paperclip version or commit**
master at
|
||
|
DottaandClaude Opus 5.5
|
d06f43b14f |
fix(workspaces): settle a resolved no-replay hold once a sibling saved its files (#20641)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a remote workspace save fails, recovery actions hold the task until the files are saved > - One failed run can have two holds: its export hold, and a no-replay hold that a later "try again" opened for the same run > - The no-replay hold is recorded resolved-but-blocked, and it is the task's execution blocker > - #20569 lets the export hold retry under the new symlink capture policy and save the files > - The sibling settlement that should then release the no-replay hold only looked at active holds, so the task stayed blocked with no working Retry > - This pull request lets the sibling settlement and its discovery include resolved no-replay holds > - The benefit is that the task continues without a person once the files are saved ## Linked Issues or Issue Description Refs #20569 **What happened?** On staging, a native run on a Daytona sandbox failed to save its workspace (`remote_workspace_archive_unsafe_symlink`). Its export hold (`native_workspace_sync_out_retry_exhausted`) was exhausted after 6 attempts and handed to the board. A user comment then opened a second hold for the same run (`native_continuation_requires_reconciliation`, `preserve_without_replay_v1`). That hold also failed 6 times and was recorded `resolved` with outcome `blocked`. The task's execution blocker is this second hold. **Expected behavior** After #20569 is deployed, the worker re-arms the export hold and saves the files. The no-replay hold of the same run then settles, and the task is no longer blocked. **Steps to reproduce** 1. Create both holds with the exact staging evidence (see the new test `stagingIncident`). 2. Run the recovery worker until nothing is due. 3. Before this change, the export hold completes, but the no-replay hold stays `resolved`/`blocked` and `getExecutionBlocker` still returns it with `canRetry: false`. ## What Changed - `workspace-recovery-fallback.ts` (`settleSiblingRecoveredHold`): a resolved no-replay hold (`isRetainedFileHold`) can now settle when a verified receipt of a sibling action saved its run's files. Settling removes `automaticRecovery`, so the hold stops blocking the task. It starts no turn. - `workspace-recovery-fallback-discovery.ts`: the worker discovers such a resolved hold when a sibling of the same run is recovered. - New test in `workspace-recovery-capture-policy.test.ts` with the exact staging shape of both holds: board owner, `active_run_watchdog`, system recovery policy version 1 without a capture policy version, stage `restoring_files`, `export_interrupted` with cause `provider_error`, 6/6 attempts, `historicalWorkspaceSource` and `workspaceRestoreRecovery` lease ids. - `doc/execution-semantics.md`: states that a resolved no-replay hold also settles from a sibling save. ## Verification - Red before the fix: the new test failed. The no-replay hold stayed `resolved`/`blocked`, and it stayed the execution blocker. - Green after the fix: `npx vitest run --root server src/services/workspace-recovery-capture-policy.test.ts` (3 passed). The test drives the worker's own discovery (`nextWorkspaceRecoveryAt` + `reconcileRetainedWorkspaces`) until nothing is due. It checks: the export hold is re-armed once and saves the files; the no-replay hold is `resolved`/`restored` by the sibling; `getExecutionBlocker` returns null; the continuation hook ran when the task was free; no new run was created; the worker goes idle. - Related suites: `workspace-recovery-fallback`, `-admission`, `-stop-proof`, `-source`, `-preparation`, `-preparation-gates`, `execution-blocker-read`, `preserved-native-response-wait.integration`: 409 passed. - `npx tsc --noEmit -p server/tsconfig.json`: clean. - `pnpm verify` exits 2 (global doc input), so CI supplies the broad coverage. ## Risks - A resolved no-replay hold is now released when another action's verified receipt saved the same run's files. The receipt check is unchanged, so an unverified sibling still does not release it. - Settling starts no turn. Waiting input and the run's own result continue the task through normal admission, which still checks every other hold. - Previous-release story: holds saved by earlier releases need no migration. The worker discovers them on its first pass after deploy. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use 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> |
||
|
DottaandClaude Opus 5.5
|
e3bc4ca798 |
fix(native): settle warm follow-ups that a restart left behind (#20631)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native warm runs on a remote sandbox continue the saved session of the previous run. Admission first proves what happened to the previous Runner. > - A deploy restarted the staging server while warm Runners were idle. Three follow-up runs then failed before provider work, each with a different refusal, and their tasks stayed blocked. > - The refusals asked for proof that a restart had made impossible. A restart is a product failure, so recovery must settle it automatically and keep the strict checks for real uncertainty. > - This pull request accepts the proof that the restart leaves behind, in three places: the warm owner on a replaced sandbox, the suspended Runner with an undelivered suspend receipt, and the restore hold of a run that saved nothing new. > - The benefit is that these follow-up runs start, and state saved by older releases carries forward without a migration. ## Linked Issues or Issue Description Refs #20567, #20346 **What happened?** On staging, a deploy replaced the webserver process at about 18:04 UTC on 2026-11-10 (lease controller boot `348ec0bb` on one deployment, then boot `3e84c195` on a new deployment that started at 18:04:16). Three runs of one Codex agent on Daytona failed between 18:08 and 18:15: 1. A warm follow-up got a newly created sandbox. The previous warm Runner stayed `ready` in its own retained sandbox. Live adoption denied it with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate` (`remote-warm-session-recovery.ts:127`), because only a resumed sandbox may be adopted. The per-turn retirement path refused warm owners (`lifecycle_not_per_turn`). 2. A warm follow-up resumed the same sandbox and directory. The previous Runner had been suspended by the shutdown, but the controller stopped before it acknowledged the suspend result. The Runner kept its terminal fence (`pendingTerminalDelivery` for `runner.suspend`). The read refused it with `native_controller_recovery_missing_evidence:runner_not_settled` (`native-session-executor.ts:7032`). 3. A continuation into a fresh workspace failed with `remote_workspace_recovery_receipt_invalid` (`remote-workspace-recovery-attachments.ts:60`). Its predecessor had restored the committed save, failed before provider work, and kept a restore hold with no recovery receipt. Its saved reference named the snapshot it restored. **Expected behavior** After a restart, the next run continues. It retires a finished owner that nobody can adopt, reads a finished suspension, and skips a source that left no unsaved files. Real uncertainty still refuses with a named reason. **Steps to reproduce** 1. Run a warm native Codex turn on a reusable Daytona sandbox and let it commit. 2. Restart the server (deploy) while the Runner is idle. 3. Start the next turn. Get a new sandbox (case 1), or resume the same sandbox after the shutdown suspended the Runner (case 2). 4. For case 3, continue a task whose previous run restored the last save and failed before provider work, into a fresh workspace. **Paperclip version or commit** Staging before `2e3cbe57` (the runs predate that deploy). The gaps remain on `master` at `9b21dcbd`. ## What Changed - `inspectRemoteWarmSessionRecovery` returns `null` when this run's sandbox is `created` or `replacement`. Such a sandbox holds no Runner to adopt. The caller then uses ordinary admission. - `inspectFinishedPerTurnRunnerdOwner` also retires a **warm** owner on a remote sandbox runner when this run's sandbox is `created` or `replacement`. It uses all the existing proofs: terminal run, committed result, released coordinator lease, stopped or fenced controller, settled journal, and quarantine copies. Local warm Runners and resumed sandboxes keep the live path. - `restoreRemoteSuspendedRunnerCheckpoint` accepts a suspended Runner whose only open fence is its own undelivered `runner.suspend` receipt. The fence must name `runner.suspend` with lifecycle `suspended`, it must be the last controller command, and the restored journal must not name that command differently. A provider cleanup, unsent events, a turn's terminal fence or a warm transition still refuse with `runner_not_settled`. A later authorized Runner process reconciles the receipt, as the Runner already does. - `attachPreservedRemoteWorkspaceSnapshots` skips a source with a receipt-less restore hold when its saved reference names the exact committed snapshot that its own succeeded `remote_workspace_restore` loaded at the same epoch. Any other saved snapshot still needs the canonical receipt. - `doc/execution-semantics.md` describes the three rules. ## Verification - Red (unfixed code), each in the exact staging shape: - `native-recovered-owner-followup.test.ts` "retires a finished warm owner left behind by a restart when this run's sandbox is created/replacement" failed with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate`. - `native-controller-recovery.integration.test.ts` "defers a remote warm continuation…" failed: live adoption rejected a created/replacement sandbox with the same error. - `native-controller-recovery.integration.test.ts` "reads a resumed sandbox's suspended Runner with an undelivered suspend receipt" failed with `native_controller_recovery_missing_evidence:runner_not_settled`. It reads through a sandbox runner that refuses paths outside the workspace base. - `workspace-recovery-preparation.test.ts` "decides a receipt-less restore hold of a source that restored_unchanged" failed with `remote_workspace_recovery_receipt_invalid`. - Green: the same tests pass. Focused files pass: `native-recovered-owner-followup.test.ts` and `native-controller-recovery.integration.test.ts` (104 tests), `workspace-recovery-preparation.test.ts` (85 tests). - Safety cases (strict checks stay strict): - A warm owner on a resumed sandbox is not retired. - A warm owner with an unsettled journal (pending command, unanswered tool input) stays held and names `retirement_refused=journal_…`. - A warm owner whose finalization lease is still held stays held. - A turn terminal fence, a provider cleanup, unsent events, a fence behind a later command, or a journal that names the command differently still fail with `runner_not_settled` and cache nothing. - A restore-hold source that saved another snapshot, whose restore did not succeed, or whose restore was at another epoch still fails with `remote_workspace_recovery_receipt_invalid`. - Neighbouring suites pass: `native-session-executor.test.ts`, `plugin-runner-release-resume.integration.test.ts`, `explicit-native-continuation.test.ts` and `native-harness-filesystem-independence-continuation.integration.test.ts` (1786 tests). The last one asserted the old warm refusal on a created sandbox. It now shows that the warm follow-up resumes the same provider session for the new run. - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` returns exit 2, because a global doc input changed (`doc/execution-semantics.md`). Required CI supplies the broad coverage. ## Risks - Previous-release state: all three rules read state that earlier releases already save (lease outcome, coordinator rows, Runner state in the sandbox, journal commands, run results and workspace operations). Rows saved by older releases carry forward. No migration. - Case 1 starts a new Runner for a warm session whose old Runner may still exist, idle, in its retained sandbox. Its controller is gone and its result is committed, so it can accept no new commands. The provider session follows the existing resume-or-replace policy. - Case 2 trusts a suspend fence only in the exact shape that the Runner writes before it sends the result. Other fences still refuse. - After case 1, a warm follow-up resumes from its own warm checkpoint. That checkpoint carries no replacement policy, so a failed provider resume is still reported as `native_session_recovery_failed` and is not replaced. This is the existing warm behavior. - Case 3 skips a source only when it provably saved nothing beyond a committed snapshot. That snapshot stays in its own workspace. - The three tasks that already failed stay blocked until their next wake or a user message. This PR does not add an automatic continuation for a completed no-replay hold. A follow-up can add that. - `native-runner-restart-recovery.integration.test.ts` (runs only with a runner container) no longer probes a replacement sandbox, because that probe now retires the owner by design. - Not verified live on Daytona. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, read-only browser access to staging). Extended reasoning enabled. ## 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> |
||
|
DottaandClaude
|
24661683db |
fix(recovery): bootstrap a remote follow-up that a restart cut before its Runner launched (#20642)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a control-plane restart, restart recovery classifies each claimed native run: reattach a live Runner, resume from a checkpoint, or bootstrap again when durable records prove no provider work > - A follow-up run on a continued session gets a rebound admission checkpoint. It names the new run, but it keeps the previous turn's provider session ID > - Recovery read that copied provider session as this run's provider work. A follow-up killed after `native.process_start_requested` but before its remote Runner launched was classified `reattach_remote_runner` > - The reattach then had no Runner to find, failed with `native_continuation_requires_reconciliation`, and the task needed Retry > - This pull request stops the copied session from counting as this run's work, and keeps the existing durable proofs as the authority > - The benefit is that these restarts bootstrap again by themselves and do not repeat provider work ## Linked Issues or Issue Description Refs #20561, #20555 **What happened?** Two Daytona restart-chaos tasks needed a Retry click: - Wave `w-20261110T173243-s9202` t02 (claude C1, K2): the follow-up run was killed at `stage.asset.home`, after `native.process_start_requested`. Recovery chose `reattach_remote_runner`. Verification failed with `native.remote_runner_recovery_failed {stage: runner_state, reason: state_read_failed}` (`runner_remote_state_unavailable: exit=1`): `runner-state.json` did not exist because the Runner never launched. The storage fault window (ST7) had ended 30 s before, so it was not the cause. - Wave `w-20261110T174930-s9205` t04 (codex C1, RR1): the scheduled retry of a `process_lost` run was killed at `session.checkpoint.restore`, after `native.process_start_requested`. Recovery chose `reattach_remote_runner`. Verification failed with `native_remote_recovery_lease_mismatch: stage=runner_verification field=identity_run_id`: the local Runner identity belonged to the previous run, because this run's Runner never started. **Expected behavior** A run whose durable records prove that no Runner received a provider turn bootstraps again without a person. **Steps to reproduce** 1. Start a remote follow-up run on a continued session (for example, a comment after a completed turn). 2. Kill the server after `native.process_start_requested` and before the remote Runner launches. 3. Observe `reattach_remote_runner`, a failed verification and a Retry banner. **Paperclip version or commit** master close to `6d6b2d0e` (waves); still reproduced by the new tests on master `90dd864bff`. ## What Changed - `native-restart-recovery.ts`, no controller checkpoint for the run (`remote_runner_never_received_commands`): a follow-up with `sessionIdBefore` qualifies when it carries only its continued-session checkpoint, and a predecessor's settled Runner identity (`carriesInheritedRunnerIdentity`) no longer disqualifies it. A recorded process identity, a Runner-sent event, a provider event or a pending request still keeps the run held. - `native-restart-recovery.ts`, controller checkpoint exists: the existing Postgres proof (`nativeRunnerCheckpointRemoteBootstrapProof`) now also runs when the run carries only a rebound checkpoint whose provider session a settled earlier run of the same session saved (`carriesSettledPredecessorSession`). The copied provider session then does not count as this run's provider evidence. The controller checkpoint stays the authority: any turn command or turn event keeps the run held. - `doc/execution-semantics.md`: describe the rule. ## Verification - `server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts` (real Postgres): - "restarts a follow-up whose controller never saved a checkpoint (s9205 t04)" and the variant "with its predecessor's remote runner identity": red on master (`reattach_remote_runner`), green now. - "restarts a follow-up whose saved controller checkpoint proves no provider turn (s9202 t02)": red on master, green now. - Held cases stay held: own recorded Runner identity, a carried active turn, a provider session that no settled earlier run saved, a `turn.start` command, a projected `turn.started` event. - Whole file: 171 passed. - `npx tsc --noEmit -p server` passes. - The executor's existing `bootstrap_incomplete` path handles both shapes: a scoped root that belongs to the previous run goes through the existing prior-owner verification, and a root of this run restored from the controller checkpoint goes through `runnerdStateProvesIncompleteBootstrap` / `runnerdStateProvesNoRemoteProviderTurn`. ## Risks - A run with any recorded Runner process, Runner-sent event, provider event, turn command or pending request keeps today's reattach or hold. - A bootstrap claim charges one provider attempt, as today. The bounded limit (3) still applies. - No persisted shape changes. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use 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 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
fd6e71a7ff |
fix(native): restore a warm backup when its source sandbox is idle (#20627)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs on remote sandboxes keep a warm sandbox and a verified harness backup so the next run continues the same provider session > - A run that starts while the previous run's workspace export is still saving gets a new (created) sandbox lease, because reuse skips an exporting sandbox > - When that run prepares the harness, it finds a backup but no recorded reusable lease, and it fails closed with `runner_harness_state_mismatch: backup_without_reusable_lease` > - #20278 accepted a destroyed source and a settled restore hold as retired backup sources, but not an idle warm source that is still `retained` for reuse > - This pull request accepts an idle warm source under the same proofs, claims it so it is never resumed again, and covers the resumed lease that the failed run leaves behind > - The benefit is that these recoveries continue the same session automatically instead of blocking the task ## Linked Issues or Issue Description Refs #20278 **What happened?** A monitor wake started about 90 seconds after the previous run on the same task finished. The previous run's warm sandbox lease was `retained` with policy `reuse_by_environment`, but its workspace export was still saving. Reuse skipped it and created a new sandbox. Workspace realization waited for the export (about 136 s) and restored the saved files. Then harness preparation found the previous run's backup (its lease stamp names this exact manifest, and no provider work followed it). The source lease was not destroyed and was not a restore hold, so the run failed with `native_session_recovery_failed: Error: runner_harness_state_mismatch: backup_without_reusable_lease` and the task was blocked. **What did you expect?** The run continues the same provider session. The idle source sandbox holds exactly the backed-up state, so restoring the backup into the new sandbox is safe. **Steps to reproduce** 1. Finish a warm native run on a remote sandbox. Its workspace export is still saving. 2. Start the next run on the same task before the export settles. 3. Reuse skips the exporting sandbox and creates a new lease. Harness preparation fails with `backup_without_reusable_lease`. 4. A retry resumes that new lease, which has no harness state, and fails with `resumed_unavailable`. ## What Changed - `native-retired-backup-source.ts`: an idle warm source is a third retired backup source. It must be `released` or `retained` with policy `reuse_by_environment`, its own run must be terminal and not the current run, and it must have no cleanup in flight, no restore hold or pending shutdown settlement, and no pending native workspace export. The backup stamp, single-owner chain, and later-provider-work checks still apply. - Before the restore, the idle source is claimed with a compare-and-swap on the checked row. The claim moves its `reusableSandboxLease` scope into a `reusableSandboxLeaseSuperseded` receipt. Reuse never resumes that older state again, and the next acquisition treats it as obsolete and destroys it. If a run resumed or changed the source first, the claim matches no row and the backup is not restored. - `environments.ts`: a cross-run reuse handoff now requires that the source row has no superseded receipt. The supersede claim and the handoff compete on the same row, so an acquisition that selected the source before the claim cannot resume it afterward. - The proof now rejects a backup whose source is the target allocation itself (`targetProviderLeaseId`). - `native-session-executor.ts`: a resumed lease whose sandbox has no harness state at all (`unavailable`) restores the backup under the same proof. This recovers the lease that the failed run left behind, so rows saved by older releases recover after deploy without a migration. - `doc/execution-semantics.md`: documents the idle-source rule, the supersede claim, and the resumed stateless lease. ## Verification - Red before the fix, green after: - `native-session-executor.test.ts` "restores a warm backup into a created lease only when an idle warm source finished its run and its export" failed with `backup_without_reusable_lease`. - "... a resumed lease never held the session and the warm source is idle" failed with `resumed_unavailable`. - `native-retired-backup-source.integration.test.ts` (real Postgres) "authorizes an idle retained/released warm source and claims it as superseded" failed (returned false). - Integration test "refuses a reuse handoff of a source superseded while its provider resume was pending" fails without the handoff guard and passes with it. - Safety rows stay refused: source run still running, workspace export pending, cleanup in flight, restore into its own allocation, stamp names another checkpoint, provider work after the backup, two host leases for one allocation, claim lost to another run, and a resumed lease that lost its own state. Existing destroyed-source and restore-hold rows are unchanged. - `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts src/services/native-runtime/native-retired-backup-source.integration.test.ts src/services/native-runtime/remote-warm-session-recovery.test.ts` passes (1408 tests). - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` exits 2 because a global CI input (`doc/execution-semantics.md`) changed. Required CI supplies the broad coverage. ## Risks - Previous-release story: no persisted shape changes. Lease rows saved by earlier releases carry forward. The new `reusableSandboxLeaseSuperseded` metadata key is additive. - The idle source is destroyed at the next acquisition for its workspace. Its state is the verified backup, which the restored lease now holds, so no work is lost. - Not covered: when the source cannot be proven retired or idle (for example a live owner or later provider work), the run still fails closed with the same named code. An automatic fresh-session fallback for that case is not in this PR. - The root trigger (reuse skips a sandbox while its export saves, then waits for that export) is unchanged. This PR does not change the reuse lookup. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, with tool use and code execution in Claude Code. ## 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> |
||
|
DottaandPaperclip
|
76c787fffc |
fix(runner): report monitor notes length limits (#20604)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents use task monitors to resume a future check. > - The monitor schema limits notes to 500 characters. > - The field description did not state the limit. Rejection showed only the schema path. > - This pull request adds the limit to the description and to safe length errors. > - Agents can correct notes before they retry a rejected call. ## Linked Issues or Issue Description Related prior fix: #16868. **What happened?** A native `set_task_monitor` call with 501 characters in `monitor.notes` failed with a path that ended in `notes/maxLength`. The error did not state 500. The schema already had `maxLength: 500`, but the field description did not state it. **Expected behavior** State the limit in the field description and in the rejection. Accept 500 characters. Reject 501 characters before dispatch. Preserve a saved monitor when a replacement is invalid. **Steps to reproduce** 1. Supply a future `monitor.nextCheckAt` and 501 characters in `monitor.notes`. 2. Read the native provider error. 3. Before this fix, it shows the constraint path without the maximum. **Paperclip version or commit** Reproduced on `7368a0b15c2f02107778a961144d86e2eb59a952` in a Linux source checkout, with Node 25.9.0 and Rust 1.97.1. ## What Changed - Use one local value for the published notes limit and its description. The server test checks agreement with the persisted shared validator. Keep the standalone Runner package boundary. - Read the numeric maximum from the authorized JSON Schema validation error. Keep submitted values private and preserve the 512-byte feedback bound. - Test the published schema and real bridge dispatch path at 500 and 501 characters. Test saved contents and rejection without monitor changes with real PostgreSQL. - Refresh generated contracts and document the limit. - Sync two SDK proof lock identities after the reviewed master lock refresh in #20615. This fixes the remaining CI proof mismatch. Keep compiler, parser, Zod, and all rejection checks unchanged. ## Verification - Final head: `bac60234ab3a8fc7043a90c7f9c81b02e250428b`. - Red: the server regression failed because the field description omitted `500`. The Rust regression failed because the error omitted `maximum 500 characters`. - Local passes after the final base update: 44 PostgreSQL monitor/scheduler tests, 14 real-source Rust bridge tests, 11 cache/Docker guard tests, and 56 SDK proof tests. Runner TypeScript build and generated contract checks pass. - The focused Rust check compiles the actual source in a small Cargo workspace. The full Rust CI suites also pass on Linux and Windows. - Final-head [PR CI run](https://github.com/paperclipai/paperclip/actions/runs/42881633978) passes. This includes typecheck, build, Runner support, Runner Rust and Vitest, server tests, serialized tests, and all eight e2e shards. - Final-head [Windows Rust run](https://github.com/paperclipai/paperclip/actions/runs/42881633419) passes. - Greptile: 5/5 on the final head, with no open findings. There are 61 successful checks and five conditional skips. The separate Paperclip Review remains queued; its comment says work is paused. No CI or test check is pending or failed. - `pnpm verify` returned exit 2 and selected broader CI. The full local suite was not run. Final-head CI supplies that coverage. Earlier failed attempts were repaired and are not counted as passes. ## Risks - Low risk. The existing 500-character bound, scheduling semantics, and permissions stay in place. - Length feedback uses only a schema path and numeric bound. It does not print submitted text. - The Runner binary and tool contracts must reach an instance for the new guidance to apply. This PR does not deploy or merge. ## Model Used OpenAI Codex, GPT-6, with reasoning, tool use, and code execution. The runtime does not expose a more specific model ID or context window size. ## 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
38944cff1a |
fix(environments): retry and safely replace a reusable sandbox that will not resume (#20626)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A task that runs on a reusable (warm) cloud sandbox resumes the same sandbox on the next wake > - When a follow-up arrives while the provider is auto-stopping or archiving the idle sandbox, the resume can fail > - Each failed attempt waited the provider's whole five-minute start deadline and then failed with `reusable_sandbox_resume_failed`, even when the workspace was saved and nothing lived only on the sandbox > - Two such runs in a row made a user wait for many minutes and recorded two failed runs > - This pull request bounds the resume, retries a state change inside the run, and replaces the sandbox from the saved snapshot when that is safe > - The benefit is that recovery works automatically and the first turn is not delayed by minutes ## Linked Issues or Issue Description **What happened?** Two consecutive runs failed before provider dispatch with: `Reusable sandbox lease "<id>" could not be resumed (cause=provider_error); the lease was preserved and no replacement was created.` Each run took about 5.5 minutes (320s and 330s) in preparation. The third run resumed the same sandbox from `stopped` in 4.7 seconds after the provider settled. The previous run had ended with a clean final save and a verified harness backup. **Steps to reproduce** 1. Let a task's reusable sandbox go idle so that the provider auto-stops or archives it. 2. Send a follow-up while the provider still reports the sandbox as changing state. 3. The Daytona worker waits in `ensureSandboxStarted` until its 300s start deadline and throws "is still <state> at its start deadline". The host does not classify that as transient and it has no safe fallback, so the run fails. **Expected behavior** The run retries briefly. If the sandbox still cannot start and nothing lives only on it, the run continues on a replacement sandbox that restores the saved workspace. **Root cause** `server/src/services/environment-runtime.ts` (reusable resume block, about line 1820): the resume deadline was the provider's full start deadline (300s plus RPC margin), the transient classifier did not recognize a state transition, and the catch always threw `ReusableSandboxResumeError`. #20212 waits out a state change inside the same 300s start deadline, so it covers the start but not the time budget or the fallback. ## What Changed - Add `server/src/services/reusable-sandbox-resume-replacement.ts`: - `REUSABLE_SANDBOX_RESUME_BUDGET_MS` (90s): the total resume time for one run, retries included. - `sandboxStateTransitionFailure`: "state change in progress" and "is still <state> at its start deadline" are transient. - `retireUnresumableReusableSandboxLease`: replace only when the lease is `released`/`retained` with no live owner, the task workspace is clean with a committed saved snapshot and no other writer, and a `paperclip_runner` session has a verified lease-bound harness backup. Access refusals (401/403) and an unavailable worker stay strict. After the replacement lease (which carries the harness backup) is recorded, the old lease moves to `pending_cleanup` (reason `resume_failed`) in one guarded write, so the existing cleanup sweep destroys the old sandbox and no provider delete runs before the first turn. A failed creation or a restart before that write leaves the old lease `retained` for the next attempt. - `environment-runtime.ts`: cap the resume deadline at the budget, treat a state transition as transient, and call the replacement policy before failing. A replacement records `sandboxLeaseAcquisition: { outcome: "replacement", reason: "resume_failed" }`; workspace preparation then restores the saved snapshot. - `remote-workspace-transport.ts`: add the closed failure label `sandbox_state_transition`, so a strict failure names the cause instead of `provider_error`. - `doc/execution-semantics.md`: document the budget, the in-run retry, the replacement conditions, and the strict cases. - Update the two slow-start tests: the resume RPC timeout is now 90s instead of 330s/150s. A 61s start still resumes the same sandbox. ## Verification - Red before the fix (new file `server/src/__tests__/environment-runtime-reusable-resume-replacement.test.ts`, staging shape: lease created by the real acquisition path, then retained after a clean save with a backup stamp, 300s provider deadline, worker refusal with no status): 3 tests failed with `could not be resumed (cause=provider_error); the lease was preserved and no replacement was created.` - Green after the fix: 10/10 pass, including a case where the replacement creation fails and the next run replaces the sandbox with the backup carried forward. The safe case replaces the sandbox, keeps every resume RPC at or below 90s, makes no synchronous destroy call, and moves the old lease to `pending_cleanup`. A single state-change refusal resumes the same sandbox inside the run. Six strict cases keep the lease and fail with `cause=sandbox_state_transition`: unsaved work, no saved version, an uncommitted saved version, a live owner, no verified backup, and a 403. - `pnpm exec vitest run src/__tests__/environment-runtime.test.ts` (197 pass), `heartbeat-retry-scheduling`, `legacy-execution-recovery`, `remote-workspace-transport` (251 pass). - `tsc -p server/tsconfig.json --noEmit` passes. - `pnpm verify` exits 2 (a doc change is a global CI input). Required CI supplies the broad coverage. - Not run: a live Daytona run on staging. The provider message is kept off the run record, so the exact transition state on staging (stopping or archiving) is inferred from timing, not observed. ## Risks - A strict resume now gives up after 90s instead of 300s. The provider start continues in the background, and the bounded bootstrap retry resumes the same sandbox. A stopped sandbox resumes in seconds and an archived one in about 25s on Daytona, so 90s leaves headroom. - A replacement leaves the old sandbox to the cleanup sweep. If the old start finishes later, the sweep still destroys it. The guarded write only claims a `released`/`retained` lease with no live owner. - Upgrade safety: no new persisted fields are required. Leases saved by older releases are read with the same rule; one without a verified backup stays strict. The replacement reason `resume_failed` already exists in the acquisition outcome. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## 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> |
||
|
DottaandClaude Opus 5.5
|
9b21dcbdda |
refactor: remove SPEC docs and repoint spec citations (#20565)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors and agents read the docs under `doc/` before they change code > - `doc/SPEC.md` and `doc/SPEC-implementation.md` were out of date. `doc/SPEC-implementation.md` became an append-only log: in 60 days it got about 1,645 new lines and lost 181, across 140 commits > - #20557 changed `AGENTS.md` so the code and its tests are the source of truth, and the specs are only background > - This pull request removes the two spec docs. It moves only the sections that live code comments cite into focused docs, and it repoints all other references > - The benefit is one less stale document that agents read and append to ## Linked Issues or Issue Description Refs #20557 (merged). This PR is the follow-up that removes the docs. It is based on `master`, which already contains #20557. **Issue type** Outdated content. The docs are out of date and conflict with the code. **Where is the issue?** `doc/SPEC.md`, `doc/SPEC-implementation.md`, and the code comments and docs that cite them. **What's wrong?** `doc/SPEC.md` and `doc/SPEC-implementation.md` are out of date, but agents still read them and append a paragraph with each fix. **Suggested fix** Delete both files. Move the sections that code cites into focused docs. Repoint the references. ## What Changed - Deleted `doc/SPEC.md` and `doc/SPEC-implementation.md`. - Moved these spec sections, because live code comments cite them: - §9.3.1 "Shared default-open issue writes" → new section in `doc/agent-permission-defaults.md`. - §9.8.1 "Issue-thread interaction resolver contract" → `doc/execution-semantics.md`, "Issue-thread interaction resolution". That section already had most of the contract. I added only the missing parts: the `supersedeOnUserComment` default, the canonical policy list, the addressee and cap rules, the rule that surfaces show the server's effective audience, and the cross-issue budget charge. The short policy summary at the top of the section now points to the list, so the definitions occur one time only. - §9.9 "Task Watchdog Authority Contract" → new "Authority contract" section in `doc/TASK-WATCHDOG.md`. It holds the allowed and disallowed mutation lists and the deferred capabilities. The atomic recovery batch, restoration verification, and interaction rules were already in `doc/execution-semantics.md` §11, so the new section links there and does not repeat them. I did not move the "Downstream acceptance criteria" checklist or the sentence about the CTO and Security sign-off. These were implementation-time notes; the tests now hold that coverage. - §9.10 "Company Skill Policy Contract" → new `doc/company-skill-policy.md`. No focused doc had this contract. I did not move the "Required regression tests" phase checklist. - Did not move any other part of the specs. - Repointed code comments that cited spec sections: - `packages/shared/src/issue-write-denial.ts` (§9.3) → `doc/agent-permission-defaults.md` - `server/src/routes/issues.ts` (§9.9) → `doc/TASK-WATCHDOG.md`, "Authority contract" - `server/src/routes/issues.ts` (§9.3, §9.8.1 cross-issue budget) → `doc/agent-permission-defaults.md` - `ui/src/lib/interaction-audience.ts` and `ui/src/components/InteractionGovernancePanel.tsx` (§9.8.1) → `doc/execution-semantics.md` - `ui/src/lib/skill-policy-denial.ts`, `ui/src/pages/CompanySkills.tsx`, `ui/src/pages/CompanySkills.production.tsx`, `ui/src/pages/SkillStudio.tsx` (§9.10) → `doc/company-skill-policy.md` - `server/src/services/recovery/ownership-invariant-report.ts` (monitor deadline wait path) → `doc/execution-semantics.md` §8. That section already lists the one-shot monitor as a valid wait path. - Resynced the git blob pin for `packages/shared/src/issue-write-denial.ts` in `tests/runner-e2e/source-sdk-public-inputs.json`. The comment edit changed the file's hash, and the Runner support suite checks that pin. I used `node tests/runner-e2e/source-sdk-public-inputs.mjs <HEAD> --write`; only that one pin changed. - Updated `AGENTS.md` §1 so it does not name the deleted files. - Removed the links to the deleted files from `doc/plugins/PLUGIN_SPEC.md`. - Updated the `doc-maintenance` agent skill (`.agents/skills/doc-maintenance/`) so it no longer audits or stages `doc/SPEC.md`. - Removed or replaced references in `maps/README.md`, `maps/feature-map/README.md`, `maps/storybook-map/README.md`, `doc/PRODUCT.md`, `doc/CLIPHUB.md`, `doc/DEPLOYMENT-MODES.md`, `doc/DEVELOPING.md`, `doc/evals.md`, `doc/spec/agent-runs.md`, `doc/AGENTCOMPANIES_SPEC_INVENTORY.md`, `doc/connections/CONNECTOR-PLAYBOOK.md`, `doc/plugins/ideas-from-opencode.md`, `doc/execution-semantics.md`, `doc/TASK-WATCHDOG.md`, and `.devin/wiki.json`. - In `doc/execution-semantics.md`, the "admission is independent of cleanup and accounting" reference now states the rule inline: only a hard stop, an explicit pause, or an inactive company refuses admission. - Kept these references as they are: dated plans in `doc/plans/` and `docs/plans/`, the dated log `doc/logs/2026-05-24-cli-api-parity-e2e-log.md`, `releases/`, the transcript fixture `ui/src/fixtures/runTranscriptFixtures.ts` (it uses the filename only as sample text), and `docs/specs/external-task-protocol.md` (it refers to the Symphony project's own `SPEC.md`). ## Verification - `git grep -n "SPEC-implementation\|SPEC\.md"` (without `releases/`, and ignoring `PLUGIN_SPEC.md` citations) finds only the kept references listed above. - `grep -rn "§9\.\(3\|8\|9\|10\)" ui/src server/src packages/shared/src` finds no results. - `node scripts/check-maps.mjs`: "Maps aligned: 35 areas, 192 capability IDs". - `pnpm check:token-gates`: "All gates clean". - `pnpm exec vitest run --config tests/runner-e2e/vitest.config.ts tests/runner-e2e/source-sdk-public-type.test.ts tests/runner-e2e/source-sdk-evidence.test.ts`: 45 passed. These failed in CI before the pin resync. - `pnpm verify`: exit 2 ("Broader verification required") because `AGENTS.md` is a global CI input. Required CI supplies the broad coverage for this revision. - All code changes are comments only. No runtime behavior changes. The title uses `refactor:` because comments in source files changed. ## Risks - Low risk. The change removes docs and edits comments only. - A reader can lose context from spec sections that I did not move. The text stays in git history. The code and its tests are the source of truth. - External links to the two deleted files will return 404. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), in Claude Code with tool use (shell, file edits). Standard context window. ## 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 checks above; CI supplies broad coverage) - [x] I have added or updated tests where applicable (not applicable: docs and comments only) - [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> |
||
|
DottaandClaude Opus 5.5
|
86cc3e3a11 |
fix(workspaces): save remote workspaces that contain outside symlinks (#20569)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents on a remote sandbox (for example Daytona) save their workspace after each run, and recovery retries a failed save > - The save refused the whole workspace when one symlink was absolute or pointed out of the workspace > - Ordinary agent work makes such links: a headless Chromium profile links `SingletonSocket` by absolute path into `/tmp` > - Recovery then spent its full budget on the same refusal, handed the hold to the board, and the task stayed blocked > - This pull request keeps confined links, records and omits outside links, and retries saves that an older policy exhausted > - The benefit is that these saves succeed and blocked tasks continue without a person ## Linked Issues or Issue Description **What happened?** On staging, a native run (OpenCode on `paperclip_runner`, Daytona sandbox) built Storybook and drove Chromium with `--user-data-dir=<workspace>/.review-chrome`. Chromium wrote `.review-chrome/SingletonSocket -> /tmp/.org.chromium.Chromium.*/SingletonSocket`. The save failed three times with `remote_workspace_archive_unsafe_symlink` (`native_workspace_sync_out_retry_exhausted`). System recovery then tried six more times. It recorded each failure as `export_interrupted` with cause `provider_error`, became `exhausted`, and gave the hold to the board. The task and its parent stayed blocked. **Expected behavior** The save succeeds for an ordinary workspace that contains symlinks. Links inside the workspace stay links. Absolute and escaping links are dropped and recorded, and are never followed. Holds that an earlier release exhausted recover automatically after deploy. **Steps to reproduce** 1. Create a workspace with `node_modules/.bin/tool -> ../tool/cli.js`, an absolute link to a file outside the workspace, and a link `escape -> ../outside/file`. 2. Capture it with `captureRemoteWorkspaceArchive` (or the serialized sandbox helper). 3. Before this change, the capture fails with `remote_workspace_archive_unsafe_symlink`. ## What Changed - `remote-workspace-snapshot-archive.ts`: capture records an absolute or escaping symlink in the manifest `excluded` list and omits it, instead of failing the save. It never follows a link. Confined links are kept. Restore validation is unchanged and still refuses an archive that names an outside link. - Added `REMOTE_WORKSPACE_CAPTURE_POLICY_VERSION = 2`. Each new system recovery generation records `capturePolicyVersion`. Generations without the field count as version 1. - New `workspace-recovery-capture-policy.ts`: an exhausted save whose last failure was in `preserve_files` (`export_interrupted`) under an older capture policy gets one new bounded generation. Restore refusals (`archive_unsafe`) are not re-armed, because restore validation is unchanged. It shares the run-over guard (`sourceRunProvablyOver`) with the stop-proof re-arm. It writes a `capturePolicyRecovery` marker and `issue.workspace_recovery_rearmed` activity with reason `capture_policy_changed`. The new generation has the current version, so the rule cannot repeat for it. - The recovery worker discovers these holds (`capturePolicyRearmPendingCondition` in `nextWorkspaceRecoveryAt` and the obligations query). - Docs: `doc/native-workspace-finalization-recovery.md` (symlink policy) and `doc/execution-semantics.md` (capture-policy re-arm). Previous-release story: archives saved by the previous release never contain outside links, and restore of them is unchanged. Recovery state saved by the previous release has no `capturePolicyVersion`; it is read as version 1 and, if exhausted in `preserve_files`, is re-armed once on the first worker pass after deploy. No migration is needed. A capture already in flight at the cutover uses the helper that its operation installed; the next attempt installs the new helper. ## Verification Red before the fix (same tests, unfixed code): - `remote-workspace-snapshot-archive.test.ts` "keeps confined links and drops outside links" (local and serialized helper): failed with `remote_workspace_archive_unsafe_symlink`. - `remote-workspace-transport.test.ts` "saves a workspace with outside links through the sandbox helper" (split and Daytona one-shot output): failed with `remote_workspace_archive_unsafe_symlink`. - `workspace-recovery-capture-policy.test.ts` "retries once under the new policy": failed, because the worker did not discover the exhausted hold (`nextWorkspaceRecoveryAt` returned null). Green after the fix: - `npx vitest run --root server src/services/remote-workspace-snapshot-archive.test.ts src/services/remote-workspace-transport.test.ts src/services/workspace-recovery-capture-policy.test.ts`: 271 passed. - The recovery test uses the exact staging evidence shape (6 attempts, `export_interrupted`, `provider_error`, board owner, no policy version). It ends `complete` and `recovered`, saves a snapshot, and leaves the source untouched. - Safety cases: the archive does not contain the outside bytes; restore refuses a hand-made archive with an escaping link; a hold exhausted under the current policy is not re-armed and no sandbox resume is attempted. - `npx tsc --noEmit -p server/tsconfig.json`: clean. - `pnpm verify`: see CI for the broad selection. ## Risks - A tracked symlink that is absolute or escaping is now omitted. After restore it shows as a deletion in `git status`. Before, the whole save failed. - On deploy, every hold exhausted in `preserve_files` with `export_interrupted` gets one more bounded recovery generation, including holds that failed for provider reasons. This is one generation per hold per policy version. - The manifest exclusion label stays `paperclip.remote-workspace-exclusions.v1`. Workspaces that the old policy accepted produce the same manifest, so unchanged-snapshot reuse still works. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use 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 - [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> |
||
|
DottaandClaude
|
c37eacbee3 |
fix(native): Retry re-finalizes a saved result that ran out of finalization retries (#20578)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs save an accepted result. Then a finalizer applies that
result to the task.
> - Finalization has its own budget of 3 failed attempts (#20555). When
the budget runs out, the coordinator goes to `terminal_failure` with
`native_finalization_retry_exhausted`.
> - For a provider-succeeded run, the task then rests `in_review` with a
board recovery card. The task shows "resolved" and offers no Retry. The
user is stuck.
> - The ordinary Retry is also wrong here: it starts a new provider run,
which repeats work the agent already finished.
> - This pull request adds two things. First, contention failures on
such a result get two longer automatic delays before the board gets the
task. Second, when the state is still exhausted, the task offers Retry,
and that Retry applies the same saved result again.
> - The benefit: fewer tasks need a Retry, and when one does, Retry
works and does no duplicate provider work.
## Linked Issues or Issue Description
Refs #20555 (it names this gap under Risks).
**What happened?**
In restart-chaos wave `w-20261110T044338-s20385`, task t03 ended with a
succeeded run, an accepted result, and a coordinator in
`terminal_failure` / `native_finalization_retry_exhausted`. The issue
stayed `in_review` with a board-owned `active_run_watchdog` card and
`canRetry=false`. #20555 stopped the early exhaustion. A real exhaustion
still leaves the same stuck state. Coordinators that older builds
already left in this state are stuck the same way.
**Expected behavior**
Transient finalization failures recover by themselves when that is safe.
When they do not, the task offers Retry. That Retry re-finalizes the
saved result. It does not start a provider run and does not charge a
provider attempt.
**Steps to reproduce**
1. Let a native run save an accepted result and end `succeeded`.
2. Make each finalization of that result fail until the budget of 3 runs
out.
3. Before this fix, the task presentation is `resolved` with
`canRetry=false`, and the board card has no action that applies the
result. The new test
`server/src/__tests__/native-exhausted-finalization-retry.test.ts`
reproduces this.
**Paperclip version or commit**
master at
|
||
|
DottaandClaude Opus 5.5
|
be0d100a8b |
fix(native): settle cleanup-gate refusals after a completed turn without a dead-end Retry (#20572)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs execute a provider turn, then close the provider session
and prove it stopped
> - When close cannot prove that the old runner, provider and tools
stopped, the runner holds a cleanup gate
(`native_session_cleanup_quarantined`) that refuses the next session on
that task
> - A run finished its provider turn and saved its work. A server
restart had left one tool callback without a saved result, so close
could not prove the drain, and the gate refused the run's own same-run
recovery 30 seconds later
> - The task then showed "Couldn't finish this attempt" with a Retry
button, but the retry route refuses this error code with 409. The saved
error text told an operator to act
> - This pull request makes the task projection, the execution blocker,
the executor and the retry route agree, and replaces the operator text
with plain wording
> - The benefit is that finished work no longer looks like a failed
attempt, and the UI never offers a Retry that the server refuses
## Linked Issues or Issue Description
**What happened?**
A remote native run completed its provider turn and saved its comments
and documents. A server restart during the turn left one semantic tool
callback without a saved result. Session close then reported
`native_session_settlement_incomplete` (runner suspended, provider drain
unproven), and the runner kept an operator-required cleanup gate. The
same-run disposition recovery was refused at admission, and the run
ended `failed` with `native_session_cleanup_quarantined`.
**Expected behavior**
The task should show its real state (here, waiting on a child task). It
should not show a failure banner or a Retry that the server refuses. No
text should ask an administrator to act.
**Steps to reproduce**
1. Start a remote native run and restart the server while a tool
callback runs.
2. Let the resumed turn complete without a semantic result, so a
same-run recovery is scheduled.
3. Observe the refusal at admission, the "Couldn't finish this attempt"
banner, and a Retry that returns 409.
**Paperclip version or commit**
master at
|
||
|
Nicky LeachandPaperclip
|
2e3cbe57a4 |
fix(native): preserve parked Runners across controller restarts (#20586)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native execution parks a released sandbox Runner for a later turn. > - The warm idle timer detaches Core and leaves Runner retention to the host. > - Controller shutdown instead stops recently parked Runners through checkpoint cleanup. > - This makes reuse depend on whether the idle timer fired before shutdown. > - This pull request uses the same detach operation for idle expiry and shutdown. > - The benefit is consistent Runner retention without a new retention or recovery mechanism. ## Linked Issues or Issue Description Fixes: #20012. Related: #20005 unified cached and adopted process reuse. Closed #19975 included this shutdown change within a broader retention proposal. ## What Changed - Detach confirmed released entries during controller shutdown. Keep busy, unreleased, and close-marked entries on the existing cleanup path. - Share the detach operation with idle expiry and cached-owner handoff. - Record that a local handle has started detaching. Later cleanup cannot fall back to stopping its Runner after a partial detach failure. - Preserve explicit environment deletion, release proof, admission, and process-reuse checks. - Add Codex and Claude restart, detach-failure, and environment-deletion regressions. Add real Runner adoption cases after the restart sweep, including managed GitHub rebinding. - Document the lifetime rule and make the shutdown warning cover both detach and checkpoint failures. ## Verification - Final source revision: `e4bb6ecbd7`. Local environment: macOS arm64, Node 26.12.0. - New controller-restart and detach-failure tests fail against unchanged master: 4 expected failures. Old shutdown calls close or reports success instead of reporting detach failure. - Focused executor tests: 22 passed. - Full native session executor suite: 1,328 passed. - `cd server && node scripts/typecheck-layers.mjs`: passed all source and test layers. - `node scripts/check-module-boundaries.mjs` and `git diff --check`: passed. - Manual diff review and added-line credential/PII scan: passed. - `cd server && node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/plugin-runner-release-resume.integration.test.ts --bail=1 --reporter=verbose`: 14 passed on this revision. This includes new Codex and Claude restart cases and managed GitHub rebinding. - Failure history: an earlier concurrent run had 4 passes and 10 failures. Its first failure was the existing Claude cached case exceeding 60 seconds. Later cases encountered a leftover prototype spy, recursive calls, and startup timeouts. No source changed before the passing full rerun. A clean worktree of unchanged master (`e01fe7f84a`) passed all 11 original cases. The first timeout's exact cause remains unknown; the later pass does not establish that cause. - Standard Greptile review: 5/5 on `e4bb6ecbd7bd965b5f56507d2c42c580e8a1d547`, with no actionable findings. - `node scripts/local-verify.mjs --explain` returned exit 2 because the execution-semantics document requires broad CI. Required CI will supply broad typecheck, test, and build coverage. - A hosted deployment and an actual controller process replacement have not been run. The added integration cases exercise the shutdown sweep and a new adopting transport against a real Runner. ## Risks - SIGINT and SIGTERM preserve confirmed parked Runners, including ordinary service stops. The host still owns eviction and retention. - Detach failure retains a fenced local owner and reports failure. Core does not infer permission to stop the Runner from that failure. - Reuse remains conditional on current admission, process configuration, and host availability. - No persisted schema, checkpoint format, launch identity, or migration changes. State from the previous release uses the existing adoption or verified restore paths. ## Model Used - OpenAI Codex, GPT-6, with reasoning, repository inspection, tool use, and local test execution. The exact deployment identifier and context-window size 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 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 Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
96ab5e3d38 |
fix(recovery): read a retained Runner from its own allocation and continue unverifiable no-replay holds from the last save (#20567)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native warm runs on a remote sandbox continue a saved session in the next run. The next run first reads the retained Runner lifecycle from the sandbox. > - #20346 abandons the workspace epoch of a run that died during preparation. The next run then restores the last save into a new directory of the same sandbox. > - The retained-sandbox binding check required the old and the new directory to be the same. The warm follow-up failed with `native_controller_recovery_missing_evidence:retained_sandbox_binding` before provider work. > - The failed run's no-replay hold then refused its files with `allocation_ownership_ambiguous`. The task stayed blocked. The text said that Retry "succeeds only after Paperclip is updated", so no click could help. > - This pull request binds the Runner read to the directory that the retained lease recorded, and lets the existing retained-source fallback settle these holds from the last save. > - The benefit is that a moved workspace continues its session, and a hold that cannot be verified continues automatically instead of dead-ending. ## Linked Issues or Issue Description Refs #20346 **What happened?** On staging, a warm native continuation ran about 10 minutes after a deploy. The run before it died during preparation (database deadlock), so its workspace epoch was abandoned. The continuation restored the last save into a new allocation directory of the same Daytona sandbox. It then failed with `native_controller_recovery_missing_evidence:retained_sandbox_binding`, because the retained lease of the committed source run recorded the old directory. The failure recorded a no-replay hold (`native_continuation_requires_reconciliation`, resolved with outcome `blocked`). Workspace recovery refused the source with `allocation_ownership_ambiguous`, because the sandbox's reusable-lease chain had predecessors in another directory and a non-native preparation-only run. The task stayed blocked with "Retry runs the check again and succeeds only after Paperclip is updated." **Expected behavior** The continuation reads the retained Runner state from the directory where that Runner wrote it, and continues. When retained files cannot be verified, the task continues from the last committed save without a person. **Steps to reproduce** 1. Run a warm native session on a reusable remote sandbox and let it commit. 2. Fail the next run during preparation, so that its epoch is abandoned (#20346). 3. Start the next warm continuation. It restores the last save into a new allocation directory and fails with `retained_sandbox_binding`. 4. See the hold refuse its source with `allocation_ownership_ambiguous` and stay blocked. **Paperclip version or commit** Staging `c874cfa4dacc` (includes #20346). ## What Changed - `assertNativeRetainedSandboxBinding` now returns the directory that the retained lease recorded. It accepts a different target directory only when both are canonical sibling allocations of the same execution workspace under the sandbox's `.paperclip-workspaces` (`<workspaceId>` or `<workspaceId>-<epoch>-<leaseId>`). Same sandbox (`providerLeaseId`), environment, run, issue and committed owner are still required. Other directories stay refused. - `restoreRemoteSuspendedRunnerCheckpoint` reads the Runner state from that recorded directory. A moved source must be suspended and settled. A Runner left in the old allocation is never adopted live. - The retained-source fallback (`continueFromLastSavedWorkspace`) and its discovery now also cover the native no-replay hold (`native_continuation_requires_reconciliation`, active or resolved with outcome `blocked`, replay blocked for the same run). Rows saved by earlier releases are discovered without migration. - The fallback owns the Blocked status of such a hold only while the recorded `nativeFailureBlock` status version still matches the task. Otherwise it keeps the newer block and starts no continuation. - The refusal text no longer says "succeeds only after Paperclip is updated", and it promises no fallback. - `doc/execution-semantics.md` describes both rules. ## Verification - Red (unfixed code): `native-controller-recovery.integration.test.ts` "binds a retained Runner read to the source's directory after the workspace moved within the same sandbox" failed with `native_controller_recovery_missing_evidence:retained_sandbox_binding` at `native-controller-recovery.ts:145`, the staging error. - Red (unfixed code): `workspace-recovery-fallback.test.ts` "continues a resolved-but-blocked native no-replay hold from the last save, discovered by the worker" failed: the worker found no due work for the exact staging row. The newer-block safety case and the message case in `system-recovery-state.test.ts` also failed. - Provider boundary: a new test drives `restoreRemoteSuspendedRunnerCheckpoint` through a sandbox runner that refuses paths outside the sandbox workspace base. The suspended case reads `runner-state.json` from the source directory, never the moved root. The ready case is refused with `runner_not_settled` and caches nothing. - Green: `native-controller-recovery.integration.test.ts`, `workspace-recovery-fallback.test.ts` and `system-recovery-state.test.ts` pass (45 tests). The neighbouring suites `workspace-recovery-retry-copyback`, `workspace-recovery-preparation-gates` and `workspace-recovery-stop-proof` pass (135 tests). - Safety cases: another workspace, another parent, a non-canonical path or name (`-0-`, trailing text), `/unrelated` and another sandbox stay refused. A hold whose task moved to a newer Blocked decision keeps the block and starts no continuation. Exactly one continuation is delivered. Native replacement is denied for the abandoned source, so no second executor starts. - `npx tsc --noEmit -p server/tsconfig.json` passes. `pnpm verify` returns exit 2 (a global doc input changed), so required CI supplies the broad coverage. ## Risks - Previous-release state: rows saved by earlier releases carry forward without migration. The binding reads existing lease metadata. The fallback discovers existing resolved-but-blocked holds by their existing evidence fields. - The binding now accepts a sibling allocation directory. The read still binds to the exact sandbox, run, environment and committed owner, and it reads only the source's recorded directory. A non-suspended Runner in a moved allocation is refused, not adopted. - The fallback abandons the unverified epoch and continues from the last committed save with a fresh session. In the staging shape the run did no provider work after it restored that save, so no work is lost. Unverified files are attached to the task when they can be read, as before. - Not verified live: a real warm Codex resume after a moved allocation on Daytona. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, read-only browser access to staging). Extended reasoning enabled. ## 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> |
||
|
|
f4337bb5fb |
test(evals): land the chat catalog eval harness on master (#19614)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The chat-feature evals measure whether agents complete real chat requests through the product > - The chat catalog eval harness and its repairs live on the `chat-eval/master-candidate` branch, not on master > - Today's work made 139 of 242 chat evals pass in verified single-case live runs. Each product fix landed on master as its own pull request > - The eval-side changes (setups, graders, fault seams, evidence capture) still exist only on the candidate > - This pull request lands the harness on master. Master stays the source of truth for product behavior > - The benefit is one tree: evals run against master commits, and product fixes reach the evals as soon as they merge ## Linked Issues or Issue Description Refs #19441, Refs #19599, Refs #20270, Refs #20334, Refs #20342, Refs #20344, Refs #20354, Refs #20360, Refs #20373, Refs #20382, Refs #20409 **What existing behavior does this improve?** The chat catalog eval harness and the product it evaluates live on a long-running branch outside master. Eval fixes do not reach master, so master-based eval runs use old setups and graders. **Subsystem affected** `tests/runner-e2e` chat catalog harness, plus a small set of product changes that exist only on the candidate. **Proposed change** Land the harness on master. Every product fix from this eval work already merged to master as its own pull request. Conflicts keep master's product behavior. ## What Changed - Most of the diff (808 of 851 files) is under `tests/runner-e2e/`: chat-catalog domains, scenarios, descriptors, the TestOps manifest, private evidence tooling, and their tests. - Graders now check saved state, setup literals and rendered or clicked links. Many keyword and regex checks on free-form replies are removed. Each changed grader has a bumped `oracleVersion` and calibration tests (a valid reply, a reworded valid reply, and broken variants that must fail). - The shared link helper `hasLink` no longer treats a list link (for example `/COM/issues?view=mine`) as a link to every task without a URL key. - Setup fixes build real state through supported product routes: routine history, agent grants through the board grant route, budget holds with a real cap, and a storage-layer archive fault for `inbox-search.archive-partial-failure`. - Browser boundary journeys (sign-in, chat-disabled, onboarding) now write the standard pass evidence files. - Retargeted cases, approved by the maintainer: `agent-chat.chat-disabled` (Conference Room gate), `agents.restore-instructions` (no saved instruction history), `access.manage-membership` (member permission edits stay board-only). - The private evidence trusted files (`manual-private-handoff.ts`, `manual-private-retention.ts`, `manual-private-report-assets.ts`) are byte-identical to master. - Product files outside `tests/` (43 files, +1,328 −49) carry candidate-only changes that master does not yet have. Most are tests. The main code changes are in `request-human-input.ts`, `heartbeat-run-summary.ts`, `native-completion-feedback.ts`, `native-run-finalizer.ts`, `openapi.ts`, `secrets.ts` and `IssueChatLongThreadPerf.tsx`. ## Verification - `pnpm vitest run chat-catalog-domains chat-catalog-contract.test.ts chat-catalog-consistency.test.ts unit-launcher.test.mjs --testTimeout=180000` in `tests/runner-e2e`: 269 files pass, 3,550 tests pass. - `node --test chat-full-catalog-testops-manifest.test.mjs`: 10 pass. - `node tests/runner-e2e/source-sdk-public-inputs.mjs <HEAD> --check`: no change. - `tsc` for `server` and `tests/runner-e2e`: clean. - Live single-case verification: 139 of 242 cases passed on paid runs, and a person read the saved evidence for each. These are single-case runs, not one full-cohort run. ## Risks - Large diff. Most of it is test-only, but the 43 non-test files change product code paths. Review those files first. - Some graders are now less strict on reply wording. A person must read replies for meaning on those cases. - 42 cases need disposable outside accounts (Notion, Slack, GitHub, Daytona and others) and stay blocked. `access.invite-teammate` stays failing until local agent isolation lands, because the runner's durable state keeps a live invite token. - The paid workflow compares the trusted private-evidence files with master. They are unchanged, so this risk is low. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution, in Claude Code. Subagents used the same model. ## 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> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
fea4530a30 |
fix(recovery): retry a preparation killed by a restart in the uncharged lane (#20534)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When the control plane restarts, an interrupted run must continue on its own. A user must not have to press Retry because a deploy or a crash happened > - A preparation that a restart stops before the provider starts spent no provider work. It is retried in the uncharged `interrupted_preparation` lane (12 retries), not the charged `transient_failure` lane (2 retries) > - #20496 made that work for a graceful drain (SIGTERM). A hard kill (SIGKILL) during workspace preparation still landed in the charged lane > - Two killed preparations in one chain then spent the failure budget, and the next interruption could not continue: the task needed a manual Retry > - This pull request lets restart recovery hand a killed preparation to the uncharged lane once the killed controller is proven gone > - The benefit is that a task survives any number of restarts during workspace preparation (up to the crash-loop cap) without a Retry click ## Linked Issues or Issue Description **Subsystem affected** Heartbeat restart recovery: the process-loss reaper in `server/src/services/heartbeat/recovery.ts`. **Problem or motivation** The restart-chaos harness on Daytona with `--storage s3 --storage-trigger snapshot-download` (seed 603, task t02, a codex C1 follow-up) kills the server while a workspace snapshot downloads. On builds that contain #20496, two replays still ended in `R3:retry_needed`. The run chain in replay `w-20261110T061107-s603-replay-t02-1794309789008`: 1. K1 SIGKILL during preparation: `process_lost`, then "Scheduled bounded retry 1/2 … transient_failure". 2. K2 drain during preparation: `interrupted_preparation` 1 (the #20496 path, correct). 3. K4 SIGKILL during preparation: `process_lost`, then "Scheduled bounded retry 2/2 … transient_failure". 4. K2 drain during preparation: the failure budget was spent (`failureRetries` 2), so the hold could not continue. The harness had to click Retry. **Root cause** The reaper does record the no-provider-work certificate (`executionRecovery: { kind: "bootstrap", providerWorkStarted: false }`) for a preparing run from another boot. But before it fails the run, `revokeExpiredLegacyControllerWithState` swaps in a random boot id with a 60 s lease, a tombstone that no controller holds. `scheduleBoundedRetryForRun` then asks `isInterruptedNativePreparation`, whose `preparerMayStillUnwind` check sees that live lease and says the old preparer may still be unwinding. The lane falls back to `transient_failure` and charges the failure budget. Every killed preparation is charged, whether or not its controller is provably gone. **Proposed solution** When the reaper has already proven the old controller's recorded process gone (`legacyControllerGone`, the shared owner prover that #20496 also uses) and the revoked state shows the run was still preparing with no process recorded (`restartInterruptedBeforeDispatch`), expire that tombstone lease in the same `process_lost` write. The lane decision then sees an expired lease and applies its existing checks (no coordinator, result or execution events; leases released with stop receipts; workspace repair when needed). Any evidence of provider work, or a controller that cannot be proven gone, keeps today's charged behaviour. **Alternatives considered** - Revoking with an already expired lease: the 60 s revocation lease fences other servers while this pass fails the run, so the expiry belongs in the terminal write. - Teaching `preparerMayStillUnwind` to ignore unstamped leases: that would also skip the wait for a lease of a live controller saved by an older release. ## What Changed - `server/src/services/heartbeat/recovery.ts`: the `process_lost` write sets `controllerLeaseExpiresAt` to now when `restartInterruptedBeforeDispatch && legacyControllerGone`. - `server/src/__tests__/heartbeat-process-recovery.test.ts`: - New: a chain whose preparation is killed twice (controllers proven gone) and then drained. Each kill schedules `interrupted_preparation` (attempts 1 and 2, `failureRetries` 0), and after the drain the restart settlement schedules `interrupted_preparation` 3 without a Retry. - New old-saved/new-resumed case: a preparing run saved by an older release without a controller process stamp keeps the charged `transient_failure` lane. - `doc/execution-semantics.md`: one additive sentence saying the lane also covers a hard kill once the killed controller is proven gone. ## Verification - Red/green: `npx vitest run src/__tests__/heartbeat-process-recovery.test.ts -t "killed twice"` fails on master (the first kill schedules `transient_failure`) and passes with the fix. - Related suites pass: `heartbeat-process-recovery`, `interrupted-preparation-hold-settle`, `execution-recovery-dead-controller`, `legacy-controller-lease` and `ownership-reconciliation-report` (5 files, 521 tests). - Live (Daytona, `--storage s3 --storage-trigger snapshot-download`, replay of `w-20261110T061107-s603/t02` with K1/K2/K4/K2, image `paperclip-daytona-runner@sha256:05a4a092…` whose runner content matches this branch): running; the result will be posted as a comment. ## Risks - Low. The change applies only when the shared owner prover returns `gone` for the exact recorded controller and the run was still preparing with no process recorded. The lease's boot id stays the random revocation id, so the old controller can still never renew it. Old rows without a controller stamp keep today's behaviour. - Previous-release story (upgrade-safe-run-state): no persisted shape changes. A run saved mid-preparation by an older release has no `controllerProcess` stamp, cannot be proven gone, and keeps the charged lane (covered by the new old-saved test). Retries an older release already scheduled in the charged lane keep their counters. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## 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> |
||
|
DottaandClaude
|
ca472d543a |
fix(runner): settle a saved governed wait whose shutdown drain left sequence gaps (#20494)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native agent run can ask the board a question and park on that saved (governed) wait > - After the question is saved, the runner interrupts the provider and drains its usage and terminal for a bounded time > - The drain keeps only usage, the terminal and cancellation facts of the parked turn. It discards every other numbered provider event > - If the control plane restarts before the drain settles, recovery replays the saved journal. That replay required strictly contiguous `sourceSeq`, so the discarded events made a valid saved wait look like the wrong conversation > - This pull request accepts a gap after the saved trigger only when every event after the gap is a drain-retained fact of the same turn. Identity checks do not change > - The benefit is that the asking run settles its saved wait after a restart instead of recording a failure ## Linked Issues or Issue Description **What happened?** Restart-chaos wave `w-20261110T063253-s208`, tasks t02 (Codex Q1, PV2: provider terminal dropped) and t03 (Codex Q1, PV1: provider terminal held), each with K1 (`kill -9` of the control plane about 3-4 s after the fault). The question, the board answer and the final brief were all correct. But the asking run was recorded `failed: native_session_interrupted`, and the grader failed `T:asking_run_settled`. **Cause** The run log of the asking run shows `failureDetail.message: native_governed_wait_replay_mismatch`. It is thrown at `packages/paperclip-runner/src/native-session-runtime.ts:2344` (master). The persisted runner events of the asking run have `sourceSeq` 33..40 and then 43 (t02), and 41, 42 and then 45 (t03). The two missing numbers are events that the accounting drain (`consumeTurn` → `settleDurableResult`, around line 1206 on master) read and discarded, because they are not usage, a terminal or a cancellation fact. Runs without a restart have the same gap (for example `w-20261110T032350-s201/t01`: 34, then 37). Those runs settle without replay, so the gap was not seen before. With a held or dropped terminal and a kill before the shutdown settlement receipt, recovery must replay. Its strict contiguity check then fails on a gap that the runner made itself. **Expected behavior** The question was saved before the provider was interrupted. Recovery must settle that same saved wait, as #20336 does after verified shutdown. Recovery must not start a new model turn. **Related** Refs #20336 (settle saved handoffs after verified shutdown). Not related in cause or code: #20477, #20482, #20483. ## What Changed - `isGovernedDrainRetainedEvent` (new, in `native-session-runtime.ts`) names the facts that the governed-wait drain persists: usage, the turn terminal, the interrupt acknowledgement, cancelled or expired runtime requests, and a failed or cancelled `run.terminal`, all for the parked turn. The drain now uses it in place of the same inline condition. Drain behavior does not change. - Governed-wait recovery replay accepts a `sourceSeq` gap only when: - the trigger is the first replayed event (a gap before it still fails), - sequence numbers strictly increase, - every event after the first gap is a drain-retained fact of the trigger's turn. - The run, normalized session and source instance identity checks do not change. Any mismatch still fails with `native_governed_wait_replay_mismatch`. - `doc/execution-semantics.md`: the governed-wait text said that shutdown keeps a contiguous journal. It now describes the gaps and the replay rule. Note for the sibling Claude/ACPX `turn.cancel` work: this change touches `settleDurableResult` only in the drain filter line (same condition, shared helper). It does not change cancel, the drain timeout or `settleCancelledTurn`. ## Verification - Red/green: `packages/paperclip-runner/src/native-session-runtime.test.ts` - `settles a saved governed wait whose drain left sequence gaps before a restart (terminal-after-restart)` - `settles a saved governed wait whose drain left sequence gaps before a restart (late-terminal-saved)` The first run uses the real drain. It discards two numbered events, so the history is trigger, acknowledgement, then usage with a gap. The stream then ends without a terminal, as with PV1/PV2 plus K1. On master, recovery rejects with `native_governed_wait_replay_mismatch` (the live error). With this change, recovery settles the saved wait as yielded. It does not call `resolveGovernedWait` again. It starts no second turn. The terminal is appended once. When the terminal arrived late and is already saved, recovery does not cancel the provider again. A further recovery of the settled checkpoint does not submit or settle again. - Fail closed: `keeps rejecting an unproven governed-wait replay with a sequence gap: …` covers a gap before the trigger, a foreign run, a foreign session, a foreign source instance, usage of another turn, a non-accounting event after a gap, and an accounting event followed by a non-accounting event. - `cd packages/paperclip-runner && npx vitest run src/native-session-runtime.test.ts`: 261 passed. `npx tsc --noEmit -p .`: clean. - `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts -t governed`: 12 passed. - `pnpm verify` returned exit 2 (test file outside resolved graphs). Required CI supplies the broad coverage. - Live restart-chaos wave: see the comment below with the wave id and result. ## Risks - Low. The rule loosens only the sequence check, and only after the saved trigger, and only for facts that the drain itself retains and that carry no work authority. Identity checks stay strict. A non-accounting event after a gap still fails closed. - No change to the normal start path or turn startup (fast-turn rule). - No schema, protocol or env change. The pack `contractDigest` and provider `commandDigest` do not change. No Daytona image rebuild is necessary. ## Model Used - Anthropic Claude, model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file edits, test runs). ## 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 <noreply@anthropic.com> |
||
|
Devin FoleyandPaperclip
|
53c9be8d41 |
Reuse identical captured workspace archives (#20476)
Reuse the current committed snapshot when a fresh capture matches its manifest and plaintext archive hash and size. Preserve the original encryption binding and verify each new run receipt for recovery, concurrent replay and storage request statistics. Verified with 338 focused save and recovery tests, server typecheck and build, all final-head CI checks, and Greptile 5/5. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
b09f17fbfd |
fix(agents): repair queued setup through task connection cards (#20361)
## Thinking Path > - Paperclip manages AI agents doing work for people. > - Agent verification must finish before an accepted task can start. > - A missing AI account default can prevent verification before a provider starts. > - Lifecycle policy treated that owned configuration refusal as a temporary failure and retried each minute. > - The accepted turn remained Queued without explaining the prerequisite. > - Park account configuration failures and reuse the task's existing inline connection card. > - Successful repair rechecks verification and releases the original queued turn under its original authority. ## Linked Issues or Issue Description **What happened?** An OpenCode agent selected the responsible user's OpenRouter default, but that default did not exist. Verification retried unchanged configuration indefinitely and its accepted task stayed Queued. **Expected behavior:** Preserve accepted work, explain the setup prerequisite, and let the appropriate person repair it using the existing task connection UI. Completion must verify the agent before starting the same queued turn. Genuine pause, approval and access gates remain effective. **Reproduction:** Create an OpenCode Runner agent with a responsible-user OpenRouter binding and no personal default; assign a task while verification is pending. The base retries verification and shows an unexplained queue. Related: #18702 handles pending task connection recovery after account repair. This PR handles the earlier agent-verification boundary. #20330 addresses a separate Codex governed-wait timeout and is already merged. ## What Changed - Share owned account-selection failure evidence through a pure library contract. Lifecycle policy parks those configuration failures with fixed, redacted guidance; temporary infrastructure errors retain their retries. - Batch setup-state reads for unstarted queued turns. Distinguish manual repair from automatic setup retries and preserve Stop, started/history state and lifecycle holds. - Reuse the existing `connection_intent` card, account selection and completion APIs. Its controller-only `agentSetup` marker accepts only never-started legacy turns; runtime tokens still require a running run. - Address the card to the lifecycle verifier's account owner, preserving the task's execution principal and private-task visibility. Attach missing cards on admission, verification, startup and full recovery, including retained retries and continuations. Repeated delivery and publication remain bounded. A new queued turn supersedes the setup card of a stopped turn; generic connection cards retain cross-run reuse. - Recheck lifecycle verification after usable-account completion, then release the existing turn through the normal readiness path. Preserve decline, Stop, reassignment and changed setup-owner gates. Fence an in-flight verification failure from overwriting a repair; add no provider wake or recurring worker. - Park owned account refusals during resume as well as initial verification. Bind setup cards to the lifecycle operation, settle obsolete deliveries, and keep explicit Retry guidance accurate. - Reuse the router’s existing lifecycle pin for setup repair without rerouting the task, advancing the pool cursor, or replacing its saved binding. Legacy direct-account cards remain readable. - Hide the incorrect Resume banner during setup, retaining genuine manual/budget pause controls. Document the execution and retry contract. ## Verification - Red: eight original behavior assertions failed on the base, including the real selector's retry decision and the real batched queue projection. Six owned selection cases failed classification. Temporary-error presentation, different task/verifier users, retained retry/continuation recovery and the setup Resume banner also have failing-before/passing-after regressions. - Green: 430 affected backend, lifecycle, shared-contract and UI tests passed before the final review corrections. Subsequent focused runs cover one inline card, real account selection/completion, duplicate delivery/publication, verification before readiness, unchanged run authority, private tasks, no extra provider wake, decline/Stop/reassignment, company isolation, invalid runtime tokens, transient retries and redacted diagnostics. - Final setup/card and generic connection-service suites: 49 tests passed. The stopped-turn case failed before the correction, then passed with real account completion, the stopped source remaining cancelled, and the replacement source remaining queued. - After rebase, all 129 focused assertions passed. The local queue suite’s 88 execution assertions passed repeatedly, but its database-fixture teardown exceeded both 30-second and 120-second hooks. An isolated queue case completed, and the setup/connection suites fully passed. The original queue test and cleanup remain unchanged; broad CI supplies the full-suite result. - Server/UI typechecks, module boundaries, token gates and diff checks passed. `pnpm verify --explain` requested broad CI because the execution-contract document changed; no redundant full local suite was substituted. - Real source acceptance: macOS arm64, Node 26.4.0, isolated embedded PostgreSQL and local board identity. Created an OpenCode/OpenRouter agent and task through the API with no default. Admission retained one unstarted turn and produced the existing inline Connect OpenRouter card. The browser opened its credential form and closed it without entering credentials. The same card/run persisted after server restart and reload, with the setup label and no incorrect Resume. The browser also reproduced Stop followed by a new queued turn retaining the stale card on the prior head. On `12f609c98372fd1e50644028db75cfcc8deb210d`, startup superseded the stale card and retained the same replacement turn, with the original turn still cancelled and no provider execution. - The earlier CI case-search failure also reproduces on exact base `0e3fdebea8257bdff11573081c76b306f12db230` without this PR. Its repair is now in master via #20371; this branch is rebased onto it and contains no API-search changes. - Staging's actual Runtime test returned “Connect an account and choose your personal default” before provider startup. The original turn later started independently on deployed code, then hit a separate workspace-export failure. Automatic file recovery subsequently marked that run successful; its task remains Blocked and the trial’s useful output is unverified. This PR is not deployed; provider authentication through its card and downstream publication remain unverified. No account, credential, default, permission or binding was changed during diagnosis. - Previous revision review: Greptile 5/5 on `12f609c98372fd1e50644028db75cfcc8deb210d`, with all four inline findings addressed and resolved. [Full CI passed](https://github.com/paperclipai/paperclip/actions/runs/42713494508) on that revision: 59 successful checks, one neutral check and three intentional skips, with no pending or failed checks. - Latest revision `976bcae993805a8a2222395bbff9394f6791b424`: resume, router and Retry-copy regressions failed before repair. Green: 92 lifecycle/setup/shared tests, 83 setup/generic-connection/router tests, and 137 UI/shared tests. Server/UI typechecks, module boundaries and token gates passed. [Full CI passed](https://github.com/paperclipai/paperclip/actions/runs/42720731137) on this exact revision: 59 successful checks and three intentional skips, with no pending or failed checks. Greptile reviewed this head at 5/5; all six review findings are addressed and resolved. Real Stop/retry acceptance retained the same replacement turn and current-operation card through two actual server restarts, with no provider execution. ## Risks - Known account configuration failures now require repair or explicit Retry. Temporary failures keep automatic retries; accepted work and authority remain intact. - The projection adds one company-scoped batch read only for unstarted queued turns. Card attachment examines at most 100 active candidates and adds no timer, worker, schema or migration. - The optional stored setup-card marker requires an aware reader during rollback. No run/session/checkpoint identity format changes. Setup presentation grants no access and cannot bypass verification or other gates. ## Model Used - OpenAI GPT-6 (Codex), with reasoning, tool use, code editing and test execution. The exact serving model identifier and context-window size 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 - [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> |
||
|
Nicky LeachandPaperclip
|
2550f399d9 |
fix(lifecycle): align stop proof for pre-dispatch resource waits (#20384)
## Thinking Path > - Paperclip manages AI agents and controls when their work can run. > - An agent pause waits until every earlier run has stopped. > - Recovery already treats verified workspace and AI-capacity waits as pre-dispatch outcomes. > - The stop proof did not accept those same receipts. Historical waits could keep an agent pausing forever. > - This pull request shares the resource-wait classification and the existing execution-event check. > - The benefit is consistent stop evidence without a new lifecycle command, database backfill, or host-boot assumption. ## Linked Issues or Issue Description Refs #18147 and #17718. This fixes the verified resource-wait case of #18147. It does not certify ambiguous historical runs that have no positive non-execution evidence. An agent can have an old cancelled legacy run with `errorCode: workspace_busy`, `executionStage: preparing`, and `executionRecovery: { kind: workspace_wait, providerWorkStarted: false }`. No process, native coordinator, or retained resource exists. Recovery recognizes this as a resource wait, but pause previously required a stop acknowledgment from an executor that never started. ### Credit and related work Thanks to @br1an-c0des (#17692) and @crownlayerapp (#15383) for the focused preparation and shared-admission analysis, @lucasweismann-ai (#18072) for the durable dispatch-evidence and race analysis, and @Pdesengrini (#18905) for the cancellation-receipt and process-identity guard cases. We reviewed these alternatives before implementing this smaller change. No contributor commits were cherry-picked. The design uses receipts already written by core. It does not adopt a reboot-time bypass, a new adapter-entry journal, or a pause-abort route. #17236 and #20037 address the separate cost of history reads. ## What Changed - Add one shared classifier for matching workspace and AI-capacity wait receipts. Use it in legacy recovery and cancellation stop proof. - Reuse the existing dispatch-event predicate instead of maintaining another event list. - Retain checks for native execution references, unresolved workspace obligations, controller leases, and resource cleanup. A live controller or cleanup still in flight cannot pass the proof. - Add a focused PostgreSQL suite. It drives the real pause worker with historical wait records and checks Retry eligibility against the same evidence. - Document the contract. No schema, API, UI, dependency, or scheduling change is required. ## Verification Source checkout on macOS arm64, Node 26.12.0, isolated local PostgreSQL 18, synthetic data only. ```sh node node_modules/vitest/vitest.mjs run --root server \ src/modules/agent-lifecycle/__tests__/resource-wait-stop.test.ts \ src/services/legacy-execution-recovery.test.ts ``` - 61 tests passed across these two files. - All eight positive regressions failed on unchanged master. The fix restores them. - After rebasing onto current master, the 61 focused tests passed again. Another 53 selected tests passed in `explicit-native-continuation.test.ts` with `-t 'pre-dispatch review wait|unclaimed workspace retry|historical native preparation'`. The other 361 cases in that file were not run. - Guard cases cover wrong/missing receipts, dispatch events, process identity, native ownership, live controller leases, workspace recovery, cleanup in flight, unreceipted remote resources, and an unknown historical HTTP dispatch. - A remote lease passes only after its exact termination receipt is stored. - Secret/PII diff scan and `git diff --check` passed. - Standard Greptile reviewed commit `201a61d2bb63290ca9b9b9d524e6c6715a872310`: [5/5 with no actionable findings](https://github.com/paperclipai/paperclip/pull/20384#issuecomment-6753165178). No review threads are unresolved. - Local testing is limited to affected cases. GitHub CI owns broad typecheck, build, and test coverage. Verification setup failures remain accounted for: worktree preflight could not complete because Cargo is unavailable; the first bundled PostgreSQL run failed because its macOS binary lacks a shared library. The focused tests passed on a separate, temporary local PostgreSQL 18 cluster. An early local server typecheck could not resolve the unbuilt Runner package; it is not claimed as a pass. ## Risks - Stop proof now accepts existing positive resource-wait receipts. Missing PIDs, terminal status, elapsed time, and a different boot ID remain insufficient. - The receipt must match its error code. The runtime must remain unresolved, with a preparation or settled stage and no conflicting native or workspace references. - The shared cancellation proof is also used by explicit continuation and Retry. Those paths keep their existing user-intent, ownership, pause, budget, and authorization gates. A stop proof does not itself start another run. - Runs that lack usable evidence remain held. This PR does not claim to repair every run shape reported on #18147. - No migration or live data edit is needed. No hosted deployment is included. ## Model Used OpenAI GPT-6 through Codex, with repository inspection, code execution, PostgreSQL regression tests, and GitHub tools. The exact serving revision, context window, and internal thinking configuration 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 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 - [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> |
||
|
DottaandPaperclip
|
3cc1032f6a |
fix: deliver saved answers to dependency-blocked tasks (#20363)
## Thinking Path > - Paperclip lets people direct agent work through tasks and structured answers. > - Dependencies stop task execution until prerequisite work finishes. > - A person must still be able to answer or redirect the assignee of a blocked task. > - Ordinary comments already get a bounded answer/triage turn. A saved question response has no comment ID, so the same gates reject it. > - Later dispatch also requires an active task and tries to check it out. That conflicts with bounded interaction on a blocked task. > - This pull request verifies the stored human response at each gate and keeps the turn in bounded interaction mode. > - The assignee can receive the answer without starting dependency work or removing its blockers. ## Linked Issues or Issue Description Refs #19923. That change covers dependency waits and comment retries. This change covers stored interaction responses that have no comment ID. **What happened?** A parent task waits on a parked child. The assignee asked a setup question. After a person answers, the response stays queued. Sending that saved response is refused as `issue_dependencies_blocked`. Ordinary new messages can reach the same assignee. Passing admission alone is insufficient. The dispatch status check rejects a blocked task, and automatic checkout then tries to start work whose prerequisites are unfinished. **Expected behavior** Deliver the stored human answer or confirmation to its requesting assignee as a bounded answer/triage turn. Keep the task blocked and keep its dependency relationships. Preserve current-owner, pause, approval, budget, recovery, and execution gates. **Steps to reproduce** 1. Assign a parent task and make a backlog child block it. 2. Save a question on the parent and answer it as a person. 3. Dispatch its resolved-interaction wake, or send the saved answer from the task queue. 4. Observe that the answer does not reach the provider while an ordinary comment can run in answer/triage mode. **Paperclip version or commit** Reproduced on master at `653ab8b18f76`. This branch is rebased onto master at `0e3fdebea8`. ## What Changed - Verify the stored response by company, task, requesting agent, status, kind, human resolver, and resolution time. A context flag alone is insufficient. - Allow a verified response through dependency admission and claim in the existing bounded interaction mode. - Recheck that response during scheduled retry promotion and final provider dispatch. - Keep bounded responses from checking out the blocked task or setting it to `in_progress`, including when the last blocker finishes during preparation. - Preserve continuation classification through workspace and AI-account resource waits, so the final response and owner checks still run. - Correct the task-case API summary so the existing `case for task` discovery regression passes on the first search page. This fixes a pre-existing failure exposed by full CI, without changing search ranking or assertions. - Ignore obsolete Retry notices after newer conversation work, without acknowledging saved input or changing execution authority. - Document the response contract. Add real PostgreSQL and assembled-heartbeat regressions. ## Verification - Final commit `ba4b337cf1bb0236b7c0a742665b222ee894ff51`: [required CI passed, attempt 2](https://github.com/paperclipai/paperclip/actions/runs/42706363120/attempts/2), including typecheck, production build, server/chat tests, Runner checks, and all 8 browser shards. PR status: 59 passing checks and 3 normal skips. - Red: answered questions and accepted/rejected confirmations return no admitted run before the fix. All three new positive cases fail at the dependency gate. A resource-wait retry also reproduces as refused before its gate is fixed. - Green: at delivery revision `9375a603e4`, focused heartbeat integration and classification passed 14 tests; retry scheduling and dispatch safety passed 241 tests. These use real PostgreSQL and controlled adapter and resource-availability boundaries. The wider initial dependency, native-continuation, and queue suites also passed before the final refinements. - Negative cases keep pending, missing, wrong-task, and agent-resolved responses blocked. Changing the response proof immediately before dispatch cancels the run without calling the provider. Policy controls retain the current assignee, task status, and pending approval checks. - Positive cases execute once, leave the parent blocked, preserve the dependency edge, and leave the child in backlog with no child execution. The same saved response survives a resource wait; removing its human proof refuses promotion. - The resource-wait provider-boundary and blocker-completion regressions also failed before their fixes. All 14 focused integration and classifier cases passed at delivery revision `9375a603e4`, including a changed response during retry preparation. - Red: a retained Retry for an older run makes a newer successful conversation display as failed. Green: all 78 database-backed query and interruption-presentation tests pass, including a later real failure and independent queued input controls. - Server layered typecheck and module-boundary checks passed at `11992aad0c7fbaf13e7548d71ee633560bd8bb6f`, before the one-line API summary correction. That CI passed build, typecheck and every browser shard, but exposed the pre-existing case-search assertion and a report-assets test timeout. Final-head CI passed as linked above. - Fresh Greptile review is 5/5 on final commit `ba4b337cf1bb0236b7c0a742665b222ee894ff51`, with no unresolved threads. - The existing API discovery suite reproduced its first-page assertion before the summary correction; all 3 cases pass afterward. The timed-out report-assets privacy test passed locally in isolation and the full support job passed on the final-head rerun. Runner shutdowns interrupted the first attempt; affected jobs were rerun once without code changes. A separate onboarding timeout also passed on that rerun. - `pnpm verify --explain` requests broad CI because the execution contract is a global input. The final CI run supplied that coverage. - Separately recovered the reported staging conversation through an ordinary message and the existing steering control. The agent acknowledged the message and the saved-answer queue drained. The fresh Notebook action subsequently succeeded. This was on the previously deployed revision; it does not claim this code is deployed. ## Risks - This changes admission and dispatch. Each gate checks the persisted response; no caller-supplied marker grants the exception. - The bounded turn does not check out the task, resolve a dependency, or approve another action. Existing pause, ownership, budget, approval, and stopped-execution controls still apply. - Each additional lookup is one primary-key read and runs only for a response on a dependency-blocked path. There is no new polling worker. - No migration is required. Older queued contexts get the exception only when their stored human response still verifies. No prior run or action is replayed by this change. - This PR does not roll out the code to staging. ## Model Used OpenAI Codex, GPT-6 family, with repository tools, code execution, and browser inspection. The runtime does not expose the exact model ID, context-window limit, or reasoning-mode label; those values are not asserted. ## 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
c67909cb0f |
fix: keep tool approvals valid through OAuth refresh and delayed review (#20340)
## Thinking Path > - Paperclip lets people manage agent work and approve app actions. > - An approval must bind the action that the person reviewed. > - OAuth access tokens can expire while that person is away. > - The old target snapshot treated each token refresh as a new authorization. It also imposed a one-hour review deadline. > - This pull request separates token renewal from authorization identity and keeps the human decision durable. > - Current access, the reviewed arguments, meaningful target changes, task closure, and execution receipts still control dispatch. > - People can approve later without repeating a decision after routine connection maintenance. ## Linked Issues or Issue Description **What happened?** A pending app action returned `approved_tool_target_changed` after its OAuth tokens refreshed. The tool and connection settings were unchanged. The same card also expired after one hour. A sign-in problem could discard an accepted decision before provider dispatch. **Expected behavior** The original card remains reviewable after routine OAuth refresh and a server restart. A later approval runs the exact reviewed action once. A connection repair preserves the decision while Paperclip verifies the same authorization. Meaningful changes require a new review. **Steps to reproduce** 1. Connect an OAuth MCP app and configure an action to require approval. 2. Request the action, then refresh its access and refresh tokens before approving. 3. Approve the original card. Before this change, it returns HTTP 409 even when the authorization is unchanged. 4. Wait more than one hour. Before this change, the card expires without a task-specific deadline. **Paperclip version or commit** Reproduced from master at `76254ead2229`. The final branch is rebased onto current master. ## What Changed - Add versioned approval target comparison. New snapshots bind credential identity epochs, consent, permissions, ownership, connection configuration, tool contracts, and credential references. Authenticated OAuth refresh preserves identity. Manual credential replacement does not. - Remove the implicit one-hour deadline from ordinary app action reviews. Keep explicit deadlines and terminal-task expiry. Synchronize the nullable interaction deadline in shared contracts and UI. - Keep accepted, undispatched actions when sign-in is required. Show a saved-approval repair state. Resume on committed connection changes after target and access revalidation, without an idle retry loop. - Preserve the existing execution claim, result receipt, and uncertain-outcome no-replay rules. - Explain meaningful target changes without inviting the person to retry the stale card. - Add a production task-thread Storybook example and document compatibility and recovery behavior. ## Verification - Red: production-refresher invalid-grant handling and formal-approval withdrawal during a sign-in wait also reproduced as failing regressions before their fixes. - Red: before implementation, the local OAuth HTTP fixture failed for both public and confidential clients with `approved_tool_target_changed` after token refresh. - Green: the extended fixture passes for public and confidential clients with personal and shared grants. It covers actual local HTTP token exchanges, multiple refreshes, a rebuilt gateway, an ended originating run, approval after 25 hours, duplicate approval, preserved sharing arguments, sign-in waiting, and event-driven repair. The gateways use the production OAuth refresher; an invalid refresh preserves the accepted decision before provider dispatch. Omitted-scope responses cover equivalent scope-reporting changes. The provider is an isolated local fixture, not a deployed Notebook service. - Negative controls refuse changed consent, scopes, tool definition, destination, and manual credential replacement before provider execution. - Existing gateway service, connected-tool acceptance, secret owner, secret service, unsigned-review queue, and connection-health suites passed. Explicit-deadline fixtures still prove expiry behavior, including deadlines on accepted sign-in waits. A withdrawn formal approval cancels a waiting action before connection recovery. - Approval card, task-chat, error-copy and target-comparison tests passed. Server and UI typechecks passed. Token gates and Storybook catalog checks passed. - Inspected the new saved-approval repair card in local Storybook using the production task thread. It shows a reconnect link and no second approval button or false running state. - `pnpm verify --explain` returned exit 2 and requested broad CI coverage. [Full CI](https://github.com/paperclipai/paperclip/actions/runs/42677855507) passed on `eb4f1c7caa9ee6e802c314ffc5eadec475a5de0c`, including build, typecheck/release registry, general and serialized tests, and all eight browser shards. - Greptile reviewed the same final head at 5/5. Both findings from the first review are fixed and their threads resolved; no open follow-ups remain. ## Risks - No database migration is needed. The new comparison reuses the vault identity epoch. Only authenticated OAuth refresh code can preserve that epoch; HTTP credential-write schemas cannot select that behavior. - Earlier signed requests keep their original exact-version checks and saved deadlines. Already-expired cards are not revived. A historical card whose credentials already changed needs a fresh review. - A new consent, account, permission set, destination, or manually replaced credential must not inherit the old authorization. Reconnection continues automatically only when revalidation proves the same saved authorization. - The interaction enrichment now permits a null deadline. Existing string deadlines remain valid. The web client and shared validator change together. - This PR does not deploy to staging or perform a live Notebook publication. ## Model Used OpenAI Codex, GPT-6 family, with repository tools, code execution, and browser inspection. The runtime does not expose the exact model ID, context-window limit, or reasoning-mode label; those values are not asserted. ## 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
0e325e8d52 |
fix(runner): settle saved handoffs after verified shutdown (#20336)
## Thinking Path > - Paperclip manages AI agents and their work. > - An agent can save a question or approval handoff before its provider stops. > - A missing provider terminal notification does not prove that the saved work failed. > - Runner already verifies provider exit, event drain, tool settlement and suspension. > - This change uses that proof to settle the same saved handoff. > - Unproven shutdown holds continuation and explains that the work is saved. > - Recovery must never repeat the completed work or allow overlapping execution. ## Linked Issues or Issue Description Refs #20330. That PR extends the Codex terminal drain window. This change handles the separate case where no terminal arrives but the owned shutdown can prove stopped execution. **What happened?** A native run saved a human handoff, accepted cancellation, and later stopped its provider. The missing terminal notification still made the attempt fail and show “Couldn’t finish this attempt.” **Expected behavior** Keep the saved handoff. Accept its yielded result after either an authoritative provider terminal or verified shutdown of the exact execution. If shutdown remains uncertain, retain the handoff and explain the hold without offering a duplicate Retry. **Steps to reproduce** Save a governed wait, acknowledge interruption, and omit the provider terminal. The new regression uses the real Runner binary and a controlled Codex process. The old runtime rejects the handoff instead of asking the existing shutdown owner to settle it. **Deployment mode** Native Runner with Codex. Local verification used macOS arm64 and Node 26.4.0. Hosted sandbox recovery has not been qualified. ## What Changed - Add an optional driver shutdown-settlement operation. Runner returns proof only after its existing stop, drain, tool settlement and suspension barrier passes. - Bind the receipt to the run, normalized session and turn. Persist it with the saved wait and validate it during recovery. - Complete the same yielded handoff after verified shutdown. Retire the stopped session from warm reuse. Do not invent a provider terminal or complete usage. - Retain a named shutdown-pending condition under the existing bounded same-run recovery policy. Preserve actual provider failures and integrity failures. - Show saved-work shutdown guidance in task and inbox presentation. Keep genuine approvals authoritative and disable duplicate Retry. - Document the completion and restart contract. The checkpoint field is additive; older checkpoints retain the terminal-or-shutdown path. ## Verification - Red: the new runtime regression failed against the original implementation because verified shutdown was never requested. Server presentation and error-classification regressions also failed before the fix. - Red: both inbox shutdown states showed generic text before the inbox change. - Green: Runner runtime, shutdown and harness-backend suites pass 301 tests. Tests cover missing terminals, failed stop proof, wrong identity, cancellation, late proof, genuine provider failure, and restart after receipt persistence. Recovery completes once without another model turn. - Green: the native executor and task-presentation suites pass 1,371 tests. Execution projection, inbox and blocker presentation pass 125 tests. Two test-authoring mistakes were corrected: an integrity-error text expectation and an assertion for a nonexistent projection field. Their initial failures remain in local logs. - Runner TypeScript and full server layered type checks pass. Token gates and diff whitespace checks pass. - Live provider acceptance: real Runner and Codex with gpt-6.1-sol, one disposable local workspace, one tool call, and one deliberately dropped terminal notification. The saved file, bound settlement receipt, one accepted yielded result, one provider turn, confirmed provider exit and suspended Runner all passed. The tool handler and persistence port were controlled. This does not claim a full server or hosted journey. - The full browser question-and-answer cell stopped before execution because OPENAI_API_KEY was absent. No model work ran in that attempt. Hosted sandbox recovery and the full browser continuation journey remain unverified. - `pnpm verify` returned exit 2 because the execution-spec change requires broad CI. Required CI will supply that coverage; a full local repository suite was not repeated. - Review correction in `8553da244e`: a failed trailing-event write could hide a provider failure. A new regression reproduced red, then passed after all ordinary drain errors retained the named pending condition. No settlement receipt or result can be accepted after that error. The live missing-terminal check passed again on the corrected runtime. - The real PostgreSQL control-plane regression and three existing governed-wait controls pass. The new case persists one accepted yielded result, a bound shutdown receipt, and the original pending human question without a provider terminal. - First CI revision `7c2cb9b1`: test-program typecheck found a missing `PrpEvent` annotation in the new fixture. The annotation is fixed and the Runner test-program typecheck passes. Runner support also timed out in the unchanged `registered roots remain protected even when equal to genuine public static bytes` case after 30 seconds. One unchanged local diagnostic passed in 3.69 seconds overall. The hosted timeout cause remains unknown; no timeout, fixture, or assertion was weakened. [Original failed support job](https://github.com/paperclipai/paperclip/actions/runs/42670851821/job/129346900133). - Rebased onto the merged #20330 and current recovery changes. The combined branch passes 1,402 executor/presentation tests. Both prompt and 18-second provider terminals still pass. A warm session with no terminal passes after the full 30-second drain and verified stop/suspension. Its initial overly broad assertion rejected a nonterminal turn event; the corrected assertion rejects every terminal type. The rebased CI gate was cancelled before any runner or step started; its cause is unconfirmed and it supplies no validation credit. Final revision `b49ec5a144` starts fresh CI and review. - Final-head CI attempt 1 failed before creating jobs. GitHub reports an internal server error (correlation `2c576b07-b67f-4179-a69f-74f7d99cd926`), followed by empty runner-selection outputs for the aggregate jobs. One unchanged-commit retry was requested; the initial admission failure remains recorded. [CI run](https://github.com/paperclipai/paperclip/actions/runs/42673159868). - **Current blocker:** final head `b49ec5a144e94e41e96e2508a79bd9aec91fe45a` has a fresh 5/5 Greptile review and zero unresolved threads, but required CI is not complete. GitHub reports an active [Actions and Checks API incident](https://www.githubstatus.com/incidents/tp8cghg7tlc4). Attempt 2 ran only part of the full selected graph. Build, canary, Rust, sandbox providers, admitted browser shards and other completed test jobs passed. The E2E aggregate correctly failed because `policy` and `e2e_shards` had `abandoned` results. Server shard 5/12 received a runner shutdown signal at 18:36:55 UTC and its test step was cancelled; its job still reports in progress. The shutdown cause is not established. The separate review workflow had the same pre-execution internal-error class and passed one bounded retry. No further retry was submitted during the incident. Resume with the missing/cancelled coverage after GitHub recovers; do not count the absent typecheck, Runner or support jobs as passes. [Failed coverage gate](https://github.com/paperclipai/paperclip/actions/runs/42673159868/job/129354318236), [interrupted server shard](https://github.com/paperclipai/paperclip/actions/runs/42673159868/job/129350854174). ## Risks - A missing terminal can use one additional bounded shutdown interval. Unknown shutdown still blocks continuation. - Provider terminal and usage accounting keep their separate meaning. The receipt proves stopped execution and settled tool outcomes, not model success. - Recovery rejects a foreign receipt, a later turn, an active turn with a settled receipt, and an actual failed provider terminal. - There is no database migration. Older checkpoint shapes remain readable. Drivers without this optional proof retain a named pending state. - This PR does not deploy the fix or repair an existing hosted task. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository tools and local code execution. The exact serving model ID and context window are not exposed in this session. Live test inference used gpt-6.1-sol. ## 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 - [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> |
||
|
DottaandClaude Opus 5.5
|
f56b097fbd |
fix(recovery): move stranded legacy subtasks off a parent's held workspace (#20314)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents split work into subtasks. Since #19860, every subtask gets its own execution workspace from the project base, and a subtask that an older release bound to its parent's workspace moves to its own workspace on its next admission > - Some legacy subtasks never get a next admission. Another task held the shared workspace with unsaved files, so their runs failed setup before any provider work. Stranded-work recovery then blocked them and handed them to the board > - On staging, a parent and two of its children stayed blocked for four days this way. The children could have run in their own workspace at any time > - This pull request makes the scheduled recovery-action sweep move such a child to its own workspace and continue it, with no person > - The benefit is that a legacy subtask never stays blocked on a workspace that it should not share, and the parent keeps its own workspace ## Linked Issues or Issue Description Refs #19860 (subtasks get their own workspace; legacy children move at admission) Refs #20295 (the held workspace's exhausted save now settles automatically) Refs #20273 (stall wakes wait while a workspace hold refuses a run) **What happened?** A parent task and three children shared the parent's remote workspace, because an older release created the children before #19860. One child's ended run held the workspace with unsaved files. The other two children were woken, but each run failed setup with "The workspace still has an unfinished writer or save operation." (`setup_failed`, `executionRecovery.kind=bootstrap`, `providerWorkStarted=false`). Stranded-work recovery then set each child to Blocked with a board-owned escalation (`board_escalation_no_takeover_v1`) that asked a board operator to retry. The #19860 move happens only at the next admission, and a blocked child gets no admission. **Expected behavior** A subtask that failed only because of a legacy shared-workspace binding continues in its own workspace from the project base automatically. The parent and the shared workspace stay unchanged. **Steps to reproduce** 1. With a release before #19860, create a parent task and children that share its remote workspace. 2. Let one child's run end with unsaved files whose save is not settled. 3. Wake another child. Its run fails setup on the workspace hold, and stranded-work recovery blocks it with a board escalation. 4. See that the blocked child is never moved to its own workspace and never runs again. **Root cause** `server/src/services/execution-workspaces.ts` (`releaseAncestorWorkspaceBindingAtAdmission`) moves a legacy child only when a run of the child is admitted. `server/src/services/recovery/service.ts` (`reconcileStrandedAssignedIssues`) ignores Blocked tasks, so nothing admits a child that a setup failure on the shared hold blocked. ## What Changed - `server/src/services/execution-workspaces.ts`: extract the binding release into `releaseAncestorWorkspaceBinding(tx, …)`, which runs inside the caller's transaction. Admission uses it unchanged (same guards: no other running run of the task, no unsettled workspace recovery, only a relative's workspace). - New `server/src/services/recovery/legacy-workspace-rehome.ts` (`rehomeStrandedLegacyWorkspaceTasks`). It selects a board-owned stranded escalation whose latest run failed setup on the workspace writer hold before any provider work, for a Blocked task bound to a relative's workspace. In one transaction it releases the binding, resolves the escalation with the outcome `legacy_shared_workspace_rehomed`, sets the task to `todo`, records a pending wake (`legacyWorkspaceRehome.wake: "pending"`), and records `issue.legacy_workspace_rehomed` system activity. After commit it wakes the assignee once (`issue_recovery_action_restored`, with a fresh session) and marks the wake `delivered` only when a run is created. A wake lost to a crash, refused (`null`), or failed stays pending and backs off (5 minutes, doubling, at most an hour); the pass returns the earliest owed wake as its retry deadline, so the recovery worker schedules it. Candidates are paged by action id. - Guards: the Blocked status must come from that escalation (set after the failed run, and no status decision at or after the block), the shared dependency-readiness rule must allow work (cancelled blockers and done blockers that still save files keep it blocked), the failed run must belong to the current assignee, and the binding release must be allowed. Otherwise nothing changes. - `reconcileActiveRecoveryActions` (the company-scoped sweep that the scheduled issue-recovery worker runs for each company with an active recovery action) runs this pass first. `recoveryActionSweepCondition()` also selects a company whose moved task still owes its wake. - `doc/execution-semantics.md`: documents the move and what a moved child sees. ### What a moved child sees A fresh workspace from the project base, exactly like a new top-level task in the project. It does not get the parent's changes. It does not get its own earlier files from the shared workspace either, because nothing of its work ran there (its runs failed setup before any provider work). The child that still holds the shared workspace with unsaved files is not moved by this change: its save recovery owns the hold (#20295 settles it automatically), its files stay in the shared workspace and in that recovery's saved-work receipts, and it moves on its first admission after that recovery settles (the #19860 rule). ### Previous-release story (`rules/upgrade-safe-run-state.mdc`) - **Migrated, no schema change.** The change reads escalations and task bindings that older releases saved, and it changes only the task's binding row, the task status, and the escalation. It does not change saved run identity (native execution input, leases, checkpoints, sessions). The regression test seeds the exact staging shape (Nov 4–5 evidence fields, `retryReason: workspace_busy`, `latestRunErrorCode: setup_failed`). - **Mid-run deploy.** A child with a running run or an unsettled workspace recovery is not moved (the same guards as the admission release). ## Verification - Red (the move disabled), `npx vitest run src/__tests__/legacy-workspace-rehome.test.ts`: 1 failed, 2 passed. The staging-shape case failed because the stranded child stayed Blocked and bound to the parent's workspace. - Green: 8 of 8 pass. All cases run the scheduled path: the company is selected by `recoveryActionSweepCondition()` and the pass is `recoveryService(...).reconcileActiveRecoveryActions({ companyId })` with the real heartbeat wake. - Staging shape: a parent and 3 children share the parent's workspace. One child holds it with a dirty writer run. Two children are blocked by board escalations after setup failed on that hold. After the stranded-work sweep, both children are unbound from the shared workspace, their escalations are resolved by the system with text that asks no person to act, and each child is woken exactly once with a fresh session. Each child runs once through the real heartbeat path outside the shared workspace and completes. The parent and the writer child keep their bindings, the shared workspace row is unchanged, and nothing runs for them. A later pass changes nothing. - A move whose wake was lost to a crash: the scheduled worker still selects the company, delivers the wake once, and the child runs and completes. - A refused wake (`null`) or a thrown enqueue stays pending, backs off, does not hold up other wakes, is reported to the worker as the next deadline, and keeps the company selected after a restart; once accepted, it settles. - Thirty rejected escalations do not hide a later eligible one (paging). - A child with a cancelled blocker stays blocked with its board escalation. - A child whose last status decision came before the block moves; one decided after the block stays blocked. - A stranded child whose setup failed for another reason stays blocked with its board escalation. - A task bound to the workspace it created (the parent) is never moved. - Related suites pass: `subtask-workspace-independence`, `heartbeat-workspace-busy`, `heartbeat-remote-workspace-busy`, `heartbeat-held-comment-wake`, `recovery-deadline-acceptance`, `heartbeat-retry-scheduling`, `task-workspace-binding` (278 tests), and after the review changes `issue-blocker-stall-wakeups`, `issue-recovery-deadlines`, `issue-recovery-work-lifecycle`, `recovery-durable-wait-projection`, `owner-gone-attestation` (124 tests with the two workspace suites). - `pnpm verify`: exit 2 ("broader verification required", because `doc/execution-semantics.md` is a global CI input). Required CI supplies the broad coverage. - `npx tsc --noEmit -p server/tsconfig.json`: passes. ## Risks - The pass changes a Blocked task to `todo` only under narrow guards, and only for a task bound to a relative's workspace whose last run failed setup on that workspace's writer hold. A Blocked status from another decision, or an unresolved blocker, keeps the task blocked. - The moved child starts again from the project base. This is the product rule for subtasks. Nothing of its own work is dropped, because its runs failed before any provider work. - The pass runs inside the existing active recovery action sweep and selects at most 25 escalations and 25 pending wakes per pass. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, browser read-only API checks) and extended reasoning. ## 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> |
||
|
DottaandPaperclip
|
76254ead22 |
fix(runner): allow Codex interruption to settle governed waits (#20330)
## Thinking Path > - Paperclip manages AI agents and their work. > - A native run must stop provider work before it waits for a human response. > - The Codex transport allows 30 seconds for a control command. > - The governed-wait loop used its 15-second default for Codex. > - A valid interruption terminal at 18 seconds therefore failed the run. > - This change gives Codex the existing 30-second terminal drain allowance. > - The run still fails if it cannot prove that the provider turn ended. ## Linked Issues or Issue Description **What happened?** A Codex run can save a human wait, accept an interruption, and fail before the provider sends its terminal. Its saved wait remains intact, but normal finalization does not occur. **Expected behavior:** Allow terminal delivery through the transport's bounded command window. Keep missing-terminal cases closed. **Steps to reproduce:** Run the new protocol regression against the old backend. The real Runner binary uses a controlled Codex process that delays its interruption terminal for 18 seconds. The run fails at the 15-second drain deadline. The same process passes with this change. **Paperclip version or commit:** Reproduced against `1b7fe33432`. **Deployment mode:** Local test of the native Runner path. The incident was observed in a hosted sandbox. **Installation method:** Source checkout. **Operating system:** macOS arm64 for local verification. Hosted incident runtime was Linux. **Additional context:** No matching repair was found. This addresses a reproducible deadline mismatch. The hosted log ends terminal observation at 15 seconds, so it cannot prove whether that provider would have settled within 30 seconds. The approved local live-provider check passed. This PR does not claim recovery of the hosted task. ## What Changed - Give Codex a 30-second governed-wait terminal drain through the existing backend option. - Keep other providers and the caller's shorter execution bound unchanged. - Add real Runner protocol cases for prompt, delayed, and absent terminal delivery. Check retained wait state and one provider turn. - Document the bounded wait and terminal-proof requirement. ## Verification - Red: the 18-second controlled-provider case failed with `governed_wait_provider_terminal_unproven` on the old backend. The prompt case passed. - Green: 282 tests passed across the native runtime, harness backend, and new Runner protocol suite, including a repeat on the rebased PR revision. `pnpm verify --explain` returned exit 2 because the execution-contract document requires broad CI; the [required CI run](https://github.com/paperclipai/paperclip/actions/runs/42668235142) passed on the final head. All 58 applicable checks passed; the three skipped checks were the two unrelated Storybook jobs and the selected-test planner replaced by broad coverage. - The missing-terminal case still fails without `completeRun` or a successful run terminal. The saved governed wait remains available. Only one provider turn and one tool call occur. - Existing runtime tests cover shorter deadlines, foreign or unproven terminals, complete usage without a terminal, and checkpoint recovery without another model turn. - Live acceptance at `f3da2b87425d4fd45c649bfc98785c61f82abbe0`: real Runner binary, Codex CLI 0.165.0, gpt-6.1-sol, Node 26.4.0, macOS arm64, disposable local workspace. One `request_human_input` tool call returned a pending interaction fixture. The real provider emitted `turn.interrupted`; the controller accepted the yielded wait and its run terminal. The check passed in 8.14 seconds. The tool callback and persistence port were controlled; no hosted server permission or Notebook publication claim is made. - Initial local probe setup failed before model execution: first the explicit launch lacked `app-server`, then it omitted the Runner permission configuration. The successful check used the Runner's normal launch arguments and isolated home with the approved existing account. - Hosted sandbox acceptance and recovery remain unverified. The 18-second delayed shutdown is a controlled protocol regression; the live model check proves normal shutdown, not that specific latency. ## Risks - A Codex provider that never stops can occupy the settlement window for up to 15 additional seconds. It still fails closed and is quarantined. - No persisted schema, provider identity, permission, or recovery rule changes. Previous-release checkpoints use the same format and existing recovery checks. - This change cannot fix a provider that never emits a terminal. It also does not grant missing setup permissions or decide pending approvals. ## Model Used OpenAI Codex, GPT-6 family. The exact serving model ID and context window are not exposed in this session. Used reasoning, tool calls, code edits, and local 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 - [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> |
||
|
Dotta
|
a3e7e63330 |
Guard Native recovery with process-space proof and explicit containment (#19813)
## Thinking Path > - Paperclip manages agents and their work. > - Recovery must preserve execution ownership and accepted results. > - A saved PID proves nothing outside its recorded process namespace. > - Uncertain unfinished work must stay held until its original owners stop. > - Operators need a source-bound containment action and a separate ordinary Retry. > - Accepted results must use the existing finalizer without another provider turn. > - This PR keeps those paths safe, recoverable, and quiet between real obligations. ## Linked Issues or Issue Description Refs: #19713 Refs: #19591 Recovery could use a local PID to release or replace a controller from another process namespace. Connected-chat Retry also rejected successful containment on a task in `todo`. The new containment action used a broader permission than the existing owner-stop action. A later review found that result-less guards also rejected the existing accepted-result confirmation action. ## What Changed - Record and verify the controller's kernel/PID namespace. Keep unknown and foreign unfinished ownership held. Preserve independent durable stop evidence and the existing remote lease fences. - Use the shared accepted-result ownership reconciler. Bind settlement to the observed lease token, generation, boot, PID, namespace and process birth. Retain finalizer, cleanup, current-controller, live-owner and remote exclusions. - Share the remote-owner reader across recovery consumers. Scope runner leases to the same company, task and run. Do not make local PID probes for remote accepted-result ownership. - Gate original-host containment on `runtime:confirm_owner_stop`. Bind it to the current task, source, assignee, original expiry, lease and Stop state. Audit separate controller/provider containment and reviewed action outcomes. - Keep containment separate from ordinary Retry. Use one canonical committed receipt and the normal admission, budget, workspace and lineage checks. Preserve the recorded external-action outcomes. - Keep the controller-only confirmation actionable for both result-less work and accepted results. Result-less work gets its next source-bound attention in the same transaction. Accepted results keep their original result and use normal shared settlement/finalization. - Keep identity inspection quiet. Preserve the original foreign-owner expiry and use the existing scoped finalizer in accepted-result recovery controls. - Update the recovery card, validators, execution projection, docs and access maps. Reuse existing canonical columns; add no migration or ownership backfill. ## Verification Current publication head: `71ea76b568de09d6313d8992e803131d9d5c54f3`. **The current fix has a causal red/green check.** On the test-only revision, both foreign-local stopped/fenced controller cases failed because recovery probed saved runner, process-group and provider PIDs in this host's namespace. This measured the wrong probes; it did not run duplicate providers. After the fix, the same two cases passed every no-probe, no-claim, no-controller-stamp, no-stop-receipt and unchanged-authority assertion. Three existing remote preparation, grace and fenced-controller progress cases also passed. Total: **5 passed, 0 failed;170 filtered rows excluded**. All three normal affected App TypeScript programs passed on this head (660,681 and428 roots). The fix separates a controller wait from execution stop proof. Independent live/grace evidence still schedules recovery. A stopped/fenced foreign controller can pass this namespace guard only for remote execution, which keeps its later checkpoint and never-launched checks. Foreign local execution stays held before runner or provider PID inspection. Earlier source-scoped results remain evidence, not fresh executions on the current head: - On `c86dd741`, the complete failed-owner file passed **25/25**, with no skips or filters. Empty scans wrote no controller stamps. Foreign ownership kept its original expiry. Normal returned-claim settlement and attention at the exact deadline passed. - On `c86dd741`, **15 selected accepted-result/restart controls passed**. The81 filtered rows are excluded. Both completion cases used the existing scoped finalizer and kept their accepted result without another provider turn. - Trusted CI at synthetic merge `50d072a5` (actual master `1b7fe334` plus `c86dd741`) passed the complete **17-case maintenance-server file**. This includes the original90-real-second quiet observation and two qualified hourly windows. The quiet case took118,918ms. The four jobs that failed on the earlier publication all passed on that synthetic merge. - On `6243b32c`, the complete ownership-report file passed **29/29**. Native NULL namespace stayed unprovable and generic compatibility remained. Three selected recovery controls passed;170 filtered rows are excluded. - On `f81f2c3c`,356 business cases passed: whole ownership-route221, recoveryUI70, owner-proof38 and controller-confirmation8; selected accepted-result authority18; and one real connected-chat containment, separate authorized Retry, useful Native result and downstream reply case. The2,024 filtered rows are excluded. A separate earlier accepted-result causal case kept the original result and completed after restart without replay. Normal Shared exports, App programs, UI typecheck, token gates and map checks passed on their recorded revisions. Unchanged DB, adapter, Eval and Runner inputs/outputs have explicit carry proofs. The clean normal merge with fetched master `8fe524f7` preserves all31 exclusive owned paths and14,827 unrelated master entries. The nine overlapping paths were reviewed, including the upstream unidentified-run stop action. Fresh CI must validate that actual merge composition. PG cases used existing external PG17, fresh owned databases, pools at most4, one worker and no SQL observer during cases. Registered clients and recorded process groups closed. Independent final SQL audits found zero campaign sessions before normal database cleanup. Original failing fixtures remain retained. Original failures are preserved. One new test first failed compilation due to a missing getter import; an import-only correction passed its full owning programs before the causal red/green check. A local quiet attempt stopped in the normal prerequisite hook before any business case and its collector rejected two normal build markers. The later17-case CI pass establishes quiet recovery; it does not turn that failed local collector into a pass. An earlier useful case passed before its private cleanup collector failed on an owned read-only directory; normal cleanup completed after all owners closed. Earlier runner-shutdown cancellation and a separate private whole-suite failure remain unexplained. Current-head checks are complete:57 success,1 neutral and3 skipped, including the successful aggregate CI gate. The legacy status passed. Greptile is5/5 and Ponytail gives an explicit5/5 Ship verdict for this head. All seven review threads are resolved. Full recursive typecheck, the full test suite and release build were not repeated locally. No result here proves deployed suspension, a live provider or termination on another host. ## Risks - Unknown physical ownership can require an owner/admin action. A false containment statement can be unsafe. - New work, reassignment, Stop, renewed authority, accepted results or pending cleanup can invalidate an action. The operator must refresh. - Controller confirmation does not prove provider/tool containment. Completed external actions must not be replayed. - Generic legacy and child-process compatibility remains separate from the strict Native controller policy. - Existing saved state and canonical schema are preserved. Permanent safety refusals remain held with their normal named recovery path. ## Model Used OpenAI `gpt-6.1-sol` agents implemented and tested the changes with reasoning, repository tools and code execution. A GPT-6-based Codex managing agent reviewed and integrated them. The host does not expose the manager's exact build or the context-window sizes. ## 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 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 |
||
|
DottaandClaude Opus 5.5
|
47dccbf3eb |
fix(recovery): re-prove an exhausted save's agent stop and free the workspace (#20295)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When an agent run ends, Paperclip saves the files that the run changed in its remote workspace. Until that save settles, the run holds the workspace, so no other task can write it > - When the save fails, a bounded system recovery proves that the agent stopped (a provider stop receipt) and then saves the files again > - If the stop cannot be proven before the 30-minute recovery window ends, the recovery is exhausted with `agent_stop_unconfirmed`. Exhaustion was terminal: Paperclip never proved the stop again, even after the run ended and its sandbox had a stop receipt > - The hold then stayed forever. On staging, a child task's save was exhausted this way on Nov 5. Its parent and its two sibling tasks share that workspace, and all of them stayed blocked for four days > - This pull request re-arms such an exhausted save automatically, on a bounded backoff, once the run is provably over. After the bounded re-arms, the hold continues from the last save with the unsaved files attached > - The benefit is that a dirty workspace hold always settles without a person, and no unsaved work is discarded silently ## Linked Issues or Issue Description Refs #20273 (the stall sweep stops failing wakes against this kind of hold) Refs #20278 (a settled restore hold returns its warm sandbox) Refs #19860 (subtasks get their own workspace) **What happened?** A child task's run wrote its remote workspace and failed to save it (`native_workspace_sync_out_retry_exhausted`). The save recovery made one attempt. That attempt failed with `agent_stop_unconfirmed`, and the recovery window ended before a second attempt. The recovery became `exhausted`. Ten minutes after the first attempt, the sandbox lease got a provider stop receipt, so the agent was provably gone. But nothing looked at the exhausted recovery again. The run kept the workspace's writer hold. The parent task and every other task on that workspace could not start ("The workspace still has an unfinished writer or save operation."). The blocker told a person to press Retry. **Expected behavior** An unproven stop is a wait, not a refusal of the saved files. When the run is over, recovery must prove the stop again and finish the save, so the hold settles automatically. If the stop still cannot be proven, Paperclip must keep the unsaved files and then release the hold. A person must not need to act. **Steps to reproduce** 1. Run a task in a remote (sandbox) workspace. Make its final save fail. 2. Keep the workspace recovery from proving the agent stop until its 30-minute window ends (for example, keep the finalization ownership busy). 3. See that the recovery action is `exhausted` with `agent_stop_unconfirmed`, and that it stays so after the lease gets a stop receipt. 4. Wake another task bound to the same workspace. See that its setup fails because the workspace is held. **Root cause** - `server/src/services/workspace-recovery.ts` (`recoverRetainedWorkspace`): an `exhausted` state only goes to the sibling settlement and to the retained-source fallback. The fallback accepts only permanent source refusals (`isUnverifiableSourceFailure`). Every other exhausted state is handed to the board and never attempted again. - `recheckStoppedWorkspaceRecovery` skips `exhausted` states, and it only accepts acknowledged cancellations. A failed run whose lease got a stop receipt later is never re-examined. - `retainedWorkspaceCandidates(…, obligationsOnly)` and `nextWorkspaceRecoveryAt` do not select exhausted `agent_stop_unconfirmed` holds, so the recovery worker never wakes for them. ## What Changed - New `server/src/services/workspace-recovery-stop-proof.ts` (`rearmExhaustedStopProof`). For an exhausted save whose last failure is `agent_stop_unconfirmed` (`recheck_evidence`), it starts a new bounded system recovery generation. It does this only when the source run is provably over: the run is terminal, no local execution remains, and no controller lease or finalizer lease is live. The new generation repeats the strict stop proof (a provider stop receipt) and the save. It records an `issue.workspace_recovery_rearmed` system activity. - Re-arms are bounded: at most 4. The first is due at once. The next ones are due 30 minutes, 2 hours and 8 hours after the previous re-arm. The `stopProofRecovery` marker in the recovery action evidence records the count and the next due time. - `workspace-recovery-fallback-discovery.ts`: new discovery conditions (`stopProofRearmPendingCondition`, `stopProofRearmDueAt`, `stopProofRearmDueCondition`) and `retainedHoldFallbackEligible`. `retainedWorkspaceCandidates` (worker pass) and `nextWorkspaceRecoveryAt` use them, so the recovery worker wakes when a re-arm is due. - `workspace-recovery-fallback.ts`: when every re-arm is spent and the stop is still unproven, the hold takes the existing retained-source fallback. The unsaved files are attached to the task (unapplied, secrets omitted), the unverified epoch is abandoned, and the task continues from the last save. - `system-recovery-state.ts`: the exhausted `agent_stop_unconfirmed` text now says that Paperclip tries again automatically. It no longer asks a person to press Retry. Retry stays available. - Review follow-ups: the re-arm applies to every workspace recovery the worker owns (restore and historical holds too). A due re-arm is scheduled no earlier than the source run's controller or finalizer lease expiry, so the worker is not woken every 15 seconds while a lease is live. A fallback whose bounded checks are spent is reconsidered once when an allocation of the source run changes, and `settleStopOnlyCleanup` wakes the workspace recovery worker when it records a stop receipt. The automatic-retry text is shown only while a re-arm remains. - `doc/execution-semantics.md`: documents the re-arm, its bounds, the fallback, and the upgrade behavior. ### Previous-release story (`rules/upgrade-safe-run-state.mdc`) - **Carried forward, no migration.** A hold exhausted by an older release has no `stopProofRecovery` marker. The recovery worker treats it as due at once and re-arms it on its first pass after deploy. The new regression test saves the exact Nov 5 evidence shape (`attempts: 1`, `retriesUsed: 0`, `state: exhausted`, no marker) and resumes it with the new code. - **Mid-run deploy.** A run that is still running, or whose controller lease or finalizer lease is live, is not re-armed. The test covers a live controller lease. The re-arm uses the same CAS (generation and revision) and the same row locks as the other recovery writers, so an old worker's late write cannot overwrite the new generation. - **No new refusal codes.** No saved run identity changes. The re-arm writes only recovery-action evidence. ## Verification - Red (before the fix), `npx vitest run src/services/workspace-recovery-stop-proof.test.ts`: 3 failed, 1 passed. The staging-shape test failed because the worker did not wake (`nextWorkspaceRecoveryAt` returned `null`) and the hold stayed. The backoff and fallback tests failed for the same discovery gap. - Green (after the fix): 4 of 4 pass. - Staging shape: a parent and 3 children that share one remote workspace. One child's save is exhausted with `agent_stop_unconfirmed`, and its run ended days ago with a stop receipt. Before recovery, the hold refuses the parent (`remoteWorkspaceHoldRefusesStart` is `true`). The recovery worker discovers the hold, re-arms it, proves the stop, saves the files and completes. The workspace is not dirty, it has a new snapshot, and the hold is `free` for the parent. The action stays system-owned and its text asks no person to act. - A live controller lease keeps the hold. No re-arm happens and no lease is resumed. - A re-arm that is not yet due is not discovered. The worker's next wake is in the future, and the text says that Paperclip tries again automatically. - After 4 re-arms, the hold continues from the last save. The unsaved files are captured as a task attachment, and the hold is `free`. - A live controller lease defers the worker's next wake to the lease expiry, and the hold is not selected before it. - A spent fallback is not selected, and a later lease change (a late stop receipt) makes it selected again; the worker then settles it. - `system-recovery-state.test.ts`: the automatic-retry text appears only while a re-arm remains. - Related suites, all pass: `src/services/workspace-recovery*`, `src/services/remote-workspace*`, `src/services/system-recovery-state.test.ts`, `src/services/task-retry.test.ts`, `src/__tests__/heartbeat-observed-workspace-preservation.test.ts` (34 files, 1510 tests), plus the stop-cleanup suites (`environment-retained-orphan-sandbox`, `live-recovery-work`, `retained-native-session-cleanup-work`, `legacy-workspace-restore-recovery`, native workspace warm-reuse, cancelled preservation, reconstruction and failed recovery; 11 files, 622 tests). - `pnpm verify`: exit 2 ("broader verification required", because `doc/execution-semantics.md` is a global CI input). The focused suites above cover the change, and required CI supplies the broad coverage. - `npx tsc --noEmit -p server/tsconfig.json`: passes. ## Risks - A re-armed generation resumes the stopped sandbox to save its files. This is the same physical path as a manual Retry. It runs only when the source run is provably over. - Re-arms are bounded (4) and spaced, so a stop that can never be proven does not loop. After that, the existing retained-source fallback settles the hold. If the retained files cannot be read, the fallback records why. This is the same behavior as for a refused source. - The parent's workspace receives the child's saved files, because the child (bound by an older release) wrote that workspace. This is the existing #19860 behavior: the files that a legacy child left in the shared workspace stay there. #20314 moves the stranded legacy children to their own workspace. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, browser read-only API checks) and extended reasoning. ## 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> |
||
|
DottaandClaude Opus 5.5
|
fd2e7482c0 |
fix(runner): stop a lost runnerd's provider and tools from a durable lineage journal (#20270)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A local native run uses one runner daemon (`paperclip-runnerd`).
runnerd starts the provider (for example OpenCode), and the provider
starts tool processes
> - When runnerd dies in the middle of a tool call, the provider and its
tools are re-parented. The only record of that process tree was in
runnerd's memory
> - Close then cannot prove that those processes stopped. It quarantines
the session, and the next attempt stops with
`native_session_cleanup_quarantined`: "Automatic recovery stopped /
operator recovery required"
> - Restarts and runner loss are product failures. Interrupted work must
recover by itself, and it must never get two writers in one workspace
> - This pull request makes runnerd journal its process lineage to disk,
and makes close stop and prove gone every journalled process after
runnerd is lost
> - The benefit is that a killed runnerd leads to an automatic, safe
retry within the normal attempt budget, instead of an operator stop
## Linked Issues or Issue Description
Refs #20259 (runner loss classification and the lineage sampler panic).
Refs #20260 (automatic continuation of an inert cleanup quarantine).
**What happened?**
A chat eval killed only the owned local `runnerd` process (SIGKILL)
during an OpenCode tool call
(`chat-features.runner-opencode.local.recovery.retry-exhausted`,
`recovery.explain-stall` and `recovery.source-action`). Attempt 1
recorded `native_runner_process_exited` with "Retry scheduled 1/3".
Attempt 2 stopped with `native_session_cleanup_quarantined`. The task
was blocked with no owner, and the next action was operator recovery.
**Expected behavior**
After runnerd is lost, the product stops the provider and tool processes
that runnerd started, proves them gone, and continues the run within its
retry budget. When it cannot prove that, it keeps the fail-closed
quarantine.
**Steps to reproduce**
1. Start a local native OpenCode (or Codex) run.
2. Let the agent run a tool that leaves the provider's session (for
example `nohup job &`).
3. Send SIGKILL to the `runnerd` PID only.
4. Close rejects with `runner did not durably suspend before
checkpoint`, and the next attempt fails with
`native_session_cleanup_quarantined`.
**Paperclip version or commit**
master at
|
||
|
DottaandClaude Opus 5.5
|
1004e55180 |
fix(recovery): stop stall wakes from failing setup on a held workspace (#20273)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Recovery tells a waiting parent or dependent when a child or blocker stalls on the board (`issue_blocker_stalled`) > - A legacy child that an older release bound to its parent's remote workspace failed its save, and its save recovery was exhausted. Its ended run still holds the shared workspace, days past the one-hour holder cutoff > - Workspace reservation refuses a run on such a hold and fails setup with "The workspace still has an unfinished writer or save operation." Every stall wake of the parent created a run that could only fail this way > - The stall sweep treated each failed run as an undelivered wake and retried it every ten minutes, with no backoff and no limit. On staging this made 72 failed runs (58 of 60 failures since a deploy), in pairs every ten minutes > - This pull request makes the stall sweep hold a wake while the waiter's workspace hold refuses a run, and bounds retries of a wake whose run fails > - The benefit is that a held workspace no longer produces a failed run every ten minutes, and the wake still arrives once the hold settles ## Linked Issues or Issue Description **What happened?** A parent task was woken by `issue_blocker_stalled` twice every ten minutes. Each run failed in setup (`setup_failed`, `executionRecovery.kind=bootstrap`, `providerWorkStarted=false`) with "The workspace still has an unfinished writer or save operation." The parent's remote workspace was held by a child's ended writer run whose save recovery was exhausted (`native_workspace_sync_out_retry_exhausted`, last failure `agent_stop_unconfirmed`). The child shared the parent's workspace because an older release created it before subtasks got their own workspace (#19860). **Expected behavior** A wake that cannot start because its workspace is held must not create a failed run every time. It should wait until the hold settles. Automatic retries must be bounded. **Root cause** - `server/src/services/heartbeat/workspace-preparation.ts` rethrows `RemoteWorkspaceWriterActiveError` as a setup failure when the ended writer is silent past `WORKSPACE_BUSY_HOLDER_STALE_AFTER_MS` (documented, intended). - `server/src/services/recovery/blocker-stall-wakeups.ts` retried every non-delivered wake row (including `failed`) after `REFUSED_WAKE_RETRY_MS` (10 minutes) with no backoff and no limit, and it did not check whether the waiter could start at all. ## What Changed - `remote-workspace-writer-hold.ts`: add `remoteWorkspaceHoldRefusesStart`, the one rule for a hold that reservation refuses outright (unattributed writer, or a writer silent past the cutoff with no live owner fence). - `workspace-preparation.ts`: use that rule instead of an inline copy, so reservation and wake senders cannot drift. - `blocker-stall-wakeups.ts`: before waking a waiter, check for proof that its run would be refused. The proof is that the assignee's own latest run on the task failed setup with the unfinished-writer error, and the bound workspace hold still refuses a run. Then do not wake it; count it as deferred and re-check at most hourly. A different assignee (for example a controller-only agent that never uses the workspace), a later run, or a settled hold lets the wake through. Reservation already tried and failed the unchanged-writer release for that run, so the sweep does not repeat that proof. - `blocker-stall-wakeups.ts`: count failed runs that carry the stall (the keyed wake's run and keyless resource-wait retries that keep its context). Retry after 10, then 20 minutes, and stop after the third failed run. Refusals (wake-on-demand off, agent paused) keep the 10-minute retry. A new stall (new recovery action) wakes again. - `remote-workspace-writer-hold.ts` / `remote-workspaces.ts`: share the unfinished-writer error text as one constant. - `doc/execution-semantics.md`: document both rules. ## Verification - Red before the fix (`server/src/__tests__/issue-blocker-stall-wakeups.test.ts`): 3 new tests failed. The held-workspace test got a delivered wake; the backoff test got a third wake 11 minutes after the second failure. - Green after the fix: `npx vitest run src/__tests__/issue-blocker-stall-wakeups.test.ts` passes 22/22. Added after review: a waiter with no refused run, or a reassigned waiter, is still woken; failed resource-wait retries count toward the limit. - The new tests use the staging shape: a legacy `shared_workspace` remote workspace owned by the parent, a child bound to it, the child's failed native writer run ended four days ago with `native_workspace_sync_out_retry_exhausted`, a dirty authority, and the child's board-owned exhausted recovery action. The parent's own earlier run failed setup on the hold, as on staging. They also prove that a writer inside the cutoff still gets its wake (the run parks in a workspace wait), and that the wake is delivered once the hold settles. - Related suites pass: `heartbeat-remote-workspace-busy.test.ts`, `heartbeat-dependency-scheduling.test.ts`, `native-cancelled-workspace-preservation.test.ts`, `src/modules/run-dispatch`. - `npx tsc --noEmit -p server` passes. - `pnpm verify` exits 2 because `doc/execution-semantics.md` is a global CI input; required CI supplies the broad coverage. ## Risks - Low. No schema or persisted-shape change. Saved state from the previous release (existing wake rows, existing workspace authority) is read as-is: earlier `failed` stall wake rows count toward the new three-run limit, which stops the current loop at once after deploy. - A parent whose workspace stays held is not woken for the stall until the hold settles. The stall is still listed on its page and the child's recovery action stays with the board. - Not fixed here: the exhausted save hold itself is never settled automatically (`agent_stop_unconfirmed` exhaustion in workspace recovery), and legacy children that still share a parent's workspace are not moved. Both touch workspace recovery code that is under concurrent change and are left for a follow-up. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, browser read-only inspection of staging). ## 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> |
||
|
DottaandClaude Opus 5.5
|
2dd7a98753 |
fix(recovery): return a settled restore hold's warm sandbox to reuse (#20278)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Recovery tells a waiting parent or dependent when a child or blocker stalls on the board (`issue_blocker_stalled`) > - A legacy child that an older release bound to its parent's remote workspace failed its save, and its save recovery was exhausted. Its ended run still holds the shared workspace, days past the one-hour holder cutoff > - Workspace reservation refuses a run on such a hold and fails setup with "The workspace still has an unfinished writer or save operation." Every stall wake of the parent created a run that could only fail this way > - The stall sweep treated each failed run as an undelivered wake and retried it every ten minutes, with no backoff and no limit. On staging this made 72 failed runs (58 of 60 failures since a deploy), in pairs every ten minutes > - This pull request makes the stall sweep hold a wake while the waiter's workspace hold refuses a run, and bounds retries of a wake whose run fails > - The benefit is that a held workspace no longer produces a failed run every ten minutes, and the wake still arrives once the hold settles ## Linked Issues or Issue Description **What happened?** A parent task was woken by `issue_blocker_stalled` twice every ten minutes. Each run failed in setup (`setup_failed`, `executionRecovery.kind=bootstrap`, `providerWorkStarted=false`) with "The workspace still has an unfinished writer or save operation." The parent's remote workspace was held by a child's ended writer run whose save recovery was exhausted (`native_workspace_sync_out_retry_exhausted`, last failure `agent_stop_unconfirmed`). The child shared the parent's workspace because an older release created it before subtasks got their own workspace (#19860). **Expected behavior** A wake that cannot start because its workspace is held must not create a failed run every time. It should wait until the hold settles. Automatic retries must be bounded. **Root cause** - `server/src/services/heartbeat/workspace-preparation.ts` rethrows `RemoteWorkspaceWriterActiveError` as a setup failure when the ended writer is silent past `WORKSPACE_BUSY_HOLDER_STALE_AFTER_MS` (documented, intended). - `server/src/services/recovery/blocker-stall-wakeups.ts` retried every non-delivered wake row (including `failed`) after `REFUSED_WAKE_RETRY_MS` (10 minutes) with no backoff and no limit, and it did not check whether the waiter could start at all. ## What Changed - `remote-workspace-writer-hold.ts`: add `remoteWorkspaceHoldRefusesStart`, the one rule for a hold that reservation refuses outright (unattributed writer, or a writer silent past the cutoff with no live owner fence). - `workspace-preparation.ts`: use that rule instead of an inline copy, so reservation and wake senders cannot drift. - `blocker-stall-wakeups.ts`: before waking a waiter, read its bound remote workspace hold. When the hold refuses a run, do not wake it; count it as deferred and re-check at most hourly. A workspace with an unchanged-release baseline is left to reservation, which can release it. - `blocker-stall-wakeups.ts`: a stall wake whose run failed is retried after 10, then 20 minutes, and not after the third failed run. Refusals (wake-on-demand off, agent paused) keep the 10-minute retry. A new stall (new recovery action) wakes again. - `doc/execution-semantics.md`: document both rules. ## Verification - Red before the fix (`server/src/__tests__/issue-blocker-stall-wakeups.test.ts`): 3 new tests failed. The held-workspace test got a delivered wake; the backoff test got a third wake 11 minutes after the second failure. - Green after the fix: `npx vitest run src/__tests__/issue-blocker-stall-wakeups.test.ts` passes 20/20. - The new tests use the staging shape: a legacy `shared_workspace` remote workspace owned by the parent, a child bound to it, the child's failed native writer run ended four days ago with `native_workspace_sync_out_retry_exhausted`, a dirty authority, and the child's board-owned exhausted recovery action. They also prove that a writer inside the cutoff still gets its wake (the run parks in a workspace wait), and that the wake is delivered once the hold settles. - Related suites pass: `heartbeat-remote-workspace-busy.test.ts`, `heartbeat-dependency-scheduling.test.ts`, `native-cancelled-workspace-preservation.test.ts`, `src/modules/run-dispatch`. - `npx tsc --noEmit -p server` passes. - `pnpm verify` exits 2 because `doc/execution-semantics.md` is a global CI input; required CI supplies the broad coverage. ## Risks - Low. No schema or persisted-shape change. Saved state from the previous release (existing wake rows, existing workspace authority) is read as-is: earlier `failed` stall wake rows count toward the new three-run limit, which stops the current loop at once after deploy. - A parent whose workspace stays held is not woken for the stall until the hold settles. The stall is still listed on its page and the child's recovery action stays with the board. - Not fixed here: the exhausted save hold itself is never settled automatically (`agent_stop_unconfirmed` exhaustion in workspace recovery), and legacy children that still share a parent's workspace are not moved. Both touch workspace recovery code that is under concurrent change and are left for a follow-up. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, browser read-only inspection of staging). ## 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> |
||
|
DottaandClaude Opus 5.5
|
8525bb534c |
fix(chat): post a turn's answer when a card it did not create is pending (#20220)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People talk to agents in Agent Chat. The agent's final message becomes the chat reply. > - A chat thread can hold a pending card that the board addressed to another person, for example a human-only confirmation for Riley. > - When Alex asks the agent to act on that card, the agent correctly gets `403 interaction_human_only` and writes a final answer that explains the denial. > - The shared answered rule (`conversationTurnAnswered()`, from #20179 and #20204) counted any pending human-only card in the thread as the turn's own response. So the answer was suppressed, and the chat stayed `active` with no reply. > - This pull request limits that rule to cards that the turn's own run created. Other cards still hold the task status. > - The benefit is that the person always gets the turn's answer, unless the turn itself asked a question with a card. Live completion and restart recovery stay consistent. ## Linked Issues or Issue Description Refs #20179, #20204 (the shared answered rule this PR corrects). **What happened** In an Agent Chat, the board created a pending `request_confirmation` card with `resolverPolicy: human_only`, addressed to another user (Riley). The card has no source run. Alex wrote "I approve Riley's request." The agent tried to accept the card and got `403 interaction_human_only`. It then wrote a correct final answer: the card needs Riley. The run succeeded with a `yielded` result and a `response_wake` continuation. Run completion recorded `presentationDecision: none`. No comment was created. The conversation stayed `active`, so Alex never got a reply. Cause: `conversationTurnAnswered()` reused the status-gate query `pendingNativeInteraction()`. In conversation mode, that query excludes another run's card only when it is an ordinary question or an `anyone` confirmation. A human-only card addressed to someone else is kept. The rule then treated that card as the card that "presents the turn", although this run did not create it. **Expected behavior** A successful chat turn posts its final answer exactly once, and the conversation moves to `waiting`. Only a pending question or confirmation card that the turn's own run created presents the turn in place of its prose. A card from an earlier turn, or one the board addressed to another person, can hold the task status, but it does not suppress the answer. **Steps to reproduce** 1. Open an Agent Chat as Alex. 2. As the board, create a pending `request_confirmation` card in the chat with `resolverPolicy: human_only` and `addresseeUserId` set to another person. 3. Send "I approve Riley's request." 4. Let the agent try to accept the card, get `403 interaction_human_only`, and end its turn with a final answer. 5. See that no reply appears, `presentationDecision.commentAction` is `none`, and `conversationState` stays `active`. ## What Changed - `server/src/services/native-runtime/native-governance.ts`: `conversationTurnAnswered()` now checks only pending cards whose `sourceRunId` is the turn's run. `pendingNativeInteraction()` gets an optional `createdByRun` filter for this. The status gate (`pendingNativeGovernance()`) does not change, so another person's card still holds the task. - Both callers get the fix through the one shared rule `conversationTurnPresentsAnswer()`: live run completion (`run-completion.ts`) and background presentation recovery (`recoverCommittedNativeChatResponse`). - `server/src/services/heartbeat/run-completion.test.ts`: new live cases "someone else's confirmation card" (board-created, human-only, addressed to another user, no source run) and "earlier turn's question card" (created by an earlier run of the same agent). Each case now also checks that the conversation moves to `waiting`. - `server/src/services/native-runtime/agent-chat-response-recovery.test.ts`: new restart case for the board-created card, plus a `waiting` check after recovery. - `doc/execution-semantics.md`: state that only the turn's own card presents the turn. ## Verification - Thesis: the reply is suppressed because the answered rule counts any pending human-only card in the thread, not only this run's card. If this is true, a turn with only a board-created card addressed to someone else posts no comment on master, and restricting the rule to `sourceRunId = runId` posts exactly one comment and settles the conversation to `waiting`, with the own-card cases unchanged. - Failing first: without the fix, the two new live cases failed (`expected [] to deeply equal [answer]`). The new recovery case also failed (no comment). - After the fix: - `cd server && npx vitest run src/services/heartbeat/run-completion.test.ts -t "posts a waiting conversation turn"`: 9 passed (includes the two own-card cases, which still post nothing). - `cd server && npx vitest run src/services/native-runtime/agent-chat-response-recovery.test.ts`: 62 passed. - Both suites use a real embedded PostgreSQL database. - Turn start: no change. The rule runs only after the provider turn ends. ## Risks - Low. Only the presentation rule changes. The status gate, the conversation settle rule (it already required `sourceRunId = run.id`) and the external-chat path do not change. - Behavior shift: a turn now posts its answer while another run's or the board's card is pending. Before, the answer was lost. A card that the turn itself created still presents the turn, as before. - Previous-release state: no persisted shape change. A run that the previous release already recorded as `none` keeps that decision. Recovery does not re-present it. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use (shell, file edits, test runs) in Claude Code. ## 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> |
||
|
DottaandClaude Opus 5.5
|
ad2304dc88 |
fix(chat): recover a held conversation turn's answer after a restart (#20204)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People talk to agents in Agent Chat. The agent's final message becomes the chat reply. > - A chat turn can end while its task stays held: a pending approval, an execution stage or a dependency blocker keeps the status. > - #20179 made live run completion post that held turn's answer. It added `conversationTurnAnswered()`. > - Background presentation recovery still required a `conversation_turn_finished` decision and a `done` result. > - If the server restarts between the status commit and the live presentation, recovery skips the held turn. The person never gets an answer. > - Restarts are product failures. A committed answer must not be lost. > - This pull request makes recovery use the same answered rule as the live path, through one shared helper. > - The benefit is that a held turn's answer reaches the chat exactly once, with or without a restart. ## Linked Issues or Issue Description Refs #20179 (live fix; this PR closes the "Known gap" in its Risks section). **What happened** In an Agent Chat, the agent resubmitted a hire approval and ended its turn with a yielded result. The finalizer committed `governed_response_waiting` and moved the task to `in_review`. The server restarted before run completion posted the reply. Background presentation recovery found the run as a candidate, but declined it, because it accepted only `conversation_turn_finished` with a `done` result. No reply was posted. The run stayed a pending presentation candidate. **Expected behavior** After a restart, recovery posts the held turn's final answer exactly once. It never posts a second copy when the live path already presented the answer. When the turn's own question or confirmation card presents it, recovery posts nothing and records that decision, as the live path does. **Steps to reproduce** 1. Open an Agent Chat. Link a pending hire approval to the chat task. 2. Let the agent end the turn with a yielded result and a `response_wake` continuation. The status decision commits as `governed_response_waiting`. 3. Stop the server after the status commit and before run completion presents the reply. 4. Start the server. Presentation recovery runs. 5. See that the chat shows no reply. ## What Changed - `server/src/services/native-runtime/native-governance.ts`: add `conversationTurnPresentsAnswer()`. It is the one rule for whether a successful conversation turn's prose is its answer: `conversation_turn_finished`, or a committed decision with no pending card of the turn (`conversationTurnAnswered()`). - `server/src/services/heartbeat/run-completion.ts`: the live path calls the shared helper. No change in behavior. - `server/src/services/native-runtime/native-run-finalizer.ts` (`recoverCommittedNativeChatResponse`): - A current conversation turn no longer needs `conversation_turn_finished`. The turn must still be the unsettled current turn: same agent, user, session generation, `conversationState: active`, and no other execution run. A `conversation_turn_finished` turn still needs `in_progress`. - A conversation turn with a `yielded` result is accepted next to `done`. - The resolver gets `conversationTurnFinished` from the shared helper. - When the turn's own card presents it, recovery records `presentationDecision: none` (reason `committed_conversation_interaction_presented`) and settles the conversation turn, like the live path. The run then leaves the pending presentation set. Before, such a run stayed a candidate. - `server/src/services/native-runtime/native-response-presentation-query.ts`: the candidate prefilter matches the same turns: any committed reason for an active conversation turn, and `yielded` conversation results. - `server/src/services/native-runtime/agent-chat-response-recovery.test.ts`: restart regressions with the real finalizer and a real database. - `doc/execution-semantics.md`: state that recovery applies the same rule. ## Verification - Failing first. With only the new tests applied, 5 of 7 new cases failed: pending approval, revision-requested approval, execution stage (all yielded), pending approval with `done`, and the own-card case. The run was a candidate, but recovery posted nothing and did not settle it. - After the fix: `cd server && npx vitest run src/services/native-runtime/agent-chat-response-recovery.test.ts src/services/heartbeat/run-completion.test.ts src/__tests__/idle-native-finalization.test.ts src/services/presentation-ownership-read.test.ts src/services/chat-monitor-continuation.test.ts src/services/native-runtime/native-preserved-monitor-wait.test.ts src/services/native-runtime/native-response-publication-retry.test.ts src/services/heartbeat/run-lifecycle.test.ts src/__tests__/scoped-deadline-work.test.ts src/services/native-runtime/paperclip-control-plane-port.test.ts src/__tests__/heartbeat-run-summary.test.ts src/__tests__/agent-conversations.test.ts`: 12 files, 627 tests passed. - The new cases check: - Held turns: the run is a candidate, the check posts the exact final answer once, the presentation decision is recorded, the run leaves the candidate set, and a second recovery returns false with one comment. - Live already presented: no candidate, no second comment. - Own question card: no comment, `presentationDecision: none`, the card stays pending, the conversation settles to `waiting`, and the run leaves the candidate set. - Previous-release live decision `none` (`yielded_control_plane_wait`): kept, not re-presented. - `tsc --noEmit -p server`: no errors in changed files. Four unrelated errors remain in this local worktree (`esbuild` not linked in `native-harness-prestage.test.ts`, and a linked-package type mismatch in `run-dispatch/adapters/postgres.ts`). - Turn start: no change. The helper runs only in run completion (after the provider turn ends) and in background presentation recovery. Nothing new runs before `turn.start`, and no files move. The live path keeps the same queries: it short-circuits on `conversation_turn_finished`, and otherwise runs the same card read as before. ## Risks - Low to medium. Recovery can now post the answer of a conversation turn whose task is held. This matches the live path since #20179. - A card-presented turn found by recovery is now settled with `presentationDecision: none`. Before, it stayed a pending candidate and kept the presentation worker (and idle-sleep checks) busy. - A `conversation_turn_finished` turn keeps the `in_progress` check. Held turns accept any status their gate committed. The unsettled-turn checks (active conversation state, session generation, execution run) still block stale or superseded turns. - Previous-release state: no persisted shape changes. A run committed by the previous release with no saved presentation decision (the restart window) is recovered by the new code. The test fixture uses the unchanged finalizer, so it saves exactly the previous release's shape. A run whose live path already recorded a decision keeps it. A held turn that the previous release recorded as `yielded_control_plane_wait` is not re-presented, so no late duplicate or stale reply appears. - Not changed: generic stranded-work recovery in `recovery/service.ts` still skips only `conversation_turn_finished` runs during the publication window. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use (shell, file edits, test runs) in Claude Code. ## 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> |