mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 19:35:13 +01:00
4d8297128ddeaecba185d04b329fcf855e06e8ef
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
ca6416da81 |
fix(server): preserve hot-restart shutdown snapshots (#10815)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A guarded hot restart must preserve or finalize every active agent run. > - The server uses embedded PostgreSQL when `DATABASE_URL` is not set. > - The database dependency installs signal handlers before Paperclip installs its coordinated shutdown handler. > - Those handlers can stop PostgreSQL before Paperclip writes the shutdown snapshot. > - ACP runs also use server-owned stdio and cannot be adopted after that server exits. > - This pull request keeps PostgreSQL available through snapshot and drain, then uses the existing ordered stop. > - The benefit is a complete restart report with no false adoption and no missing snapshot loss. ## Linked Issues or Issue Description **What happened?** A guarded hot restart with a valid marker can report a live preflight run as lost with reason `missing_shutdown_snapshot`. The `embedded-postgres` package imports `async-exit-hook`. That package registers `SIGINT` and `SIGTERM` listeners before Paperclip registers its own shutdown listener. The dependency can close PostgreSQL while Paperclip queries active heartbeat runs and writes the snapshot. **Expected behavior** Paperclip must keep its database available until it persists the shutdown snapshot and completes any required run drain. A detached CLI run must remain eligible for adoption. An ACP run must finish as interrupted and queue a retry because its server-owned stdio cannot survive the server. **Steps to reproduce** 1. Run Paperclip from source with embedded PostgreSQL. 2. Start a local ACP-backed agent run. 3. Write a valid hot-restart marker for the current server process. 4. send `SIGTERM` through the service manager. 5. Inspect the restart report and server log. 6. Observe that PostgreSQL can close before the shutdown snapshot query completes. **Paperclip version or commit** The defect reproduces on `2ab797dcbed0031c45c7335a0f497fea2a20bd9a`. **Deployment mode** Self-hosted server built from source, with embedded PostgreSQL and a systemd service. Related work: #9628 introduced hot-restart continuity. #10556 explores a broader database ownership transfer. #10775 addresses ACP continuity after replacement startup. This pull request uses a smaller path: it keeps the current database owner alive through snapshot and drain, then performs the existing explicit database stop. ## What Changed - Remove only the `SIGINT` and `SIGTERM` listeners added by the embedded PostgreSQL import. - Preserve Paperclip's existing ordered database stop after heartbeat snapshot and drain. - Detect active ACP and server-stdio local runs before shutdown. - Persist their complete snapshot before changing the marker to an ACP drain request. - Drain only ACP runs to an interrupted terminal state and queue their retry. - Keep detached CLI runs eligible for adoption in the same mixed restart. - Quiesce already-running scheduler queue claims before capturing the snapshot and selective drain set. - Report a selected ACP run as lost if process termination succeeds but its terminal database write does not persist. - Add the drain reason to the restart report. - Document the normal path and the one-time recovery path across an older affected build. ## Verification - `PAPERCLIP_TEST_DATABASE_MODE=native pnpm --filter @paperclipai/server exec vitest run src/__tests__/heartbeat-process-recovery.test.ts` — 100 passed. - `pnpm exec vitest run server/src/shutdown.test.ts server/src/services/hot-restart.test.ts` — 24 passed. - The shutdown suite imports the real `embedded-postgres` package and verifies that its eager signal listeners are absent after the guarded import. - The embedded PostgreSQL recovery suite verifies snapshot, pre-snapshot scheduler quiescence, selective ACP drain, detached CLI adoption, queued retry, original-run finalization, `lostRunIds=[]` in a mixed restart, and fail-closed reporting when terminal persistence fails. - `pnpm --filter @paperclipai/server typecheck` — passed. - `git diff --check` — passed. - A full workspace typecheck reached the UI and stopped because the shared local install does not contain its declared `@base-ui/react` dependency. All server and preceding package checks passed. CI uses a clean install and remains the authoritative full gate. ## Risks - Low to moderate risk. This changes shutdown signal ownership and local run behavior during guarded restarts. - Paperclip already stops its managed embedded database explicitly. The change removes only the dependency listeners that race the coordinated path. - ACP runs now retry instead of receiving an unsafe bare-process adoption. Detached CLI runs keep their existing adoption behavior. - The report adds one field. There is no schema migration or breaking API 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 - OpenAI Codex with GPT-5. The runtime did not expose a more specific model revision or context-window size. Reasoning, repository editing, shell execution, and test execution 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> |
||
|
|
263316609e |
fix(server): avoid hot restart shutdown deadlock (#9670)
## Thinking Path > - Paperclip is the open source control plane people use to manage AI agents and their work > - The server coordinates agent heartbeats and preserves eligible live runs during a hot restart > - Shutdown previously waited for all heartbeat scheduler work before capturing the hot-restart snapshot > - A deployment heartbeat can itself be in that scheduler set while waiting for the restart, creating a circular wait > - The missing snapshot prevents startup from classifying and adopting the still-running agent process > - This pull request captures the snapshot first and skips scheduler/drain waits only for an eligible hot restart > - The benefit is a single SIGTERM can restart the server without losing eligible live agent runs ## Linked Issues or Issue Description - **Preflight:** Searched open and closed PRs for the hot-restart shutdown deadlock; no duplicate found. Reproduced on `master` and confirmed this is core Paperclip behavior. - **What happened:** During a hot restart initiated by a running deployment heartbeat, the SIGTERM handler waited for `heartbeatSchedulerInFlight` before calling `prepareHotRestartShutdown()`. The heartbeat was itself in that set and waited for restart completion, so shutdown never wrote the adoption snapshot. - **Expected behavior:** An eligible hot restart captures its snapshot before waiting for scheduler work, preserves live child processes, and exits after one SIGTERM. - **Steps to reproduce:** 1. Start a heartbeat that remains active while requesting a hot restart. 2. Send SIGTERM to the server process. 3. Observe shutdown waiting on the active scheduler task and startup finding an intent without a shutdown snapshot. - **Paperclip commit:** `992389480a243b97bda214227e0767eb8c3672af` - **Deployment/install:** Self-hosted server built from source. - **Adapter:** Not adapter-specific; reproduced with a Codex heartbeat. - **Database/access:** Embedded PGlite; agent bearer context. - **Environment:** Node `v22.22.2` on `Linux 6.17.0-1015-aws aarch64 GNU/Linux`. - **Privacy:** No secrets, private logs, user paths, or internal issue references are included. ## What Changed - Add a focused shutdown coordinator that prepares hot-restart state before waiting for heartbeat scheduler idleness. - Skip scheduler-idle and graceful-drain waits only when the hot-restart service returns `skipDrain: true`. - Preserve normal graceful shutdown behavior when no eligible intent exists or preparation fails. - Add regression coverage for pending scheduler work, normal shutdown, and preparation failure. ## Verification - `pnpm exec vitest run server/src/shutdown.test.ts` — 3 passed. - `pnpm --filter @paperclipai/server typecheck` — passed. - `pnpm exec vitest run server/src/__tests__/heartbeat-process-recovery.test.ts -t 'hot-restart'` — 3 passed, 88 skipped. - `git diff --check origin/master...HEAD` — passed. ## Risks - Low-to-moderate risk: shutdown ordering changes, but only the explicitly eligible hot-restart path bypasses scheduler-idle and run-drain waits. - Normal shutdown and hot-restart preparation failures retain the existing graceful behavior. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. This is a focused bug fix and does not duplicate planned roadmap work. ## Model Used - OpenAI Codex coding agent; exact runtime model ID and context-window size are not exposed to the agent. Tool use, shell execution, repository editing, and test execution 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 (no documentation change required for this internal shutdown-order fix) - [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> |