mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
DottaandClaude Fable 5.2
a56b648602
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A deployer can replace the server while agent runs continue. Before the swap, it reads the upgrade-readiness report of the old server > - The version 1 report is ready only when every active run is adoptable and no wake admission is in flight > - On a busy instance, new runs arrive each minute. Some runs are always in preparation, so the report is almost never ready and the instance does not upgrade > - A run in preparation has done no agent work. Restart recovery starts it again. A swap loses nothing > - This pull request adds a `handoff` object to the report. It separates recoverable, restartable and blocking runs > - The benefit is that a deployer can wait only for work that a swap would lose ## Linked Issues or Issue Description Refs #20852 (reference only; this change does not depend on it and does not use its version 2 shape). **What happened?** A busy instance had 16 running runs and 1 queued run. Fourteen had a live process. Two or three native runs had not launched a process yet. One was a legacy run. `upgradeReadiness.ready` needs `blockingRuns === 0 && pendingWakes === 0` in one probe. That state almost never occurs, so the deployer never swaps the server. **Expected behavior** The report tells the deployer which runs a swap would lose. Runs that only wait for a launch, and wake admissions in flight, do not block a graceful replacement. **Steps to reproduce** 1. Keep a steady flow of new runs on an instance. 2. Read `GET /api/instance/task-drain?upgradeReadiness=1` repeatedly. 3. See `ready: false` each time, because one or more runs are in preparation or a wake admission is in flight. ## What Changed - `summarizeUpgradeReadiness` returns an additive `handoff` object: `version`, `ready`, `recoverableRuns`, `restartableRuns`, `blockingRuns`, `pendingWakes`, `unsavedWakes`. - `handoff.ready` is true when the instance is not draining, the census is complete, and `handoff.blockingRuns` and `handoff.unsavedWakes` are 0. Saved pending wakes do not change it. - The wake queue tracks each public wake admission until its wake request and queued run commit. `unsavedWakes` is the count of admissions before that commit. Without a valid count, every pending wake is unsaved. - The top-level version 1 fields keep their values and meaning. `ready` and `blockingRuns` are unchanged, so a current consumer sees no looser verdict. - The census query marks a run `restartable` only with all of these: `paperclip_runner` adapter; no process identity; no provider session before or after; no checkpoint header; no dispatched runner task; no execution-start event. It must also be a native run in the `observed` phase with preparation fencing and an unsettled finalization record, or a legacy run in the `preparing` stage under a recorded controller. - All other runs without adoption evidence stay blocking. This includes local adapters, legacy runs with a child process, follow-up runs that carry a provider session, and runs the census could not observe. - The OpenAPI description and `doc/testing-native-remote-continuity.md` describe the new fields and their limits. Why these classes are correct on master: - Restartable native runs: restart recovery claims a run with no checkpoint and no provider evidence as `bootstrap_incomplete` and executes the same run again (`native-restart-recovery.ts` `classifyNativeRunnerRecoveryEvidence`; `heartbeat/recovery.ts` executes each claim). A graceful shutdown suspends the run and notifies recovery. Existing tests: "recovers an unclaimed remote startup once without requiring a provider checkpoint" and "recovers an unclaimed remote startup that a graceful shutdown suspended during preparation". - Restartable legacy runs: process-loss recovery retries a `preparing` run with `providerWorkStarted: false`. Existing test: "continues a chain whose preparations were killed twice and then interrupted again, without a Retry". - Pending wakes: the count is wake admissions in flight in the process, not queue rows. A wake commits its wake request and its queued run in one transaction. After the commit, the next server dispatches the queued run at startup, so a saved wake does not block. Before the commit, nothing is saved. Shutdown waits at most 30 seconds and then exits, so an unsaved wake can be lost and blocks. ## Verification - `cd server && npx vitest run src/services/heartbeat/upgrade-readiness.test.ts src/services/heartbeat/upgrade-readiness-post-result.test.ts`: 28 passed. - Red/green: before the implementation, the 7 new tests failed (6 summary tests, 1 PostgreSQL census test). After it, all pass. - Red/green for unsaved wakes: a test holds the agent row lock while a wake admission waits. Without the fix, `handoff.ready` was true with no saved wake. With the fix, it is false with `unsavedWakes: 1`, and the wake saves after the lock releases. - New cases: only recoverable runs; recoverable plus unlaunched runs plus pending wakes (`handoff.ready` true, top-level `ready` false and `blockingRuns` unchanged); a started run without recovery evidence; an unobserved execution; draining; truncated census. - The PostgreSQL test changes one fact at a time (process identity, provider session, phase, fencing, checkpoint, adapter, each execution-start event) and expects the run to become blocking. - `cd server && npx vitest run src/__tests__/openapi-routes.test.ts src/__tests__/instance-settings-routes.test.ts`: 157 passed (187 with the two readiness files). - `cd server && npx tsc --noEmit`: passed. - `pnpm verify` exited 2 (a documentation path is a global CI input). Required CI supplies the broad coverage. - Not run: a live server replacement. The classification relies on the existing recovery tests named above. ## Risks - Low risk for current consumers. The change is additive and the version 1 fields are unchanged. - `handoff.ready` is an observation at one instant. A wake admission that starts after the probe is unsaved until it commits. - Restart recovery applies its own proofs and a bounded attempt budget before it starts a restartable run again. The report is an observation, not a guarantee for each run. - The probe adds one indexed `not exists` on run events per running row, inside the existing two-second statement timeout. > 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 Fable 5.2 (`claude-fable-5-2`), 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 Fable 5.2 <noreply@anthropic.com>