mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 10:26:26 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The Paperclip Runner keeps durable run and provider state outside one server process. > - A server restart can leave that runner alive or can interrupt it after a provider checkpoint. > - The old startup path used handoff intent and PID evidence, but it did not reconstruct native ownership. > - That gap could block the issue, create a replacement run, or start duplicate provider work. > - This pull request adds durable same-run recovery for coordinated and uncoordinated restarts. > - The benefit is exact recovery of the run, runner, session, provider, steering, and finalization state. ## Linked Issues or Issue Description Refs #9628. That pull request added earlier local-adapter hot-restart work. This change adds native PRP authority reconstruction and same-run provider resume. Refs #10935. That pull request handles missing hot-restart snapshots. This change also supports hard restarts with no snapshot. Refs #11624. That pull request prevents unsafe retry after an adopted legacy process exits. This change reconciles native terminal evidence before provider recovery. Refs #12070. That pull request improves process liveness checks. This change also binds recovery to a process-start fingerprint and fails closed on ambiguity. **What happened?** The server could record hot-restart intent, but startup did not rebuild native runner ownership. A live runner could not re-register its PRP authority. A dead runner could not resume the exact native and provider session on the same heartbeat run. Generic recovery could then block the issue or create replacement work. **Expected behavior** A live native runner must reconnect with the same PID and logical identities. A dead runner must resume the same durable session and heartbeat run with only a new operating-system PID. A proposed or terminal result must finalize once before any provider turn starts. Ambiguous process or session evidence must stay blocked without a signal or duplicate spawn. **Steps to reproduce** 1. Start a Paperclip Runner heartbeat and wait for an active provider turn. 2. Restart only the Paperclip server, with or without a hot-restart marker. 3. Observe that the old startup path does not reconstruct the native control-plane authority. 4. Kill both the server and runner after a provider checkpoint. 5. Observe that the old path cannot resume the exact native session on the original heartbeat run. **Paperclip version or commit** The defect was reproduced from commit `1991f31fd53e7f7794d5c2e4b93be384ade2b41d`. This branch is rebased onto the current `master`. **Deployment mode** Local development and self-hosted server deployments that use the local Paperclip Runner. ## What Changed - Added correlated hot-restart requests and version-compatible native handoff fields. - Added controller boot identity, process-start identity, controller generation, recovery state, request id, and bounded history to the native finalization ledger. - Added transactional recovery claims for live-runner reattach, dead-runner resume, and incomplete bootstrap. - Added fail-closed ownership takeover rules and process identity validation. - Added live runner adoption to the local runner transport without a duplicate spawn. - Added same-run provider checkpoint resume and legacy retry-row compatibility. - Reconciled proposed and terminal results before runner or provider recovery. - Bound the HTTP and PRP listener before startup recovery and delayed scheduling and generic reapers until classification completes. - Added restart-aware health diagnostics, run-log recovery transitions, durable runner diagnostics, and bounded shutdown finalizer draining. - Moved restart-survivable diagnostics into runner-owned, pre-redacted bounded writes; raw stdout and stderr are never persisted. - Added process-start fencing for controller, runner, and provider PIDs; startup classifies every candidate without an implicit cap. - Added crash-recoverable, contention-safe development restart-request coordination and failed-startup listener cleanup. - Added a credential-free real-process restart suite for eight restart, scale, and identity scenarios. - Documented native restart operation, persistence, diagnostics, and verification. ## Verification - The documented native restart commands passed. They ran eight real-process/database recovery scenarios and the live runner adoption transport test. - Native executor tests passed: 111 tests. - Heartbeat recovery tests passed: 124 tests. - Hot restart, health, and shutdown tests passed: 52 tests. - The broader affected server suite passed: 350 tests. - Focused native recovery and startup tests passed: 49 tests. - Runner transport and control-plane tests passed: 63 tests. - Runner-owned diagnostic tests passed for write-time bounding, credential redaction, private file modes, and raw stream non-persistence. - Development restart coordination tests passed: 11 tests. - Database migration checks and the partial-application/replay regression test passed. - Server, database, and Paperclip Runner typechecks passed. - `git diff --check` passed. - Full Paperclip PR CI passed, including build, canary, all five general server shards, all five serialized server shards, all three browser E2E shards, workspace suites, and release-registry verification. - Greptile completed at 5/5 with no outstanding findings, recommendations, follow-ups, or open review threads. ## Risks - Moderate risk. This changes startup ordering and ownership transfer for active native runs. - The migration adds nullable columns and does not rewrite existing rows. - Recovery fails closed when process or durable session identity is incomplete or contradictory. - The first implementation supports the local Paperclip Runner. Remote targets keep their existing behavior. - The real-process suite covers cleanup and asserts that no runner or provider process survives each test. > 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. Repository editing, shell execution, database tests, and real-process 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 (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
367 lines
12 KiB
TypeScript
367 lines
12 KiB
TypeScript
import { EventEmitter } from "node:events";
|
|
import { describe, expect, it, vi } from "vitest";
|
|
import {
|
|
coordinateHeartbeatSchedulerShutdown,
|
|
drainRunExecutionFinalizersForShutdown,
|
|
finalizeServerShutdown,
|
|
loadWithoutCoordinatedShutdownSignalHooks,
|
|
} from "./shutdown.js";
|
|
|
|
function deferred<T = void>() {
|
|
let resolve!: (value: T) => void;
|
|
let reject!: (reason?: unknown) => void;
|
|
const promise = new Promise<T>((res, rej) => {
|
|
resolve = res;
|
|
reject = rej;
|
|
});
|
|
return { promise, resolve, reject };
|
|
}
|
|
|
|
function stubLogger() {
|
|
return { info: vi.fn(), error: vi.fn() };
|
|
}
|
|
|
|
describe("finalizeServerShutdown", () => {
|
|
it("awaits the setup-token cleanup before the database stop and the process exit", async () => {
|
|
const order: string[] = [];
|
|
// The held promise models the setup-token session cancellation and its
|
|
// sandbox lease release. The teardown must not continue while it is pending.
|
|
const release = deferred();
|
|
const shutdownAppServices = vi.fn(async () => {
|
|
order.push("appServices:start");
|
|
await release.promise;
|
|
order.push("appServices:settled");
|
|
});
|
|
const stopEmbeddedPostgres = vi.fn(async () => {
|
|
order.push("postgres:stop");
|
|
});
|
|
const shutdownInstrumentation = vi.fn(async () => {
|
|
order.push("instrumentation:flush");
|
|
});
|
|
const shutdownSentry = vi.fn(async () => {
|
|
order.push("sentry:flush");
|
|
});
|
|
|
|
let exited = false;
|
|
const finalize = finalizeServerShutdown({
|
|
signal: "SIGTERM",
|
|
shutdownAppServices,
|
|
stopEmbeddedPostgres,
|
|
shutdownInstrumentation,
|
|
shutdownSentry,
|
|
log: stubLogger(),
|
|
}).then(() => {
|
|
// This models the caller's `process.exit(0)` continuation.
|
|
exited = true;
|
|
order.push("exit");
|
|
});
|
|
|
|
// The cleanup is in flight. The database stop, the instrumentation flush,
|
|
// and the process exit continuation must all wait for it to settle.
|
|
await vi.waitFor(() => expect(shutdownAppServices).toHaveBeenCalledOnce());
|
|
expect(stopEmbeddedPostgres).not.toHaveBeenCalled();
|
|
expect(shutdownInstrumentation).not.toHaveBeenCalled();
|
|
expect(exited).toBe(false);
|
|
|
|
release.resolve();
|
|
await finalize;
|
|
|
|
expect(exited).toBe(true);
|
|
expect(order).toEqual([
|
|
"appServices:start",
|
|
"appServices:settled",
|
|
"postgres:stop",
|
|
"instrumentation:flush",
|
|
"sentry:flush",
|
|
"exit",
|
|
]);
|
|
});
|
|
|
|
it("keeps the teardown durable and still exits when the setup-token release fails", async () => {
|
|
const order: string[] = [];
|
|
// The held promise rejects, which models a lease release that failed. The
|
|
// reaper owns the durable retry, so the teardown must log the failure and
|
|
// continue rather than swallow it or block the exit.
|
|
const release = deferred();
|
|
const releaseError = new Error("lease release failed");
|
|
const shutdownAppServices = vi.fn(async () => {
|
|
await release.promise;
|
|
});
|
|
const stopEmbeddedPostgres = vi.fn(async () => {
|
|
order.push("postgres:stop");
|
|
});
|
|
const shutdownInstrumentation = vi.fn(async () => {
|
|
order.push("instrumentation:flush");
|
|
});
|
|
const shutdownSentry = vi.fn(async () => undefined);
|
|
const log = stubLogger();
|
|
|
|
let exited = false;
|
|
const finalize = finalizeServerShutdown({
|
|
signal: "SIGTERM",
|
|
shutdownAppServices,
|
|
stopEmbeddedPostgres,
|
|
shutdownInstrumentation,
|
|
shutdownSentry,
|
|
log,
|
|
}).then(() => {
|
|
exited = true;
|
|
});
|
|
|
|
await vi.waitFor(() => expect(shutdownAppServices).toHaveBeenCalledOnce());
|
|
expect(stopEmbeddedPostgres).not.toHaveBeenCalled();
|
|
expect(exited).toBe(false);
|
|
|
|
release.reject(releaseError);
|
|
await finalize;
|
|
|
|
// The teardown surfaced the failure in the log, then finished the ordered
|
|
// teardown and reached the exit continuation.
|
|
expect(log.error).toHaveBeenCalledWith(
|
|
expect.objectContaining({ err: releaseError, signal: "SIGTERM" }),
|
|
expect.any(String),
|
|
);
|
|
expect(order).toEqual(["postgres:stop", "instrumentation:flush"]);
|
|
expect(exited).toBe(true);
|
|
});
|
|
|
|
it("skips the database stop when no embedded PostgreSQL runs in this process", async () => {
|
|
const shutdownAppServices = vi.fn(async () => undefined);
|
|
const shutdownInstrumentation = vi.fn(async () => undefined);
|
|
const shutdownSentry = vi.fn(async () => undefined);
|
|
const log = stubLogger();
|
|
|
|
await finalizeServerShutdown({
|
|
signal: "SIGINT",
|
|
shutdownAppServices,
|
|
stopEmbeddedPostgres: null,
|
|
shutdownInstrumentation,
|
|
shutdownSentry,
|
|
log,
|
|
});
|
|
|
|
expect(shutdownAppServices).toHaveBeenCalledOnce();
|
|
expect(shutdownInstrumentation).toHaveBeenCalledOnce();
|
|
expect(log.info).not.toHaveBeenCalled();
|
|
});
|
|
});
|
|
|
|
describe("drainRunExecutionFinalizersForShutdown", () => {
|
|
it("awaits bounded execution finalizers", async () => {
|
|
const release = deferred();
|
|
const drain = vi.fn(() => release.promise);
|
|
const pending = drainRunExecutionFinalizersForShutdown({
|
|
signal: "SIGTERM",
|
|
drain,
|
|
timeoutMs: 1_000,
|
|
log: stubLogger(),
|
|
});
|
|
await vi.waitFor(() => expect(drain).toHaveBeenCalledOnce());
|
|
release.resolve();
|
|
await expect(pending).resolves.toBe("drained");
|
|
});
|
|
|
|
it("returns after the bounded timeout when an adopted run remains active", async () => {
|
|
vi.useFakeTimers();
|
|
try {
|
|
const log = stubLogger();
|
|
const pending = drainRunExecutionFinalizersForShutdown({
|
|
signal: "SIGINT",
|
|
drain: () => new Promise<void>(() => undefined),
|
|
timeoutMs: 250,
|
|
log,
|
|
});
|
|
await vi.advanceTimersByTimeAsync(250);
|
|
await expect(pending).resolves.toBe("timed_out");
|
|
expect(log.info).toHaveBeenCalledWith(
|
|
expect.objectContaining({ timeoutMs: 250 }),
|
|
expect.stringContaining("timed out"),
|
|
);
|
|
} finally {
|
|
vi.useRealTimers();
|
|
}
|
|
});
|
|
});
|
|
|
|
describe("loadWithoutCoordinatedShutdownSignalHooks", () => {
|
|
it("removes the eager signal handlers from the real embedded-postgres import", async () => {
|
|
const before = {
|
|
SIGINT: process.rawListeners("SIGINT"),
|
|
SIGTERM: process.rawListeners("SIGTERM"),
|
|
};
|
|
const moduleName = "embedded-postgres";
|
|
|
|
await loadWithoutCoordinatedShutdownSignalHooks(() => import(moduleName));
|
|
|
|
expect(process.rawListeners("SIGINT")).toEqual(before.SIGINT);
|
|
expect(process.rawListeners("SIGTERM")).toEqual(before.SIGTERM);
|
|
});
|
|
|
|
it("keeps the database available for a marker-backed SIGTERM snapshot", async () => {
|
|
const signalTarget = new EventEmitter();
|
|
const preexistingSignalListener = vi.fn();
|
|
signalTarget.on("SIGTERM", preexistingSignalListener);
|
|
|
|
let databaseAvailable = true;
|
|
const embeddedPostgresExitHook = vi.fn(() => {
|
|
databaseAvailable = false;
|
|
});
|
|
await loadWithoutCoordinatedShutdownSignalHooks(
|
|
async () => {
|
|
signalTarget.on("SIGINT", embeddedPostgresExitHook);
|
|
signalTarget.on("SIGTERM", embeddedPostgresExitHook);
|
|
return { default: class EmbeddedPostgres {} };
|
|
},
|
|
signalTarget,
|
|
);
|
|
|
|
let shutdown: Promise<unknown> | null = null;
|
|
let snapshotCaptured = false;
|
|
signalTarget.once("SIGTERM", () => {
|
|
shutdown = coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: async () => {
|
|
// This models the real failure path: a valid intent exists, and the
|
|
// snapshot must query embedded PostgreSQL after SIGTERM is delivered.
|
|
expect(databaseAvailable).toBe(true);
|
|
snapshotCaptured = true;
|
|
return { mode: "hot_restart" as const, skipDrain: true };
|
|
},
|
|
waitForHeartbeatSchedulerIdle: vi.fn(async () => undefined),
|
|
});
|
|
});
|
|
|
|
signalTarget.emit("SIGTERM");
|
|
await shutdown;
|
|
|
|
expect(preexistingSignalListener).toHaveBeenCalledOnce();
|
|
expect(embeddedPostgresExitHook).not.toHaveBeenCalled();
|
|
expect(snapshotCaptured).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe("coordinateHeartbeatSchedulerShutdown", () => {
|
|
it("quiesces active scheduler work before capturing a hot-restart snapshot", async () => {
|
|
let snapshotCaptured = false;
|
|
let releaseScheduler!: () => void;
|
|
const schedulerIdle = new Promise<void>((resolve) => {
|
|
releaseScheduler = resolve;
|
|
});
|
|
const waitForHeartbeatSchedulerIdle = vi.fn(() => schedulerIdle);
|
|
|
|
const shutdown = coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: vi.fn(async () => {
|
|
snapshotCaptured = true;
|
|
return { mode: "prepared" as const, skipDrain: true };
|
|
}),
|
|
waitForHeartbeatSchedulerIdle,
|
|
});
|
|
|
|
await vi.waitFor(() => expect(waitForHeartbeatSchedulerIdle).toHaveBeenCalledOnce());
|
|
expect(snapshotCaptured).toBe(false);
|
|
releaseScheduler();
|
|
|
|
const result = await shutdown;
|
|
expect(snapshotCaptured).toBe(true);
|
|
expect(result).toEqual({
|
|
hotRestart: { mode: "prepared", skipDrain: true },
|
|
preparationError: null,
|
|
waitedForSchedulerIdle: true,
|
|
});
|
|
});
|
|
|
|
it("quiesces scheduler work before selecting server-stdio runs to drain", async () => {
|
|
const waitForHeartbeatSchedulerIdle = vi.fn(async () => undefined);
|
|
|
|
const result = await coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: vi.fn(async () => ({
|
|
mode: "acp_drain_required" as const,
|
|
skipDrain: false,
|
|
drainRunIds: ["acp-run"],
|
|
})),
|
|
waitForHeartbeatSchedulerIdle,
|
|
});
|
|
|
|
expect(waitForHeartbeatSchedulerIdle).toHaveBeenCalledOnce();
|
|
expect(result).toEqual({
|
|
hotRestart: {
|
|
mode: "acp_drain_required",
|
|
skipDrain: false,
|
|
drainRunIds: ["acp-run"],
|
|
},
|
|
preparationError: null,
|
|
waitedForSchedulerIdle: true,
|
|
});
|
|
});
|
|
|
|
it("preserves the scheduler idle wait for normal graceful shutdown", async () => {
|
|
let releaseScheduler!: () => void;
|
|
const schedulerIdle = new Promise<void>((resolve) => {
|
|
releaseScheduler = resolve;
|
|
});
|
|
const waitForHeartbeatSchedulerIdle = vi.fn(() => schedulerIdle);
|
|
let settled = false;
|
|
|
|
const shutdown = coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: vi.fn(async () => ({
|
|
mode: "not_requested" as const,
|
|
skipDrain: false,
|
|
})),
|
|
waitForHeartbeatSchedulerIdle,
|
|
}).finally(() => {
|
|
settled = true;
|
|
});
|
|
|
|
await vi.waitFor(() => expect(waitForHeartbeatSchedulerIdle).toHaveBeenCalledOnce());
|
|
expect(settled).toBe(false);
|
|
|
|
releaseScheduler();
|
|
|
|
await expect(shutdown).resolves.toEqual({
|
|
hotRestart: { mode: "not_requested", skipDrain: false },
|
|
preparationError: null,
|
|
waitedForSchedulerIdle: true,
|
|
});
|
|
});
|
|
|
|
it("waits for scheduler idle when hot-restart preparation is unavailable", async () => {
|
|
const waitForHeartbeatSchedulerIdle = vi.fn(async () => undefined);
|
|
|
|
const result = await coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: null,
|
|
waitForHeartbeatSchedulerIdle,
|
|
});
|
|
|
|
expect(waitForHeartbeatSchedulerIdle).toHaveBeenCalledOnce();
|
|
expect(result).toEqual({
|
|
hotRestart: null,
|
|
preparationError: null,
|
|
waitedForSchedulerIdle: true,
|
|
});
|
|
});
|
|
|
|
it("falls back to the scheduler idle wait when hot-restart preparation fails", async () => {
|
|
const preparationError = new Error("snapshot failed");
|
|
const waitForHeartbeatSchedulerIdle = vi.fn(async () => undefined);
|
|
|
|
const result = await coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: vi.fn(async () => {
|
|
throw preparationError;
|
|
}),
|
|
waitForHeartbeatSchedulerIdle,
|
|
});
|
|
|
|
expect(waitForHeartbeatSchedulerIdle).toHaveBeenCalledOnce();
|
|
expect(result).toEqual({
|
|
hotRestart: null,
|
|
preparationError,
|
|
waitedForSchedulerIdle: true,
|
|
});
|
|
});
|
|
});
|