mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 13:20:54 +01:00
DottaandPaperclip
76a796e964
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native Runner keeps provider execution separate from the main server. > - A server replacement must reconnect to the original execution. > - ACPX recovery also needs the original provider session to remain alive. > - We need to distinguish a controller outage from a provider crash or an explicit close. > - This pull request adds five process-level regression tests for those boundaries. > - The tests prevent duplicate launches and preserve the scope of the evidence. ## Linked Issues or Issue Description Refs #17097, #17439 and #17455. These changes add coverage for the existing recovery contract. They do not add another recovery implementation. **What existing behavior does this improve?** Native ACPX execution across authenticated PRP controller replacement. **Current behavior** Runner has recovery coverage, but this ACPX sequence needed a focused test: lose the controller, restore its checkpoint, adopt the same process, and close the old controller. A provider that has already closed can return `ACPX request snapshot requires the surviving provider`. That message alone does not identify why it closed. **Proposed behavior** Run five credential-free regressions with the production Rust Runner and a scripted ACPX peer. Preserve active work and offline completion. Check native transport adoption and old-controller cleanup. Keep provider-crash and explicit-close controls separate from controller recovery. **Reason and benefit** Catch a future change that kills or replaces a live execution during controller replacement. Keep a matching error control from being mistaken for proof of a hosted root cause. ## What Changed - Add five process-level ACPX controller recovery tests. - Restore controller state from an external checkpoint after deleting local controller storage. - Check that native transport adoption does not launch a replacement Runner. - Build the debug Runner before the tests. Pass only the test bootstrap credential to its process. - Verify scripted-peer shutdown through its lifetime sockets instead of signaling a saved provider PID. - Document the command, dependencies, optional prebuilt binary/profile overrides and verification limits in the Runner README. - Give the setup hook 30 seconds of margin above the bounded Cargo build. ## Verification - Focused test: `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/control-plane/acpx-controller-recovery.test.ts --maxWorkers=1` — 5 passed. - Adjacent regression selection: `src/control-plane/`, `src/live/runnerd-controller-outage.test.ts`, and `src/live/acpx-request-recovery.test.ts` — 5 files and 195 tests passed. - Protocol manifest and package ownership checks passed. The standalone forbidden-import checker reports existing imports in unchanged files; this PR changes none of those files or the checker. - `pnpm -r typecheck` and `pnpm build` passed. The full `pnpm test:run` attempt is retained. Its first database cases failed because the local macOS PostgreSQL package lacked required dylib aliases. Direct native probes identified the missing libraries. Local dependency aliases restored initialization, and one unchanged failing warm-session database case then passed. The original full-suite attempt was stopped after that diagnosis. Its failures remain retained; full-suite completion is not claimed. - Current-head CI repair: browser shard 5 failed after the task reload because the result was still behind the conversation loading veil. The saved snapshots show this state. The UI has a 15-second history reveal limit, but the result assertion had a 5-second wait. The test now returns to the task tab and waits for its existing history-content readiness before it checks the result. `PAPERCLIP_E2E_PORT=38741 pnpm exec playwright test --config tests/e2e/playwright.config.ts tests/e2e/connection-reviews.spec.ts --grep 'connection review: (approve|decline|always|failure),'` passed all four journeys in 2.9 minutes against a fresh local server and database. Master independently received the same readiness fix in #17923. This branch merged that exact base version, so the browser file is not part of the final PR diff. The five ACPX tests passed again after that merge. No production UI code changed in this PR. The original failed CI report is retained. - Retained CI failure: the first Linux run rejected the scripted peer's automatically assigned ports because they can be below the protocol minimum of 49152. The fixture now binds explicit random ports in the permitted range, with bounded collision retries. No production validation was relaxed. The fixed peer also passed a network-isolated Linux arm64 container check; that check used the actual generated peer and verified permitted ports and clean EOF shutdown. - Review follow-up: both native-transport reads require the original active turn ID with status `inProgress`. Adoption always runs for a selected prebuilt binary. The crash control accepts only explicit provider-loss diagnostics, not any snapshot-error wrapper. Focused tests cover these changes. - Final review follow-up: the setup hook has a 150-second limit around the 120-second Cargo build. The README documents both optional overrides. All five focused tests passed in default mode (17.72 seconds) and with the prebuilt binary plus profile overrides (10.90 seconds). The fixture version is explicitly passed through `--runner-version`; it is not a check against Cargo package metadata. The crash control verifies the original PID receipt and released lifetime fence ports before it accepts only known provider-loss diagnostics. - CI completed successfully for `d31647fb95f38b7d9d56bc8c13a9a8c0b391d247`, including all eight browser shards. Final head `949a70596f07042cf7b39a5a3ccb9a9445b6e855` also passed [all CI gates](https://github.com/paperclipai/paperclip/actions/runs/40904592159), including all eight browser shards, and received fresh 5/5 Greptile and Paperclip reviews. All six review threads are resolved. - Separate manual provider evidence used real Claude Opus 5.5, ACPX 0.13.1, Claude ACP 0.73.0 and Claude SDK 0.3.286 on macOS arm64. Source `929e840595c8933ca485ac7a2e99413e1e11ca64` survived 10-second and 300-second controller outages. Original source `4f8104dec6a0bb37009080daa8984a80f58b7a04` survived a 10-second outage. The five-minute case retained the original processes and session, made useful progress, reattached at 303.793 seconds from disconnect, and completed once. These runs preceded this branch rebase and do not qualify every change in the rebased server. - Retained manual failures: the first historical attempt failed installation admission because scratch dependency symlinks escaped their root. Correcting that setup allowed the next attempt to start. The five-minute harness initially checked the final message before asynchronous projection finished. Saved final-item and terminal events passed an independent evidence check. The original failed result remains retained. ## Risks - This PR adds tests and documentation only. It changes no production protocol, recovery logic, database schema or browser behavior. - The automated tests use a scripted ACPX peer. They do not establish real Claude, PostgreSQL, Daytona or Cloud rollout acceptance. - The tests require Cargo and a POSIX host. Their waits are bounded. They stop only their own Runner and scripted peer. - The earlier hosted provider loss remains unexplained. The separate real-provider experiments did not reproduce it. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, tool use and code execution. The host does not expose a more specific serving model ID or context-window size. Anthropic Claude Opus 5.5 (`claude-opus-5-5`) performed the separate synthetic verification workloads. ## 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>