mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
4612fa9fb46d31afddea34892aff69aa45a29a89
6809
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandClaude Opus 5.5
|
4612fa9fb4 |
fix(native): restart a drained Daytona run whose runner opened its session (#19711)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs on Daytona start a `paperclip-runnerd` process in a sandbox. A graceful shutdown (every rolling deploy) suspends live native runs "without cancelling provider work". The next boot then decides how each run continues. > - #19583, #19616 and #19684 let restart recovery start a remote bootstrap again when durable evidence proves that no provider work exists. > - A chaos replay found another window. The drain suspended a remote run just after its sandbox runner opened the provider session, before the controller sent a turn and before a session snapshot existed. > - Recovery accepted only an unfinished `session.open` in the controller checkpoint, so it blocked the task as ambiguous. A Retry recovered it, but automatic continuation was lost. > - A related replay showed that the same-run bootstrap reuses the sandbox. The interrupted runner can still hold the runner ingress port there (`runner_ingress_bind_conflict`). > - This pull request proves the "session opened, no turn" case and stops the interrupted runner before relaunch. > - The benefit is that a deploy during Daytona startup continues automatically, without a port conflict. ## Linked Issues or Issue Description **What happened?** A chaos replay (ACPX Claude on Daytona) sent SIGTERM while a remote run was starting. The drain recorded `restart_suspended`. The dying controller still launched the runner and finished `session.open`. On the next boot, restart recovery returned `ambiguous_remote_recovery_evidence` with disposition `blocked`. The decrypted controller checkpoint of that run showed: - `run.prepare` completed and `session.open` completed (provider session opened). - Committed events `session.started`, `session.capabilities.updated` and `session.goal.snapshot` (`ackedSourceSeq: 3`). - No turn command. No run-profile session snapshot and no projected provider events. On the next run in that replay, a same-run bootstrap relaunched the runner in the same sandbox (one lease for all attempts). It failed with `runner_ingress_bind_conflict: failed to bind listener: Address already in use`, because the interrupted runner from 51 s earlier was still listening there. The bounded same-run retry recovered it on its last attempt. **Expected behavior** When the durable controller record proves that no turn was ever sent, recovery starts the bootstrap again. It first stops the interrupted runner in the reused sandbox. **Steps to reproduce** 1. Start a native run on a Daytona environment. 2. Send SIGTERM to the control plane while the runner opens its provider session. 3. Restart the control plane after the controller lease expires. 4. See that restart recovery blocks the task with `ambiguous_remote_recovery_evidence`. Root cause: in `server/src/services/native-runtime/native-restart-recovery.ts`, the remote checkpoint proof (`remoteBootstrapProvenIncomplete`, #19583) required an unfinished `session.open` with no committed events, and no provider events at all. A completed `session.open` with only session-open events had no rule, so it fell through to `ambiguous_remote_recovery_evidence`. Refs #19583, #19616, #19684, #19524 ## What Changed - `native-bootstrap-proof.ts`: new `runnerdStateHasNoProviderTurn` proof. The state must hold only `run.prepare` and `session.open` (pending or completed) and no warm, staged or adopted state. Every committed event must be a session-open event, with contiguous sequence numbers from 1, so no event is hidden. Only the controller issues turn commands, and it writes each one to Postgres before sending it. - `native-runner-checkpoint.ts`: `nativeRunnerCheckpointRemoteBootstrapProof` replaces `nativeRunnerCheckpointProvesIncompleteBootstrap`. It also reads a bounded number of `event:N` parts and returns which proof holds. It still rebuilds the state in memory. - `native-restart-recovery.ts`: a remote run can now claim `bootstrap_incomplete` with reason `remote_session_opened_without_turn`. Projected provider events may only be session-open events (`harness.ready`, `session.started`, `session.resumed`, `session.updated`), and only together with the stronger session-opened proof. A turn or provider event still blocks. - `native-session-executor.ts`: - For a remote sandbox target, the executor accepts the same proof on the restored record. - Before the same-run bootstrap relaunches, `stopStaleRemoteRunnerBeforeBootstrap` stops the interrupted runner in the reused sandbox. It uses the verified identity marker and process check from reattachment, which signals only a process whose identity matches this run: SIGTERM, a wait of up to 10 s, then SIGKILL. - The stop is best effort. If no prior runner can be verified, nothing is stopped, and the bounded same-run retry still handles any conflict. - `doc/run-log-events.md`: document the reason and the stop. ## Verification - Red, then green: `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "drained remote"`. Before the fix, the opened-session case (with a projected `session.started`) returned `blocked` / `ambiguous_remote_recovery_evidence`. - These keep the hold: a `turn.start` command, a committed `turn.started`, an acknowledged event that is missing from the record, and a projected `turn.started`. The executor test restores the checkpoint and checks that the local proof is false and the remote proof is true. - Regression: `native-runner-restart-recovery.integration`, `native-session-executor`, `native-restart-recovery`, `native-bootstrap-retry.integration` and `heartbeat/recovery`: 1460 passed, 14 skipped. - Stale runner stop: controlled sandbox-runner tests (`native-session-executor.test.ts`, "stale runner stop before a same-run remote bootstrap") run the real reattachment identity and process scripts against a fake sandbox runner: - a verified runner that exits on SIGTERM gets SIGTERM only; - a runner that ignores SIGTERM gets SIGTERM, then SIGKILL; - a runner with another identity gets no signal; - no live prior runner means no signal. - Live Daytona proof (wave `w-20261107T021837-s29`, ACPX Claude, `--faults K2 --triggers time,result`, 4 tasks, concurrency 1). The image was built from current master (`paperclip-daytona-runner@sha256:71f711ab…`), and its content id matches this branch. - All 4 tasks crossed a graceful restart, and all passed R1. Two of the K2s fired on the `result` trigger, during a running remote turn. - The K2 during a turn reattached to the live remote runner (`reattach_remote_runner`, generation 2), as designed. - Task t01 failed R6 with an orphan Daytona sandbox that no lease references. The graceful restart came during sandbox creation. This is a separate signature, outside this change. - The timed faults did not land in the exact "session opened, no turn" window. The integration tests cover that window with the evidence from the saved replay database. ## Risks - The proof relies on two facts: only the controller issues turn commands, and the controller writes each command before sending it (`#sendJson` waits for the checkpoint flush). A turn always leaves a `turn.start` command in the record. - The provider session opened in the old sandbox runner is not resumed. The new bootstrap opens a fresh session. No turn ran in the old one, so no work is lost. - The runner stop sends signals inside the run's own sandbox. It signals only after the existing reattachment identity and process checks pass. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, GitHub CLI) 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
|
3fedfe8763 |
fix(runner-e2e): wait out a briefly uncertain owned group instead of failing the stop (#19692)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The restart chaos harness and the runner e2e suites stop and restart the control plane through one process-tree owner (`tests/runner-e2e/process-tree-owner.ts`) > - The owner signals only process groups whose identity it has proven, and refuses everything when a group's identity becomes uncertain > - Under load a group can turn over between two `ps` snapshots: the observed members exit, and a straggler that nobody saw start is left in the group, reparented away from every owned anchor > - The stopper treated that as fatal on the first inspection, so a K2 graceful stop at load 28 failed in under a second, and a chaos storm wave was marked as a harness error although the server came back > - This pull request makes the stopper re-inspect within its existing deadline until the uncertainty settles. It still never signals the uncertain group, and it fails with the original error if the uncertainty persists > - The benefit is that chaos waves under load stop failing on a harness race, without weakening the rule against signaling unproven processes ## Linked Issues or Issue Description Refs #19470 **What is the problem?** `stopOwnedProcessTree` fails at once with "Owned process group identity became uncertain" when, under load, a group's observed members exit and an unobserved straggler remains for a moment. In a chaos storm this failed a K2 stop 0.9 s after SIGTERM (load 28), and the wave exited as a harness error. **What is the expected behavior?** The stopper waits (within the same grace or forced deadline) for a later inspection that settles the group, and it never signals the uncertain group. If the straggler outlives the deadline, the stop still fails with the same error. **Why does it matter?** A harness-only race must not invalidate a chaos wave, and the fix must not let the harness signal a process whose ownership is not proven. ## What Changed - `tests/runner-e2e/server-stop.ts`: the chaos and e2e server stopper (the actual K2 path) uses the same bounded settle-wait for its graceful observe/signal and its SIGKILL escalation. Live evidence: a K2 at load 13.9 failed in 0.5 s on an ACPX Claude provider group (launcher, reaper, module-hook loader, `claude` runtime) that tore itself down during the drain and was gone 0.4 s later. - `tests/runner-e2e/process-tree-owner.ts`: the uncertainty error now names the unowned members (pid, ppid, pgid, kind, start). `settleUncertainIdentity` is exported and shared. Export the `UNCERTAIN_GROUP_IDENTITY` message prefix. `stopOwnedProcessTree` retries the grace signal, the SIGKILL escalation and the stop check every 100 ms while that error persists, bounded by the existing grace deadline and one forced window (set before escalation and reused by the escalation retry, the stop check and the fallback), so a stop never exceeds gracefulMs + forcedMs. Other errors, and an uncertainty that outlives the deadline, take the existing fallback path unchanged. The owner's identity checks do not change, and neither does `signal()`. - `tests/runner-e2e/process-tree-owner.test.ts`: three tests. An uncertainty that starts only at SIGKILL escalation fails within one forced window and never sends SIGKILL. A brief turnover settles and the stop succeeds without signaling the uncertain group or killing the direct child. A persistent turnover still rejects with "identity became uncertain" and never signals. ## Verification - `pnpm exec vitest run --config tests/runner-e2e/vitest.config.ts tests/runner-e2e/process-tree-owner.test.ts tests/runner-e2e/server-stop.test.ts`: 35 pass across `process-tree-owner.test.ts` and `server-stop.test.ts`, including 5 new tests; the new brief-turnover tests fail on the old stoppers. With the old stopper, the first new test fails: it rejects on the first inspection and then kills the direct child. - `node scripts/typecheck-test-projects.mjs tests/runner-e2e`: passes. ## Risks - Low risk, test infrastructure only. - A stop that hits a persistent uncertainty now fails at the deadline (up to the grace time) instead of at once. It sends no extra signals. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits). ## 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> |
||
|
|
18be7ac696 |
fix(server): retire idle native sessions before tenant sleep (#18988)
## Thinking Path > - Paperclip lets people manage AI agents and their work. > - Native agents can retain a warm session between useful turns. > - That owner must close and save its files before the tenant can sleep. > - A failed close must keep the owner visible until retirement succeeds. > - This PR prepares idle resources under the exact current admission hold, then takes a fresh safety snapshot. > - Busy turns, uncertain closure and changed holds still prevent sleep. > - CI also exposed a chat authorization race. The fix repeats full authorization after the exact delivery commits. ## Linked Issues or Issue Description Refs #18639 for local work and incoming wake boundaries. Related #18829 covers suspension receipts after admission. **What happened?** Idle preflight did not retire warm Native owners. Three failed-close paths could forget an owner without proving that its provider stopped. CI also found a false chat rejection when a delivery changed from processing to processed between two validation reads. **Expected behavior** Retire only idle owners under the exact bounded hold. Wait for closure and file collection. Retain failed owners. Scan safety after preparation. Keep diagnostic reads passive. A newly committed chat delivery must pass the full current authorization checks before attestation succeeds. **Steps to reproduce** Finish a warm Native turn. Start an idle drain and request safety with its owner ID. Delay or reject provider closure, or replace the hold during preparation or scanning. The report must keep the instance awake. For the chat race, commit the exact delivery between the first binding read and its fallback read. **Paperclip version or commit** Current head `7f6f02b611dcf5a9a2b05173a44e9631eaf78058` normally integrates actual master `1aa0c3639714f4976cfa56f95aae6e2b7d1235ac`. The merge preserves every unrelated master path. Fresh exact-head CI and complete review remain required. ## What Changed - Retain uncertain Native owners after failed restart, idle timeout and failed-turn closure. - Prepare Native retirement through the existing heartbeat facade. Reuse provider closure and stopped-file collection. - Track preparation under the exact idle owner and expiry. Reject new activity, changed holds and expired holds before and after awaited work. - Take the final durable and local safety snapshot after preparation. Keep diagnostic inspection passive. - Cover ephemeral workspace retirement and useful next use, supported plugin drain and SDK persistence, busy owners, failed closure and hold replacement. - Keep dedicated recovery storage explicit. Use the same validated options for support and storage. Probe-free helpers retain the original implementation bodies and preserve default callers. - Complete the Native fixture through real retirement and budget evaluation. Keep its rejection, file, credential, accounting and strict NONE assertions. - Repeat the complete chat authorization once if the fallback sees the exact batch now processed. Preserve owner locks, principal and binding checks, denial handling and the original retry deadline. ## Verification - The support-mode regression reproduces one failure with four passes under the old policy, then passes all five controls. It retains prior allocation and cleanup cases. The explicit mode does not probe embedded PostgreSQL or use an admin alias. - The deterministic chat race baseline reproduces two authorized failures with all four current-authority denial controls passing. Both Slack and Discord use real PostgreSQL and the real authorization function. The negative setup commits before the attestation transaction; it does not claim to revoke access through a row lock. - The product fix passes normal own App declaration emission with no public declaration changes, plus both current normal owning compiler programs (399 and 611 roots). The same six controls pass after the fix. The full original file plus these controls passes all 30 tests with no skips. - The original controlled Native continuation passes: one test, no failures or skips, 1.718 seconds in its body and 23.138 seconds in its launcher. The original 40-second case and 180-second outer bounds are unchanged. It verifies held closure, strict NONE, an ordinary authorized comment, saved session/files, useful output, encrypted credentials, accepted results and settled accounting. All ten observed processes and groups close; worker connections and listeners reach zero. A separate post-owner audit finds zero sessions; the successful owned database is normally removed. - Earlier preparation, HTTP, lifecycle, workspace and plugin controls remain source-specific evidence. They are not a claim that every final-tree test ran locally. - The original CI assertion log is retained. The controlled race proves a possible cause; the original CI SQL schedule was not captured. Two other jobs lost their self-hosted runners and provided no test verdict. Earlier asset timeouts, readiness failures and unexplained later recovery remain separately recorded. No timeout, failure predicate or authority check was weakened. Focused public commands: ```sh pnpm exec vitest run server/src/__tests__/idle-sleep-preparation.test.ts server/src/__tests__/instance-settings-routes.test.ts --maxWorkers=1 --no-file-parallelism pnpm exec vitest run server/src/__tests__/heartbeat-reviewed-chat-binding.integration.test.ts --maxWorkers=1 --no-file-parallelism ``` Local database qualification uses fresh owned test databases on existing PostgreSQL, bounded pools and one worker. The local allocation adapter changes only database allocation for this test module; it runs normal migrations and client cleanup. It is not part of the published source. No new cluster is created. Local acceptance measured the `0a69efa6` integration with the same committed authorization bytes. The later normal master merge adds startup-timeout classification and recovery changes. Those paths are outside the controlled healthy-backend witness; retirement and authorization hunks are preserved. Exact-head remote CI at `7f6f02b611dcf5a9a2b05173a44e9631eaf78058` has completed with 56 successful contexts and four skips. The complete current Greptile summary is5/5 with no new actionable findings; all review threads are resolved. Ponytail’s current turn is explicitly paused without a current-head verdict; earlier actual findings are fixed and accounted for. No whole final-tree local repository build/test run, live-provider restore or deployed suspension is claimed. Actual master subsequently advanced to `d5c4f7dd02d408a261cedd9a1926b9900a4aedc4`. Its local normal-merge projection with this reviewed head preserves all20 owned paths and every unrelated master blob and mode. Current Runner TypeScript, App declarations and their full owning compiler programs pass. Focused Native cleanup and warm-continuation composition checks are being completed before merge; the new upstream suspension-retry behavior is not inferred from the older controlled-backend witness. The remote PR head remains unchanged. ## Risks - Privileged idle preparation can close idle warm Native sessions. It requires the exact current hold. - Failed closure retains its owner and can keep the tenant awake until normal retry succeeds. - A changed hold cannot authorize sleep. Started cleanup remains tracked until it settles. - The chat repair can perform one extra full authorization read. It keeps the existing deadline and does not grant authority from the fallback query. - Unsupported resources and active work retain their existing blockers. This PR does not certify every transport, plugin or deployed host. ## Model Used OpenAI `gpt-6.1-sol` produced the delegated changes and verification. OpenAI Codex, based on GPT-6, performed managing review and integration. Both used repository tools and code execution. The coordinator's exact backend snapshot and context-window limits were not exposed. ## 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 at exact head `7f6f02b611dcf5a9a2b05173a44e9631eaf78058` - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups at the exact current head - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Codex <noreply@openai.com> |
||
|
DottaandPaperclip
|
55c9c44e41 |
fix: preserve database-only failed controller evidence (#19714)
## Thinking Path > - Paperclip preserves failed work as well as successful work. > - Ephemeral enrollment must retain each selected controller source. > - A failed source can already exist entirely in an encrypted database checkpoint. > - Its original inline snapshot does not create a local filesystem dependency. > - The migration owner must prove that both legacy files are absent before accepting that case. > - This change preserves the failed evidence without creating execution authority. ## Linked Issues or Issue Description Refs: #19678. **What happened?** Startup still reports `native_controllers_require_migration` for a failed retained source with a valid database checkpoint, its original inline snapshot, and no local controller or supervisor. The online archive owner reports that the source is already absent. The existing database-only preservation case accepts only a missing inline snapshot. **Expected behavior** Accept the exact database-only failed evidence after full checkpoint and identity validation. Keep a present local payload blocked. Retain the failed run, original snapshot, coordinator, task pointer and queued work. **Steps to reproduce** 1. Store a valid encrypted checkpoint for a failed native run. 2. Keep its matching inline session snapshot and terminal-failure coordinator. 3. Remove both legacy controller files. 4. Check local controller obligations. 5. The old owner reports an obligation even though the evidence is already in the database. **Paperclip version or commit** The fix builds on current master `d5c4f7dd02`. **Deployment mode** Hosted native Runner with durable controller checkpoints. The local regression uses the existing real PostgreSQL and encrypted checkpoint fixture. ## What Changed - Add a narrow database-only failed-source preservation case to the existing migration owner. - Require the decoded checkpoint, protocol header, admitted execution, selected run and inline snapshot to match their exact scope. - Require both the controller and supervisor paths to be absent. A file or link keeps the local obligation blocked. - Retain the earlier null-snapshot predicate and all original failed execution evidence. Create no recovery metadata or executable authority. - Exercise the actual startup and maintenance consumers. Document the preservation boundary. ## Verification - Final revision `1b1da543dafd576e3c420657e3c2c6d0c882172d`: all checks are terminal, with 56 successful and four skipped, zero pending and zero failures. The [required CI aggregate](https://github.com/paperclipai/paperclip/actions/runs/42329501690/job/128275664414) passed. Build, types, server/Runner/serialized suites, end-to-end aggregate and Snyk passed. [Fresh exact-head review](https://github.com/paperclipai/paperclip/pull/19714#issuecomment-6701018015) is 5/5 with no open threads. - Review found that the first non-recovery assertion stopped at the queued fixture's missing current binding. The fixture now binds the exact native issue/session before calling the real resolver and asserts `unsupported:metadata_format` from the selected failed source. That corrected case passes in 4.58 seconds, with wrapper exit 0 and cleanup complete. The production predicate is unchanged. The original startup RED remains valid. The final source/test type graph, module boundaries and diff checks pass. Fresh revision CI and review remain required. - The actual startup regression failed before the source change: one case, 16.62 seconds, expected the absent-files obligation to be false but received true. The wrapper exited 1 and completed cleanup. This failure is retained. - The final focused owner run passed nine cases in 10.79 seconds. It covers the new DB-only case, controller/supervisor presence, every inline scope field, foreign checkpoint Runner/session identities, the unchanged null-snapshot case, and existing online/offline archival controls. The wrapper exited 0 and completed cleanup. - The new case verifies unchanged run, coordinator, checkpoint and task-session rows, retained queued work, no upload, and no invented recoverable session. - The full qualified server/test source type graph passes with db/shared/Runner source aliases. Module boundaries and diff checks pass. Required broad package builds and regression suites remain in CI. - Read-only source diagnosis validated one exact existing checkpoint with the canonical decoder and matching header, run, Runner and session bindings. No source profile or secret values were emitted. No hosted mutation, admission hold, cancellation, status change, or writers-stopped claim was performed. ## Risks - A false database-only classification could hide local bytes. Both exact legacy paths must be absent, and every protocol and database binding must match. - Corrupt or mismatched checkpoints still fail closed. Existing failed evidence does not become successful or executable. - The original null-snapshot case is unchanged. Present local files still require the existing preservation path. ## Model Used OpenAI GPT-6 through Codex, with reasoning, tool use and code execution. The host does not expose a more specific backend 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
|
a5fe5deebe |
fix(environments): reclaim sandboxes left by a restart during acquire or teardown (#19696)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote agent runs execute in provider sandboxes (for example Daytona) that Paperclip creates and must clean up > - Before each provider create, Paperclip saves a durable creation intent (`sandbox_creation_operations`). Recovery uses it to find and destroy a sandbox whose lease was never committed > - Recovery waited 30 minutes after creation before it checked an intent, because a live server could still be waiting for the provider > - After a restart, the creating server is gone, but the intent still waited the full 30 minutes. A short-lived or redeployed instance never reclaimed those sandboxes > - The teardown that follows has the same problem: a teardown claim fences other sweeps for 15 minutes, and a restart during the claim kept the sandbox alive until the fence expired > - This pull request saves the server process identity with each intent and each teardown claim, and ends the wait when same-host proof shows that process exited > - The benefit is that a restart during sandbox acquire or teardown no longer leaves paid provider sandboxes behind ## Linked Issues or Issue Description **What happened?** A control-plane restart during Daytona sandbox acquire left the created sandbox without an `environment_leases` row. In one restart chaos wave, 6 creation intents stayed `pending` with `next_attempt_at` = creation + 30 minutes. The wave ended (and its environment was deleted) before that time, so 4 sandboxes stayed `stopped` in Daytona. Only Daytona's own auto-delete would remove them. Two of the runs did not exist in `heartbeat_runs` because they were non-run acquires; their intents still had the run id. **Expected behavior** After a restart, each sandbox that this instance and company created, with no committed lease and no live creator, is found by its exact creation name and labels and destroyed through the plugin, after the existing ownership checks. Sandboxes from another instance, another host, or a live server stay untouched. Nothing acts org-wide. **Steps to reproduce** 1. Start a Daytona-backed run. 2. Kill the server (SIGKILL) after the creation intent commits and before the lease commits. 3. Restart the server. On master, the intent stays `pending` for 30 minutes and the sandbox stays in Daytona. 4. The regression tests in `server/src/__tests__/sandbox-creation-operations.test.ts` reproduce this with the controlled provider fixture. Refs #18357 (added the durable creation intent this change completes). **Paperclip version or commit** mastercanary/v2026.1107.0-canary.3 |
||
|
Nicky Leach
|
03498f90ee | Add explicit Runner session release and resume support (#19683) | ||
|
DottaandClaude Opus 5.5
|
f09b4704b6 |
fix(recovery): keep the agent's work when Retry continues after exhausted or blocked recovery (#19666)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Task recovery restarts agent work after a control-plane restart or a failed run > - When automatic recovery runs out or is blocked, the user clicks Retry > - Retry then started a fresh isolated attempt cloned from committed HEAD, so the agent lost its uncommitted files, its task branch and the worktree Git link > - A restart must never lose the agent's work > - This pull request makes Retry keep the task's own workspace when that is safe, and tells the agent where the old files are when a fresh attempt is still required > - The benefit is that a Retry continues the real work instead of starting over ## Linked Issues or Issue Description **What happened?** After `native_restart_recovery_blocked` or an exhausted recovery generation, a Retry ran in `isolated-workspace-attempts/<company>/<issue>/<run>/workspace`. That directory was a detached clone of the base ref. It did not contain the uncommitted edits from the issue's git worktree. It was not on the task branch. The agent was not told where the old files were. A restart-chaos wave graded this as an R2 failure (`pre_fault_files_lost`). A second defect: when a restart hit the bootstrap of a fresh attempt, preparing the same run again failed with `isolated_task_directory_binding_mismatch`, so the Retry could not finish. **Expected behavior** Retry continues in the task's own workspace with its edits and `.git` intact when no other writer can hold it. When a fresh attempt is really needed, the old directory stays as it was and the agent gets its path and its changed files. Preparing the same attempt again after a restart is idempotent. **Paperclip version or commit** `master` at `e482f0f632`, restart-chaos waves on 2026-11-06. **Steps to reproduce** 1. Start a native task in a git worktree. Let the agent write an uncommitted file. 2. Kill the control plane so recovery ends in `native_restart_recovery_blocked` or an exhausted generation. 3. Click Retry. 4. The successor runs in a fresh clone without the uncommitted file or the task branch. Root cause: - `server/src/services/fresh-workspace-continuation.ts` `canSelectFreshWorkspace` returns a fresh intent for every task Retry that has a system recovery record. - `server/src/services/explicit-native-continuation.ts` (fresh selection in the source loop) then always persisted `fresh_isolated_workspace`. - `server/src/services/fresh-workspace-seed.ts` `materializeFreshWorkspaceSeed` clones the project source at the base ref with `--no-local`, checks out a detached commit and removes `origin`. Uncommitted work and the branch are not carried over. - RC-35: `server/src/services/heartbeat/workspace-preparation.ts` rejected any reusable existing workspace while `fresh` was true, including the attempt directory that the same run had already created and bound. ## What Changed - New continuation mode `reuse_issue_workspace`. At Retry admission, after all stop proofs pass, `retainedIssueWorkspaceForRetry` checks that the task's bound workspace is the single local directory the failed run used, owned by the task, not a shared workspace, not under remote storage authority, with no other lease, failed cleanup or running workspace operation, a settled recovery generation, no workspace-export failure, and a readable Git state rooted at that directory. Then the Retry continues there with a fresh session. Otherwise the existing fresh attempt is kept. - The reuse decision consumes the generation's one alternative attempt (`freshAttemptRunId`), sets the Retry strategy to `continue`, and records `issue.retry_workspace_reused`. - Environment selection pins the reused workspace binding and fails with `retry_workspace_binding_mismatch` if the binding changed before dispatch. - Fresh fallback: the run's continuation context gets `priorWorkspace` (path, branch, bounded list of uncommitted files). Git status is read with optional locks disabled. The prompt tells the agent to inspect it and carry over the work. The old directory is not changed. - RC-35: preparing the same fresh attempt again after a restart reuses the attempt directory that this run already bound. - Reuse checks run Git through the shared workspace scan scheduler with short deadlines, because admission can hold the issue lock. - `doc/task-recovery.md` and `doc/execution-semantics.md` describe the new contract. ## Verification - `server/src/services/retry-workspace-continuity.test.ts` (embedded Postgres, real git worktree with uncommitted edits): - exhausted-generation Retry and blocked-recovery Retry continue in the same worktree, `.git` file and branch intact. Both failed before the fix (readiness was `fresh_isolated_workspace`). - Blocked recovery without a generation and duplicate holds for one run also reuse the worktree. A running operation orphaned by a restart of the stopped source does not block reuse. - The fresh attempt is kept when the workspace is unsafe: live lease, running workspace operation, corrupt `.git`, missing directory, remote authority, shared workspace, running export generation. `priorWorkspace` is omitted for the unsafe writer and shared cases. - `workspace-preparation.test.ts`: - A `reuse_issue_workspace` Retry prepares in the original worktree with its uncommitted file and branch. - RC-35 red/green: re-preparing the same fresh attempt reuses its bound directory and keeps files written before the restart. Another run still gets `isolated_task_directory_binding_mismatch`. - The fresh fallback sets `priorWorkspace` with the old path and `unpublished.txt`. - `environment-selection.test.ts`: the new mode pins the binding and rejects a changed binding or cwd. - `packages/adapter-utils/src/server-utils.test.ts`: the prompt names the retained directory and its files. - Regression files run locally: `workspace-preparation`, `environment-selection`, `task-retry`, `quarantined-task-retry`, `explicit-native-continuation`, `workspace-recovery`. - Restart-chaos (`tests/restart-chaos`, local, B1+B5, codex+claude, K1/K4/K6, concurrency 2, rc06-masked): - Final head: wave `w-20261107T014633-s7206` on `c6776283f3`: 8/8 PASS, all 8 restart-crossing. 5 tasks needed a Retry (t03-t07: codex B1 K6, codex B1 K1, codex B5 K6, codex B5 K1, claude B5 K6). All five finished in the original git worktree with `.git` and every pre-fault file present (`workspaceMovedTo` empty, `lostFiles` empty); no "Retry keeps a fresh attempt" log line. - Wave `w-20261107T004829-s7205` on `29373a6b6a`: 8/8 PASS, all 8 restart-crossing. 3 tasks needed a Retry (t02, t06, t07). All three finished in the original git worktree with `.git` and every pre-fault file present; no "Retry keeps a fresh attempt" log line. (`cc178492ef` only makes the operation filter stricter for run-less jobs.) - Wave `w-20261107T000412-s7204` on `d197082107`: 8/8 PASS, 7 restart-crossing. 3 tasks needed a Retry (t02, t04, t06). All three finished in the original git worktree (`workspaceMovedTo` is empty), R2 passed with `.git` and every pre-fault file present, and no "Retry keeps a fresh attempt" log line appeared. - Wave `w-20261106T231416-s7203` on the first commit: 6/6 PASS, but its t01 Retry still moved to `isolated-workspace-attempts`. That instance was not kept, so the exact rejection was not recorded. The likely cause is a `running` workspace operation that a killed server left on the stopped source (the source restarted three times). The second commit treats such restart orphans as not live and logs the rejection reason. A `--replay` of that t01 on `d197082107` finished in the original worktree with its `.git` file and no attempt directory. - Review follow-ups in `d197082107`, `29373a6b6a`, `cc178492ef` and `c6776283f3` (run-less workspace jobs still block reuse; red/green covered by the `running_operation` case): holds are grouped by source run; the readiness fallback (blocked recovery without a generation) also reuses the task workspace; Git probes go through the shared scan scheduler with 10-15 s deadlines; `priorWorkspace` is only set for a task-owned directory with no other writer; the reuse rejection reason is logged; `priorWorkspace` uses the same task-owned, settled, single-writer predicate as reuse; Git probes share one budget per workspace; the RC-35 check is a pure path check; renames are parsed from either porcelain column. ## Risks - A Retry now writes into the old directory instead of a new one. The checks above prevent a second writer. A stale binding fails the run with `retry_workspace_binding_mismatch`; it does not write to another directory. - If the cause of the failure was inside the workspace itself, reuse could repeat it. Workspace-export failures, restore failures and required workspace recovery keep the fresh attempt. - A workspace operation of the stopped source that started before the source settled and before the current server process no longer blocks reuse (a restart orphan). Operations with no run, of another run, or started in the current process still block it. - `priorWorkspace` is an optional new field in the continuation envelope. Older adapters ignore it. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, 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 Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
d5c4f7dd02 |
fix: preserve shared Git sources during online workspace migration (#19688)
## Thinking Path > - Paperclip preserves the work produced by its agents. > - Remote workspace migration must preserve legacy Git history and local edits. > - Linked checkouts can share administration and objects with other tasks. > - A task lock alone cannot exclude writers of a shared or borrowed store. > - The online source owner must derive and retain the full physical ownership closure. > - This change reuses task, project, lifecycle and finalizer locks for that closure. > - Idle legacy Git work can then migrate while unrelated remote turns continue. ## Linked Issues or Issue Description Refs: #19642. **What happened?** Online migration keeps legacy Git sources blocked because several task checkouts can share a project base or a private Git cache. A read-only source observation found 160 pending Git records, 120 distinct paths and nine common stores. Its largest project cohort contained 126 records and 110 task owners. An ordinary-directory fence cannot preserve these sources by itself. **Expected behavior** Derive the physical source owners from stable Git metadata. Hold their existing source and lifecycle keys through publication. Retain all history and recovery obligations. Reject foreign aliases, unknown stores and changed sources. Let unrelated object-backed work continue. **Steps to reproduce** 1. Create a managed project checkout and linked task worktrees. 2. Keep a settled prepared recovery source on a sibling task. 3. Keep unrelated remote work running. 4. Request an online import of an idle leaf checkout. 5. The existing owner keeps the shared source blocked. **Paperclip version or commit** This branch builds on merged #19642 (`7ac99718a4`) and fresh master `fb3cf1a072`. It adds the Git closure. It does not add a writers-stopped shortcut. **Deployment mode** Hosted native Runner with remote workspace storage. Local qualification uses owned synthetic Git repositories, disposable PostgreSQL databases and the actual archive and storage owners. ## What Changed - Read bounded stable Git pointers, common-directory metadata and local alternates. Derive the complete physical source ownership cohort. Preserve all object stores. - Retain common and borrowed store owners in preparation, lifecycle, runtime, expiration and actual reopen paths. Keep company boundaries and the existing 128-key bound. - Hold exact sibling native recovery proofs under their existing physical finalizer keys. Recheck descriptors, allocations, recovery actions and source identities before capture and publication. Preserve original states and warnings. - Require leaf-first nested imports. Reuse the existing immutable child download, restore and source comparison before parent exclusion. - Bind the self-contained archive owner to the held Git namespaces. Keep its three walks, history normalization, fsck, bytes and digest format. Reject bare, detached, network, unknown or changed Git storage. - Check both foreign path fields. Keep foreign metadata private. Document the proof, bounds, source observation and remaining eligibility limits. ## Verification - Final revision `2934e41c923b93e16508b978c849b747c86be645`: all checks are terminal, with 56 successful and four skipped, zero failures and zero pending. The [required CI aggregate](https://github.com/paperclipai/paperclip/actions/runs/42326464408/job/128266279210) passed. Build, types, all server/Runner/serialized suites, end-to-end aggregate and Snyk passed. [Fresh exact-head Greptile review](https://github.com/paperclipai/paperclip/pull/19688#issuecomment-6699760104) is 5/5 with no open threads. GitHub reports MERGEABLE and CLEAN. - Review then found two correctness gaps: an ordinary managed directory inside an unrelated host repository was blocked, and a Git parent could miss a completed nested operation after its child snapshot became missing or deleted. Three actual-owner regressions reproduced both gaps in 9.70 seconds. The corrected owner passes 12 focused cases in 17.08 seconds. It imports and restores only the ordinary root bytes, preserves the outer repository, rejects both missing/deleted child receipts before a parent journal, and retains valid/changed child coverage and private-cache contention. Its wrapper exited 0 and cleanup completed. The first bootstrap attempt failed before tests ran because the host exhausted its 32 shared-memory segment limit; no shared services were stopped and no pass is claimed for that attempt. The final qualified source/test type graph, module boundaries and diff checks pass. - Receipt scans keep canonical path resolution. A lexical SQL prefix can omit children reached through symlinked ancestors. The observed full metadata closure completed in 1.17 seconds; no measured bottleneck justifies weakening that alias proof. Writer admission still rejects uncertain Git ownership before mutation. Expiration returns its typed skip, and reopen returns its structured failure. Runtime and lifecycle do not bypass an unknown source. No broad error-contract change was added. - Full CI at `720104ea975e` exposed five existing boundary cases across three server shards. Four exact cases reproduced locally as failures in 25.98 seconds. Reopen now preserves its structured failure when source inspection rejects the path, without reconstructing it. Runtime recognizes only the exact regular ignore-only `.git/info/exclude` fragment; migration still rejects that fragment. The original arbitrary-path and file-URL import/delete-host/resume fixtures keep both supported input forms and use the genuine offline setup path after their sole writer finishes. The production mode gate is unchanged. A new real-owner control proves the offline flag still rejects a running legacy database writer. - The final focused run passed all ten cases across four suites in 22.75 seconds, including all five CI failures, invalid-pointer admission, fragment rejection, offline writer exclusion and actual nested reopen lock ordering. Its wrapper exited 0 and cleanup completed. The known invalid-pointer case remains fail-closed; the separate unexpected-I/O diagnostic retains safe error metadata without command text or credentials. Existing Git stderr redaction coverage remains intact. The final metadata reader suite passed 20 cases in 2.68 seconds. The qualified server/test source type graph, module boundaries and diff checks pass. No production assertions or timeout defaults were weakened. Fresh CI and review are required for the repair revision. - A later review found that the ordinary early return still used only one foreign path field. The actual ordinary providerRef-only regression failed as incorrectly eligible. One source-path owner now enumerates both local locators for company/foreign ordinary and Git scans, layout checks, early admission, lifecycle/reopen/runtime and expiration/move/preparation retention. Capture and display keep their original preferred path. The actual owner canary passed 49 cases across two suites in 109.15 seconds. It covers cwd-only/ref-only/different/symlink locators, same-company mismatches, foreign owners, lifecycle ordering, Git/recovery imports and expiration/reopen. Three selected-locator fallback cases also failed before their guard; the final eight-case control passed in 31.05 seconds, including the configured alias and valid import/private-child controls. All wrappers exited 0 and completed cleanup. The metadata/locator suite passed 19 cases. The final source/test type graph, module boundaries and diff checks pass. - Review found two P1 gaps. Both configured-directory symlink cases failed before canonicalization. The actual parent import lost an already-imported child's unique private cache and failed its archive fence instead of retaining that owner. The corrected real-owner canary passed four cases in 20.78 seconds: configured symlink runtime provisioning, private-cache contention before the parent journal, successful parent publication after release, and unchanged/changed child coverage. The metadata suite passed 15 cases in 4.14 seconds. Both wrappers exited 0 and cleanup completed. - A fresh source read-only observation found 160 pending Git records and nine common stores. An additional exact receipt query found 1,243 completed migration receipts and 557 nested source paths under pending Git roots. All 126 records in the largest project were present in the captured Git metadata set. For all 110 present candidate workspace records, the derived closure fits the unchanged 128-key cap. The worst case needs 110 issue keys and one project key, with 17 keys of headroom. Its 173 imported children introduce no physical issue owner outside those 110 logical task owners. This checks the observed key bound only. It does not claim migration eligibility or a coherent snapshot. - The initial stacked CI did not run four Runner test lanes: trusted master passed new --package-lane/--native-server options to the old base lane script. The branch is now rebased onto actual merged master and retargeted before synchronization. It includes the canonical current lane parser without an unrelated Runner patch. Fresh new-head CI is required. - The actual importer/source owner run passed 36 cases across three suites in 187.75 seconds. It covers sibling prepared recovery, unrelated running remote work, common-cache contention, changed coordinator/allocation/path state during upload, exact nested coverage, dirty unpushed work, and existing expiration/reopen owners. Wrapper exit 0 and cleanup completed. - A separate previously hanging lifecycle-worker restore passed in 17.33 seconds with the unchanged 15-second test-body limit. An earlier run timed out in three expiration/reopen cases and blocked cleanup; its wrapper later exited 1. These failures remain recorded. They are not counted as passes. - Two boundary controls first failed: a foreign providerRef-only alias was incorrectly eligible, and actual reopen did not admit its distinct cache owner at the initial source boundary. The narrow guards now inspect both foreign locators and derive physical Git keys before the raw lifecycle transaction. The foreign alias and existing real nested reopen contention passed. The private-cache control first retained contradictory recorded source provenance and stayed fail-closed after release. The corrected present legacy-worktree fixture passed in 12.61 seconds. These failed attempts remain recorded; they are not passes. - The metadata reader and serialized/local archive controls use actual Git repositories. They compare archive bytes and digests, remove every original source, restore staged and unstaged work, and run fsck. The metadata reader passed 13 cases in 2.85 seconds. The local/serialized archive controls passed nine cases in 9.27 seconds. All processes exited 0. An additional actual importer control changed a relative commondir pointer to the absolute name of the same store during upload. Publication was rejected, the uploaded journal remained available, and original files stayed intact (one case, 5.86 seconds; wrapper exit 0 and cleanup complete). - The final rebased full local server/test type graph passes with db/shared/Runner source aliases because reused dependency declarations predate new master exports. The first rebased attempt reported those stale declaration fields/tables, and is not counted as a pass. Module boundaries and diff checks pass. Standard package builds, types and broad regression checks remain required in CI. - No hosted import, admission hold, cancellation, provider mutation, source filesystem write or shared-cache Git command was performed for this change. The source observation is read-only and is not a locked eligibility proof. ## Risks - A missing writer owner could make physical source exclusion incomplete. Existing mutation owners retain the same company/task/project keys, and uncertain metadata fails closed. - A project can contain a large cohort. Limits remain finite. A bound failure keeps the source blocked. - Bare, detached, unowned or network Git stores remain blocked online. Missing archived sources retain their existing authenticated reconstruction path. - Exact recovery proofs preserve authority and obligations. They do not infer provider termination or mark a prepared descriptor finalized. - This change does not claim all observed Git records are immediately eligible. Existing preparation, provider, recovery, runtime and source-change blockers remain. ## Model Used OpenAI GPT-6 through Codex, with reasoning, tool use and code execution. The host does not expose a more specific backend 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
|
a894b11a2b |
fix(runner): retry a rejected runner suspension and log every cleanup quarantine (#19653)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A native run's close must prove that its runner durably suspended
before the session's next run may start. If it cannot, the runtime
quarantines the session (`NativeSessionCloseUnrecoverableError`)
> - In a restart chaos storm, an ACPX Claude run succeeded and its
normal close quarantined its own session. The follow-up run on that
session failed 2 s later with `native_session_cleanup_quarantined`, and
no control-plane restart came between the two runs
> - The close sent `runner.suspend` once, the ACPX sidecar rejected it
right after the turn completed, and the barrier gave up at the first
rejected command. No log line showed the quarantine or its cause
> - This pull request retries a rejected suspension within the close
deadline, and logs every cleanup quarantine with its cause when it is
created
> - The benefit is that a successful run no longer blocks its own
follow-up because of a one-time suspension rejection, and any remaining
quarantine can be traced
## Linked Issues or Issue Description
**What happened?**
Run A (ACPX Claude) succeeded. The run log of its close shows:
- `runner did not prove durable suspension before checkpoint`
- `native_session_settlement_incomplete
{"runnerSuspended":false,"providerDrained":true,"suspensionState":{"commandStatus":"failed","runnerLifecycle":"suspended",...},"semanticTools":{"pending":[]},"finalProviderState":{"activeProviderTurnId":null,"providerSettled":true}}`
The runner journal shows `runner.drain` completed, then `runner.suspend`
failed with `failed to suspend ACPX provider: ACPX sidecar command
session.suspend was rejected (retryable=false,
classification=unclassified)`. The follow-up run B on the same session
was created 200 ms after A finished and failed 2 s later at admission
(`retryQuarantinedSessionCleanups`, operator recovery branch). The
server log had no line for the quarantine. One Retry completed the task.
**Steps to reproduce**
1. Run an ACPX task whose sidecar rejects the first `session.suspend`
after the turn completes.
2. Start a follow-up run on the same session.
**Expected behavior**
The close asks again, proves suspension, and the follow-up runs. If a
quarantine is still needed, it is logged with its cause.
**Actual behavior**
The close fails on the first rejection, quarantines the session, and the
follow-up fails with no logged cause.
## What Changed
- `runner/.../durable/runner.rs` (`process_command`): a failed
`runner.suspend`, on first execution or replay, now keeps the runner in
`Continue` instead of `Suspend`. Before, the runner recorded `suspended`
over a provider whose session was still open, then exited. The RC-34
journal shows exactly that: runner lifecycle `suspended`, ACPX provider
`session_open`. A failed `runner.shutdown` still stops the runner.
- `durable-prp-control-plane.ts`: a failed or rejected `runner.suspend`
now lifts the connection's terminal-lifecycle fence, so the next pending
command (the fresh suspend) is delivered on the same connection.
Completed suspensions and every shutdown keep the fence. New test:
"delivers a fresh suspension after a rejected one on the same
connection". Before the fix, the retry was never sent and the test timed
out waiting for the frame.
- `runnerd-codex-transport.ts` (`awaitRunnerSuspensionBarrier`): after a
rejected `runner.suspend`, the barrier sends a fresh suspend command, up
to 3 attempts, 250 ms apart, inside the same close deadline. An exited
runner or a spent budget still fails closed. A rejection changes no
state: the runner returns before it records suspension.
- `native-session-runtime.ts`: new
`onNativeSessionCleanupQuarantined(listener)`. It is called when a
cleanup quarantine is created or escalates to operator recovery, with
company, run, issue, session, `operatorRecoveryRequired`, a bounded
cause (error name, code, message) and bounded settlement facts.
- `server`: `logNativeCleanupQuarantines` logs each quarantine as a
warning. The subscription is process-wide and installed once, so closes
that fail during shutdown, after HTTP closes, are still logged.
## Verification
- Red before the change: `pnpm --filter @paperclipai/paperclip-runner
exec vitest run src/live/runnerd-codex-transport.test.ts -t "rejected
suspension"`.
- "retries a rejected suspension…" got `false`.
- "stops retrying…" saw 1 attempt instead of 3.
- Green after the change: both pass.
- Rust: new test
`rejected_suspend_keeps_the_runner_serving_its_controller` covers a
failed suspend, its replay, and an accepted retry with a fresh command.
`failed_lifecycle_commands_replay_their_terminal_transition` now expects
`Continue` for suspend and `Shutdown` for shutdown. The `pnpm run
check:runner` unit suites pass (559). `acpx_park_readiness` has two
timing tests (`< 500ms`) that fail at this host load on unchanged master
as well.
- New runtime test: "announces every cleanup quarantine with its cause
when it is created" (operator-required quarantine, cause and settlement
reported once). `src/native-session-runtime.test.ts` passes (212).
- The suspension cases in `runnerd-codex-transport.test.ts` pass, except
"proves local suspension after an event backlog… (1024 suffix deltas)".
That case also times out on unchanged master at this host load (27–35);
I confirmed this by stashing the change and rerunning.
- Server `tsc --noEmit` passes after the runner package build.
## Risks
- A provider that keeps rejecting suspension now gets three tries
instead of one, within the existing close deadline. The final outcome
and quarantine are unchanged.
- The cause of the ACPX sidecar's first rejection is not proven. The
logged evidence fits the sidecar's "not at a safe suspension point"
guard (`turnId`, tools or inputs still set) racing the just-completed
turn. This change makes close tolerate it. It does not change the
sidecar.
- Log volume: one warning per quarantine.
- The Rust change ships in the runner binary. Hosted images need a
rebuild to pick it up.
- After a failed suspend, the runner stays alive with its provider. If
every retry fails, close still treats suspension as unproven and kills
the owned runner process, exactly as before.
## 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
Refs #19532, #19609.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
30857db55e |
fix(native): recover a run killed between runner launch and its identity record (#19694)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs start a local `paperclip-runnerd` process. The server records the request to start it, then records its process identity after it spawns. > - If the server is killed between those two records, there is no process ID to check after the restart > - Restart recovery then restarted the bootstrap, but the executor failed with `durable_identity_unreadable` because the runner never wrote its state > - The user's Retry then waited forever with "Retry is waiting for Paperclip to verify that the previous execution stopped", because the stop check needs a process ID > - This pull request restarts that bootstrap and proves on the host that no runner for the run exists > - The benefit is that a restart in the runner launch window no longer loses the task or leaves Retry permanently stuck ## Linked Issues or Issue Description **What happened?** In restart-chaos testing, an ACPX Claude run was created and the server was SIGKILLed 1.3 s later, right after `native.process_start_requested` and before the runner identity was recorded. On restart, recovery claimed `bootstrap_incomplete` (`dead_runner_before_provider_identity`). The executor then threw `runner_state_identity_mismatch: durable_identity_unreadable`: the state root existed, but `control-plane/control-plane-state.json` had never been written. The run failed with `native_continuation_requires_reconciliation`. A Retry was accepted (HTTP 200), but no run was ever created. The issue stayed blocked with `retryWait.reason=process_identity_missing` for over 20 minutes and through another restart. **Expected behavior** When the runner never wrote state, no provider started, so the bootstrap can start again. A Retry should be admitted once the host proves that no runner for the run exists. **Paperclip version or commit** `master` at `fb3cf1a072`. **Steps to reproduce** Create a native local task and SIGKILL the server about 1 s after the run starts, after the runner launch is requested and before its identity is recorded. Restart the server, then click Retry. ## What Changed - `migrateRunnerdStateRootForExecution` (`native-session-executor.ts`): for a `bootstrap_incomplete` restart on a local target, a root is quarantined (kept for inspection) and the bootstrap starts again only when both of these hold: - the root has no `control-plane-state.json`; - no live runner for the run exists on this host. A state file that exists but cannot be read, or a runner that started but has not connected yet, still fails closed with `durable_identity_unreadable`. - `unidentifiedLocalRunnerProvenAbsent` (`native-session-executor.ts`) proves that a runner is absent when both of these hold: - no host process carries the run's `--run-id <runId>` argument (`paperclip-runnerd` is always started with it, detached and unreferenced); - no saved control-plane state for this run exists, in the canonical root or in quarantine. A provider starts only after its runner connects and writes that state. - `native.process_start_requested` now records the launching `hostname`. - `recordUnidentifiedLocalRunnerStop` (`native-local-process-stop.ts`) applies the proof only when all of these hold: - the run is native; - the run has no process ID; - its latest launch event is `native.process_start_requested`, from this host; - all of its leases are local. A launch recorded without a host stays blocked. - The process listing runs asynchronously with a 5 s deadline. A failed or slow listing proves nothing. It then writes `native.local_process_stopped` with null process IDs and `basis: "unidentified_runner_absent"`. A later launch request invalidates it as usual. - The continuation and Retry gate (`explicit-native-continuation.ts`) uses this proof before returning `process_identity_missing`. A dry run checks the proof but records nothing. - `doc/run-log-events.md` documents the new receipt basis. ## Verification - `native-session-executor.test.ts`: - The bootstrap restart with absent state was red before the fix (`durable_identity_unreadable`) and is green after it (root quarantined, backend admitted). - Unreadable state, and a live runner that has not connected, still reject. - Proof cases: absent → true; a live process with `--run-id <runId>` → false; this run's saved state → false; another run ID → false. - `explicit-native-continuation.test.ts` (embedded Postgres, real admission): - The `absent` and `dry_run` cases were red before the fix (blocked) and are green after it (admitted). Only the real admission records the receipt. - The `live_runner`, `saved_state`, `foreign_host` and `unbound_launch` cases stay blocked. - Broader local run after the review fixes: `explicit-native-continuation`, `native-session-executor`, `native-recovered-owner-followup`, `native-runner-restart-recovery.integration` and `task-retry` gave 1836 passed, 0 failed. - A chaos replay of the original task (`--replay w-20261106T232854-s7103/t26`) passed (R1 PASS). Its kill landed after the runner identity was recorded (`resume_dead_runner`), not in the launch window, so the tests above are what reproduce that window. - `tsc --noEmit` for `server` passes. ## Risks - The proof depends on `ps -axww` listing every process on the launching host. If `ps` fails or times out, the proof fails closed (still blocked). Remote runners and other hosts are excluded. - A runner that was spawned but has not yet written its state could be mid-start when admission runs. It would still carry `--run-id` in its arguments, so the proof refuses. - Only the bootstrap path with no state file changes. Ownership, checkpoint and reattach checks are unchanged. > 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, test execution). 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> |
||
|
DottaandClaude Opus 5.5
|
aca908a165 |
fix(restart-chaos): stop the timeline echo loop when stdout breaks (#19695)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The restart chaos harness (`tests/restart-chaos/`) runs waves of
real agent tasks while it restarts the control plane on purpose
> - A wave writes a timeline file and also echoes every timeline entry
to stdout
> - When the process that reads that stdout exits, every echo fails with
EPIPE. The uncaught-exception handler then records the error with
another echo, so the loop never ends
> - In one Daytona wave this wrote 3.2 million entries (2.5 GB) in 30
minutes. It also stopped the wave from grading its tasks
> - This pull request stops the echo after the first stdout error, and
records the stdout loss once
> - The benefit is that a wave keeps running and grading when its log
reader goes away
## Linked Issues or Issue Description
**Bug description**
`Timeline.add` echoes with `console.info`. After stdout breaks, each
echo raises EPIPE as an uncaught exception. The handler in `main.ts`
calls `timeline.add("harness_uncaught_exception")`, and that call echoes
again.
**Steps to reproduce**
Run a wave as a child process that reads its stdout, for example from a
wave-loop script. Kill the parent while the wave runs. `timeline.jsonl`
then grows without bound with `harness_uncaught_exception` / `write
EPIPE` entries.
**Expected behavior**
The wave goes on writing its files (timeline, grades, ledger) and stops
writing to the broken stdout.
Refs #19544
## What Changed
- `tests/restart-chaos/timeline.ts`: a `process.stdout` error listener
and a guard around `console.info` turn off the echo for good after a
broken-pipe error. The timeline file is still written. On the first
entry after the echo stops, the timeline writes one
`harness_stdout_lost` entry (file only) with the error. The file adds
`isBrokenStdoutError` and `markTimelineEchoBroken`.
- `tests/restart-chaos/main.ts`: the uncaught-exception handler passes
broken-stdout errors to `markTimelineEchoBroken`. It does not mark the
wave as interrupted and does not set a harness error.
- `tests/restart-chaos/timeline.test.ts` (new): the error
classification, and the file-but-no-echo behaviour with exactly one
`harness_stdout_lost` entry.
## Verification
- `pnpm exec vitest run --config tests/restart-chaos/vitest.config.ts`:
102/102 pass.
- `node scripts/typecheck-test-projects.mjs tests/restart-chaos`: clean.
- Incident: wave `w-20261107T001552-s13`. Its timeline had 76 real
entries, then 3,242,762 `harness_uncaught_exception` entries. Those 3.2
million entries came from the loop this pull request removes.
## Risks
- This change touches the test harness only.
- After stdout breaks, the console shows no wave progress, but every
result is still in the wave's files. A different uncaught exception
still stops the wave as before.
> 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, in Claude Code with
tool use (shell, file edits, GitHub CLI). Default context window,
extended thinking.
## 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
|
803cb3bfe3 |
fix(native): re-bootstrap an OpenCode runner drained before its first turn (#19704)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs start a local `paperclip-runnerd`. After a restart, recovery may start the bootstrap again when the durable state root proves that no provider work started. > - #19665 added a proof for a graceful drain that lands after `session.open` started a fresh session and before the first turn. It requires every recorded provider process to have exited. > - OpenCode records provider startup ownership twice during `session.open`. The first record comes before launch, with `processId: null` and `processGroupId: null`. > - The proof treated `processId: null` as a process it could not prove gone. An OpenCode run drained in this window still failed with `runner_state_identity_mismatch: bootstrap_not_proven_incomplete`. > - This pull request treats a null process id as "no process yet". The benefit is that a graceful restart during OpenCode bootstrap continues automatically, as it already does for ACPX. ## Linked Issues or Issue Description Refs #19665, #19524 **What happened?** A restart chaos wave sent SIGTERM to the control plane about 11 s after an OpenCode run started. The drain suspended the run. At the next boot, recovery verified that the local runner had stopped and chose `bootstrap_incomplete`. The executor then failed the run with `adapter_failed` and `runner_state_identity_mismatch: bootstrap_not_proven_incomplete`. The kept state root had: - `run.prepare` and `session.open` completed, with a freshly started session (`status: "started"`); - only `session.started`, `session.capabilities.updated` and `session.goal.snapshot` committed; - two `harness.diagnostic` events of type `provider_startup_ownership` in the runner outbox, the first with null process ids and the second with the server pid and process group; - no turn command, and the OpenCode server process had exited. **Steps to reproduce** 1. Start an OpenCode task on a local runner. 2. Send SIGTERM to the control plane after `session.open` completes and before the first turn. 3. Let the control plane restart. **Expected behavior** Recovery starts the bootstrap again, and the task continues. **Actual behavior** The run fails. The task needs a manual Retry. ## What Changed - `native-bootstrap-proof.ts`: the process-id collector skips `processId`, `pid` and `processGroupId` values that are `null`. A null value records a launch that has not produced a process yet. Any other non-number value still fails the proof, and every numeric id must still be proven gone. - Tests: a red test with the OpenCode startup-ownership shape, plus counter-tests for a live OpenCode server process, a non-numeric process id, and a started turn. ## Verification - Red before the fix: "allows a fresh bootstrap after a drain once OpenCode opened its session before the first turn" failed with `expected false to be true`. Green after the fix: the `native incomplete-bootstrap evidence` block passes 36/36. - I ran the proof read-only against kept real roots: - the OpenCode drained root now returns `true`; - an OpenCode root from the same wave where a turn ran (108 events) still returns `false`; - the three roots from #19524 and #19665 still return `true`. - `native-runner-restart-recovery.integration.test.ts` and `native-restart-recovery.test.ts`: 134 passed. `tsc --noEmit -p server/tsconfig.json` passes. - Live replay: see the PR comment. ## Risks - Low. The change only skips null ids. Numeric ids still need a proven exit. Any turn, provider output, resumed session, or remote runner still fails closed. > 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`), used through Claude Code with tool use (shell, file edits, test execution). Default context window. Extended reasoning was available. ## 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> |
||
|
Devin FoleyandPaperclip
|
1aa0c36397 |
Fix execution after onboarding plan approval (#19698)
## Thinking Path > - Paperclip manages agents and their work. > - First-task onboarding turns a user's goal into an approved proposal. > - The interview offers a plan and a team to carry out that plan. > - The instructions leave execution after acceptance unclear. > - An agent can add a plan-only approval boundary and finish without doing the requested work. > - This change makes the approval scope explicit and tests both execution and plan-only outcomes. ## Linked Issues or Issue Description **What happened?** An onboarding agent can treat an interview about building a product as a request for a plan only. It can then retain the approved plan and close onboarding without creating execution tasks. The interview evaluation previously accepted that result. **Expected behavior** Acceptance of an execution proposal starts its selected work. An explicit plan-only request stops after planning. User-requested separate approvals and existing permission and budget controls still apply. **Steps to reproduce** 1. Choose the onboarding interview and describe a deliverable. 2. Accept the saved plan proposal. 3. Check that the agent starts the approved work instead of closing with only a plan. ## What Changed - Clarify execution intent, approval wording, continuation, and completion in the first-task prompt. - Require assigned child work and saved output in the interview acceptance evaluation. Use checkbox acceptance in the regular interview journey and retain conversational acceptance in the approval-reply suite. - Add an explicit plan-only acceptance journey and nine grader regression checks. - Update evaluation counts, grading versions, and onboarding verification guidance. ## Verification - Passed: all applicable CI checks on `198a6b47cac7`, including typechecks, the general and serialized server tests, Runner checks, build, and browser E2E shards. [CI run](https://github.com/paperclipai/paperclip/actions/runs/42324855376). - Passed locally: the full Runner support suite, 6,860 tests across 323 files (three skipped), plus 13 onboarding asset tests. - Passed locally: the full workspace build, workspace package typechecks (server rechecked after restoring excluded-plugin SDK links), and the Runner evaluation TypeScript check. - Passed: map validation and whitespace checks. - Initial broad local checks failed during checkout preparation because PostgreSQL library symlinks and runtime/SDK assets were missing. Restoring the dependency setup made the affected controls pass. The full root test suite passed in CI; it was not rerun locally after that repair. - Live model journeys have not been run. Grader tests do not prove model compliance. ## Risks - Prompt behavior needs live qualification across providers. This change adds no server-side enforcement or permission bypass. - The new plan-only case adds four local evaluation cells. A full campaign now has 56 first-task cells. - Existing proposals that reserve later approval keep that restriction. This change does not rewrite tenant data or start existing onboarding tasks. - No schema migration or deployment is included. ## Model Used - OpenAI `gpt-6.1-sol`, xhigh reasoning effort, with repository tools and local test execution. Context window size is not exposed by this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass (focused and full Runner support locally; full root coverage passed in CI) - [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
|
c37bc749ac |
fix(native): keep restart continuations alive through early results and repeat restarts (#19699)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs continue a turn lost to a control-plane restart once, in the same conversation (#15297, #19501) > - In restart storm 3, two Codex runs each crossed one restart and still needed a Retry > - The continuation was attempted, but the driver failed it with `turn_binding_mismatch`, and the run then reported "runnerd exited before provider turn startup" > - A second restart that loses the continuation turn itself also needed a Retry, because the one-shot marker was already used > - A restart should not cost a click when continuing is safe, and a failure should name its real cause > - This pull request fixes the start race, reports the driver's own failure, and gives each admitted restart its own continuation > - The benefit is that interrupted tasks finish without a Retry, within the existing recovery budget ## Linked Issues or Issue Description Refs #19501, #15297, #19658, #19539 **What happened?** Restart storm 3 (seed 7103, RC06 mask on, load 9–12). Codex B5 Retry runs crossed one K1 restart mid-turn. After the restart, recovery resumed the dead runner. The runner restored the conversation and recorded `turn.failed` with `provider_turn_lost_on_restore` (`recoverable: true`). The control plane started the same-conversation continuation (`resumeInterruptedTurn` → `startTurn`). About 1 s later the transport closed with `protocol_failure:turn_binding_mismatch`. The run failed as `native_runner_process_exited: runnerd exited before provider turn startup`. The task needed another Retry. I reproduced it with the focused cell (`--workloads B5 --faults K1 --triggers retry`) and a temporary diagnostic. The failure is "Provider semantic result did not name the active turn": a `paperclip/runResult` reached the Codex driver while the continuation's `turn/start` was still pending and no turn was bound. The driver waits for a pending start before it handles `turn/completed`, but not before it handles `paperclip/runResult`. **Expected behavior** After a restart that loses a turn, the run continues automatically when that is safe. Each admitted restart attempt can continue once, within the provider-attempt budget. A failure names its real cause. **Steps to reproduce** 1. Run `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --adapters codex --workloads B5 --faults K1 --triggers retry --tasks 2 --concurrency 2 --seed 4040 --rc06-masked`. 2. When a lost turn's continuation receives a semantic result before its start settles, the run fails with `turn_binding_mismatch` and the task needs a Retry. The race is intermittent: 1 of about 14 lost turns in my runs. **Root cause** - `packages/paperclip-runner/src/drivers/codex/codex-session-notifications.ts`: the `paperclip/runResult` branch checked the active-turn binding without waiting for `state.turnStartSettled`. The `turn/completed` branch already waits. - `codex-harness-session.ts` `startTurn`: the driver's own protocol failure closed the transport. The pending start then reported the runner exit, which hid the cause. - `codex-session-terminal.ts` `mapTerminalTurn`: a consumed one-shot marker blocked every later continuation in the run, even when the newly lost turn was that continuation itself. The one-continuation allowance is per run, not per issue or session lineage: attaching a new run resets the marker. The storm runs did attempt their continuation; the start race made it fail. ## What Changed - Codex driver: a `paperclip/runResult` waits for a pending turn start to settle before it is bound, like a terminal. - Codex driver: when this session's own protocol failure ends a pending turn start, `startTurn` throws `codex_turn_start_protocol_failure: <code>: <message>` instead of the runner exit. CodexRpc errors and protocol-integrity errors keep their own types. The server now classifies this as `native_session_interrupted` instead of `native_runner_process_exited`; both are retryable. - A later restart that loses the continuation turn itself re-arms the one-shot marker: the driver resets it as it was before the first continuation. It does this when it maps the lost terminal, including during recovery from a checkpoint taken while the continuation ran. The checkpoint gate `nativeRestartInterruptedTurnId` stays strict: a consumed marker still means the allowance is used. Each continuation still needs an admitted dead-runner restart. The existing provider-attempt budget bounds the repeats, and an exhausted budget still blocks with Retry. - `doc/execution-semantics.md`: an additive paragraph. ## Verification Red before the fix, green after: - `codex-app-server-driver.events.test.ts` "binds a semantic result that arrives before its pending turn start settles". Red: `session.failed`. Green: the result binds to `turn-1` and the turn completes. - `codex-app-server-driver.events.test.ts` "names its own protocol failure when that failure ends a pending turn start". Red: "runnerd exited before provider turn startup". Green: `codex_turn_start_protocol_failure: turn_binding_mismatch`. - `codex-app-server-driver.recovery.test.ts` "offers another restart continuation when a later restart loses the continuation turn itself". Red: `null`. Green: `turn-2`. A checkpoint with a consumed marker still returns `null`. - `codex-app-server-driver.recovery.test.ts` "re-arms the continuation when recovery finds the checkpointed continuation turn lost". This recovers from a checkpoint taken during the continuation, then starts the next continuation. Red without the re-arm (the marker stays consumed); green with it. - `native-runner-restart-recovery.integration.test.ts`: the same checkpoint (continuation active, consumed marker naming it) is claimed as `resume_dead_runner`. This is regression coverage. Suites run locally on the final code: - Runner: Codex driver, `native-session-runtime`, `runnerd-restart-continuation` and contracts (745 tests). - Server: `native-runner-restart-recovery.integration.test.ts`. - `tsc` for the runner tests and the server. - A few real-process tests timed out at about 6 s while six live agents ran. Each passed when rerun alone. The settlement suite passed both with and without this change at the same load. Live (local, real Codex, macOS arm64), focused restart-chaos cell `--adapters codex --workloads B5 --faults K1 --triggers retry,time --tasks 6 --seed 4042 --rc06-masked`: 6/6 crossed a restart and passed. R1 passed for all 6, so each continued without a Retry. ## Risks - A run can now continue more than once in the same conversation, once per admitted restart. Each continuation asks the agent to reconcile unfinished actions before repeating them. The provider-attempt budget still caps the repeats. - A semantic result that arrives during a pending start now waits for that start. If the start fails, the result is still rejected by the existing binding check. - The start race was intermittent (1 of about 14 lost turns), so the live cell shows absence of the failure, not a forced repro. The driver regression reproduces it deterministically. - No schema, API or migration change. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use, code 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 - [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
|
11c07c622c |
fix(native): restart a remote bootstrap retry that shutdown cut before runner launch (#19684)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs on Daytona start a `paperclip-runnerd` process in a
sandbox. Restart recovery decides what to do with each live or due run
after the control plane restarts.
> - #19616 added the `remote_runner_never_launched` proof. A claimed
remote run with no runner process request and no controller checkpoint
starts its bootstrap again.
> - A graceful shutdown closes the local runtime listener before its
drain. A remote run that is still starting then fails with
`local_runtime_api_listener_unavailable`. The executor settles it as a
same-run `bootstrap_retry` and releases its lease. #19596 treats this as
a restart detach only for local runners.
> - When the server restarted with that retry due, restart recovery
claimed the row. The proof accepted only rows that still held a lease,
so recovery blocked the task as ambiguous.
> - This pull request lets the same proof also admit that settled
`bootstrap_retry` row.
> - The benefit is that a graceful restart during Daytona startup
continues automatically.
## Linked Issues or Issue Description
**What happened?**
A chaos wave restarted the control plane gracefully (SIGTERM) while an
ACPX Claude run on Daytona was starting. At 2 s after `stage.workspace`,
the run failed with `local_runtime_api_listener_unavailable`, and the
executor persisted "same-run resume persisted" with `recoveryMode:
bootstrap_retry`. After the restart, restart recovery returned
`ambiguous_remote_recovery_evidence` with disposition `blocked`. The
task rested blocked.
Evidence in the saved database at that point:
- Coordinator `phase: retryable_failure`, `attempt: 1`,
`controllerGeneration: 1`, controller identity recorded, and no lease
owner.
- `failureDetail` recorded `recoveryMode: bootstrap_retry`,
`checkpointExists: false`, `providerEventsExist: false` and
`providerSessionEstablished: false`.
- The run had no runner checkpoint rows, no
`native.process_start_requested`, no process identity, and no
runner-sourced event. It also had no PID and no provider session.
**Expected behavior**
Restart recovery starts the bootstrap again, as it does for a claimed
run that holds a lease (#19616) and for a local run.
**Steps to reproduce**
1. Start a native run on a Daytona environment.
2. Send SIGTERM to the control plane after workspace staging and before
the runner is requested. The run settles as a same-run
`bootstrap_retry`.
3. Restart the control plane while that retry is due.
4. See that restart recovery blocks the task with
`ambiguous_remote_recovery_evidence`.
Root cause: in
`server/src/services/native-runtime/native-restart-recovery.ts`, the
`remoteRunnerNeverLaunched` condition required `phase === "observed"`
and a lease owner. The settled `retryable_failure` / `bootstrap_retry`
row failed that check. The remote classifier then returned
`ambiguous_remote_recovery_evidence`.
Refs #19616, #19596, #19583
## What Changed
- `native-restart-recovery.ts`: `remoteRunnerNeverLaunched` now accepts
one of two coordinator shapes:
- an `observed` claim that holds a lease (unchanged);
- a `retryable_failure` with `recoveryMode: bootstrap_retry`, run status
`failed`, and no lease owner.
- All other proof conditions are unchanged. The run must still have no
process start, process identity, adapter, tool or runner-sourced event.
It must still have no process fields, no provider session, no provider
evidence and no checkpoint. The same three-attempt budget applies.
- An `exact_checkpoint_resume` retry keeps its own admission path.
- Admission: before claiming a settled retry, restart recovery applies
the same admission as the resumption reconciler
(`readNativeSessionResumptionAgent`): task pause hold, agent status,
dependencies, ownership, and competing runs. An ineligible retry is
deferred with `native_retry_admission_ineligible` and keeps waiting.
- `doc/run-log-events.md`: note the settled `bootstrap_retry` case.
## Verification
- Red, then green: `cd server && npx vitest run
src/services/native-runtime/native-runner-restart-recovery.integration.test.ts
-t "bootstrap retry"`. Before the fix, the proven case and the budget
case returned `blocked` / `ambiguous_remote_recovery_evidence`.
- New cases: the proven case gets `bootstrap_incomplete`. The
coordinator returns to `observed`, `nextAttemptAt` is cleared, the run
returns to `running`, and no recovery action is created. A requested
process start or a committed runner event keeps the hold. A spent budget
blocks with `execution_recovery_budget_exhausted`. An
`exact_checkpoint_resume` retry does not take this path.
- Regression: `native-runner-restart-recovery.integration`,
`heartbeat/recovery`, `native-restart-recovery`,
`native-bootstrap-retry.integration` and
`native-remote-retry-admission`: 337 passed, 14 skipped.
- Admission: a paused agent's due `bootstrap_retry` is deferred with
`native_retry_admission_ineligible`. It is not started (red before the
follow-up commit, green after).
- Live Daytona replay of `w-20261106T234719-s12/t02`, 3 runs, ACPX
Claude B1, using the recorded K4, K2 and K1 sequence. Image: the wave's
own `paperclip-daytona-runner@sha256:2767e91c…`, with runner inputs
pinned to its revision
|
||
|
DottaandClaude Opus 5.5
|
d26372dc8a |
test(restart-chaos): add a result trigger and recovered-run comment-wake guards (#19689)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The restart chaos harness kills the control plane at chosen moments and grades whether each task still finishes correctly > - One wave lost a user's follow-up comment after a kill that landed after the run accepted its result but before the run settled > - No existing trigger aims a fault at that moment, so the case was hard to reproduce on purpose > - The recovery path that settles such a run had no regression test that covers the saved comment > - This pull request adds a `result` trigger to the harness and a regression test for the recovery path > - The benefit is that the harness can aim faults at the result-accepted window, and a change that drops, repeats, or re-wakes a saved comment fails a test ## Linked Issues or Issue Description **What happened?** In a restart-chaos storm wave, a user posted a comment while a native run was active. A hard kill (K1) landed after the run accepted its result and before the run settled. The run settled `succeeded` and the task stayed `done`, but no follow-up run read the comment. **Steps to reproduce** The original failure did not reproduce. Three targeted waves ran 12 tasks with K1 at comment time or in the new result window. Every follow-up comment was consumed exactly once. The new regression test also passes on current `master`. The test and the trigger guard the path and do not claim a fix. **Expected behavior** A comment posted during a run is consumed exactly once after the run ends, including when restart recovery settles the run. ## What Changed - `tests/restart-chaos`: add `--triggers result` (C1 only). K1 fires 0–3 s after the run that received the follow-up proposes or accepts its result (`run.result.proposed` or `run.result.accepted`). The fault fires only while that run is still `running`: the observer checks the latest sample and rereads the runs right before dispatch. A run that settled during the delay, or a follow-up posted at rest, records a `result` miss. Add poll-driven unit tests for these cases and a README row. - `server/src/__tests__/native-run-projection-work.test.ts`: add embedded Postgres tests in which background recovery, not the executor, settles the run (projection, environment settlement, and deferred-receipt workers), with a deferred user comment and an active local lease left by the killed executor. The tests check: - The comment is promoted into exactly one follow-up run, and a second restart adds none. - A comment the run already received as a same-turn steer is not woken again. The other saved comment still is. - A queue whose only comment was steered stays cancelled, and no follow-up starts. - A Retry click does not consume the comment a second time. A click that observed the source is stale. A click that observed the follow-up joins it. ## Verification - `cd tests/restart-chaos && npx vitest run --config vitest.config.ts` (96 tests pass) - `cd server && npx vitest run src/__tests__/native-run-projection-work.test.ts` (54 tests pass) - Live, local, ACPX Claude, Codex and OpenCode: `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --workloads C1 --triggers comment --faults K1 --adapters codex,claude,opencode --tasks 4 --concurrency 2 --rc06-masked` passed 4 of 4. Two waves with `--triggers result` consumed each follow-up exactly once. One task in those waves found a separate defect: a kill 1.4 s after the result commit left the run `running` behind the dead controller's 20-minute finalization lease. A separate pull request fixes that defect. ## Risks - Low risk. These are test and harness changes only. No production code changes. - The `result` trigger depends on the observer's poll interval (about 3 s), so the exact kill point within the window varies. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, run as an agent in Claude Code with tool use (shell, file editing, local test 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> |
||
|
DottaandPaperclip
|
5e4d24b6c8 |
fix(recovery): preserve admitted workspace on native restart (#19693)
## Thinking Path > - Paperclip manages AI agents and their work. > - A Retry can admit a new workspace before native provider work starts. > - The native execution input records that admitted workspace. > - A server restart must recover the same run under that recorded contract. > - Retry readiness previously evaluated fresh admission again after the run had started. > - This could replace the workspace and make the retained lease fail its correct identity checks. > - This change keeps the admitted workspace and leaves reattachment authority with the existing lease and controller checks. ## Linked Issues or Issue Description **What happened?** A native run started by Retry could do useful work, then fail after a server restart. Preparation evaluated the original fresh-workspace receipt again. It reserved a replacement workspace under the Retry intent ID. The existing runner still owned the original workspace, so setup failed with `native_remote_recovery_lease_mismatch`. **Expected behavior** Restart recovery must retain the workspace recorded in the same run's native execution input. A later configuration or admission decision must not replace it. **Steps to reproduce** 1. Admit a remote workspace for a Retry run. 2. Persist its native execution input and keep its original controller and environment lease records. 3. Recover the same run while its canonical Retry receipt selects fresh admission. 4. Preparation must reach the original lease with the original workspace. Related: #19340 preserves verified Retry workspaces. This change handles an already admitted native run instead of selecting a workspace for a new turn. Public duplicate searches for admitted/fresh workspace restart found no matching repair. ## What Changed - Stop fresh admission when a parsed native input names a real workspace in the run's company. - Validate the company, run, agent, and task binding before using that input. - Keep synthetic run-ID bootstraps on the existing admission path. Hold missing real bindings. - Test both recovery-action and ordinary user-Retry receipts against real workspace, controller, and lease rows. Check malformed, foreign, missing and opaque bindings, synthetic bootstrap behavior, and no-files API controllers. - Align the resume-owner fixture with the real transient run-ID binding. Give each invocation a unique issue label so a CI test retry cannot mask the original failure. ## Verification - RED on unchanged production: the new reservation regression failed with `native_remote_recovery_lease_mismatch` instead of reaching the admitted lease. - Review RED: an opaque task-workspace ID reached the UUID query. The new guard returns the controlled missing-admission error before the query. - Review RED: a valid no-files Cursor controller was treated as a missing task workspace. The existing task-workspace projection now keeps that controller on its prior path. - Clean final production-source regression: both restart cases passed in a fresh embedded PostgreSQL cluster. These cover both receipt paths and all binding, bootstrap and API-controller controls. - The initial focused checks passed 11 cases. Seven existing ordinary Retry and environment selection cases also passed. A later combined final-base run had 17 assertions pass but failed workspace teardown at 90 seconds and task-Retry setup at 30 seconds. That attempt is retained as a failure; the two new restart cases then passed alone with exit 0. Earlier PostgreSQL support-probe failures are also retained as setup failures. - The worktree's own Runner, shared and SDK dependencies were built. All seven server typecheck layers passed for the final production source. The final additional commit changes only the existing resume-owner fixture. - Required CI found an invalid fixture binding in [server shard 11](https://github.com/paperclipai/paperclip/actions/runs/42322379917/job/128250151095). The first attempt failed before lease acquisition because the fixture named a random workspace with no row. The automatic test retry then failed on its fixed issue label. The fixture now uses the transient run-ID contract and a unique label. Its exact original acquisition and owner-release assertions pass locally with exit 0. - Whitespace and added-code-comment checks passed. - Final commit `f0e41e8c6b949f43863ec60fc7b96b97025eafc6` passed required [CI](https://github.com/paperclipai/paperclip/actions/runs/42323826555): 56 successful checks, four intended skips, one neutral result, and no pending or failed checks. Greptile and Ponytail both gave this exact head 5/5 with no actionable findings. No review threads remain open. Hosted deployment and original-task recovery remain separate from local proof. ## Risks Malformed or missing native admission records now hold rather than selecting fresh work. Valid initial admissions and synthetic run-ID bootstraps retain their prior path. The change does not grant reattachment authority or settle provider effects. Existing controller, workspace writer, lease, Stop, and trust checks still apply. No API, schema, or configuration contract changes. ## Model Used OpenAI GPT-6 family through Codex. The runtime does not expose the exact serving model ID or context window. No model override was used. The assistant used reasoning, code editing, shell tools, and local tests. ## 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
|
9a52a2dfeb |
fix(native): retry ACPX session.open startup timeouts and give cold Claude a 60 s bound (#19661)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs start an ACPX provider through Runner. `session.open` opens the provider session before any prompt. > - On a loaded host, a cold ACPX Claude `session.open` took longer than its 30-second bound. The controller reported `runner_local_connect_failed: PRP command session.open timed out`. > - The server classified this as the generic `native_session_interrupted` with `ambiguous_state`, so the task was blocked at once. A person had to click Retry, sometimes several times. In one restart-chaos storm, one task needed three Retry clicks, and on another task both Retry runs timed out too. > - The server already has a bounded same-run bootstrap retry for failures that come before provider work (#18854). It did not recognize this timeout. The 30-second bound also cut off normal slow startups. > - This pull request gives cold ACPX Claude a 60-second `session.open` bound, like Pi and OpenCode (#19487). It also retries a startup `session.open` timeout automatically on the same run, from clean state. > - The benefit is that a slow cold start on a busy host completes, or retries itself, instead of waiting for a person. ## Linked Issues or Issue Description Refs #18854 (bounded bootstrap retry for definite `session.open` failures), #19487 (60-second cold OpenCode admission), #19665 (the bootstrap proof that this change reuses). **What happened?** ACPX Claude runs failed with `runner_local_connect_failed: PRP command session.open timed out` about 35 to 40 seconds after they started. The coordinator recorded `native_session_interrupted` with `recoveryMode: "ambiguous_state"` and `providerSessionEstablished: false`. The task was blocked with a board-owned action. No provider event and no checkpoint existed. The host 1-minute load was 17 to 44, sometimes with no restart nearby. **Expected behavior** A normal slow cold start completes. A `session.open` that still times out during runner startup, with no provider event, no checkpoint and no turn, retries on the same run with backoff. At most two automatic attempts follow the first. A run where provider work may have started is never retried automatically. **Steps to reproduce** 1. Run ACPX Claude tasks on a host with a 1-minute load of about 25 or more. 2. Observe a run fail after about 35 to 40 seconds with `PRP command session.open timed out`. 3. Observe the task blocked with `ambiguous_state` and a manual Retry. **Paperclip version or commit** `master` at `6c7cc2c5d6`. ## What Changed The bound (evidence below): - `coldAdmissionTimeoutMs` gives cold ACPX Claude 60 seconds (live adoption keeps 30 seconds). The runner's Rust sidecar `session_open_timeout` and the sidecar's provider handshake (`acpxSessionHandshakeTimeoutMs`) give Claude and Pi 60 seconds too. All three bounds must agree, or the shortest one fails first. Claude's warm run attachment uses the same cold budget, like Pi. The automatic retry: - `nativeSessionFailureSourceCode` returns `provider_session_open_timeout` only for exactly `<startup failure code>: PRP command session.open timed out`. The startup failure codes are `runner_local_connect_failed`, `runner_direct_wss_failed` and `runner_ingress_unavailable`. The transport uses them only before startup completes. After startup the code is `provider_transport_failed`, which is unchanged. - `nativeProviderRecoveryEvidence` treats the cause as pre-session. Durable evidence still decides: a bootstrap retry needs no provider event (`session.started`, `turn.started` and the others) and no checkpoint. The existing budget applies: three attempts with backoff of about 30 seconds, then about 60 seconds. When the budget is used, the task is blocked with the existing "Provider startup failed on 3 attempts before any provider work began" action. - A timeout is indeterminate: the runner can open a provider session after the controller stops waiting. Live runs showed both leftovers, so the retry now starts from clean state: - `prepareNativeSessionOpenTimeoutRetry` (server) runs when the failure settles. It quarantines a local runner root only when the restart-recovery proof (`runnerdStateProvesIncompleteBootstrap`, #19665) shows that no turn began and every recorded process is gone. Any other root, and any remote runner, stays `ambiguous_state` as before. - `resetAbandonedBootstrapRecord` (runner ACPX adapter) runs on a fresh open (no runner-recorded identity). If ACPX has a saved record for the session key with no messages, no prompt and no events, it is marked `reset_on_next_ensure`. Claude saves nothing before the first prompt, so reusing such a record fails the first turn with `session_resume_required`. A record that holds any conversation is unchanged. - A chat retry of an exhausted `provider_session_open_timeout` run is accepted like the other pre-provider causes. - `doc/execution-semantics.md` and the Runner README describe the cause and the bound. ## Verification **Measured startup distribution** (ACPX Claude `runner.session.bootstrap` / `runner.session.startup` spans from restart-chaos run events, 1-minute load 8 to 44): | Source | Bound | Successful cold bootstraps (s) | Failures | | --- | --- | --- | --- | | Earlier storm evidence | 30 s | 2.9, 4.7, 7.4, 8.6, 8.7, 26.2, 27.4, 27.6, 29.2 | startups ended at 34.9 to 40.3 s (the 30 s `session.open` bound plus spawn and `run.prepare`) | | This branch, wave s3601 | 60 s | 6.3, 7.8, 8.1, 13.4, 14.4, 22.3, 22.8, 22.9, **37.3** | 0 `session.open` timeouts | | This branch, wave s3602 | 60 s | 13 samples, p50 10.5, p90 17.9, max 25.7 | 0 `session.open` timeouts | | This branch, wave s3610 (final head, load 28 to 42) | 60 s | 10.4, 11.3, 11.7, 12.0, 13.3, 14.2, 29.6, **30.4**, **30.9** | 0 `session.open` timeouts | Under load, successes clustered just under 30 s and failures sat just above it, so the 30 s bound was cutting the distribution off. With 60 s, cold starts of 30.4, 30.9 and 37.3 s succeeded that the old bound would have failed. 60 s is about 1.6 times the largest observed success, and it matches Pi and OpenCode. **Live restart-chaos waves** (local, ACPX Claude only, 4 in flight, `--rc06-masked`, real provider, K1/K2 restarts): - s3601 (8 tasks, load 19 to 32): 7/8 PASS. No `session.open` timeout. The one failure is an R3 Retry signature after K1 (`native_restart_recovery_blocked`), not this cause. - s3602 (8 tasks, load 17 to 39): 8/8 PASS. No `session.open` timeout. Three Retry clicks were for `native_provider_terminal_failed` (`provider_turn_lost_on_restore`), a different restart signature. - Forced timeout, s3609 (4 tasks, B5, K2): a local, uncommitted fault injection gave every other cold startup a 1.5 s `session.open` deadline. Result 4/4 PASS. Run 4fe6537a timed out, retried itself on the same run, and then succeeded through four K2 restarts with no person involved. Run 02799a85 hit the injection three times in a row; the budget stopped it after attempt 3, and one Retry completed it. - Before the two clean-state fixes, the same forced wave (s3607 and s3608) showed the retry reusing leftover state: `turn.start` failed with `session_resume_required`, and then "ACPX session is not open". Those runs are the red evidence for the two clean-state fixes. - Final head, s3610 (8 tasks, K1/K2, no injection, load 28 to 42): **8/8 PASS**, 0 `session.open` timeouts. The one Retry click (t06) was for `provider_turn_lost_on_restore`, a different restart signature. **Tests:** - Red before the fix: `native-startup-retry.integration.test.ts -t timeout` (embedded PostgreSQL, real executor, real resume claim). The 3 retry cases failed with `native_session_interrupted` / `ambiguous_state`. - Green: `native-startup-retry.integration.test.ts` passes 36/36. New cases: - for each startup failure code, a timeout retries on the same run with backoff, keeps the task `in_progress` with an agent-owned `resume_native_run` wake, and blocks after attempt 3 with a board-owned action; - no automatic retry when `session.started` or `turn.started` is durable, when the code is `provider_transport_failed`, or when another command timed out; - a real runner root with an opened session and no turn is quarantined; a root with a turn command stays `ambiguous_state` and intact; a remote runner is never prepared. - `codex-runtime-adapter.test.ts`: 113 pass. New cases: a 45 s delayed handshake is admitted for Claude and rejected for Codex; a never-prompted record is reset only on a fresh open; records with messages, a prompt or events are kept. - `abandoned-bootstrap-record.test.ts` (real `createAcpRuntime` with `createRuntimeStore` and a controlled ACP peer that, like Claude, cannot load a never-prompted session): without the reset, a fresh open reuses the record and the first turn fails with SESSION_RESUME_REQUIRED after `session/load`. With the reset, the provider receives `session/new` and the turn completes. - `vitest run src/drivers/acpx src/cli` in the runner: 107 files, 1,935 tests pass. - `native-session-executor.test.ts -t "session.open|startup|bootstrap"`: 144 pass. `runnerd-codex-transport.test.ts -t "cold process admission"`: 4 pass. `cargo test -p paperclip-runner-core --lib admission_budget` passes. - `npx tsc --noEmit -p server` and the runner `tsc -p tsconfig.test.json` pass. ## Risks - **This changes Runner Rust code. The Daytona runner image must be rebuilt after merge** for remote sandboxes to get the 60-second sidecar bound. Until then, a remote Claude `session.open` still fails at 30 seconds in the sidecar (as a definite failure, which already retries). A remote timeout stays operator-owned, because a remote root cannot be proven clean here. - A cold ACPX Claude start that hangs now fails after about 60 seconds instead of 30 seconds. - A timed-out `session.open` may have created an empty provider session. No prompt was sent. The retry quarantines that runner root and asks ACPX for a new session. - Same-run retries of a fresh-attempt Retry run can still fail with `isolated_task_directory_binding_mismatch`. This is RC-35 (workspace re-preparation), which has its own fix in progress. - No schema or API change. The new error code appears on runs and in run error codes. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, test 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> |
||
|
DottaandPaperclip
|
b94963f91e |
fix: retry temporary repository catalog failures before dispatch (#19690)
## Thinking Path > - Paperclip manages AI agents and their work. > - A task must check repository access before it starts agent work. > - Repository discovery can fail while the user's existing grant remains valid. > - Discovery previously replaced a known temporary provider refusal with an empty catalog. > - The task then reported lost access and did not schedule recovery. > - This change keeps temporary discovery failures distinct and uses the existing bounded retry owner. > - Each retry must pass a fresh access check before it can use the saved repository. ## Linked Issues or Issue Description **What happened?** A task with a saved catalog repository stops with an access denial when GitHub returns an exhausted-quota response or HTTP 503. The task does not start provider work, but the discovery layer discards the temporary failure and returns an empty catalog. **Expected behavior** Known temporary failures before provider dispatch use the existing bounded infrastructure retry. The successor checks the same responsible user's current grants and reads the saved files only after fresh discovery succeeds. A completed denial remains terminal. **Steps to reproduce** 1. Bind a task to an authorized catalog repository with saved local changes. 2. Make its authorized GitHub discovery return HTTP 403 with zero remaining quota, or HTTP 503. 3. Let discovery recover before the existing retry deadline. 4. Observe one successful successor using the original repository bytes. **Paperclip version or commit** Reproduced on master at `b8d94b3ad1`. A search found no existing PR for this temporary catalog failure. ## What Changed - Keep a safe typed failure for known HTTP availability and quota responses. Honor the later provider deadline and bound it to one day. - Propagate that failure only when fresh discovery cannot find a required repository. Preserve successful partial catalogs and the existing connection setup error contract. - Classify the failure only before provider dispatch. Reuse existing bounded retry, interaction ownership, and bootstrap settlement rules. - Keep current grant checks, saved repository selection, normal Stop, and provider-started failure policy. - Keep provider deadlines when accepted interactions use their own retry owner. Recognize secondary HTTP 403 rate limits with valid retry advice while keeping ordinary permission denials terminal. This matches [GitHub rate-limit guidance](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api). - Keep direct repository request responses outside the automatic bootstrap retry contract. Test successful partial discovery and missing-required-repository recovery with two independent eligible connections. - Document task repository discovery recovery. ## Verification - Behavioral RED: real heartbeat and PostgreSQL fixtures reached actual HTTP 403 and503 discovery responses, started no adapter, and found zero successors instead of one. Both cases failed before the production change. - Behavioral GREEN: both cases waited for the real retry deadline, used the normal queue worker, ran the controlled adapter once after fresh discovery, and read the original saved bytes. Two cases passed in 74.73 seconds. No external model or provider request was made. - Deadline RED/GREEN: a combined Retry-After and quota-reset response initially selected the earlier deadline. The regression now selects the later deadline. - Initial repair at `d8d4714a18` (same production source as `018e8c52d345d79972d7907468080dc9908c299f`): seven affected files passed, 69 cases in 185.16 seconds. That test-only cleanup then passed all five real-database acceptance cases in 134.76 seconds. - Commands: `pnpm exec vitest run` with `heartbeat-repository-catalog-recovery.test.ts`, `github-catalog-timing.test.ts`, `github-grant-metadata.test.ts`, `project-repositories-persistence.test.ts`, `task-workspace-repositories-persistence.test.ts`, `github-repository-catalog-failure.test.ts`, and `services/heartbeat/retries.test.ts` from the server source tree. The acceptance file also ran fresh at `018e8c52d3`. A fresh five-case run after the P1 repairs passed at `f490ca6b78` in 122.06 seconds. The final eight-case file passed fresh at `e5b2efab11919467850f55fd6345317132b9b672` in 124.62 seconds, using real owned embedded PostgreSQL. It covers automatic recovery with saved bytes, genuine denial, provider-started classification, normal Stop, the direct HTTP route, and both partial-catalog outcomes. - Review repairs at `f490ca6b787cf77df96d3d6edfb2ab9115eb19a6`: both reported P1s reproduced RED. The real database interaction scheduler chose a retry after 24 seconds before the provider deadline of one hour. A secondary HTTP 403 with valid Retry-After lost its retry classification. The narrow fixes then passed 44 cases across scheduler, header, and metadata-contract files in 37.66 seconds. - A real direct request through the task workspace router and normal error middleware reproduced a 500 response instead of the existing 403. Required-ID retry mode now applies only to admission and its rechecks. The route and both two-connection partial-catalog cases then passed in 29.09 seconds. Both partial cases use actual owned grants and credential bindings. - All seven server type layers passed again at final head `e5b2efab11919467850f55fd6345317132b9b672`. The affected test type layer passed again after the test-only cleanup. Environment: macOS arm64, Node 26.4.0, pinned pnpm 9.15.4, Vitest 5.0.3, owned disposable PostgreSQL 18 on loopback. The current checkout's shared packages, plugin SDK, and Runner were built first. - One local acceptance run under concurrent owned test/typecheck load hit the unchanged 15-second Stop-case timeout. Its four other cases passed. The unchanged complete file then passed serially. The timeout receipt remains recorded; no assertion or timeout was weakened. - A provider-started failure retains the existing generic adapter retry policy. It does not receive the catalog no-provider certificate or a provider retry deadline. Fixture cleanup cancels its scheduled retry through the normal owner. - Final head `e5b2efab11919467850f55fd6345317132b9b672` has 59 successful checks, 3 allowed skips, no pending or failed check, and Greptile 5/5 with no open findings. [Completed CI workflow](https://github.com/paperclipai/paperclip/actions/runs/42322202977). - Additional local diagnostic for the late recovery-owner advisory: on unchanged `e5b2efab`, the dedicated scheduler returned `retry_exhausted` with attempt 3/max 2. The normal stranded sweeper then escalated once, created no extra run, and started no adapter, even after catalog access recovered. One case passed in 17.32 seconds. The existing failed-predecessor path delegates to that bounded scheduler and returns; it does not enter a new generic retry budget. No production registry change was made. This diagnostic was not a CI case. Its local harness is preserved separately at `74e3854eea3f7c4121fdcd9c8a8ad522cd8c7477` on `codex/catalog-exhaustion-acceptance`; its production source is identical to `e5b2efab`. - Module boundary and whitespace checks passed. ## Risks - A temporary failure from another eligible connection can delay a truly missing repository for the existing limit of two attempts. It cannot grant access. - Unknown network errors and genuine access denials retain their existing behavior. This change handles known HTTP failures only. - Historical provider quota causality is unproven. This repair has a separate local producer regression. It does not rewrite past failures or queued user intents. - Acceptance uses real local database and heartbeat owners with controlled HTTP and adapter fixtures. It does not claim a live native-provider recovery. ## Model Used OpenAI GPT-6.1 Codex (`gpt-6.1-sol`), with high reasoning, repository tools, code execution, and controlled local PostgreSQL tests. The host did not expose a 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
|
20fc37778f |
fix(runner): keep settled OpenCode sessions across a controller proxy rebuild (#19641)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - The native Runner keeps an OpenCode conversation in durable state,
so a task can continue after a control-plane restart.
> - Saved OpenCode state binds one digest of the launch profile: the
Node command, the proxy bundle and the OpenCode executable, each by path
and SHA-256.
> - The Node command and the proxy bundle are controller runtime. A
rebuild or deploy between a hard kill and recovery replaces their bytes.
> - The recovery probe then refused a settled conversation with
`provider_state_incompatible`, although the OpenCode release and the
conversation were unchanged.
> - ACPX already re-binds a settled session after a Runner upgrade
(#18879). OpenCode did not.
> - This pull request records the OpenCode release digest in saved state
and re-binds a settled session when that release is unchanged.
> - The benefit is that a task continues automatically after a restart
that also changed the controller build. A real release change still
fails closed.
## Linked Issues or Issue Description
Refs #19558, #19501, #18879.
**What happened?**
A restart-chaos wave killed the control plane with SIGKILL while an
OpenCode run was settled between turns. The first recovery attempt
restored the session. The controller's runner `dist` was then rebuilt in
the same checkout. The next recovery attempt failed in the
`runner.drain` resume probe with `OpenCode runner launch profile changed
across durable recovery` (`native_provider_state_incompatible`). The
saved state matched the original proxy bytes exactly. Only the proxy
bundle bytes had changed. The OpenCode executable, the paths and the
model were the same.
**Expected behavior**
A settled OpenCode session continues after a restart when only
controller runtime bytes changed. A changed OpenCode release, or a
session with work in flight, fails closed. The existing disposition then
offers one Retry that continues in a fresh provider session.
**Steps to reproduce**
1. Start an OpenCode run and let it settle between turns.
2. Kill the runner and the controller.
3. Replace the qualified proxy bundle with valid new bytes (rebuild or
deploy).
4. Recover the run. The resume probe fails with
`provider_state_incompatible`.
**Paperclip version or commit**
`master` at
canary/v2026.1107.0-canary.2
|
||
|
DottaandPaperclip
|
254b44f72b |
fix(remote-workspaces): recover retired roots that cannot be renamed (#19674)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Remote workspaces must retain saved files across process and sandbox replacement. > - Restore uses identity receipts to distinguish a retired owned root from an unrelated directory. > - Some overlay filesystems reject a directory rename with EXDEV even within one mount. > - That error currently prevents a valid saved workspace from being restored. > - This pull request replaces only a verified retired root in place when that rename fails. > - Runtime state stays in place. Each restore holds a root-specific OS lock. > - A durable workspace epoch fence rejects delayed older commands. A pending marker keeps interrupted cleanup recoverable. ## Linked Issues or Issue Description **What happened?** Workspace restore fails with `remote_workspace_restore_failed: stage=publish code=EXDEV syscall=rename` when the retired directory comes from a lower overlay layer. Retry repeats the same rename. **Expected behavior** Restore should publish the verified saved workspace and preserve runtime state. An interrupted restore should remain safe to retry. **Steps to reproduce** 1. Create an owned retired root with a valid identity receipt, runtime state, and stale files. 2. Make directory rename return EXDEV for the existing lower-layer root. 3. Restore a saved workspace through `restoreRemoteWorkspaceDirectory`. 4. Repeat with an interruption before the new restore receipt is published. **Paperclip version or commit** The regression fails before this change on base `a2aa69bc6230c8b81de3588d7d5571b38db4dd04`. **Deployment mode** Remote sandbox with an overlay filesystem. Local regression: macOS arm64, Node 26.4.0, source checkout. Refs #19231. Refs #19251. Those changes establish the retained-root restore path. This fix handles its EXDEV publication failure. ## What Changed - Keep the EXDEV fallback limited to a verified retired workspace root. Preserve runtime state and the existing ownership checks. - Hold a private SQLite OS lock across receipt checks, file replacement, cleanup, and receipt publication. Keep the lock file outside the replaced root. Process exit releases ownership; a lost reply does not. - Pass the existing workspace authority epoch from the real restore caller. Write and sync a separate epoch fence before root changes. Reject lower epochs, conflicting identities within one epoch, and missing epochs on fenced roots. The fence survives SQLite rollback after process death. - Publish a pending marker without a completed identity until stale preparation-receipt removal and directory-mode cleanup finish. A retry can finish that cleanup. A higher authorized epoch can then restore its selected save. - Preserve later file and permission edits when adopting a completed restore. Refuse unsafe lock files and malformed authority records. Document compatibility and the stop-evidence boundary. ## Verification - RED before the original fix: real restore commands failed with EXDEV. The test preload changes only the kernel rename result. - RED before the review fixes: a live command with an unconfirmed timed-out reply overwrote edits made after retry. Tests also reproduced incomplete cleanup after receipt publication. - RED before the authority fence: a delayed older command replaced a newer completed restore. It also passed after the newer command was killed before root mutation. The final test uses real separate Node processes, OS lock contention, SIGKILL, and a durable on-disk fence. - Final local checks: 142 transport filesystem tests, 16 real PostgreSQL restore-caller cases, and all seven server typecheck layers. The caller cases verify the published receipt uses the current workspace epoch. - Late review RED: an actual separate SQLite owner made the real PostgreSQL restore caller mark an expired lock wait non-retryable. Corrupt fence and receipt bytes also produced a raw syntax error without the required refusal reason. Both regressions now pass. Only the test clock advances the lock deadline; actual OS lock contention and the production caller run unchanged. - Lock contention stays retryable only under the existing same clean writer, epoch, and snapshot proof. Four negative cases change those fields and remain non-retryable. After the owner stops, normal restore returns the useful saved files. Corrupt authority records stay untouched and non-retryable. - Safety cases cover same and different authorized identities, process-death lock release, an unrelated root making progress, pending cleanup interruption, malformed or missing fences, invalid input, and unsafe lock parents/files. - The first final transport attempt passed 139 cases and failed only because the restricted shell could not open a Unix socket (`listen EPERM`). The full suite was then run with the required local socket permission. No production change was made for that setup failure. - Reproduce the filesystem checks from `server/`: `../node_modules/.bin/vitest run --config vitest.config.ts src/services/remote-workspace-transport.test.ts`. Run the caller file with `-t "restores a newer save over this workspace|preserves the published workspace path"` and the repository test PostgreSQL setup. Run `node scripts/typecheck-layers.mjs` from `server/` for all seven layers. - Agent preflight, module boundary audit, and `git diff --check` passed. No added code comments or private issue references occur in the patch. - Local EXDEV proof uses macOS arm64 and Node 26.4.0. It does not claim a new Linux overlay-kernel run or hosted recovery. Final source: [`8092cc6b2ec448ff372337b1e7a43854eae7bb08`](https://github.com/paperclipai/paperclip/commit/8092cc6b2ec448ff372337b1e7a43854eae7bb08). [Exact-head CI](https://github.com/paperclipai/paperclip/actions/runs/42321313715) completed with 59 successful checks, three conditional skips, and one neutral check. Greptile gave this exact head 5/5. The independent review passed. No review threads remain open. ## Risks The EXDEV path changes files in place. It must finish before execution admission. Participating restore commands use a private permanent SQLite lock file and a separately synced epoch fence. The qualified sandbox Node runtime must support `node:sqlite`. Lock contention has a 120-second limit and does not clear live ownership. A legacy command that never acquired this lock remains outside its protection. Hosted recovery must confirm that command stopped; a timeout is insufficient. The new fence does not backfill that evidence. A conflicting identity in the same epoch or a malformed fence stays blocked. A higher epoch can recover an interrupted pending restore only with the existing retired-root replacement authority. No controller database schema, execution permission, or provider-state guard changes. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`) authored the recovered patch. OpenAI Codex, GPT-6 family, recovered, reproduced, repaired the review findings, and verified the change with filesystem tests, shell tools, and code execution. The Codex exact serving model ID and context-window size are not exposed in this session. No value is inferred for them. ## 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
|
e05bced39f |
fix(runner): adopt an accepted Codex turn whose start outlives the bound (#19608)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native Codex runs submit each turn through runnerd. The transport
answers `turn/start` only after it sees the provider's `turn/started`
for the turn the durable command result named
> - That wait has a fixed 30 s bound
> - At host load around 50, after two hard restarts, Codex accepted a
turn and started it about 27 s after the command, beyond the bound once
scheduling delay was added. The transport failed the start, but the
provider kept running the turn and made a tool call
> - No controller owned that turn. Its close could not settle, and the
task's Retry was refused at once
> - This pull request adopts the accepted turn when the bound expires,
but only when the runner still reports that turn active
> - The benefit is that a slow but live provider turn stays owned, so
its events, Stop and close keep working
## Linked Issues or Issue Description
**What happened?**
In a restart chaos replay (two SIGKILLs about 20 s apart, local runner,
real Codex, load 50), a resumed run failed with `runnerd did not report
the provider turn identity`. The runner journal shows the durable
`turn.start` result with the provider turn id, then `turn.started` for
that same turn 27 s later, followed by items and a semantic tool call.
The run's close then stayed incomplete (`semanticResultsSettled:
false`), and a Retry of the task failed with
`native_session_cleanup_quarantined: prior session cleanup remains
incomplete`.
**Steps to reproduce**
1. Use a Codex provider that answers `turn/start` and emits
`turn/started` later than the transport's turn-start bound.
2. Start a turn through the runnerd transport.
**Expected behavior**
The accepted turn is owned and observed by the controller.
**Actual behavior**
The start fails after the bound, while the provider still runs the turn.
## What Changed
- `runnerd-codex-transport.ts` (`#startTurnWork`):
- Near the end of the bound, if `turn/started` has not arrived, the
transport reads an authenticated `session.snapshot`. It reserves at most
5 s (or a quarter of the bound) for this read.
- It binds the accepted turn only when the turn is provably live:
- the runner reports that exact provider turn as active;
- the runner attributed no provider launch
(`provider_startup_ownership`) to a command after this `turn.start`;
- for a local provider, the same process (pid and start time) that
accepted the turn is still alive.
- Otherwise it keeps waiting for `turn/started` until the configured
bound and fails exactly as before. A turn lost across a provider
relaunch keeps the same-run resume path.
- `fake-codex-app-server.rs`: new test options `--turn-started-delay-ms`
(a delay between the `turn/start` response and `turn/started`) and
`--exit-before-turn-started`.
## Verification
- Red before the fix: `pnpm --filter @paperclipai/paperclip-runner exec
vitest run src/live/runnerd-codex-transport-deadlines.test.ts -t "adopts
a durably accepted"` (real runnerd and fake Codex, 12 s start delay, 8 s
bound) failed with `runnerd did not report the provider turn identity`.
This is the replay's error.
- Green after the fix: the start resolves with `provider-turn-1`, and
`turn/started` and the turn's terminal follow in order. There is one
provider `turn/start`.
- Negative case (`does not adopt an accepted turn the runner no longer
reports active`): the provider accepts the turn and exits before
starting it. The runner relaunches the provider and still lists the
turn, but the turn is not adopted. The start fails within the bound plus
scheduling slack. A snapshot-only version of this change adopted that
dead turn, and it hung for 20 s with no terminal.
- `src/native-session-runtime.test.ts`,
`src/live/runnerd-restart-continuation.test.ts` and
`src/live/runnerd-codex-transport-deadlines.test.ts` pass. The package
typecheck passes.
- Live chaos replays: results are added in a comment.
## Risks
- A provably live provider that accepts a turn and never starts it now
waits on the turn's own timeout and Stop paths instead of failing after
30 s. A dead or relaunched provider still fails at the bound.
- Remote runners can adopt only on the runner's own evidence (active
turn and no later provider launch), because their provider pid is not a
local process.
- A stale `turn/started` for a previous turn that arrives after the new
command result is still rejected. The transport cannot attribute that
turn's following events, so rejecting it fails only the transport, and
the same-run resume continues the task (seen in the replay). Delivering
it would attach the old turn's items and usage to the new turn.
## 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
Refs #19532, #19539.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
06bbcea12f |
fix(budgets): price lost receipts and stop restart-derived accounting from hard-stopping companies (#19673)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budget admission and run accounting decide whether any task in a company may start > - #19497 made pending accounting an admission hold and settles a stopped run's lost receipt as an unpriced charge > - Restart-chaos waves still stopped whole companies. A lost Codex receipt with complete token counts settled as unpriced because the rate card only prices complete receipts. Under the default blocking policy that hard-stopped the company at 0.38 of 3000 cents. In another storm, a resumed run's continuation was refused by its own task's recovering-accounting hold. Separately, unpriced charges with zero tokens or a known partial cost hard-stopped a company at 120 of 2000 cents > - This pull request prices a lost receipt's observed usage from the rate card, lets a task's continuation pass its own recovering accounting, and stops restart-derived or evidence-free unpriced charges from tripping the blocking policy > - The benefit is that a restart no longer pauses a company by itself, while genuinely unpriced usage still blocks ## Linked Issues or Issue Description Follow-up to #19497. Refs #19606. **What happened?** - Wave A (K6 crash loop): native Codex run `5037b8d1` was killed mid-turn. Its receipt was lost, but `usage_json` held gpt-6.1-sol with 28690 input and 220 output tokens. The charge was written `cost_status=unpriced`, `pricing_provenance.source=unknown`, because `priceCodexReceipt` returns early when `!receipt.complete`. 0.2 s later the default `unpricedUsagePolicy=block` opened a hard incident at 0.38 of 3000 cents. The company was paused, and 6/6 tasks were blocked with `canRetry=false`. - Wave B (K1 storm): Claude run `2a7a8abf` resumed after the restart. `reserveRunBudget` refused its interaction continuation with "New work must wait while a native run recovers its unfinished accounting", so the task could not settle its own recovery. A 0-token unpriced charge and an unpriced charge with a known 0.81-cent cost then hard-stopped the company at 120 of 2000 cents. **What did you expect?** A restart-lost receipt with known numbers is priced as an estimate, and no restart-derived accounting state pauses a company by itself. A task's own continuation is never held by its own accounting. Real usage above the limit, and genuinely unpriced usage, still stop work. **Steps to reproduce** 1. Give a company a hard-stop budget with the default unpriced-usage policy. 2. Kill a native Codex turn after it has reported tokens, so the final receipt is lost and native recovery ends in terminal failure with a verified stop. 3. Observe an unpriced charge for known tokens, a hard incident, and a paused company. ## What Changed - `run-cost-accounting.ts`: when a lost receipt settles and its usage has known tokens and no known cost, price it with `priceCodexReceipt`/`priceAnthropicReceipt` as a complete per-run checkpoint. The charge is `estimated`, its provenance keeps the rate-card source, and its evidence says "final receipt lost; observed usage is a lower bound". The estimate is also written to the run's usage so a later receipt reconciles against it. Zero observed tokens, and per-model usage, stay unpriced (unknown is never shown as zero, and aggregate totals never take one model's rate). - `budget-exposure.ts`: - The pending/recovering accounting hold excludes the run being admitted and earlier runs of the same task (same `native_issue_id`). A task's continuation and finalization can therefore settle its own accounting. Other tasks stay held, and those runs still count as spend. - The blocking unpriced-usage policy no longer counts these unpriced charges: the settled final charge of a lost receipt (restart-derived, any usage; later growth still counts), a charge with a known partial cost, and metered API or subscription billing with no observed tokens. Unknown billing with no tokens (for example Cursor Cloud) and complete-receipt usage that has no price still block. - `budgets.ts` / `budget-reservations.ts`: admission accepts the admitted `runId` and passes it to the exposure read. `reserveRunBudget`, the final dispatch recheck and the read-only admitted recheck all supply it, so a continuation that reserves can also dispatch. - Daytona wave s12 (run f1d98917, a blocked remote recovery with a 0-cent held reservation and every lease destroyed) is closed by #19606. A Retry for that same task passes the hold through the same-task exclusion here. ## Verification - `closed-native-receipt-accounting.test.ts` (embedded Postgres): - Lost Codex (gpt-6.1-sol) and Claude (claude-sonnet-5) receipts with observed tokens settle as `estimated` rate-card charges with the lower-bound evidence. The company stays active and admission is clear under the blocking policy. - A lost receipt for a model with no rate card stays unpriced but opens no incident and no pause. - A task's continuation reserves budget past its own recovering accounting, while another task's run is still held. - Unpriced charges with no tokens on metered billing, or with a known partial cost, do not hard-stop. - Complete-receipt usage with no price still blocks. - The six new cases fail on master and pass with this change. - Existing budget and accounting suites pass locally: budget-enforcement-scoping, budgets-service, heartbeat-cost-accounting, run-cost-accounting, native-stop-accounting, cost-data-quality, budget-live-attempts, budget-live-usage. Server typecheck passes. ## Risks - A lost receipt's estimate is a lower bound. The provenance says so, and a later complete receipt still reconciles. - Restart-derived unpriced charges no longer trip the blocking policy. Usage lost with a killed provider is not counted against that policy, which is consistent with budget enforcement being best effort. Unknown-billing and complete-receipt unpriced usage still block. - The same-task exemption relies on `native_issue_id`. Legacy runs without it keep the old behavior. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, embedded Postgres tests). ## 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
|
785ead5c7a |
fix(tasks): hold a Retry while the task's own native session close is retained (#19609)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a native provider session close fails, the runner keeps a cleanup owner or quarantine in process memory. Its admission gate refuses new sessions for that task (scoped to the task since #19532) > - The server could not see that state. A task's Retry was accepted (202), its run was dispatched, and the runner refused it at once with `native_session_cleanup_quarantined: prior session cleanup remains incomplete` > - The user saw a Retry that failed instantly, and nothing said what to do > - This pull request lets the server read the runner's per-task cleanup gate and hold the Retry with a clear reason. The Retry starts automatically when the close is released > - The benefit is that a Retry is never dispatched only to fail on a block that is already known ## Linked Issues or Issue Description **What happened?** After two hard restarts, a Codex run's close stayed incomplete. The board Retry for that task was accepted, and its run failed in about 1 s with `native_session_cleanup_quarantined: prior session cleanup remains incomplete`. The next Retry did the same. **Steps to reproduce** 1. Let a native session close fail or stay pending for a task. 2. Press Retry on that task. **Expected behavior** The Retry waits with a clear reason, and starts when the cleanup finishes. If the cleanup needs an operator, Retry is not offered and the one working action is shown. **Actual behavior** The Retry is accepted and its run fails at once. ## What Changed - `native-session-runtime.ts`: - New `nativeSessionCleanupBlock({ companyId, issueId })`. It is a read-only view of the in-process admission gate for one task: `pending` (a retained close or a bounded automatic close recovery still owns the task's provider resources), `operator_required`, or `null`. It grants no authority. - New `onNativeSessionCleanupSettled(listener)`. It is called whenever a cleanup owner or quarantine entry is released. - `explicit-native-continuation.ts` (the continuation gate for a Retry or a new message): - `pending`: the input is held with `local_cleanup` ("Retry is waiting for the previous execution to finish cleanup. Your message is saved."). It starts automatically when the close is released. - `operator_required`: handled exactly like a recorded `native_session_cleanup_quarantined` run. Retry is not offered. A new message runs the existing stopped-process proof (`verifyStoppedNativeSessionForContinuation`), which retires the runner quarantine when the old processes are proven stopped. - `native-cleanup-release-wake.ts` (registered in `server/src/index.ts`): each cleanup release wakes deferred execution for the company in a transaction, so a held Retry re-evaluates. - `server/src/vendor/paperclip-runner/index.ts`: exports the two runtime functions. ## Verification - Red before the gate (server change stashed): `pnpm --filter @paperclipai/server exec vitest run src/services/quarantined-task-retry.test.ts -t "holds a Retry"` dispatched a successor run while the task's close was retained (`expected [ { … } ] to have a length of +0 but got 1`). - Green after the gate: - The Retry is held (`deferred_issue_execution`, `executionWait.reason = local_cleanup`), and no successor is dispatched. - A different task is not blocked. - When the close settles, the release is announced, and the next pass dispatches exactly one successor. - The whole `quarantined-task-retry.test.ts` file passes (28 tests). `src/native-session-runtime.test.ts` passes. Server `tsc --noEmit` passes after the runner package build. - Live chaos replays: results are added in a comment. ## Risks - The gate reads process memory, so it applies only where the server and the runner session share a process (local runner). Other deployments keep today's behavior. - A held Retry re-evaluates on the release wake (sent in its own transaction) or on the existing 30 s deferred-execution pass. When the bounded automatic close budget is spent with nothing else scheduled, the view reports `operator_required`. Retry is then no longer offered, and a new message reaches the stopped-process recovery instead of waiting forever. - No schema or API shape changes. ## 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 Refs #19532. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
29e05f7e06 |
fix(heartbeat): sweep orphaned run scratch directories (#19671)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Each heartbeat run gets a scratch directory in the system temp folder > - Each process attempt makes a new directory with a marker file > - Cleanup removes only the directory of the current attempt > - A restart kills the old attempt, so its directory stays forever > - One restart storm left 83 directories across 78 runs > - This pull request sweeps those directories when the run ends and at startup recovery > - The benefit is that temp space no longer fills with leftover directories ## Linked Issues or Issue Description **What happened?** Run scratch directories made before a restart are never removed. A restart storm left 83 `paperclip-run-*` directories across 78 runs. Each holds only `.paperclip-run-scratch.json`. **Steps to reproduce** 1. Start a run that makes a scratch directory. 2. Kill the server process and start it again. 3. Let the run finish. 4. List `$TMPDIR/paperclip-run-*`. The old directory is still there. **Expected behavior** The old directory is removed after the run ends, or at startup. ## What Changed - Add `sweepOrphanedRunScratch` in `server/src/services/run-scratch.ts`. - The sweep removes a directory only if it has a valid regular-file marker and its run is terminal in this instance database. It keeps a directory whose run it cannot see, because another instance can share the temp root. - The sweep keeps active runs, unmarked directories, symlinks, and directories outside `paperclip-run-*`. - The sweep checks each directory again before removal and bounds the scan (500, with a rotating start so kept entries do not starve later ones) and removals (100). A per-run sweep filters by directory name first. Marker run ids that are not UUIDs never reach the database query. - Call the sweep after the current attempt's cleanup in `heartbeat.ts`, for that run only. - Call the sweep in `reapOrphanedRuns` (startup recovery) with run status from the database. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/services/run-scratch.test.ts` - New tests cover: leftover directories of a terminal run are removed, an active run is kept, an unmarked directory is kept, a symlink is not followed, and removals are bounded. ## Risks - Low risk. The sweep acts only on marked directories of terminal runs. - A directory left by a previous attempt could hold a live grandchild process. The sweep does not check old process groups. The run is already terminal. ## Model Used - Claude, claude-opus-4-5 class model via 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> |
||
|
Nicky LeachandClaude Opus 5.5
|
0b73c805e8 |
feat(environments): fill an unset instance default with a managed plugin environment (#19551)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A managed instance gets its instance-level environments from the `PAPERCLIP_MANAGED_CONFIG` document, which can declare plugin environments next to the managed sandbox > - A platform that runs managed instances wants a new instance to start with one of those plugin environments as its default, without changing the default of any instance that already has one > - #19632 made boot fill the instance default only when it is unset, local or deleted, in one locked transaction, and removed the revert, so every existing managed instance keeps a set default > - This pull request adds an `instanceDefault` flag to a plugin entry. That entry fills an unset default through the same locked fill, before the managed sandbox does > - The fill follows the same readiness rule as an admin's default write, so it never moves ready agents onto a plugin that has not prepared them > - The platform also needs a way to know whether a release understands plugin entries, so the release now ships a small capabilities file > - The benefit: new instances get the plugin environment as their default, and existing instances and admin choices are never changed ## Linked Issues or Issue Description Refs #19632 (the locked fill-if-unset rule this builds on), #19508 (plugin entries in the managed config) **Subsystem affected** Server environments: the managed config parser and the managed environment bootstrap. **Problem or motivation** The managed config can declare plugin environments, but it can't make one the instance default on a new instance. An external platform also has no way to tell whether a given release supports plugin entries before it sends one. **Proposed solution** - Add `instanceDefault: true` to at most one plugin entry. Right after that entry's boot ensure succeeds, it fills the instance default through `fillUnsetInstanceDefault`. The managed sandbox fill runs after it. - On a new instance the plugin environment wins. On an existing instance the default is already set, so neither fill changes it. - The fill declines while an active agent that follows the default hasn't been prepared by that plugin. This is the rule the instance-settings route enforces. When the plugin fill declines, the managed sandbox fills as usual. - Ship `server/src/services/managed-config-capabilities.json`, which lists `pluginEnvironments`. **Alternatives considered** - A recorded one-time decision with a retry worker. It needed pending states and extra checks for many writer and trigger combinations. Filling only an unset default, under the existing lock, needs none of that. - Rejecting the fill whenever agents exist. That is stricter than the admin route and blocks harmless cases, such as agents that are paused or already prepared. **Roadmap alignment** This extends existing managed-environment behaviour. It adds no UI. ## What Changed - `server/src/services/managed-config.ts`: - `ManagedPluginEnvironmentSpec.instanceDefault` (boolean, default `false`). - The parser refuses `instanceDefault` on more than one plugin entry, on the sandbox entry (even `false`), or as a non-boolean. - `MANAGED_CONFIG_CAPABILITIES` and `MANAGED_CONFIG_CAPABILITIES_SCHEMA_VERSION` constants. A plugin entry is refused unless `pluginEnvironments` is listed. - `server/src/services/managed-config-capabilities.json`: `{ "schemaVersion": 1, "capabilities": ["pluginEnvironments"] }`. A test pins it to the constants and to this literal contract. - `server/src/services/managed-environments.ts`: - New `fillPluginInstanceDefault`. It runs right after a successful boot ensure of the flagged entry and logs a fill or a decline. Recovery after boot never fills. - The managed sandbox fill now runs once, after the plugin entries. - `server/src/services/managed-instance-default.ts`: `fillUnsetInstanceDefault` runs `agentsNeedSetupForEnvironmentChange` inside its locked transaction and returns `declined: "agents_need_setup"`. A sandbox target never needs agent setup, so the managed sandbox fill behaves as before. - `server/src/services/plugin-environment-driver.ts`: the readiness query is factored into `agentsNeedSetupForEnvironmentChange`. `assertAgentsReadyForEnvironmentChange` wraps it and still throws the same conflict. - `doc/DEVELOPING.md`: documents `instanceDefault`, the fill order, the readiness decline, and the capabilities file. ## Verification - `cd server && npx vitest run src/__tests__/managed-plugin-environments.test.ts` (embedded Postgres), under `instanceDefault (fill only an unset default)`: - A new instance gets the plugin row as its default. - An existing sandbox default is kept, and so is an admin's choice. - If the plugin was down at the first boot and the sandbox filled, a later recovery and boot keep the sandbox default. - An admin-cleared or local default is declined while a ready or verifying agent follows the default without the plugin's setup. The managed sandbox then fills it. - It fills when the agents that follow the default are paused or already prepared, or are pinned to another environment. - Without the flag, nothing happens. - A new agent that follows the default gets the plugin in `requiredPluginIds`. - `npx vitest run src/__tests__/managed-instance-default.test.ts` (embedded Postgres): the earlier fill, lock and overlap cases pass. A sandbox fill also succeeds while active agents follow the default. - `npx vitest run src/services/managed-environments-plugin-entries.test.ts`: - The fill order is plugin then sandbox. - When the plugin is down there is no fill, and recovery never fills. - A failed ensure gives only the sandbox fill, and a failed fill keeps the ensure counted. - When the plugin fill declines, the sandbox still fills. - `npx vitest run src/__tests__/managed-config.test.ts src/services/managed-config-capabilities.test.ts`: parser refusals and the capabilities contract. - `npx vitest run src/services/managed-environments.test.ts src/__tests__/instance-settings-routes.test.ts src/__tests__/environment-routes.test.ts`: pass. The route still throws the same readiness conflict. - `pnpm --filter @paperclipai/server typecheck` passes. - I did not run the full `pnpm test:run`, `pnpm -r typecheck` or `pnpm build`. CI covers them. ## Risks - An instance whose default is unset at the first boot after this release gets the plugin environment, if its agents are ready for it. That covers a new instance, and an instance whose admin cleared the default or chose the local environment. This is the intended behaviour. - The readiness check doesn't lock agent rows. If an agent becomes ready during the boot fill, it can miss the check, which is the same race the admin route has. Agents created after the fill are prepared for the plugin, because setup reads the current default. - If the plugin is down at a new instance's first boot, the instance keeps the managed sandbox default from then on. - No schema change. > 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, through Claude Code with tool use (file edits, shell, 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 Opus 5.5 <noreply@anthropic.com> |
||
|
Nicky LeachandClaude Opus 5.5
|
fb3cf1a072 |
feat(ci): retry a failed Vitest case once in PR CI and report each retried pass (#19637)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors wait for PR CI before merge, so CI wall-clock time sets the pace of work > - PR CI runs Vitest shards with 4 workers on 4-vCPU runners, so timing-sensitive tests sometimes time out under load > - One such timeout fails the whole run, and a re-run costs another 15 minutes or more > - In one 7-hour window on 2026-11-06, about 30 failed jobs were test flakes on commits that passed on a later attempt > - This pull request retries a failed Vitest case once in PR CI only, and reports every pass that needed the retry > - The benefit is fewer whole-run re-runs, while each flaky test stays visible so that we can fix it ## Linked Issues or Issue Description **What existing behavior does this improve?** PR CI fails the whole run when one Vitest case fails once because of runner load. **Subsystem affected** CI: the PR Vitest lanes (general, serialized, workspace and Runner package lanes). **Current behavior** Vitest uses `retry: 0` everywhere. One timeout under load fails the run. A person must notice it and re-run the failed jobs. About 20% of recently merged PRs needed at least one re-run. **Proposed behavior** PR CI retries a failed Vitest case one time. A test that passes only on the retry gets a GitHub warning annotation and a row in the job summary. Local runs, release, cloud readiness and master keep `retry: 0`. **Reason and benefit** A re-run of one test case costs seconds. A re-run of the CI run costs 15 minutes or more. The report keeps flaky tests visible. Separate pull requests fix the known flaky tests. **Breaking changes** None. ## What Changed - Add `scripts/ci/vitest-retry.mjs`. It sets 1 retry when `GITHUB_ACTIONS=true` and `GITHUB_WORKFLOW=PR`, and 0 in other contexts. `PAPERCLIP_VITEST_RETRY` overrides the value. - Add `scripts/ci/retry-reporter.mjs`. It writes a `::warning` annotation and a step-summary table row for each test that passed only on a retry. - `scripts/run-vitest-stable.mjs` passes `--retry` to `vitest run`. The `vitest list` collection step does not retry. - The root `vitest.config.ts` and `packages/paperclip-runner/vitest.config.ts` add the retry reporter beside the existing reporters. The Runner config sets `retry` from the same gate. - `.github/workflows/pr-trusted.yml` reads the `CI_VITEST_RETRY` repository variable. Set it to `0` to turn retries off without a code change. - Add `scripts/ci/retry-reporter.test.mjs` to `test:release-registry`, and document the behavior in `doc/DEVELOPING.md`. ## Verification - `node --test scripts/ci/retry-reporter.test.mjs`: 11/11 pass. The tests cover the gate matrix, the reporter output, a real Vitest run with a test that fails once, the wrapper arguments, and the Runner config. - `node --test scripts/__tests__/run-vitest-stable-shard.test.mjs`: 39/39 pass. - `node --test .github/scripts/tests/*.test.mjs`: 744/744 pass. - Manual check: a temporary test that fails once ran through `run-vitest-stable.mjs` and through the Runner package config. With the PR environment it passed and wrote the annotation and the summary row. With `GITHUB_WORKFLOW=Release` it failed. The temporary test is removed. ## Risks - A retry can hide a flaky test. The annotation and the summary row reduce this risk. - A test that fails for a real reason now runs twice in PR CI, so a failed run takes slightly longer. - `pr.yml` loads `pr-trusted.yml` from master, so the `CI_VITEST_RETRY` switch works only after merge. The retry itself is in the scripts and starts with this pull request. - Low risk otherwise: release and master verification are not changed. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use and code execution. A Claude sub-agent wrote the change, and the parent session reviewed it. ## 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
|
7ac99718a4 |
fix(workspaces): fence legacy sources for online migration (#19642)
## Thinking Path > - Paperclip coordinates AI agents and preserves their saved work. > - Remote workspace storage replaces legacy host directories with durable snapshots. > - Several legacy workspace records can name one physical project directory. > - A task lock alone cannot exclude a new task that selects that directory. > - Terminal recovery records can retain valid sources after physical writers settle. > - This change fences the source through its real preparation and lifecycle owners. > - Idle ordinary sources can then migrate while unrelated remote work continues. ## Linked Issues or Issue Description **What happened?** Online migration rejects ordinary legacy directories when several workspace records name the same path. A source inspection found 45 aliases across three project roots with no root Git marker or previously imported nested child. The instance also had unrelated running remote work. Requiring the whole instance to drain prevented useful migration of these idle sources. Terminal native sources and superseded finalization attempts also remain blocked after their exact physical and durable owners settle. Their files, provider allocations and recovery records must remain available through the change of file authority. **Expected behavior** Migrate an idle ordinary source under a derived physical source fence. Let unrelated object-backed turns continue. Preserve provider and recovery obligations. Keep unknown writers, shared Git stores and uncertain source state blocked. **Steps to reproduce** 1. Create several local workspace records for one managed project directory. 2. Give the aliases different source tasks. Leave their local writers idle. 3. Keep a valid remote workspace turn running on another source. 4. Inspect or apply migration to an alias. The old owner returns `overlapping_workspace_requires_maintenance`. 5. Retain a terminal native prepared source with a null coordinator owner, or an old finalize attempt with an exact later committed receipt. The old online path keeps these sources blocked. **Paperclip version or commit** Current tested head: `7478c817f09f7eed396922c0f7831af07ae6ff14`, based on master `8621bcf53ee2e4bbefb7d193add42dd544ca06e8`. Related recovery work: #19131. **Deployment mode** Hosted native Runner with remote workspace storage. Qualification uses disposable local PostgreSQL databases, actual archive/storage owners and synthetic source trees. The source observations were read-only. ## What Changed - Retain the existing company/task source keys and add one physical managed-project key. Heartbeat source selection, materialization, reopen and lifecycle owners share it. Configured aliases of canonical task directories retain their original task key. - Fence runtime provision and each configured service CWD through both startup owners. Check all unsettled service CWDs for physical source overlap, even when workspace IDs differ. Runs with no services keep their filesystem-free path. - Derive a bounded ordinary alias closure. Lock every alias task and lifecycle key. Reject foreign companies, changed membership, unproven descendants, included Git metadata and other unresolved writer obligations. - Commit remote authority on the connection that holds source exclusion. Refresh a stale selected local workspace after admission if migration published remote authority. Publish activity after commit. - Admit exact terminal native recovery sources under the existing physical finalizer keys. Bind descriptors, allocations and owner checkpoints to journaled fingerprints. Recheck them before capture and publication. - Add a separate company-scoped online entry for exact superseded finalization receipts. Keep the offline entry's deployment prerequisites and keep inventory read-only. - Document the proof and the remaining Git/common-store extension. The original files stay in place. No new scheduler, deployment action or database migration is added. ## Verification - Review found that the real reopen path could hold lifecycle exclusion while waiting for a project source. The new actual nested-reopen regression failed before the repair: lifecycle lock count was 1 while another connection held the project fence. The repair acquires declared and physical project sources before reconstruction and lifecycle. The contention case, ordinary-fetch restore and runtime nested restore passed. The lifecycle-worker restore first completed in 16.64 seconds but exceeded its unchanged 15-second body limit; a bounded warm rerun passed at that same limit. Both wrappers exited and completed cleanup. The source type graph, module-boundary check and diff check pass. Final 7478 CI passed: 58 successful jobs, three skipped, one neutral and a successful Snyk context. Fresh Greptile is 5/5 for exact7478, with no unresolved threads. [Final CI](https://github.com/paperclipai/paperclip/actions/runs/42318150314/job/128240631573). - Full CI on `e4283b3452` passed the other server, build and type checks. One exposure owner case failed: two execution workspaces received one preview pair after source admission serialized their starts. The first local seven-case run reproduced that failure; adding the owner argument alone did not change the hash payload. The completed repair binds run-scoped reuse to its execution-workspace owner. All seven existing real-process/fake-broker cases then passed in 13.81 seconds, with wrapper exit 0 and cleanup complete. Public scope IDs and explicit project/agent sharing are unchanged. The final7478 head passed all required CI and fresh review gates. - The service/source/restore owner canary passed 25 cases across four suites before the final no-service fast-path guard (41.33 seconds). This includes actual project service processes with a different project ID and no execution-workspace ID. Their startup waits for source ownership, migration stays blocked while they run, and the source becomes eligible after their verified stop. The final three-case runtime canary passed at the final source inputs (18.70 seconds). It covers that fast path and both service-CWD variants. Both wrappers exited 0 and completed cleanup. - The boundary-repair canary passed 9 cases across four suites at `7c18f1c170`. It covers the real rehome/conversion gateway, project command provisioning, future task selection, stale imported authority, foreign-company binding, and existing standalone filesystem callers. The shared PostgreSQL wrapper exited 0 and completed cleanup. Duration: 94.15 seconds. - The P1 diagnostic passed 46 cases across five suites at the pre-rebase P1 revision. It covers the actual importer with an export warning created by `recordNativeFinalizationFailure`, strict prepared-source/action bindings, finalizer locks, source/lifecycle races, unrelated remote execution, and bare Git with external alternates. It uses the repository shared PostgreSQL wrapper, `--fsModuleCache`, and a local-only 90-second hook allowance. Duration: 245.44 seconds. No product timeout changed. - The canonical retained-directory compatibility check passed 22 cases across two suites. Missing archived task directories still import from authenticated restore points; they do not need a present-directory source proof. - Both initial P1 regressions were observed before the fix. The real finalizer warning made the importer ineligible. The old archive owner accepted a nested bare repository borrowing external objects. The new cases pass and preserve the original warning, prepared descriptor, allocations and files. - Prior attempts remain non-passes: the first 23-case canary passed on an earlier head; its exact-head rerun hit 30-second database setup/cleanup limits. A read of the owned PostgreSQL server found a template clone waiting on `DataFileExtend` for 58 seconds. A broader owner run was intentionally interrupted during slow existing heartbeat setup. One diagnostic invocation used an invalid CLI option and executed no tests. A later review canary passed 21 cases and failed the new real restore contention case: cwd-only admission missed the declared project of an external worktree. Its fixture also omitted the required file-repository authorization callback and produced an unhandled rejection after the failed assertion. The repaired run passed 22 cases, exited 0 and completed cleanup. These earlier failures are not reported as passes. - The recovery owner passed 37 PostgreSQL cases, including 14 new and 23 existing finalized-recovery cases. The operation reconciliation owner passed 40 cases. The initial archive run passed 103 cases and found a missing closed diagnostic entry; the corrected diagnostic and expanded Git rejection cases pass in the P1 run. - The full server type graph passes with the Runner source exports mapped locally because this checkout has no built Runner declarations. The latest module-boundary check and `git diff --check` pass. Standard package build/typecheck and broad final-head regression coverage remain required in CI. - The initial CI exposed missing admission scopes for standalone filesystem APIs and a changed foreign-company error. The latest repair keeps those APIs independent of a database and holds physical fences at their actual database-backed mutation owners. The branch also includes the existing master fix for unchanged Runner default-model assertions. New review findings and new-head CI are being checked. - The local canary command and bounded source selection are documented in `doc/plans/2026-11-06-online-ordinary-source-fence.md`. Source observations only read data. A fresh bounded metadata probe found 254 pending legacy records and zero unsettled local-process services. No live import, hold, cancellation, provider action or source filesystem write was performed for this change. ## Risks - Run-scoped reuse now includes execution-workspace ownership in its private key. Explicit project/agent sharing keeps its previous identity. - A missing writer owner could make a source proof unsafe. Admission derives physical aliases, includes future project selection and retains all existing source/lifecycle/finalizer locks. Unknown and changed state fail closed. - Cross-company physical alias checks use bounded minimal-path pages. Lexical-prefix filtering cannot safely replace them because a symlink can hide a foreign alias. Unknown paths or more than 10,240 foreign records leave the source explicitly unproven. More than 1,000 same-company local records also fail closed. - The project key can serialize local preparation within one project. A first project waits; additional project acquisition fails on contention to avoid a lock cycle. Remote authority keeps its remote preparation path. - Recovery preservation does not prove provider termination or successful file recovery. Existing provider allocations, checkpoints, task intent and recovery actions remain authoritative. - This slice does not finish shared Git migration. Git administrative/common-object/alternates closure and exact nested imported-child coverage remain the next owned extension. No global writers-stopped shortcut is used online. ## Model Used OpenAI GPT-6 through Codex, with reasoning, tool use and code execution. The host does not expose a more specific backend 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> |
||
|
DottaandPaperclip
|
7aced7a46c |
docs: restore task composer keyboard guidance (#19680)
## Thinking Path > - Paperclip helps people manage AI agents and tasks. > - Task composers have two keyboard modes. > - The product guide explains these modes. > - The routine feature merge removed that paragraph. > - This pull request restores the paragraph beside the routine guidance. > - Readers can find both behaviors. ## Linked Issues or Issue Description Refs #19128. This change addresses [the late documentation review](https://github.com/paperclipai/paperclip/pull/19128#discussion_r4492144137). **Issue type** Missing documentation. **Where is the issue?** The Task Management section in doc/PRODUCT.md. **What's wrong?** The routine feature merge removed the paragraph about Enter, Shift+Enter, and the optional Command/Ctrl+Enter setting. **Suggested fix** Restore the exact paragraph from before the merge. Keep the routine section. ## What Changed - Restore five lines in doc/PRODUCT.md. - Keep the routine responsibility section. ## Verification - Final qualification at `c423853f51c2bb698a65c746ed5e32d084ef9046`: [required CI passed](https://github.com/paperclipai/paperclip/actions/runs/42317059645/job/128236866986), including typecheck, build, all test lanes, E2E, and verify. The snapshot has 58 successful check runs, three conditional skips, one neutral advisory job, and a successful legacy Snyk status. No pending or failed checks remain. Greptile and Paperclip Review both score this exact head 5/5; all review threads are resolved. - A local content check confirms that the restored paragraph matches the text before #19128. Current keyboard handling and the Experimental setting still match the paragraph. Whitespace checks pass. - Local `pnpm verify` could not start because the restored checkout lacked its `tsx` dependency. The dependency-free selection command completed with exit 2 because PRODUCT.md is a global input. Neither attempt is represented as a test pass; the successful final CI supplies that broader coverage. - This restores documentation only. Runtime code, schema, and behavior do not change. Review verified the source behavior and the removed paragraph; no separate browser acceptance is claimed. ## Risks - Low risk. The change restores existing product guidance. - No runtime behavior or database change is included. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, tool use, and code execution. The exact runtime model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass (content and whitespace checks pass; runtime tests do not apply to this text restoration) - [x] I have added or updated tests where applicable (no runtime behavior change) - [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
|
a0ac81e114 |
fix(ui): show task image results in compact thumbnail grids (#19626)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Task feeds show agent messages, documents, and files in time order. > - Each image currently uses a full feed card. > - A turn with seven screenshots makes the conversation hard to scan. > - This pull request groups adjacent images from the same run in a compact grid. > - Users can scan the results and open each image in the existing gallery. ## Linked Issues or Issue Description **What existing behavior does this improve?** Image results in task history and the live task feed. **Current behavior** Each image uses a separate full-width card. Native image upload receipt messages can also separate related screenshots. **Proposed behavior** Adjacent images from one run share a wrapping thumbnail grid. New image results join the grid. Documents, video, and authored messages keep their positions. The grid opens the existing gallery. **Reason and benefit** Users can see a batch of screenshots without scrolling past one large card for each file. **Breaking changes** The feed layout changes. The stored comments, files, APIs, and permissions do not change. Related public work: Refs #17228. Refs #8785. Refs #7667. These changes supplied the artifact cards and media gallery that this pull request reuses. ## What Changed - Add one image grouping function for settled history, turn history, and live results. Group only images with the same known run. Keep the first row key and each image key when results arrive. - Add a token-based grid that reuses the media artifact cards and image gallery. - Add an optional typed attachment-preparation marker to file registration and chat-file reuse. Preserve the marker through comment normalization. Use it with matching image, author, and run links to hide generated receipts. Keep the exact historical sentence as a fallback for old receipts. - Add nine production component stories with seven sample-account screenshots. Cover mobile, Classic, mixed results, single images, missing images, streaming, gallery navigation, and the complete feed projection. - Add grouping and production-feed regressions. Add 18 browser cases across light and dark themes. - Improve contrast for the shared fallback label and gallery counter after the accessibility checks found low contrast. - Match alternate original-image paths when opening the gallery. Keep a separate Open original link beside Download in the gallery. - Record the feed layout in DESIGN.md. ## Verification - Final qualification at `09b12381ce85bee6593050bd8a2fa18cbe2c45a6`: [required CI passed](https://github.com/paperclipai/paperclip/actions/runs/42317080212/job/128237094513), including typecheck, build, all test lanes, E2E, and verify. The snapshot has 58 successful check runs, three conditional skips, one neutral advisory job, and a successful legacy Snyk status. No pending or failed checks remain. Greptile and Paperclip Review both score this exact head 5/5; all review threads are resolved. - Current-head local checks: 544 focused UI tests passed across 11 files, with one existing skip. UI/shared typechecks and token gates pass. All 18 focused Storybook browser cases passed after the CSS merge, with retries disabled. - Two database integration cases verify file registration and chat-file reuse receipt markers. All 52 issue-validator tests and six upstream default-model tests passed. - Regressions failed before the fixes for unknown-run grouping, receipt wording, and authored progress text with an image. The repair uses a typed receipt marker and requires matching author, attachment, image, and run links before hiding a generated receipt. All five review findings are resolved. - Browser checks cover loaded images, wrapping columns, overflow, missing-image fallback, streaming stability, keyboard gallery navigation, original-file links, and downloads. Fixtures use production renderers. These checks do not prove deployed persistence or provider behavior and do not compare visual baselines. - Local limits: the preflight helper could not finish because Cargo was absent. The earlier low-memory server typecheck and dependency-free selector did not replace final-head CI. The now-passing final CI supplies broad typecheck, build, server, Runner, and E2E coverage. - Prior independent CI failures remain recorded: one Slack timer case passed a single diagnosed rerun; its exact timer cause was not proved. A later chat shard lost its self-hosted runner without a test assertion failure. The current revision passes that shard and all applicable gates. Master conflict resolution retained both image-grid and patch CSS rules. ## Risks - Images without a run ID remain separate. Messages and other resource types end each batch. - Receipt suppression requires a matching preparation marker or the exact legacy sentence, plus matching attachment, image work product, run, and author. Tests preserve human messages and native progress text with attachments. The optional marker uses existing JSON metadata and needs no database migration. - Shared gallery and fallback text now have stronger contrast. ## Model Used OpenAI GPT-6, through Codex. The agent used reasoning, repository tools, code execution, and browser tests. The runtime did 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> |
||
|
DottaandPaperclip
|
72f7b42fe3 |
Preserve settled failed controller state during online migration (#19678)
## Thinking Path
> - Paperclip controls agent work across web-server releases.
> - Volume-free servers need retained controller state in shared
storage.
> - Successful settled sources already support online migration.
> - Some retained failed sources have no accepted result and must stay
available for recovery inspection.
> - This change copies those sources under task, coordinator and
publication fences while unrelated work continues.
> - The original files, failed status and recovery warnings remain
unchanged.
## Linked Issues or Issue Description
Refs #19038. Refs #19631.
**What are you trying to improve?**
Preserve retained failed controller files without waiting for global
idle.
**What problem does this solve?**
The online producer skips terminal failed controllers. Their files can
still be needed for recovery inspection even when no accepted provider
artifact proves executable continuation.
**What would the improvement look like?**
A settled exact source can enter the existing encrypted archive store.
An accepted committed failed source can use the existing checkpoint
import. Busy, changed, unowned or foreign sources remain blocked.
## What Changed
- Add a source owner bound to the selected task session, run,
coordinator generation, admitted input and file digests.
- Check the task, physical finalizer lock, released coordinator,
canonical resume source and active related turns before upload and
before publication.
- Preserve failed sources through the existing encrypted archive format.
Do not treat archived bytes as execution authority or old-writer stop
proof.
- Allow the existing accepted committed checkpoint import for a finished
failed run without changing its status.
- Validate admitted input bindings before looking up local controller
files.
- Document the online preservation path and the final static writer
fence.
## Verification
- Real PostgreSQL: 125 cases passed across migration, controller
recovery and durable backup storage. One added assertion inspected an
unadmitted queued fixture; it was corrected while retaining the
queued-turn preservation and stale-publication assertions. The final
migration rerun passes 51/51; the other three affected suites pass
75/75.
- Genuine prepatch RED: online failed-source preservation and accepted
committed failed-source import both fail on the previous producer and
recovery owners.
- New cases cover independent restore after deleting the original
source, an expired but non-null coordinator owner, source mutation after
upload, a new related queued turn during upload, foreign input binding
and an accepted committed failed result.
- The full rebased server source and test-tree typecheck passes after
restoring its missing ignored workspace SDK link and building that
existing dependency.
- Module boundaries and whitespace checks pass.
- Rebased onto current master; both provider-artifact imports and the
new archive owner are preserved. Rebased type and module-boundary checks
pass. All six archive cases pass on the exact rebased head (39.68
seconds). All CI gates pass on exact head
|
||
|
Nicky LeachandClaude Opus 5.5
|
8ed712f00a |
test(server): split workspace-recovery tests into seven feature files (#19664)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors wait for PR CI before merge, so CI wall-clock time sets the pace of work > - PR CI runs the general server tests in 12 shards, and each shard runs files in parallel on 4 workers > - `server/src/services/workspace-recovery.test.ts` takes about 699 seconds in CI by itself. A file runs whole on one worker, so shard 2/12 takes 11.5 minutes (median) when the other shards take about 7 minutes > - A different shard count cannot fix this, because the time of one file sets the minimum time of its shard > - This pull request splits the file into seven feature files with a shared fixture > - The benefit is that the shards can balance again. This removes about 2 minutes from the median PR CI time and about 5 minutes from the p90 ## Linked Issues or Issue Description **What existing behavior does this improve?** The time of the general server test shards in PR CI. **Subsystem affected** Server tests: workspace recovery (`server/src/services/workspace-recovery*.test.ts`) and the general-server shard weights. **Current behavior** One file holds 460 tests and takes about 699 seconds in CI. Its shard is the slowest in most PR runs. **Proposed behavior** Seven files of about 60 to 140 seconds each. The shard partitioner places them on different shards. **Reason and benefit** The general server shards finish close together. The PR CI critical path becomes shorter. **Breaking changes** None. No product code changes. ## What Changed - Split `server/src/services/workspace-recovery.test.ts` into seven files by feature area: - `retry-copyback` (76 tests); - `preparation` (80); - `preparation-proof` (53); - `preparation-dispatch` (25); - `preparation-gates` (43); - `warm-handoff` (36); - `admission` (147). - Add `server/src/services/workspace-recovery.test-fixture.ts` with the shared hooks, `seed`, helpers and an embedded Postgres per file. - Test bodies, hooks and helpers are moved without change. - Four lines that assigned the shared `db`, `root` or `connectionString` now call `replaceFixtureState(...)`. - `scripts/general-server-shard-durations.json`: replace the old key with the seven new files. The weights divide the current CI cost of the old file (636 seconds, master runs 42296943853 and 42294279748) in proportion to measured local time. A `splitEstimates` entry records the method, as for earlier splits. - Update the file references in `doc/task-recovery.md` and `maps/feature-map/recovery.md`. ## Verification - `vitest list`, before and after: 136 declarations each, with identical sorted names. At run time, 460 tests before and after. - Each new file passes when run alone: 76, 80, 53, 25, 43, 36 and 147 tests. - All seven files run together with `--maxWorkers=1`: 460/460 pass. - A timing check of the original file: - `afterEach` took 4.7 seconds in total and `beforeAll` 4.1 seconds, out of about 1,000 seconds, so the teardown is not the cost; - most of the time is in three `it.each` blocks. - Focused `tsc --noEmit` on the eight new files: clean. - `node --test scripts/__tests__/run-vitest-stable-shard.test.mjs`: 39/39 pass. - `node scripts/check-maps.mjs`: aligned. - With the new weights, the 12-shard partition puts the seven files on seven different shards. ## Risks - The CI time of each new file is an estimate from local runs. If `workspace-recovery-preparation.test.ts` takes more than about 150 seconds in CI, the next split point is the "keeps ancestor Retry held" block. - Each file starts its own embedded Postgres, which adds a few seconds of setup per file. - The optional external-database environment variables keep their old behavior but were not tested. CI does not set them. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use and code execution. A Claude sub-agent made the split, and the parent session reviewed it and corrected the shard weights. ## 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> |
||
|
Nicky LeachandPaperclip
|
d6f4155bad |
Refresh checkpoint writers when controller ownership changes (#19679)
## Thinking Path > - Paperclip manages AI agents and keeps their work attached to tasks. > - Core saves native Runner checkpoints so a session can continue under a later run. > - The checkpoint records can still belong to source run A after run B becomes the current owner. > - The writer cache currently keeps A's first write authority even after Core validates B. > - This pull request renews that writer when the authority, boot, generation, or lease owner changes. > - Repeated registration under the same authority keeps its work history. Stale and expired owners still cannot write. ## Linked Issues or Issue Description Refs: #19611. Related: #19225. **What happened?** A checkpoint writer is cached by source run ID. After Core validates a successor owner, a new registration still uses the cached writer for the old owner. Saving new work can then fail with `native_runner_checkpoint_owner_lost`. **Expected behavior** Use the validated current owner for later writes. Preserve the existing writer when its authority has not changed. Continue to reject stale or expired ownership. **Steps to reproduce** 1. Register a source checkpoint writer and save its first command. 2. Complete the source run and give its task to a successor with a current write lease. 3. Register the same source checkpoint under the successor and save a new command. The unchanged code rejects this write. ## What Changed - Store the write-authority fields with each cached writer. - Replace the writer only when its authority run, boot, controller generation, or lease owner changes. - Add a PostgreSQL regression for successor handover, repeated registration, generation renewal, stale writes, and expired leases. ## Verification - The new regression fails on base `913102870d` at the successor's checkpoint write. - The fixed regression passes: `cd server && node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/native-runner-checkpoint-authority.integration.test.ts`. - A focused strict TypeScript check covers the source, regression, and their imports. - The maintained general-server selector includes the regression. It needs no native process or new CI group. - The scoped diff passes whitespace and changed-line secret/PII checks. - Targeted database and focused strict TypeScript checks pass. Required CI will supply full regression coverage. - CI and standard review are pending for this draft. ## Risks A renewed writer starts a new in-memory work history. The authority comparison limits that reset to a real ownership change. Each database write still checks the current lease and generation in its transaction. This change adds no schema, API, or automatic session-release behavior. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), high reasoning, with repository inspection, code execution, and test tools. ## 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 N/A: This internal correction changes no documented interface. - [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> |
||
|
Devin FoleyandPaperclip
|
2a4a001d37 |
Preserve GitHub access repair guidance for MCP callers (#19655)
Return GitHub permission denials as failed MCP tool results with trusted, connection-specific recovery guidance. Explain the personal-account and organization App installation boundary without replaying writes or bypassing provider-content checks. Align the repair steps with the Permissions UI. Cover enabled model checks and malicious provider failures across direct calls, connection tests and stored approvals. Repair the CI-exposed synthetic onboarding environment fixture without weakening cancellation assertions. Verified 311 targeted tests, six onboarding Playwright tests, server typecheck, module boundaries, full CI and Apex 5/5 with all threads resolved. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
a3eecf1fc9 |
Preserve admitted runtime skill bundles during server replacement (#19675)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs keep the exact skill files admitted for execution and recovery. > - Older inputs can name immutable local bundles without a skill version ID. > - Installed skill heads alone do not preserve those admitted bundles. > - Server replacement needs a durable source for each retained bundle. > - This pull request copies authenticated bundles through the existing package owner and restores their exact projections. > - The benefit is preservation during online migration without stopping active work or settling recovery warnings. ## Linked Issues or Issue Description Refs #19656. Related: #19660. **What happened?** Older native execution inputs can contain skill bundles with a null version ID. Recovery skips those bundles. Startup installation checks can pass even when those retained inputs still require the old filesystem. **Expected behavior** Preserve each retained admitted bundle in object storage. Restore its original bytes, modes and digests on an empty replacement host. Keep its null pin and all run, controller and recovery state. Block startup and enrollment while a concrete local asset obligation remains. **Steps to reproduce** Retain a native run with an admitted local skill bundle and a null version ID. Convert its installed skill head to durable storage. Replace the server home and attempt recovery. The old bundle path remains in the recovered context. ## What Changed - Add a bounded `runtime-assets` phase to the existing skill package maintenance CLI. It copies exact admitted files under the runtime asset publication lock, verifies the durable package, and appends a run-bound receipt after source revalidation. - Keep original files, null pins, task intent, warnings and controller leases. Support an active run through copy-only preservation. Do not start a turn or claim provider ownership. - Verify and reuse a healthy canonical local bundle before storage reads. Restore missing or damaged bundles through the existing package and native asset materializers. Use the authenticated original manifest to preserve exact read and execute modes. Accept a later contract revision only when the run binding and exact skill selection remain unchanged. - Check retained recovery obligations and current task sessions during both startup and migration eligibility. Ignore ordinary admission that has no completed execution input. - Protect receipt-backed packages from orphan collection. Serialize receipt publication and deletion on the package row, then check references in a fresh statement after the lock. - Add real PostgreSQL and filesystem regressions. Replace a delete-method proxy with an actual upload-renewal row-lock race. Publish mutation activity only after commit. ## Verification - Baseline production startup gate failed the new concrete-asset regression as expected. The fixed gate passed. The actual before-fix preservation owner also failed recovery from a healthy local bundle during a simulated storage outage. A verified local-first path fixes that regression. - Eight preservation tests passed with real PostgreSQL, streaming storage, archive extraction and native materialization. They cover cold recovery, exact modes and binary bytes, active ownership, changed sources and bindings, contract revisions, missing or corrupt receipts, and publication/collection races. - Six affected package collection/deletion controls and three existing startup controls passed. The existing versioned recovery consumer passed a sequential retry. - One fixture attempt failed on its cleanup foreign-key order; the fixture was corrected. One existing suite hit its unchanged 30-second database startup hook before test execution. The unchanged single consumer passed on retry. These failures remain recorded. - Source, changed-test and CLI TypeScript checks passed. Rebase preserved both the local instruction-copy and runtime-asset guards. Current-head changed-test checks passed. Final PostgreSQL rerun passed all eleven preservation/startup cases. Current CLI check passed. Required CI remains pending at the trusted-runner gate. - Current canonical bundled creation uses package preparation plus atomic content/head publication. Native preparation requests immutable version snapshots before delivery. Converted installations and new canonical bundled creation therefore do not recreate the historical null-version asset dependency. - Fresh Greptile review on exact head `54c7dd048c9016c1af9fc68213d5ae053ac16499` passed at 5/5 with zero unresolved threads. - Module boundaries, whitespace, and credential/internal-reference scans passed. No live source writes, provider actions or migration apply were performed. ## Risks Missing, changed or unsafe local files remain blockers. This command does not repair a previously published package whose objects later disappear or become corrupt. It also refuses malformed preservation receipts. It retains the original local files and prior evidence and keeps removal blocked. Automatic repair requires a separate canonical package-owner recovery operation; direct database edits are not an accepted recovery path. Failed publication keeps the original files and any unreferenced package for normal collection. The readiness check is explicit startup or enrollment work, not a background scan. Its cost grows with retained admitted asset obligations. Older readers must be upgraded before the old volume is removed. This change preserves asset bytes; it does not settle business recovery or qualify unrelated volume dependencies. ## Model Used OpenAI GPT-6 (Codex). The runtime did not expose a more specific backend model ID or context-window size. Used reasoning, code editing, and tool 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>canary/v2026.1107.0-canary.1 |
||
|
DottaandClaude Opus 5.5
|
6d1ecfc420 |
fix(accounting): close a remote run's lost receipt once its sandboxes are terminated (#19606)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budget admission waits while completed runs await accounting; #19497 made that a hold on new work, never a hard stop > - #19497 also closes a lost usage receipt once a local provider is verified stopped, so the hold ends > - A remote (Daytona) run has no local stop event and keeps its recorded remote process identity, so its lost receipt never closed. The company then held all new work until a receipt that can no longer arrive > - This pull request closes a remote run's lost receipt once every sandbox it used carries a provider termination receipt > - The benefit is that a restart that kills a Daytona turn no longer stops a company's new work ## Linked Issues or Issue Description Follow-up to #19497. Refs #19544 (Daytona chaos harness). **What happened?** In a Daytona restart wave, run `a261c30b` failed after native recovery was blocked. Its usage receipt stayed incomplete (`accountingReceiptReady: false`). Both of its sandbox leases were released with a `destroyed` termination receipt, but accounting stayed pending. Admission then refused new company work with "New work must wait while a native run recovers its unfinished accounting". **What did you expect?** Once every sandbox of the run is stopped or destroyed, no runner can report usage for it. The known part of its usage should settle as an unpriced charge, as it does for a verified local stop, and new work should be admitted. **Steps to reproduce** 1. Run a native task on a Daytona environment with a hard-stop budget. 2. Kill the control plane during the provider turn so the turn's receipt is lost and native recovery ends in terminal failure. 3. After the sandbox lease is released with a termination receipt, observe that the run stays `cost_accounting_pending` and new work is held. ## What Changed - `run-cost-accounting.ts`: the closed-receipt predicate gains a remote branch. A native run in terminal failure without a result (failed, cancelled, timed out or interrupted) is closed when it has at least one environment lease and every lease carries a `paperclip.remote-termination.v1` receipt bound to that lease, run, company, provider and provider lease ID, with state `stopped` or `destroyed`, after a successful release. This mirrors `hasRemoteTerminationReceipt`. One SQL predicate still serves the reconciler scan and settlement. - The local branch is unchanged: a failed run with a verified local stop event and a cleared process identity. - A sandbox that may still run (no receipt, lease still active), a receipt bound to another sandbox, or a run with no lease keeps its debt pending. - `budget-enforcement-worker.ts`: the accounting worker also wakes when environment-cleanup or retained-session-cleanup work commits. Those are the signals the lease writers already send when they release or stop a sandbox. - `doc/SPEC-implementation.md`: documents the remote closure rule. ## Verification - `closed-native-receipt-accounting.test.ts` (embedded Postgres): - A failed and a cancelled remote run, with one destroyed and one stopped sandbox, settle as one unpriced charge, settle the reservation, and clear admission. These are red on master. - Debt stays pending for an active sandbox with no receipt, for one of two sandboxes without a receipt, for a receipt bound to another sandbox, and for a run with no lease. - `budget-enforcement-worker.test.ts`: the accounting worker wakes on environment-cleanup and retained-session-cleanup commits, and stops waking after `stop()`. These are red on master. - Existing suites pass locally: native-stop-accounting, budget-enforcement-scoping, heartbeat-cost-accounting, run-cost-accounting, cost-data-quality, budgets-service. Server typecheck passes. - Live Daytona restart wave: results in the comments. ## Risks - A cancelled remote run now closes once its sandboxes are terminated. Stop recovery could otherwise reattach a recorder, but a stopped or destroyed sandbox has no runner to reattach to. Local cancelled runs keep their drain. - Workspace export can restart a stopped sandbox and removes its termination receipt first. A receipt that arrives after settlement still posts through late-receipt reconciliation. - Reusable-lease chains (`resolveRemoteTerminationLease`) and recovered-cleanup stop receipts are not mirrored, so those runs stay pending. That is the conservative side. - Low query cost: the predicate runs only for pending terminal runs. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, embedded Postgres tests, live restart-chaos harness). ## 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
|
b8d94b3ad1 |
test(server): verify status and object deadlines across cold sleep (#19347)
## Thinking Path > - Paperclip manages agents and their useful work. > - Status cards and external objects can have future refresh deadlines. > - A stopped tenant needs a durable Cloud alarm to return for those deadlines. > - A local quiet report alone does not prove that handoff or later useful work. > - Acceptance must exercise the normal producer, authorization, claims and alarm owner. > - This PR adds opt-in signed cold-owner acceptance and bounded launcher controls. ## Linked Issues or Issue Description Refs #19001 and #19191, which have merged. This PR targets master and adds acceptance for the existing status and object owners. **What happened?** The existing local coverage did not join useful status generation or built-in GitHub refresh with signed Cloud alarm persistence, physical child retirement and cold reactivation at the original deadline. **Expected behavior** Keep the feature enabled. Complete useful work through its normal owner. Persist the exact authorized alarm before stopping the child. Wake and process the same original deadline without losing identity, claims, retries or useful results. **Steps to reproduce** Use the checked-in Node24 launcher with separate owned loopback App/Cloud test databases, a compiled Cloud checkout and the pinned Node22 executable. Select one status or object bootstrap, cycle or identity case. The command validates fresh evidence and database ownership before starting its guarded worker. **Paperclip version or commit** Current candidate `ea911278b22c855d6722e43afc026bfc5c031405` normally merges fetched master `07a548073ab48f13cb93b2fdc31553a5da88a070`. All eight owned acceptance files are byte-identical to `c08e64fa857d39bdb79352e94d156db7a4dbebf3`; all unrelated paths exactly match master. The guard-argument repair and source-specific historical cycle proof are preserved. ## What Changed - Add one named opt-in case per owner/stage/diagnostic combination. Keep the shared business body and its original deadlines, assertions and cleanup. - Drive actual managed App startup, authenticated producers, persisted claims and the signed Cloud SQL alarm before physical child stop. - Retain the generated five-secret fixture profile across cold boots. Store it in private directories and retain only names, hashes and modes in evidence. - Add optional query timestamps to the existing proxy and harness. Existing callers keep their default behavior. - Add launcher refusal controls and assert its complete child argument list. Both explicit guard preloads and the exact child `NODE_OPTIONS` must match. ## Verification - The current preflight repair has causal proof: the old controls pass all nine cases with an invalid worker preload flag. The new assertions reject that same command in both success controls (two failures/seven passes). Restoring the correct command passes all nine cases. The original refusal, freshness, symlink and byte-preservation assertions remain unchanged. All 33 recorded Node PIDs and three groups close; temporary copies and private directories are removed. These controls stop at the declared spawn stub. They verify the full command contract and do not claim to execute Vitest. - The prior real named status bootstrap passed in 72.942 seconds: one selected case/six explicit skips. It exercised the actual launcher and Vitest fork, useful normal generation, current authorization, settled claims, original 900000ms interval and signed SQL alarm generation/operation-lock acknowledgement before physical child stop. Recorded processes and final SQL sessions closed; both successful databases were normally dropped. - Earlier normal status and GitHub-object cycles qualified their real 900-second and 300-second original deadlines, cold reactivation and useful second results. Their complete original failure and recovery records remain source-specific; no shortened business clock or manufactured completion is used. - On current integrated head `ea911278`, own frozen offline setup and normal shared/SDK, Runner and App declaration prerequisites passed. The status test and its imported closure pass the affected normal test-tree compiler program with original options/references. The actual named normal status bootstrap then passed: one selected case/six skips, 82.202 seconds in Vitest and 85.616 seconds in the launcher. It used the real checked-in launcher, normal App/SDK/process agent and exact compiled Cloud `389878364b42dd129835d263f91882c5774d5123`. The original 900000ms interval and 120000ms alarm lead remain unchanged; useful v1 and signed SQL alarm generation/operation-lock acknowledgement precede physical App stop. All six recorded process IDs and the runtime group were absent; independent final test-database sessions were zero, with no observer during execution. Both fresh success databases were normally dropped. This refreshes the changed consumer seams through handoff and stop; it is not another cold-wake/useful-v2/full-cycle result. Historical cycles remain tied to their measured source. - Fresh exact-head CI and complete edited bot reviews are required before merge. A whole final-tree local build/test or deployed suspension is not claimed. Local provider, process and S3 boundaries are controlled fixtures; no live GitHub provider or Railway/Neon suspension claim follows. ## Risks - These tests use real SQL and wall clocks and are opt-in. Their fixtures must stay isolated and retain failed evidence. - Stable fixture secrets must remain private and must not appear in logs. Cleanup keeps them when owner closure is uncertain. - The acceptance body stays shared to preserve measured evidence. The named registration and command controls make selection failures visible. - This PR does not change production, schema or feature enablement. ## Model Used OpenAI `gpt-6.1-sol` produced the delegated acceptance work. OpenAI Codex, based on GPT-6, performed managing review, source integration and the preflight argument repair. Both used repository tools and code execution. The coordinator's exact backend snapshot and context-window size were not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used - [x] I have checked ROADMAP.md and confirmed this does not duplicate planned core work - [x] I have searched related PRs and linked the existing foundations - [x] I have described the issue with the bug-template fields - [x] I have not referenced internal or instance-local issues or links - [x] My branch name describes the change - [x] I have run the stated focused local tests and preserved their limits - [x] I have added or updated tests - [x] I have documented the opt-in command and evidence requirements - [x] I have documented risks - [ ] All current-head CI gates are green - [ ] Current-head Greptile is 5/5 with no open findings - [x] I will address reviewer findings before merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
7c673e695f |
fix(chat): name tasks by identifier, not UUID, in tool rows and agent text (#19635)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - New users meet Paperclip through the onboarding chat with their
first agent, and through task briefs that agents write
> - A browser walk of onboarding found internal handles and
control-plane jargon in those user-facing surfaces
> - A tool row showed `Read taskId: e3ecf9bc-…`, an agent-written brief
cited `task … (id 2b330993-…)`, and a Codex agent told a new user about
"receipts", "execution policy", and a "rejected policy patch"
> - Raw UUIDs and mechanism narration make the product look broken and
give the user nothing to act on
> - This pull request makes tool rows name a task by its identifier, and
tells agents to reference identifiers and to describe outcomes, not
mechanics
> - The benefit is that a new user sees "Read <task identifier>" and
plain progress updates
## Linked Issues or Issue Description
**What happened?**
In the task chat, a collapsed tool row rendered the raw call argument
`taskId: <uuid>`. An agent-written task description included a raw task
UUID next to the task identifier. A Codex chief of staff narrated
internal mechanics (receipts, execution policies, a rejected policy
patch) to a new user in chat.
**Expected behavior**
Tool rows name the task by its identifier. Agent-written text links
tasks and documents by identifier. Agents tell the user what happened,
what it means, and the next step.
**Steps to reproduce**
1. Run onboarding with the Codex API runner and accept a single-task
proposal.
2. Watch the handoff in the chief-of-staff chat.
3. See the task lookup tool row and the narration. With the Claude API
runner and the plan path, open a child task created from the plan and
read its description.
## What Changed
- `ui/src/components/task-chat/transcript-adapter.ts`: a tool row's
target never contains a UUID argument. A `taskId`/`issueId` UUID is kept
as `taskRefId` on the tool item.
- New `ui/src/components/task-chat/task-reference.ts`:
`useTaskReferenceLabel` reads the task from the shared issue detail
query cache and returns its identifier. It returns "task" until the task
resolves or when it cannot.
- `TaskChatRunnerActivityGroup` and `TaskChatToolCard` show that
identifier as the row subject. They load the task only for rows that
have a task reference.
- `skills/paperclip/SKILL.md` (Comment Style): two new required rules.
"No raw IDs in prose" and "Describe outcomes, not mechanics".
- First-task onboarding skill: a "Talk in outcomes" rule. Non-inline
handoff inputs use an identifier link, not a raw UUID.
- Chief-of-staff persona (`AGENTS.md` for the first agent): the same
plain-language rule, so it applies in every chat.
- `skills/paperclip-converting-plans-to-tasks/SKILL.md`: task
descriptions reference other tasks and documents by identifier link,
never by raw UUID.
- No change to `skills-releases/` snapshots. They are frozen releases.
## Verification
Red on master (fix files reverted, new tests kept):
```
ui tool-row-task-reference.test.tsx
FAIL ... > falls back to a plain word while the task cannot be resolved
AssertionError: expected 'ReadtaskId: e3ecf9bc-099a-4b10-8169-9…' not to contain 'e3ecf9bc-099a-4b10-8169-9ce490a91e3d'
Tests 3 failed (3)
server user-facing-wording-guidance.test.ts
Tests 6 failed (6)
```
Green with the fix:
```
ui tool-row-task-reference.test.tsx Tests 3 passed (3)
server user-facing-wording-guidance.test.ts Tests 6 passed (6)
```
Neighbouring suites:
- `cd ui && npx vitest run src/components/task-chat
src/components/TaskChatThread src/components/transcript`: 70 files, 1448
passed, 1 skipped.
- From the repo root: `onboarding-first-task-assets`,
`paperclip-skill-utils`, `onboarding-first-task-admission`,
`hiring-operational-examples`, `cli-invocation-safety`, and the new
guidance test all pass.
- `pnpm check:token-gates`: all gates clean. The change adds no styles.
- `ui` typecheck: no errors in the changed files.
- Not run locally: `agent-skills-routes.test.ts` hit a 30 s `beforeAll`
import timeout on a cold cache. CI covers it.
## Risks
- Low risk. A row that names a task loads that task once through the
shared issue detail cache. The thread usually has it cached already. A
failed load shows "task" and does not retry.
- Rows now show other non-UUID arguments after the identifier (for
example `<identifier> · status: done`). Rows without a task reference do
not change.
- The skill text changes the guidance that agents read. They do not
change permissions, tools, or API behavior.
> 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, run in Claude Code
with tool use (shell, file edits, test 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 Opus 5.5
|
6c7cc2c5d6 |
fix(native): do not spend a recovery attempt resuming a cleanly drained run (#19625)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs have an execution recovery budget of three provider attempts, so a run that keeps failing stops retrying > - A graceful shutdown (deploy or SIGTERM) drains native runs: it suspends them for reattachment by the next server > - When the runner did not survive, the next boot resumed the turn and charged one attempt > - Restarts that the run did not cause used up its budget, so recovery stopped and the task needed a manual Retry > - This pull request does not charge an attempt for resuming a run that a graceful drain suspended > - The benefit is that deploys and graceful restarts no longer exhaust a healthy run's recovery budget ## Linked Issues or Issue Description **What happened?** In restart-chaos testing, native runs reached `execution_recovery_budget_exhausted` (3/3) only because of server restarts. In one wave, two restarts crossed each of two runs. In a second schedule (K1, then K2 when ready, then K2 again), each drain correctly suspended the local ACPX Claude run, yet the fourth boot blocked it with `native_restart_recovery_blocked`. Every resume after a drain was classified `resume_dead_runner` and charged an attempt (`nextNativeProviderAttempt` in `native-restart-recovery.ts`, applied by the executor's execution claim). **Expected behavior** A restart that the run did not cause should not consume its recovery budget. A graceful drain records `restart_suspended` and `awaiting_runner_reattach`. The resume that follows should keep the attempt count. **Paperclip version or commit** `master` at `4ef3fa4271`. **Steps to reproduce** Start a native local run. Send the server SIGTERM three times while the run is active, with a full restart each time. The third resume is blocked with `execution_recovery_budget_exhausted`. ## What Changed - `suspendedByCleanDrain()` in `native-restart-recovery.ts` recognizes a run whose most recent recovery claim is older than the drain's `restart_suspended` entry. Ownership waits and deferred blocks between the drain and the next claim do not erase it. The next claim consumes it. - A scan that waits for the old controller to exit (`awaiting_evidence`) records one history entry per reason, so repeated scans cannot push the drain record out of the bounded history. - Restart claims carry `chargesProviderAttempt`. It is `false` for a claim that follows a clean drain. - `nextNativeProviderAttempt()` takes that flag. The executor passes it when it takes the execution lease, so a drained resume keeps the current attempt. - The first execution is always counted: a run drained at attempt 0 (no execution claimed it yet) still charges its first provider start. - A run drained during its third attempt may continue that attempt. The local resume and remote bootstrap gates skip the `>= 3` exhausted check only for a non-charging resume, and the executor still refuses a fourth attempt. - A crash restart (no drain record) still charges one attempt. Reattachment to a live runner still charges none. - `doc/execution-semantics.md` states the rule next to the three-attempt budget. ## Verification - New test in `server/src/__tests__/heartbeat-native-restart-suspension.test.ts` (embedded Postgres, real heartbeat drain and restart recovery): - Red before the fix: the claim after the drain had no `chargesProviderAttempt: false`. - Green after the fix, for drained attempts 0, 1 and 3: the crash claim charges (`true`). After the drain, a controlled backend lets the executor take its execution lease and records the saved `native_run_finalizations.attempt` at provider dispatch: 1, 1 and 3. Attempt 3 is claimed instead of blocked. - Red at the executor: when the executor ignores the flag, the drained attempt-1 resume saves 2. - Ownership-wait case: the drained controller (the test process) is still alive, so two scans wait. The history keeps `restart_suspended` followed by one `awaiting_evidence` entry. After the controller is gone, the claim does not charge, and attempt 3 continues. - Classifier cases (local resume and remote bootstrap): attempt 3 after a crash is still `execution_recovery_budget_exhausted`. Both new cases were red on the previous revision. - `heartbeat-native-restart-suspension.test.ts` 12/12, `native-runner-restart-recovery.integration.test.ts` and `native-runner-ownership.test.ts` 94 passed. - `tsc --noEmit` for `server` passes. ## Risks - A run that itself makes the server shut down gracefully, again and again, would no longer be stopped by this budget. Graceful shutdowns are operator or deploy actions, so this is unlikely. A crash loop (SIGKILL, OOM) still charges an attempt on every restart, and a resumed turn that fails still goes through the existing failure and replacement budget. - This changes only the attempt count. Ownership checks, checkpoint verification and the exhausted gate are unchanged. > 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, test execution). 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> |
||
|
DottaandPaperclip
|
19c2bebc9c |
fix(runner): settle durable provider exits on open streams (#19657)
## Thinking Path > - Paperclip manages AI agents and their work. > - Runner keeps the native session and its event stream until a turn ends. > - A provider can exit with a terminal error while its event stream stays open. > - Waiting for that stream to close leaves the run and its resource owner active. > - The existing durable failure receipt already identifies the failed current turn, including an authoritative replay. > - This pull request uses that event to enter the existing failure and cleanup path. > - Recovery and later idle retirement can then proceed from a real terminal result. ## Linked Issues or Issue Description Refs #18988 for native resource retirement. This is a separate terminal-event defect found during failed-turn recovery qualification. A search of open and closed PRs found no duplicate `provider_exited` fix. The related merged #12596 changes Rust test polling; it does not change this TypeScript session owner. **What happened?** `executeNativeSession` records a nonrecoverable `provider_exited` event, but can continue to wait on an open event iterator. A proposed result does not authorize success, and stream EOF may never arrive. The run cannot reach its existing failure and resource cleanup path. **Expected behavior** Settle the committed or authoritative duplicate terminal failure for this exact run, session and provider. Close through the existing owner. Preserve recoverable goals, completed-turn and identity guards. A duplicate must not add another committed event count. **Steps to reproduce** 1. Supply a native session whose iterator emits a current, nonrecoverable, unexpected `provider_exited` failure. 2. Keep that iterator open until the session owner closes it. 3. Observe that the old implementation does not settle within the existing test bound. 4. Apply the guard below. The same execution fails with the typed terminal error and calls the existing close owner once. **Paperclip version or commit** The original regression was reproduced against master source `05a19768ae4d337a4ae88ecdbc259f251406487c`. Current candidate `f4bb8f1b68e2793062dd640ee56b87a9208b4fd9` normally merges fetched master `07a548073ab48f13cb93b2fdc31553a5da88a070`. All unrelated master paths are preserved exactly. The two-file PR diff contains only the guarded settlement and its tests. ## What Changed - Add a guard after the existing durable event receipt. It requires a committed or duplicate receipt, a nonrecoverable unexpected exit, no completed current or previous turn, and exact provider/run/session identity. - Throw the existing typed provider failure so the existing failure and close owners run. Add no scheduler, polling loop, provider parser or cleanup owner. Event counting remains committed-only. - Cover open streams with and without a result proposal, authoritative duplicate replay, all six safety predicates, and a recoverable goal that later completes. Preserve accepted-result and close-once assertions. ## Verification - The original open-stream regression fails before the production change. Review then found that the duplicate test released EOF before asserting settlement. The corrected duplicate control fails on the prior production code at its original timely-settlement assertion; it passes after accepting the authoritative duplicate. All 21 selected exit, safety, goal and compaction controls pass. - The normal durable transport validates authenticated current run/session/turn/item identity and canonical replay equality before delivery. The App port validates current authority and binding; its durable event transaction returns duplicate only for matching source identity and canonical payload. No absent or conflicting event authorizes settlement. The six negative consumer tests use a deliberately permissive mock port; they prove this consumer guard, not that production accepts foreign events. - The first remote Typecheck job found five errors in new mocks that accessed PRP fields without narrowing a native-or-PRP event. Both mocks now explicitly reject the wrong event shape. Complete original CI evidence is retained. No cast, suppression or narrowed test compiler program is used. - On current integrated candidate `f4bb8f1`, normal Runner emission and the full normal `tsconfig.test.json` source/test program pass. All 26 selected cases pass: the same 21 controls plus all five structured provider-failure/model-retry rows affected by the incoming master change. The other 197 cases were filtered, not counted as passing. - Before master integration, all 411 declaration paths and bytes were identical across the settlement change. Incoming master independently adds one OpenCode declaration and changes three OpenCode declarations; those deltas are recorded separately. This fix changes no runtime declaration. No Rust source or binary is changed by this PR. - Actual guarded workers recorded zero PG sockets, active listeners or blocked attempts. All known worker processes/groups closed. The working tree is clean after qualification. No install, Rust build, App run, provider connection or PostgreSQL suite was part of the final freshness loop. - Fresh exact-head remote CI and complete bot reviews remain required. A local whole-repository test/build, successful task retry or live-provider acceptance is not claimed. Run the selected controls from a normal dependency installation: ```sh pnpm --filter @paperclipai/paperclip-runner exec vitest run src/native-session-runtime.test.ts --testNamePattern='settles committed or duplicate provider exits while their stream stays open|retains EOF settlement when provider exit fails the|preserves a recoverable provider-exit goal until its later authoritative completion|preserves a durable session failure when its stream closes|preserves session-goal failure priority|context compaction|never continues task work when a restart loses a compaction turn|preserves structured provider failure and model retry classification' --maxWorkers=1 --no-file-parallelism ``` ## Risks - A false terminal classification could stop useful work. The guard requires the existing durable receipt and explicit current-turn failure fields; recoverable and completed-turn controls remain covered. An exact durable duplicate may settle the same failure without repeating accounting. - Close can still fail. This change reuses the existing close owner and does not manufacture resource retirement or clear retained failure debt. - This proves Runner failure settlement under a controlled session boundary. It does not establish successful task retry, whole-tenant quiet, live provider closure, or deployed suspension. ## Model Used OpenAI `gpt-6.1-sol` produced the delegated fix and qualification. OpenAI Codex, based on GPT-6, performed managing technical review and integration. Both used repository tools and code execution. The coordinator's exact backend snapshot and context-window size were not exposed. ## 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 linked existing issues or described the issue in-PR following the issue template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run the selected tests locally and they pass - [x] I have added tests for the changed behavior - [x] I have reviewed documentation; no command or public contract changes require a documentation edit - [x] I have considered and documented the risks above - [ ] All Paperclip CI gates are green — fresh exact-head checks pending - [ ] Greptile is 5/5 with no open findings — fresh exact-head review pending - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Nicky LeachandClaude Opus 5.5
|
444b4de693 |
fix(cli): use native CRC-32 for import zips and give the e2e server time to start (#19667)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors wait for PR CI before merge, and a flaky test forces a re-run of the whole CI run > - Two CLI tests failed several times on 2026-11-06 on commits that passed on a later attempt > - Both failures came from time limits on CPU-contended 4-vCPU runners, and both appeared in the same jobs > - One cause is a slow checksum loop that the CLI's own zip writer also uses > - This pull request removes that cost and gives the e2e child server a startup budget that covers a cold start > - The benefit is fewer whole-run re-runs, and faster large-package zips for CLI users ## Linked Issues or Issue Description **What happened?** Two tests in `cli/src/__tests__` failed in PR CI and passed on a re-run of the same commit: - `company-import-transfer.test.ts` › "resolveChunkedImportZip › takes the chunked path…" failed with `Test timed out in 5000ms` (2 times). - `company-import-export-e2e.test.ts` failed with `Timed out waiting for the child server` (3 times). **Expected behavior** Both tests pass on a loaded CI runner. **Steps to reproduce** 1. Run the `cli` Vitest project with `--maxWorkers=4` in a 4-CPU container (`docker run --cpus=4 --memory=16g`), with a 4-thread CPU load in that container and a fresh `TMPDIR`. 2. Before this change, both tests fail in 3 of 3 runs. ## What Changed - `cli/src/commands/client/zip.ts`: the stored-zip writer uses `crc32` from `node:zlib` instead of a bitwise JavaScript loop. The CLI uses this writer for packages past the chunked-import threshold. Over about 64 MB, the old loop takes about 14 seconds and `zlib.crc32` takes 35 ms. `zlib.crc32` has been in Node since 22.2, and the engines field requires Node 24.11 or later. - `cli/src/__tests__/company-import-transfer.test.ts`: the fixture's zip builder uses `zlib.crc32` too. - `cli/src/__tests__/company-import-export-e2e.test.ts`: - add `SERVER_STARTUP_TIMEOUT_MS = 90_000` for `waitForServer`, which was 30 seconds; - set the `beforeAll` timeout to that value plus 30 seconds for the database start; - a comment gives the measurements. A child that exits early still fails at once. No assertions change. ## Verification - **Cause of the e2e failure:** `paperclipai run` loads the CLI and the server from source through tsx, and every CI run starts with a cold tsx cache. - The server is slow to start, not hung. With a larger budget at heavy load, startup took 50 to 55 seconds. - Passing CI jobs already used 15 to 25 seconds of the 30-second budget. - In the failed logs, nothing comes from the server after "Starting Paperclip server...". - **Container reproduction** (4 CPUs, 16 GB, a 4-thread load in the same container, cold tsx cache, full `cli` project with 4 workers): | State | Result | Dense-zip test | Oversized-folder test | | --- | --- | --- | --- | | Before | failed 3/3 (child-server timeout; the dense test also timed out in one run) | 4.6–5.5 s | 2.7–3.2 s | | CRC fix only | the e2e test still failed 3/3 | 0.8–1.3 s | — | | Full fix | all 65 files passed 3/3 | 0.7–1.3 s | 0.15–0.2 s | - `pnpm typecheck` in `cli`: passes. - Host run with `--maxWorkers=2` of the changed and related files: 4 files, 23/23 pass. ## Risks - The e2e change is a larger time budget, not a faster start. A real startup regression now takes up to 90 seconds to fail. - Each "after" result comes from 3 runs, which is a small sample. - `zlib.crc32` returns the same unsigned 32-bit value as the old loop. The existing zip round-trip tests cover this. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use and code execution. A Claude sub-agent found the causes and made the fix, and the parent session reviewed it. ## 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
|
56c2b28c04 |
Settle saved monitor results after a newer user request (#19644)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Native results must finish their durable finalization before later
work can reuse a saved workspace.
> - An accepted result can wait on a monitor while its workspace files
are recovered.
> - A newer user request can replace that old wait before finalization
resumes.
> - The old finalizer then reports lost monitor authority and blocks the
saved-workspace lineage.
> - This pull request proves the newer user request and settles only the
old result bookkeeping.
> - The benefit is released recovery work without replaying an old turn
or changing the current request.
## Linked Issues or Issue Description
Refs #17641 and #16975.
**What happened?**
An older successful native result has saved files and a server-issued
monitor receipt. Its task is now blocked after a newer user-comment
continuation failed during preparation. The old coordinator stays in
retryable_failure with monitor_wait_authority_lost. The retained
allocation ownership check correctly refuses the unresolved predecessor,
so later file recovery also stays held.
**Expected behavior**
The normal finalizer can settle the old accepted result when exact newer
user-request evidence proves the old wait is superseded. It must
preserve the current task, user input, ownership, schedule, accepted
result, and attempt history. It must not start provider work or publish
the old wait summary.
**Steps to reproduce**
Create an accepted successful monitor result and mark workspace
finalization successful. Leave the task blocked. Record a newer
canonical user comment and its ordinary user wake, either queued or
consumed by a same-task continuation that retains the exact comment
revision and source-run lineage. Call the real native finalizer. Before
the repair, both cases remain retryable_failure instead of settling.
**Paperclip version or commit**
|
||
|
DottaandClaude Opus 5.5
|
e33aa8a46a |
fix(runner): let saved Codex threads follow the current feature policy (#19659)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The Paperclip runner keeps a saved Codex thread so the next run of a
task continues the same conversation
> - On each new run, the controller sends `run.attach` with the current
Codex launch arguments, and the runner refuses any change to "protected"
arguments
> - #19516 removed `--disable image_generation` from the Codex launch
arguments, but every thread saved before that deploy still records the
flag
> - The runner treated the feature toggle as protected, so every
pre-deploy Codex thread failed with `run.attach cannot change protected
launch arguments`
> - Same-run recovery then tried to replace the thread, the replacement
failed, and the tasks became blocked for an operator
> - This pull request treats Codex feature toggles as controller launch
policy, so a saved thread adopts the current toggles and restarts on the
same thread
> - The benefit is that a release that turns a Codex feature on or off
no longer strands every saved Codex thread
## Linked Issues or Issue Description
Refs #19516
**What happened?**
After a staging deploy that included #19516, Codex agent runs on tasks
with a saved thread failed at the start. The run log showed
`native_session_recovery_failed: PRP command run.attach failed:
run.attach cannot change protected launch arguments` (error code
`native_session_interrupted`). Same-run retries then failed with
`runner_continuity_break_archive_failed` and
`runner_remote_state_unavailable`, and the tasks were blocked with
"Workspace repair required". Four tasks on one Codex agent showed this
first failure. Each one last ran successfully before the deploy.
**Steps to reproduce**
1. Run a Codex task so that its thread is saved with the launch
arguments from before #19516 (`--disable image_generation`).
2. Deploy a revision that includes #19516.
3. Wake the same task again. `run.attach` sends the current arguments
without the toggle, and the runner rejects them.
**Expected behavior**
The saved thread continues under the current launch policy.
**Root cause**
`CodexCommandExecutor::stable_launch_args` drops only the per-run
filesystem and shell-environment entries before it compares saved and
current arguments. All other arguments, including Codex
`--enable`/`--disable` feature toggles, must be identical. The
controller computes the toggles from Paperclip policy for every launch,
so any change to that policy made every saved thread impossible to
attach.
## What Changed
- `provider_backend.rs`: `stable_launch_args` also skips `--enable
<feature>` and `--disable <feature>` pairs and the
`--enable=<feature>`/`--disable=<feature>` form. The function contract
comment now lists the rebindable arguments. `run.attach` then adopts the
controller's current toggles. Because the arguments changed, the
existing path stops the provider and restarts it on the same thread.
- New test
`saved_thread_adopts_the_controllers_current_codex_feature_policy`:
- A thread saved with `--disable image_generation` attaches with the
current arguments, keeps its thread ID, and records the new arguments.
- A new `--disable` toggle (a tighter policy) is also adopted.
- A new `--enable` toggle is adopted in both flag forms, and the thread
ID stays the same.
- A sandbox bypass flag, a different `default_permissions` profile, and
a different subcommand are still rejected with "protected launch
arguments", and the saved state does not change.
## Verification
- Red on `master` (
|
||
|
DottaandPaperclip
|
752cb8d3da |
fix(ui): contain patch previews and highlight diff source (#19478)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Tasks show documents and file attachments in cards and file views. > - Patch source needs clear added and removed lines in each view. > - Long patch lines can exceed the width of an inline preview. > - This pull request keeps the preview inside the card and adds patch colors. > - Users can read a compact patch preview and open the full source. ## Linked Issues or Issue Description **What happened?** Patch text can exceed the inline document box. Patch files and patch code blocks show plain text in the document view. **Expected behavior** Keep source inside its preview. Show added lines in green and removed lines in red. Keep headers, source markers, copy controls, line numbers, and downloads. **Steps to reproduce** 1. Open a document card with a diff code block that has long unbroken lines. 2. Open a `.patch` or `.diff` attachment in a task tab. 3. Open the same patch in the workspace file view. **Paperclip version or commit** Source checkout based on `e82210386f1e`. **Deployment mode** Built from source. The browser checks use Storybook fixtures. Related work: #19269 adds inline diffs for file edits. This change uses its line classifier for document and file source views. ## What Changed - Share the unified diff line classifier with file and document views. Track hunk lengths so later file headers keep their metadata style. - Add safe patch source rendering for patch files and `diff` or `patch` code blocks. - Clip long lines with an ellipsis in compact cards. Wrap the full source in file views. - Add a Preview patch action for small `.patch` and `.diff` attachments, including generic binary MIME types. Opening a large artifact list does not fetch patch source. Bound the actual response bytes. Reject remote preview URLs and redirects. Keep download and retry fallbacks. - Add patch text colors with sufficient contrast in both themes. Keep source-line selection visible over patch row colors. Copy the original patch text without layout line breaks. - Add five production component stories and browser checks at two widths in both themes. - Document the focused Storybook browser command. ## Verification - Final base qualification at `a120e0ada3f1eee9746f47a10384b5aa95f79824`: normally integrated the already-merged #19610 model-fixture repair (`8621bcf53ee2e4bbefb7d193add42dd544ca06e8`), preserving the reviewed patch preview changes. Official preflight passes (1.8s); all 129 focused tests pass again (23.67s), with UI typecheck and token gates passing. Current revision CI and reviews are pending; prior-head review receipts below are not current-head approval. - Conflict qualification at `befc954a6116f7292506200b96623b673f9385b5`: normally merged current master `c66b4a55efdc5675b5a5146197078b94de6407da`, resolving the append-only `ui/src/index.css` conflict by retaining both the patch rules and all current agent identity rules. No feature behavior was changed during resolution. Official isolated preflight passes. The seven focused component/parser suites pass 129 tests; UI typecheck and token gates pass. Fresh head CI and reviews are pending. - Qualification setup limits: the first focused command used the repository root instead of the UI working directory, so setup failed before any test ran. It was corrected to the UI directory and actual attachment-test path. The first UI typecheck found the unbuilt local plugin SDK; building that checkout’s SDK and repeating typecheck passes. These setup attempts are not test/typecheck passes. - Focused Vitest run: 139 tests passed in eight files. This includes source preservation, HTML escaping, large patches, tabs, Markdown wrapping, line numbers, and transcript diff parsing. A 200-card check confirms no patch fetch occurs before a preview request. CRLF regressions cover no-newline markers and added lines that start with `+++`. - `pnpm --filter @paperclipai/ui typecheck`: passed after the plugin SDK build prerequisite. - `pnpm check:token-gates`: passed. - Storybook catalog check and 30 map/catalog tests: passed. - `STORYBOOK_TEST_BASE_URL=http://localhost:6006 pnpm exec playwright test patch-preview.spec.ts --config=tests/storybook-visual/playwright.config.ts --project=stories --workers=2 --retries=0`: 20 passed on Linux Chromium. The checks include accessibility, theme colors, overflow, opening a document, wrapping source, exact clipboard contents, and the selected source line. Screenshots were inspected and saved to the task. - The first browser runs found contrast failures in diff text and source gutters. Patch text tokens and patch gutter opacity now fix those failures. The final run passes without retries. - `pnpm verify`: exit 2. The new browser spec is a global CI input. Required CI must supply broad coverage. - The local full Storybook build was killed with exit 137. This is not a build pass. External screenshot baselines are not published, so no baseline comparison was run. - Reviews found selected-row background precedence, file headers in unified patches without git separators, eager patch loading across a collection, and CRLF no-newline markers. Each finding has a focused fix and regression checks. The Storybook hunk count now matches its source lines. - Prior feature commit `b3cb9d6d3c0423315540514b1ad68325a64c1cc2`: [CI typecheck, build, tests, runner checks, and E2E passed](https://github.com/paperclipai/paperclip/actions/runs/42247974154). [Security scan passed](https://github.com/paperclipai/paperclip/runs/127995160859). [Greptile is 5/5](https://github.com/paperclipai/paperclip/pull/19478#issuecomment-6688482753), and [Paperclip Review is 5/5](https://github.com/paperclipai/paperclip/runs/127998788846). All six inline review threads are resolved. ## Risks - Compact previews omit the end of long lines. The full document and downloads keep the source. - Patch colors apply to unified diff syntax. Other patch formats keep literal text and do not have full format parsing. - The shared classifier keeps the existing task chat behavior. Regression tests cover that consumer. - No database, API, or access rules change. ## Model Used - OpenAI Codex, GPT-6, with reasoning, tool use, and code execution. The runtime did 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 - [ ] 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Nicky LeachandClaude Opus 5.5
|
55244f1e9c |
test(server): remove two timing flakes in cloud adapter and native cleanup tests (#19668)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Contributors wait for PR CI before merge, and a flaky test forces a
re-run of the whole CI run
> - Four server tests failed on 2026-11-06 on commits that passed on a
later attempt
> - Two of them failed because of real slowness inside a fixed time
limit on contended 4-vCPU runners
> - This pull request removes that slowness from the two tests. It does
not raise their time limits
> - Of the other two tests, one is already fixed on master, and one is
not reproduced (see below)
> - The benefit is fewer whole-run re-runs
## Linked Issues or Issue Description
**What happened?**
These tests failed in PR CI and passed on a re-run of the same commit:
- `server/src/adapters/cloud-external-adapters.test.ts` › "Cloud image
adapter boundary › cold boots built-ins…" failed with `Test timed out in
15000ms` (3 times).
- `server/src/__tests__/retained-native-session-cleanup-work.test.ts` ›
"drains a backlog across discovery pages…" failed with `Test timed out
in 15000ms` (2 times).
**Expected behavior**
Both tests pass on a loaded CI runner.
**Steps to reproduce**
1. Run the test file in a 4-CPU container (`docker run --cpus=4
--memory=16g`) with 4 copies at the same time.
2. Before this change, the adapter case fails in 4 of 4 runs at 15,008
to 15,041 ms.
## What Changed
- `cloud-external-adapters.test.ts`: add a `beforeAll` warm-up with a
90-second limit.
- The warm-up imports `registry.js`, `plugin-loader.js` and
`adapter-plugin-store.js` with an empty temporary home and the managed
signal, so it reads and runs no persisted adapter package.
- Before this change, the first case paid the one-time transform of the
whole built-in adapter graph inside its own 15-second budget. In CI the
first case timed out at 15 seconds, and the next case took 1.1 seconds.
- Each case still gets fresh module state from the existing
`vi.resetModules()`. No assertions change.
- `retained-native-session-cleanup-work.test.ts`:
- `flush()` takes an optional poll `interval`, which defaults to 50 ms,
so other tests do not change.
- The backlog test uses 5 ms. The worker admits one cleanup pass per
1-second fake-time step, so 103 entries need 103 polls in a row.
- The next wake is scheduled before a pass releases its idle-work hold,
so a faster poll cannot see a false quiet state. No assertions change.
## Verification
| Test | Before | After |
| --- | --- | --- |
| Adapter cold boot, container, 4 copies | failed 4/4 (15,008–15,041 ms)
| passed 4/4 (537–756 ms per case; the warm-up takes about 23 s) |
| Adapter cold boot, host | failed 2/2 | passed 3/3 (0.6–1.6 s) |
| Backlog drain, container, 4 copies with load | 6.3–6.6 s | 2.0–2.1 s |
| Backlog drain, host, unloaded | 5.1 s | 1.1–1.3 s (5 runs, 29/29 pass)
|
- Final host run of both files with `--maxWorkers=2`: 33/33 pass.
- The backlog test did not reach the 15-second timeout in local runs.
The evidence for that fix is the measured poll-bound cost.
**Not changed in this PR:**
- `connection-management.integration.test.ts` (expected 3 to be 2):
- Cause: a GitHub webhook recovery worker from an earlier test fetched
through the suite-wide fetch spy 60 seconds later.
- Master already fixes this with commit
|
||
|
DottaandClaude Opus 5.5
|
f4b19e4231 |
fix(runner): finalize a durable proposed result when restore loses its Codex turn (#19658)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native Codex runs survive control-plane restarts: runnerd restores the provider conversation, and the control plane continues the task from its checkpoint > - A hard kill can land after the agent proposed its result (`run.result.proposed`) but before the provider turn ended > - The restored runner then proves that the turn ended with the accepted result. It records `session.reconciled` and `run.terminal`, but no provider turn terminal > - The Codex recovery read did not see that turn as ended, so recovery failed with "persisted active turn … is missing from thread/read". Retries ran out, and the Retry re-ran finished work > - This pull request lets recovery settle that turn with its durable result > - The benefit is that a task that already finished its work continues to finalization after a restart, instead of failing and redoing the work ## Linked Issues or Issue Description Refs #19525, #19591, #19539, #19501 **What happened?** In a restart storm, a native Codex task (B1) proposed a complete `paperclip.run_result.v1` with passing tests. The control plane accepted the completion tool call. About 9 s later, a K1 (SIGKILL and restart of the control plane) killed the controller, runnerd, and the Codex app-server. After startup, `resume_dead_runner` resumed the same run two times. Each resume spent about 25 s in `runner.session.startup` and failed with `native_session_recovery_failed: HarnessReconciliationError: persisted active turn <id> is missing from thread/read`. Then `native_session_retry_exhausted`, replay was blocked (`preserve_without_replay_v1`), and the run ended `native_session_interrupted`. The task needed a Retry, which re-ran the finished work. **Steps to reproduce** 1. Start a native Codex task through runnerd. 2. Let the agent call `paperclip_finish`, and let Paperclip accept the result. 3. Kill the controller, runnerd, and Codex before the provider turn ends. 4. Let restart recovery resume the run. Durable evidence from the kept instance: - The run events have `run.result.proposed` for the turn, then a `completion.tool_accepted` span, then the kill. - The saved checkpoint has `activeTurnId` set to that turn and `semanticResult: null`. - The Codex rollout on disk does contain the turn. The provider did persist it. - The runnerd journal for the first resume has `session.reconciled {previousProviderTurnId: <turn>, activeProviderTurnId: null}` and then `run.terminal {turnTerminalState: completed, runTerminalState: succeeded, reportedWorkDisposition: done}`. It has no `turn.*` event for that turn. The runnerd Codex provider state shows `completedTurnAuthoritative: true` and `completedProviderTurnId: <turn>`. **Expected behavior** Recovery finalizes the accepted result. It does not re-run the task. **Actual behavior** The runnerd Codex transport builds the recovery `thread/read` reply from runnerd's journal. It looked only for `turn.completed|failed|interrupted|cancelled` events that name the turn. Runnerd's "turn ended with an accepted result" branch (`provider_backend.rs`, `recovered_turn_ended_with_result`) emits only the reconciliation and the run terminal. So the reply omitted the turn, and `CodexHarnessSession.reconcile()` threw at `codex-harness-session.ts:796`. The second resume read the same journal and failed the same way. ## What Changed - `runnerd-codex-transport.ts`: a new `runnerdRecoveredTurnTerminal` helper selects the terminal for a restored turn. A provider `turn.*` terminal that names the turn wins, as before, with its cause. A `run.terminal` that follows a `session.reconciled` that names the turn as the ended previous turn (no active turn, no later `turn.started`) now also settles that turn, with the run terminal's `turnTerminalState`. The `thread/read` wait loop uses this helper. - `codex-harness-session.ts`: `reconcile()` now settles a checkpointed active turn that provider history no longer contains when the session holds the semantic result that this exact turn proposed (`resultTurnId` equals the turn). Recovery restores that result from the durable `run.result.proposed` event. Without such a result, a missing turn still fails closed. - `codex-harness-session.ts`: the same rule applies when the restored runner reports that turn `failed` with `provider_turn_lost_on_restore`. This happens when the runner cannot prove the accepted result itself. Later recoveries keep the settled outcome. Without a held result, a restore-lost turn keeps its existing failure and same-conversation continuation. - `fake-codex-app-server`: a test-only `--hold-after-terminal-result` flag keeps the turn open after an accepted completion report. - `doc/execution-semantics.md`: one additive paragraph on this recovery rule. ## Verification - Red before the fix, real `paperclip-runnerd` and fake Codex: `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-restart-continuation.test.ts -t "durable proposed result"` failed with `native_session_recovery_failed: HarnessReconciliationError: persisted active turn provider-turn-1 is missing from thread/read` at `native-session-runtime.ts:2267`. This is the same stack as the incident. - Green after the fix: the run finalizes on `provider-turn-1` with the proposed result and `runTerminalState: succeeded`. It has no `session.failed` or `turn.failed`, and Codex received only one `turn/start` (no re-run). With the transport fix alone, the test passes in about 5 s. With only the session fix, it also passes (about 30 s, through the 5 s read wait). - Driver red/green: `src/drivers/codex/codex-app-server-driver.recovery.test.ts -t "missing persisted turn"`. The settle case failed before the fix (`recovered: false`) and passes after it. The guard case (the held result belongs to another turn) still fails closed. The existing "fails recovery explicitly when the persisted active turn is missing or replaced" test still passes. - Driver red/green for the restore-lost case: `-t "restore-lost turn"`. The held=true case failed before the fix and passes after it. The held=false case still maps `turn.failed` with the lost cause. - Transport unit test: `runnerd-codex-transport.test.ts -t "reconciled result terminal"` covers the run-terminal rule and its negative cases. - Suites: `src/drivers/codex/`, `src/native-session-runtime.test.ts`, `src/backends/`, `src/live/runnerd-restart-continuation.test.ts`, and `src/live/runnerd-codex-transport.test.ts` pass. Some load-sensitive tests time out at host load 25–30 and pass when rerun alone. The two `proves local suspension after an event backlog … (1024 suffix deltas)` cases also time out on unmodified master. - `pnpm --filter @paperclipai/paperclip-runner typecheck:typescript` passes. - Live chaos replay with real Codex: K1 0–3 s after `run.result.proposed`, 3/3 PASS (R1). Each case resumed the same run, which ended `succeeded`. No Retry was needed and no work was re-run. Details are in the comment below. ## Risks - The transport rule accepts a run terminal only directly after runnerd's own reconciliation that names the ended turn with no active turn. Runnerd emits that pair only after it restored the conversation and proved the turn ended. A provider turn terminal still wins. Other run terminals do not settle a turn. - The session rules use only a result that Paperclip already holds for the exact turn. The control plane accepted that result before the kill. A later provider terminal for that turn cannot change the outcome. The finalizer can already interrupt a provider after an accepted result. - A missing turn without a durable result still fails closed. Its outcome is unknown, and the existing Retry stays available. - No profile, schema, pack contract, or database changes. The Rust change touches only the fake Codex test binary. ## 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
|
961455b9a0 |
test(restart-chaos): add the K13 restart storm mode (#19629)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Task recovery restarts agent work after a control-plane restart or a failed run > - When automatic recovery runs out or is blocked, the user clicks Retry > - Retry then started a fresh isolated attempt cloned from committed HEAD, so the agent lost its uncommitted files, its task branch and the worktree Git link > - A restart must never lose the agent's work > - This pull request makes Retry keep the task's own workspace when that is safe, and tells the agent where the old files are when a fresh attempt is still required > - The benefit is that a Retry continues the real work instead of starting over ## Linked Issues or Issue Description **What happened?** After `native_restart_recovery_blocked` or an exhausted recovery generation, a Retry ran in `isolated-workspace-attempts/<company>/<issue>/<run>/workspace`. That directory was a detached clone of the base ref. It did not contain the uncommitted edits from the issue's git worktree. It was not on the task branch. The agent was not told where the old files were. A restart-chaos wave graded this as an R2 failure (`pre_fault_files_lost`). A second defect: when a restart hit the bootstrap of a fresh attempt, preparing the same run again failed with `isolated_task_directory_binding_mismatch`, so the Retry could not finish. **What did you expect?** Retry continues in the task's own workspace with its edits and `.git` intact when no other writer can hold it. When a fresh attempt is really needed, the old directory stays as it was and the agent gets its path and its changed files. Preparing the same attempt again after a restart is idempotent. **Steps to reproduce** 1. Start a native task in a git worktree. Let the agent write an uncommitted file. 2. Kill the control plane so recovery ends in `native_restart_recovery_blocked` or an exhausted generation. 3. Click Retry. 4. The successor runs in a fresh clone without the uncommitted file or the task branch. Root cause: - `server/src/services/fresh-workspace-continuation.ts` `canSelectFreshWorkspace` returns a fresh intent for every task Retry that has a system recovery record. - `server/src/services/explicit-native-continuation.ts` (fresh selection in the source loop) then always persisted `fresh_isolated_workspace`. - `server/src/services/fresh-workspace-seed.ts` `materializeFreshWorkspaceSeed` clones the project source at the base ref with `--no-local`, checks out a detached commit and removes `origin`. Uncommitted work and the branch are not carried over. - RC-35: `server/src/services/heartbeat/workspace-preparation.ts` rejected any reusable existing workspace while `fresh` was true, including the attempt directory that the same run had already created and bound. ## What Changed - New continuation mode `reuse_issue_workspace`. At Retry admission, after all stop proofs pass, `retainedIssueWorkspaceForRetry` checks that the task's bound workspace is the single local directory the failed run used, owned by the task, not a shared workspace, not under remote storage authority, with no other lease, failed cleanup or running workspace operation, a settled recovery generation, no workspace-export failure, and a readable Git state rooted at that directory. Then the Retry continues there with a fresh session. Otherwise the existing fresh attempt is kept. - The reuse decision consumes the generation's one alternative attempt (`freshAttemptRunId`), sets the Retry strategy to `continue`, and records `issue.retry_workspace_reused`. - Environment selection pins the reused workspace binding and fails with `retry_workspace_binding_mismatch` if the binding changed before dispatch. - Fresh fallback: the run's continuation context gets `priorWorkspace` (path, branch, bounded list of uncommitted files). Git status is read with optional locks disabled. The prompt tells the agent to inspect it and carry over the work. The old directory is not changed. - RC-35: preparing the same fresh attempt again after a restart reuses the attempt directory that this run already bound. - `inspectGitCloseReadiness` is exported for the reuse check. - `doc/task-recovery.md` and `doc/execution-semantics.md` describe the new contract. ## Verification - `server/src/services/retry-workspace-continuity.test.ts` (embedded Postgres, real git worktree with uncommitted edits): - exhausted-generation Retry and blocked-recovery Retry continue in the same worktree, `.git` file and branch intact. Both failed before the fix (readiness was `fresh_isolated_workspace`). - The fresh attempt is kept when the workspace is unsafe: live lease, running workspace operation, corrupt `.git`, missing directory, remote authority, shared workspace, running export generation. - `workspace-preparation.test.ts`: - RC-35 red/green: re-preparing the same fresh attempt reuses its bound directory and keeps files written before the restart. Another run still gets `isolated_task_directory_binding_mismatch`. - The fresh fallback sets `priorWorkspace` with the old path and `unpublished.txt`. - `environment-selection.test.ts`: the new mode pins the binding and rejects a changed binding or cwd. - `packages/adapter-utils/src/server-utils.test.ts`: the prompt names the retained directory and its files. - Regression files run locally: `workspace-preparation`, `environment-selection`, `task-retry`, `quarantined-task-retry`, `explicit-native-continuation`, `workspace-recovery`. - Restart-chaos wave: WAVE_RESULT_PLACEHOLDER ## Risks - A Retry now writes into the old directory instead of a new one. The checks above prevent a second writer. A stale binding fails the run with `retry_workspace_binding_mismatch`; it does not write to another directory. - If the cause of the failure was inside the workspace itself, reuse could repeat it. Workspace-export failures, restore failures and required workspace recovery keep the fresh attempt. - `priorWorkspace` is an optional new field in the continuation envelope. Older adapters ignore it. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, 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 Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
6be71028ad |
Prepare online ephemeral enrollment through signed Cloud control (#19660)
## Thinking Path
> - Paperclip manages agent work through a control-plane web server.
> - A replacement web server needs shared durable data and the original
signing identity.
> - Existing enrollment exports that identity only after a global drain
and offline migration.
> - Online enrollment needs a separate preparation operation while agent
work continues.
> - This change adds a purpose-bound Cloud control operation with
existing data and key owners.
> - The result is an audited preparation receipt, with no permission to
stop agents or detach storage.
## Linked Issues or Issue Description
Refs #19038. Refs #19024.
**What are you trying to improve?**
Allow Cloud to prepare an already migrated tenant for volume removal
without the offline migrator or a global-idle gate.
**What problem does this solve?**
Cloud must preserve the original decision key before starting a
replacement with static ephemeral mode. A tenant session must never gain
key-export authority.
**What would the improvement look like?**
A fresh exact-action Cloud assertion obtains a private, deployment-bound
preparation receipt. Unfinished durable-data migration and invalid
replacement configuration block export.
## What Changed
- Add the signed `ephemeral-enrollment:prepare` control action and a
strict UUID-only POST route.
- Check existing durable-data eligibility and ephemeral configuration
before reading the original key. Never create a missing key.
- Audit only public preparation metadata. Keep HTTP diagnostics
secret-sensitive and the response non-cacheable.
- Bind the receipt to the attempt, stack, build commit, Railway
deployment, time, static mode and key digest.
- Always return `enrollmentReady: false`. Cloud still needs to arm a
replacement, verify old writers have stopped, repeat eligibility and
qualify native handoff before detaching the retained volume.
## Verification
- PASS: 80 focused tests across the preparation owner, signed route,
existing Cloud control, HTTP privacy and OpenAPI route coverage.
- PASS: 2 real PostgreSQL cases. Concurrent copies of one signed request
export once; the durable receipt still rejects reuse after clearing the
process fence. Normal run admission before skill pins is allowed;
missing durable skill installation remains blocked.
- Genuine prepatch RED: the same two PostgreSQL cases exported twice and
rejected ordinary admission. The fixed owner was restored automatically
after the comparison.
- PASS: server TypeScript check with workspace dependencies built.
- Fixed the first CI route-coverage failure by declaring this private
control endpoint in the existing explicit OpenAPI exclusion registry.
- Initial local HTTP execution lacked sandbox socket permission; the
reviewed run passed. Initial PostgreSQL fixtures enabled durable files
too late and reused an issue prefix; those fixture defects were
corrected before the red/green comparison.
- All required CI gates pass on
|
||
|
DottaandClaude Opus 5.5
|
e89be6ee83 |
fix(native): re-bootstrap a runner that a graceful drain suspended during bootstrap (#19665)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs start a local `paperclip-runnerd` process. The control plane keeps a durable state root for each session. > - After a restart, recovery gives a run with no provider evidence the `bootstrap_incomplete` disposition. The executor quarantines the old root and starts the bootstrap again, but only when the root proves that no provider work started. > - #19524 made that proof accept a root killed after `run.prepare` and before `session.open` (a hard kill). > - A graceful restart (SIGTERM) leaves different shapes. The drain sends `runner.drain` and `runner.suspend`, and the kill can land just after `session.open` failed or just after it started a fresh session. The proof refused both shapes. The run failed with `runner_state_identity_mismatch: bootstrap_not_proven_incomplete`, although no turn was ever sent. > - This pull request extends the proof to both drain shapes and keeps provider work fail-closed. > - The benefit is that a graceful restart during native bootstrap continues automatically. ## Linked Issues or Issue Description Refs #19524 **What happened?** Restart chaos waves (ACPX Claude, K2 graceful restart about 10 s after the run started) failed with `adapter_failed` and `runner_state_identity_mismatch: bootstrap_not_proven_incomplete`. Startup recovery chose `bootstrap_incomplete`, and the executor proof refused the state root. Two real roots were kept: 1. `session.open` failed (the ACPX sidecar timed out), then `runner.drain` completed and `runner.suspend` completed. There was 1 connection, no committed or acknowledged event, an empty runner outbox, and `nextSourceSeq: 1`. 2. `session.open` completed with a freshly started session (`status: "started"`). The committed events were only `session.started`, `session.capabilities.updated` and `session.goal.snapshot`, and the runner outbox held one `harness.diagnostic` (the sidecar process). No turn command existed, and the sidecar process had exited. The database had not yet recorded the session events, so the classifier saw no provider evidence. **Steps to reproduce** 1. Start an ACPX Claude task on a local runner. 2. Send SIGTERM to the control plane about 10 s after the run starts, before the first turn. 3. Let the control plane restart. **Expected behavior** Recovery starts the bootstrap again, and the task continues without a user action. **Actual behavior** The run fails and the task stops. ## What Changed - `native-bootstrap-proof.ts`, pre-session proof: a `session.open` that is pending or `failed` is now accepted, as are the runner lifecycle commands `runner.drain`, `runner.suspend` and `runner.shutdown` in any state. The controller journal and the runner journal must both agree. Any committed or acknowledged event, any turn command, a completed `session.open`, and any queued runner event still fail closed. - New `runnerdStateIsOpenedSessionBeforeTurn`. It accepts a completed `session.open` only when all of these are true: - the session was freshly started (`status: "started"`, not resumed); - the commands are only `run.prepare`, `session.open` and runner maintenance commands; - every committed event and every event in the runner outbox is a session setup or runner lifecycle event; - `ackedSourceSeq` equals the count of committed events, with no gap; - the runner journal is present; - every recorded provider process ID has exited on this host. - `native-session-executor.ts`: the executor applies the opened-session proof only for a local execution target. Remote runners stay fail-closed for that shape, because their process IDs belong to another host. The remote checkpoint classifier still reads only commands 1 and 2. ## Verification - Red before the fix: "allows a fresh bootstrap after a drain suspended a runner whose session.open failed" and "allows a fresh bootstrap after a drain that landed between session.open and the first turn" both failed with `expected false to be true`. Green after the fix: the `native incomplete-bootstrap evidence` block passes 32/32. - Fail-closed counter-tests: - completed `session.open`; - committed event, acknowledged event, or queued runner event; - turn command, `turn.stop`, or a turn processed by the runner; - `turn.started` or `provider.event`; - resumed session; - compacted events; - no recorded provider process; - missing runner journal; - live provider process; - remote runner. - I ran the new proof read-only against the three real kept roots: the K1 root from #19524, the K2 `session.open` failed root and the K2 opened-session root. All three return `true` for a local runner. The opened-session root returns `false` when the processes are on another host. - `native-session-executor.test.ts` and `native-restart-recovery.test.ts` filtered to bootstrap, restart, remote and drain: 333 passed. `native-runner-restart-recovery.integration.test.ts` and `native-bootstrap-retry.integration.test.ts`: 106 passed. `tsc --noEmit -p server/tsconfig.json` passes. - Live restart chaos replay: see the PR comment. ## Risks - After a `session.open` failure or timeout, the provider sidecar might have created an empty session. No turn command was sent, so no provider work can repeat. - An opened session before any turn is discarded, and a new one is started. The proof requires a fresh `started` session, so no earlier conversation is lost. Resumed sessions stay fail-closed. - The opened-session proof is local only. It requires every recorded provider process to be gone, so a live provider process blocks it. - Out of scope: the Retry offer when the proof fails legitimately (RC-05/06). > 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`), used through Claude Code with tool use (shell, file edits, test execution). Default context window. Extended reasoning was available. ## 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
|
9197abf673 |
fix(native): preserve controller state when remote archiving fails (#19663)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native continuity replacement must preserve the state needed to recover saved work. > - The current replacement moves local controller files before it knows whether the remote archive succeeded. > - A remote failure then leaves the local controller without its recovery files. > - This pull request confirms the remote archive before it moves those files. > - A user turn that never started can use the existing bounded Retry path after verified preservation and cleanup. > - The task retains its saved work and can continue once under current user authority. ## Linked Issues or Issue Description Refs #19638, #19547, #19399 and #19659. The prerequisite #19638 merged as `0819f4ea78dd37fba87341b314be9dac078c1410`. This PR now targets master. A normal dependency integration at `febed950f4` keeps only the six-file archive repair in the PR diff. Fresh master-based CI and review are required before merge. **What happened?** A native continuity replacement moved its local controller journal and provider checkpoint. The following remote archive failed. A later recovery attempt could no longer read the local recovery files. The first-attempt failure also had no normal Retry receipt for the saved user turn. **Expected behavior** A failed or timed-out remote archive retains the local recovery files and starts no replacement provider. A user turn that has not started can request one normal Retry. The existing user, provider-turn, Stop, preservation, cleanup, budget and approval gates remain in force. **Steps to reproduce** 1. Keep a local controller journal and provider checkpoint for a retained remote conversation. 2. Request continuity replacement and fail or time out the remote archive command. 3. Check the local controller files before any later recovery. 4. Settle the native failure under a current user wake and recover the saved workspace. ## What Changed - Confirm remote continuity archiving before moving the local controller state. - Add the exact remote archive failure to the shared bounded user Retry admission from #19638. - Verify failed, timed-out and successful archive ordering with real local files. - Verify ordinary failure settlement and saved-file recovery produce one fresh successor. Keep Stop, system wake and unsettled-provider holds. - Document the file preservation and Retry contract. ## Verification - RED: failed and timed-out remote archives removed the local controller files. Reading those files failed with ENOENT. The saved-user Retry request returned false instead of true. - Initial archive unit verification: eight cases passed, including the three archive outcomes and five existing Cloud archive safety cases. - Initial stacked revision: `9104ee399b`, based on #19638 at `d22d8f9d5ee3dc36550a94d55c1d8376f3b9aab2`. - Final master-based revision: `febed950f4`. All 13 affected cases passed across three Vitest files after dependency integration. They include the seven-mode real PostgreSQL completion case, saved-file successor and negative-hold cases, and real-file archive outcomes. All seven server typecheck layers passed again. Module boundary, whitespace and added-code-comment checks passed. All required CI passed on this exact head. The final snapshot has 59 successful checks, three intended skips, one neutral result, and no pending or failed checks. Current-head Greptile and the independent review are both 5/5, with no open findings. - Final local revision: `9104ee399b`, macOS ARM64, Node 26.4.0, pnpm 9.15.4 and embedded PostgreSQL. The three real-file archive outcome cases passed. The real PostgreSQL completion test passed all seven internal scenarios, including archive failure, prior provider activity, system wake, Stop before and after terminal settlement, and stale runtime promotion. Own Runner/shared/SDK builds and all seven server typecheck layers passed. Nine saved-work continuation and negative-hold cases passed. They verify exported file contents, one user-attributed fresh successor, idempotent hooks, no second automatic chain, and first-use manual Retry for held cases. - `node scripts/check-module-boundaries.mjs`, whitespace checks and the added-code-comment audit passed. - Focused commands from `server/`: Vitest with `-t 'retains the local controller checkpoint until remote continuity archiving succeeds'`; `-t 'replays saved user intent through normal Retry'`; and `-t 'continues a user turn once|does not continue automatically'`. PostgreSQL cases used a fresh local cluster and a 120-second setup hook budget. Application timeouts were unchanged. - The initial stacked head passed all test, build, typecheck and E2E jobs. Greptile and the independent review both scored it 5/5 with no findings. These results do not replace the fresh master-based qualification now running. ## Risks - A remote command can fail after it changed remote state. This change retains local evidence in that case. It does not claim a distributed atomic transaction. - The exact archive error can request one normal Retry only under the existing saved-user admission. The normal path must prove stop, saved files and cleanup before a successor starts. - The protected launch-argument rejection remains enforced. Its exact hosted argument difference was not available through ordinary access. #19659 separately addresses a saved-thread feature-policy argument difference. This PR preserves local archive evidence and does not claim direct hosted verification of that primary cause. ## Model Used OpenAI GPT-6 family through Codex, with reasoning, tool use and code execution. The model is inherited; no override was used. The exact serving model ID and context-window size are not exposed by this runtime. ## 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> |