mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
fix(server): preserve hot restart intent across path upgrade (#10593)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server can preserve eligible agent runs during a controlled hot restart. > - A path change moved restart state from the Paperclip home root to the instance root. > - A staged update can therefore make the old server and the new server read different intent files. > - The old server then drains live runs, while the new server can start without a shutdown snapshot. > - This pull request adds a correlated compatibility handoff and records the live preflight set. > - It also verifies the target process instance on Linux, macOS, and Windows. > - The benefit is complete and safe run classification across the path upgrade. ## Linked Issues or Issue Description No public GitHub issue covers this defect. **What happened?** A staged hot restart can run an older server that reads `hot-restart-intent.json` from the Paperclip home root and a new server that writes the file under the instance root. The old server misses the request and uses graceful drain. The new server later finds its marker without a shutdown snapshot. Before this change, that state could produce an empty loss list even when live runs existed before restart. **Expected behavior** The old server must receive the PID-targeted restart request at its legacy path. The new server must correlate the legacy shutdown snapshot with its instance-scoped request. Every run that was live during preflight must appear as adopted, finalized while down, or lost. A reused PID must not let a stale marker claim a different process instance. **Steps to reproduce** 1. Start a server version from before the instance-root marker change. 2. Keep one or more local-agent heartbeat runs active. 3. Stage a current build and request a hot restart from that build. 4. Observe that the old server reads only the home-root path while the staged build writes only the instance-root path. 5. Observe graceful drain and a new-server intent that has no shutdown snapshot. **Paperclip version or commit** The path transition entered `master` in #10045. The hot-restart adoption flow came from #9647. This fix targets current `master` and compatibility with the immediately preceding home-root behavior. **Deployment mode** Self-hosted server built from source with controlled service hot restarts. Related work: #9628 is the original broader hot-restart feature PR. #10556 addresses embedded PostgreSQL lifecycle behavior and does not address marker-path compatibility. ## What Changed - Write an authoritative instance-scoped intent and a correlated legacy home-root handoff marker. - Merge a legacy shutdown snapshot only when immutable request identity fields match. - Prevent a non-default instance from consuming an uncorrelated legacy-only marker. - Record preflight running heartbeat IDs and reconcile snapshot omissions from current database state. - Serialize marker claims, snapshot writes, stale recovery, and matching cleanup with recoverable per-path filesystem leases. - Read process start identity on Linux, macOS, and Windows to distinguish a reused PID from the original server. - Require identity for new restart requests and fail closed when a supported platform cannot provide it. - Classify older markers by comparing the replacement server boot time or operating-system process start time with the request time. - Close the preflight database client explicitly and use a root-safe SQL query. - Add focused unit, platform-branch, database-backed, and CI regression coverage. - Document the compatibility handoff, process identity probes, and instance-scoped report path. ## Verification - `pnpm exec vitest run server/src/services/hot-restart.test.ts server/src/__tests__/heartbeat-process-recovery.test.ts -t "hot-restart|old-server legacy|preflight live|preflight run|spawn identity before hot restart"` — 24 tests passed and 90 tests were skipped across 2 files. - `pnpm exec vitest run server/src/services/hot-restart.test.ts` — 17 tests passed. - `pnpm exec vitest run server/src/__tests__/issue-watchdogs-routes.test.ts -t "restarts a stalled claimed run"` — 1 test passed and 10 tests were skipped. - `pnpm exec vitest run server/src/__tests__/agent-action-audit-routes.test.ts -t "allows an agent with issue:delegate"` — 1 test passed and 7 tests were skipped. - `pnpm --filter @paperclipai/server typecheck` — passed. - `git diff --check` — passed. - GitHub Actions — 26 of 26 checks passed at `55a79cb029be8b1dc89926d9d89ccd2181266d5c`. - Greptile — 5/5 at the same head with no unresolved current-head review threads. ## Risks - The legacy handoff path is shared across instances. Exclusive claims and per-path leases prevent overwrite and match-before-delete races. - Process identity uses platform commands as a fallback when the health endpoint has no identity. Linux reads `/proc`, macOS and BSD use `ps`, and Windows uses PowerShell. - A supported-platform identity probe failure aborts the restart. This fails closed instead of replacing an unknown live process. - Older intent files do not contain process identity. The server compares the replacement boot or process start time with the request time when those values are available. - A preflight database read can fail before the marker is written. The command fails closed instead of claiming a restart whose live-run set is unknown. > 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 - OpenAI Codex with GPT-5. The exact deployment model ID and context-window size were not exposed by this runtime. Reasoning, repository editing, shell execution, test execution, GitHub CLI, and Paperclip API capabilities were 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 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>
This commit is contained in:
1 parent
ee9d907d01
commit
b163c6c473
9 files changed
+1172
-27
No files matched your search
+21
-1
@@ -115,7 +115,27 @@ pnpm --filter @paperclipai/server exec tsx ../scripts/request-hot-restart.ts --s
|
||||
systemctl restart paperclip.service
|
||||
```
|
||||
|
||||
Use `--drain-required` only when the deploy intentionally requires the old terminate-and-retry behavior. Without that flag, the old server verifies that the marker targets its own PID, snapshots currently running heartbeat run IDs and child PIDs, and skips the shutdown drain so eligible detached local-agent processes can keep running. On startup the new server writes `$PAPERCLIP_HOME/hot-restart-report.json` with `previousServerPid`, `newServerPid`, `previousServerVersion`, `newServerVersion`, `adoptedRunIds`, `finalizedWhileDownRunIds`, `lostRunIds`, and per-run classifications before the normal orphan reaper runs.
|
||||
The staged command records the target server's boot identity and operating
|
||||
system process start time with the PID. It reads process metadata through
|
||||
`/proc` on Linux, `ps` on macOS and BSD, and PowerShell on Windows. These
|
||||
identities let a later request reclaim an abandoned marker after the operating
|
||||
system recycles the numeric PID. Older markers stay compatible and use process
|
||||
start metadata when available. When OS metadata is unavailable, the current
|
||||
server's health-reported boot time can still prove that a legacy marker predates
|
||||
the process now using its PID. Paperclip refuses to create a new request without
|
||||
at least one identity source. Supported-platform process probes fail explicitly
|
||||
instead of silently treating a live PID as either the original owner or a
|
||||
recycled process when identity cannot be established.
|
||||
|
||||
Use `--drain-required` only when the deploy intentionally requires the old terminate-and-retry behavior. Without that flag, the old server verifies that the marker targets its own PID, snapshots currently running heartbeat run IDs and child PIDs, and skips the shutdown drain so eligible detached local-agent processes can keep running. On startup the new server writes `$PAPERCLIP_HOME/instances/${PAPERCLIP_INSTANCE_ID:-default}/hot-restart-report.json` with `previousServerPid`, `newServerPid`, `previousServerVersion`, `newServerVersion`, `adoptedRunIds`, `finalizedWhileDownRunIds`, `lostRunIds`, and per-run classifications before the normal orphan reaper runs.
|
||||
|
||||
The request command records the preflight set of running heartbeat IDs and writes
|
||||
an instance-scoped marker plus a PID-targeted legacy home-root handoff marker.
|
||||
This lets a previous server version capture its snapshot at the old path while
|
||||
the new server correlates that snapshot back to the authoritative instance
|
||||
request. If any preflight run ID is absent from the shutdown snapshot, the
|
||||
startup report includes it in `lostRunIds`; a missing snapshot therefore cannot
|
||||
look like a zero-loss restart.
|
||||
|
||||
A healthy guarded deploy must compare the report against `/api/health` (`version` or `serverVersion`) and treat any `lostRunIds` entry as a continuity failure that needs recovery before marking deployment complete.
|
||||
|
||||
|
||||
Reference in new issue
Block a user