mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 14:19:00 +01:00
feat(recovery): instrument 20 durable write boundaries with crash points (#19938)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A control-plane restart between two durable writes leaves an in-progress record owned by a dead process > - #19937 added `crashPoint(name)` and a registry of 20 named boundaries from the Phase 0 inventory > - A name without a call site cannot fire, so the crash-point suite cannot test it > - This pull request puts one call at each registered boundary and adds a test that keeps the registry and the call sites in step > - The benefit is that the on-demand suite (next pull request) can kill the control plane at every known write-ahead gap ## Linked Issues or Issue Description Refs #19831 (plan, Phase 5). Refs #19846 (inventory, "Write-ahead coverage"). Stacked on #19937 (helper and registry). Refs #19861 (ownable kind ids). **Problem** The registered crash points have no call sites. **Expected behavior** Each registered point is called at its gap: after the first durable write or outside effect returns, before the next one starts. Disarmed (always in production) each call is one comparison. **Scope** Call sites and a coverage test only. The suite runner and CI workflow follow. ## What Changed - Server call sites (`@paperclipai/shared/crash-points`): - `sandbox-creation-operations.ts`: around the provider `create` call (intent → provider → lease). - `native-session-executor.ts`: after `native.process_start_requested`, before spawn. - `paperclip-control-plane-port.ts`: after the result-accept transaction, before finalization starts (only when a result was inserted). - `native-run-finalizer.ts`: after `commitNativeStatusDecision`, before the run projection. - `workspace-operations.ts`: `workspace_finalize` only — after the running row, and after `settle()` before the done row. - `environment-runtime.ts`: provider release → lease row; teardown claim → destroy; reusable destroy → lease release; stop-and-retain claim → stop. - `heartbeat.ts`: lease release → runtime services; budget reservation hold → dispatch. - `heartbeat/queue.ts`: run claimed `running` → wake request `claimed`. - `run-cost-accounting.ts`, `usage-receipts.ts`: accounting commit → spool unlink; receipt spool → DB row → unlink. - `packages/paperclip-runner/src/crash-point.ts`: a small copy of the helper for controller code in this package, which does not depend on `@paperclipai/shared` (adding that dependency would change the runner content ID). It arms only `runner.*` names. runnerd never sees the variable: its environment is an allowlist. - Runner call sites: after the runnerd spawn, before its identity is published (both spawn paths); `turn.start` result received → controller journal saved → next send. - `server/src/__tests__/crash-point-call-sites.test.ts`: every registered point has a call site, every call names a registered point, and every `kinds` entry is an ownable kind id of the Phase 1 registry. ## Verification - `cd server && npx vitest run src/__tests__/crash-point-call-sites.test.ts` → 3 passed. - `pnpm --filter @paperclipai/server typecheck` → clean. `pnpm --filter @paperclipai/paperclip-runner exec tsc --noEmit -p .` → clean. - Behavior with nothing armed is unchanged: each call returns at its first comparison (measured in #19937: about 3.7 ns). ## Risks - Low risk. No call changes control flow unless `PAPERCLIP_CRASH_POINT` names it and `NODE_ENV` is not `production`. - Points on the path before `turn.start` (process start, runner spawn) cost nanoseconds, within plan invariant 8. - `usage-receipts.ts`: the persist-then-unlink one-liner became a block with the same statements in the same order. - The runner package change gives the runner a new content ID, as any runner source change does. No persisted run state changes (rules/upgrade-safe-run-state.mdc). ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits) and a read-only exploration subagent that mapped the boundaries. ## 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 Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
468f22ae4b
commit
18644a3263
15 files changed
+151
-4
No files matched your search
@@ -0,0 +1,52 @@
|
||||
import { readdirSync, readFileSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { CRASH_POINTS } from "@paperclipai/shared/crash-points";
|
||||
import { OWNABLE_KINDS } from "../services/recovery/ownership-registry.js";
|
||||
|
||||
const repositoryRoot = path.resolve(import.meta.dirname, "../../..");
|
||||
const SOURCE_ROOTS = ["server/src", "packages/paperclip-runner/src"];
|
||||
const CALL = /\bcrashPoint\(\s*"([^"]+)"\s*\)/g;
|
||||
|
||||
function sourceFiles(directory: string): string[] {
|
||||
return readdirSync(directory, { withFileTypes: true }).flatMap(entry => {
|
||||
const full = path.join(directory, entry.name);
|
||||
if (entry.isDirectory()) return entry.name === "node_modules" || entry.name === "__tests__" ? [] : sourceFiles(full);
|
||||
return /\.tsx?$/.test(entry.name) && !/\.test\.tsx?$/.test(entry.name) ? [full] : [];
|
||||
});
|
||||
}
|
||||
|
||||
function callSites(): Map<string, string[]> {
|
||||
const sites = new Map<string, string[]>();
|
||||
for (const root of SOURCE_ROOTS) {
|
||||
for (const file of sourceFiles(path.join(repositoryRoot, root))) {
|
||||
for (const match of readFileSync(file, "utf8").matchAll(CALL)) {
|
||||
const list = sites.get(match[1]!) ?? [];
|
||||
list.push(path.relative(repositoryRoot, file));
|
||||
sites.set(match[1]!, list);
|
||||
}
|
||||
}
|
||||
}
|
||||
return sites;
|
||||
}
|
||||
|
||||
describe("crash-point call sites", () => {
|
||||
const sites = callSites();
|
||||
|
||||
it("instruments every registered crash point", () => {
|
||||
const missing = Object.keys(CRASH_POINTS).filter(name => !sites.has(name));
|
||||
expect(missing).toEqual([]);
|
||||
});
|
||||
|
||||
it("calls only registered crash points", () => {
|
||||
const unknown = [...sites.keys()].filter(name => !(name in CRASH_POINTS));
|
||||
expect(unknown).toEqual([]);
|
||||
});
|
||||
|
||||
it("names only ownable kinds of the Phase 1 registry", () => {
|
||||
const known = new Set(OWNABLE_KINDS.map(kind => kind.id));
|
||||
const unknown = Object.entries(CRASH_POINTS).flatMap(([name, definition]) =>
|
||||
definition.kinds.filter(kind => !known.has(kind)).map(kind => `${name}: ${kind}`));
|
||||
expect(unknown).toEqual([]);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user