mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 14:19:00 +01:00
## 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 > - Random restart-chaos waves found these 20 ms–1 s windows only by luck > - #19937 added `crashPoint(name)`, and #19938 put it at 20 durable write boundaries > - Nothing yet runs a task, kills the server at a chosen point, restarts it, and checks the plan's invariants > - This pull request adds that on-demand suite, as a local command and as a manual CI workflow (not a required check, per Decision 2) > - The benefit is a repeatable work list: which boundary breaks which invariant, for Phases 2–4 ## Linked Issues or Issue Description Refs #19831 (plan, Phase 5 and Decision 2). Refs #19861 (report-only reconciler used for I1–I7). Stacked on #19938 (call sites), which is stacked on #19937 (helper). **Problem** The named crash points cannot be exercised end to end. No command arms one point, runs real agent tasks across it, restarts, and grades the result. **Expected behavior** `node scripts/crash-points.mjs --points <glob> --kinds <id> --adapters <list>` runs one wave per selected point and reports each point as pass, fail (with failing R rules and violated invariants), not reached, or harness error. A `workflow_dispatch` workflow runs the same thing on demand. ## What Changed - **Runtime arming.** `PAPERCLIP_CRASH_POINT_CONTROL=<file>`, in the shared helper and the runner mirror. Each hit reads `<token> <name>[:<nth>]`, and a new token resets the count. It is test mode only: `NODE_ENV=production` ignores it, and the disarmed path is still one comparison. - **`tests/restart-chaos/main.ts --crash-point <a>:<nth>,<b>:<nth>,...`** (config, types, ledger): - one booted server and no Vite UI; - per point: take an invariant baseline, launch the point's batch, arm the point, then on rest disarm it and check invariants as violations that are new since the baseline; - every task is graded R1–R6 and T; - `crash-point.json` (v2) holds one entry per point. - **`tests/restart-chaos/supervisor.ts`:** - every spawn gets the control file; - a SIGKILL whose receipt names the child's pid is restarted like K1: the control file is emptied first, the owned tree is killed, and a replacement starts. A failed tree cleanup fails the fault; - the isolated instance pins `PAPERCLIP_IN_WORKTREE=false` (a `.paperclip/.env` above the checkout otherwise suppressed all scheduling); - `RESTART_CHAOS_HEALTH_TIMEOUT_MS`. - **`tests/restart-chaos/invariants.ts`:** - the Phase 1 report-only pass with `record: false`: two real-clock passes for I1 and I2, and one pass 15 minutes ahead for I3, I4 and I7; - spool kinds are read from the wave's instance; - an unfinished pass is a harness error; - each violation carries its wake and run state as evidence. - **`tests/restart-chaos/crash-points.ts` and `scripts/crash-points.mjs`:** - select points by glob, kind and environment, and run them in one wave; - a per-point outcome: pass, fail, not_reached or harness_error; - `SUMMARY.md` with a violation evidence column. - **`.github/workflows/crash-points.yml`**, manual only, under the paid-workflow contract: - a credential-free `authorize` job; - a `suite` job in `runner-e2e-paid` whose first step reauthorizes; - a frozen install with `--ignore-scripts` and no shared cache; - only the selected adapters' keys; - the workflow is added to both security-test allowlists. - **Docs:** "Crash-point suite" in `tests/restart-chaos/README.md`, and "Built (Phase 5)" plus the first work list in the plan. ## Verification - Focused tests: - `pnpm exec vitest run --config tests/restart-chaos/vitest.config.ts tests/restart-chaos/crash-points.test.ts`; - `cd packages/shared && npx vitest run src/crash-points.test.ts`: 15 tests, including runtime arming in a real child process; - `node --test scripts/__tests__/release-verify-workflow.test.mjs`: 72 tests; - `tests/runner-e2e/workflow-security.test.ts`: 19 tests. - Typecheck: `node scripts/typecheck-test-projects.mjs tests/restart-chaos` passes. - Loop line: question "how long is one crash-point loop?" | loop: `node scripts/crash-points.mjs --points <one>` (codex A1) | elapsed 2.7 min end to end: boot 9 s, crash 9 s after arming, replacement ready in 6 s, rest about 2 min | result: one point alone takes about 3 min. Before this change it took about 15 min (one cold boot per point, Vite UI, host at load 77). - Full local run, 14 points, codex A1, 11.8 min in all: - every point was reached (the server killed itself at each one, and the replacement came back); - 11 pass; - `workspace.finalize.after_start_before_copyback` and `workspace.finalize.after_copyback_before_done` fail I2 and R6: a `workspace_finalize` operation stays running after the run succeeded; - `wake.claim.after_run_running_before_wakeup_claimed` reports I4. Its evidence shows the wake failed with "…retrying once", and the retry delivered the work, so this is the known refusal-regex false positive. - The 6 Daytona-only points were not run. ## Risks - Test harness and an on-demand workflow only. No product code changes. - `.github/**` needs a CODEOWNER review. The workflow uses provider keys, so it refuses non-default branches and actors that are not on the allowlist, like `runner-full-stack-e2e.yml`. - `PAPERCLIP_IN_WORKTREE=false` is pinned in every chaos wave, not only crash-point waves. A chaos instance is never a worktree dev instance, so scheduling must not be suppressed. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). ## 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>
196 lines
8.7 KiB
TypeScript
196 lines
8.7 KiB
TypeScript
/**
|
|
* Named crash points (durable ownership plan, "Testing: enumerate crash points").
|
|
*
|
|
* `crashPoint(name)` marks a durable write boundary: the gap between two durable
|
|
* writes, or between an outside effect and the record of its outcome. A restart
|
|
* that lands in that gap leaves an in-progress record owned by a dead process.
|
|
* The crash-point suite (`pnpm crash-points`) kills the control plane at each
|
|
* named gap on purpose instead of hoping random chaos timing finds it.
|
|
*
|
|
* Arming (test mode only):
|
|
*
|
|
* PAPERCLIP_CRASH_POINT=<name>[:<nth>] SIGKILL this process at the nth hit (default 1)
|
|
* PAPERCLIP_CRASH_POINT_RECEIPT=<file> optional; one JSON line is appended before the kill
|
|
* PAPERCLIP_CRASH_POINT_GATE=<file> optional; hits before this file exists are not counted,
|
|
* so setup work (seeding, probes) cannot fire the point
|
|
*
|
|
* Runtime arming (one booted server, many points), instead of PAPERCLIP_CRASH_POINT:
|
|
*
|
|
* PAPERCLIP_CRASH_POINT_CONTROL=<file> each hit reads "<token> <name>[:<nth>]" from the file;
|
|
* no file or an empty one means disarmed. Hits count per
|
|
* token, so writing a new token re-arms with a fresh count.
|
|
*
|
|
* The variable is read once, when this module is first imported. It is ignored
|
|
* when `NODE_ENV=production`, and an unknown name throws at import so a typo
|
|
* cannot silently arm nothing. Disarmed, `crashPoint` is a single comparison
|
|
* against a module constant (see crash-points.test.ts for the cost check).
|
|
*
|
|
* In-process tests use `armCrashPointForTest`, which throws `CrashPointReached`
|
|
* at the hit instead of killing the test runner. A throw still runs `finally`
|
|
* blocks and rejects the caller's promise, so it only approximates a crash: a
|
|
* test that needs real process death runs the suite instead.
|
|
*
|
|
* This is test instrumentation, not Telemetry or Observability (AGENTS.md §5.7):
|
|
* nothing is recorded or sent anywhere unless a test arms a point.
|
|
*/
|
|
import { appendFileSync, existsSync, readFileSync } from "node:fs";
|
|
import { CRASH_POINTS, isCrashPointName, type CrashPointName } from "./crash-point-registry.js";
|
|
|
|
export { CRASH_POINTS, CRASH_POINT_NAME_PATTERN, isCrashPointName } from "./crash-point-registry.js";
|
|
export type { CrashPointDefinition, CrashPointInvariant, CrashPointName } from "./crash-point-registry.js";
|
|
|
|
export const CRASH_POINT_ENV = "PAPERCLIP_CRASH_POINT";
|
|
export const CRASH_POINT_RECEIPT_ENV = "PAPERCLIP_CRASH_POINT_RECEIPT";
|
|
export const CRASH_POINT_GATE_ENV = "PAPERCLIP_CRASH_POINT_GATE";
|
|
export const CRASH_POINT_CONTROL_ENV = "PAPERCLIP_CRASH_POINT_CONTROL";
|
|
/** Printed to stderr just before the kill; the suite greps server logs for it. */
|
|
export const CRASH_POINT_LOG_PREFIX = "[crash-point]";
|
|
|
|
export type CrashPointSpec = { name: CrashPointName; nth: number };
|
|
|
|
export type CrashPointReceipt = {
|
|
point: CrashPointName;
|
|
hit: number;
|
|
pid: number;
|
|
at: string;
|
|
/** Runtime arming only: the control file token the hit counted under. */
|
|
token?: string;
|
|
};
|
|
|
|
/** Thrown by a point armed with `armCrashPointForTest`. */
|
|
export class CrashPointReached extends Error {
|
|
constructor(readonly point: CrashPointName, readonly hit: number) {
|
|
super(`crash point ${point} reached (hit ${hit})`);
|
|
this.name = "CrashPointReached";
|
|
}
|
|
}
|
|
|
|
/** Parses `<name>[:<nth>]`. Returns null for an empty value; throws on a bad one. */
|
|
export function parseCrashPointSpec(raw: string | undefined): CrashPointSpec | null {
|
|
const value = raw?.trim();
|
|
if (!value) return null;
|
|
const separator = value.lastIndexOf(":");
|
|
const name = separator === -1 ? value : value.slice(0, separator);
|
|
const nthText = separator === -1 ? "1" : value.slice(separator + 1);
|
|
if (!/^[1-9]\d{0,5}$/.test(nthText)) {
|
|
throw new Error(`${CRASH_POINT_ENV}=${value}: the hit count after ':' must be a positive integer`);
|
|
}
|
|
if (!isCrashPointName(name)) {
|
|
throw new Error(`${CRASH_POINT_ENV}=${value}: unknown crash point "${name}" (see packages/shared/src/crash-point-registry.ts)`);
|
|
}
|
|
return { name, nth: Number(nthText) };
|
|
}
|
|
|
|
type Armed = {
|
|
spec: CrashPointSpec;
|
|
hits: number;
|
|
mode: "kill" | "throw";
|
|
receiptPath: string | null;
|
|
/** Hits count only once this file exists; null when ungated. */
|
|
gatePath: string | null;
|
|
/** Runtime arming: the control file read at each hit; null for a fixed spec. */
|
|
controlPath?: string | null;
|
|
token?: string | null;
|
|
};
|
|
|
|
/** Parses a control file line, "<token> <name>[:<nth>]"; null when empty or malformed. */
|
|
export function parseCrashPointControl(text: string): { token: string; spec: CrashPointSpec } | null {
|
|
const [token, raw, ...rest] = text.trim().split(/\s+/);
|
|
if (!token || !raw || rest.length > 0) return null;
|
|
try {
|
|
const spec = parseCrashPointSpec(raw);
|
|
return spec ? { token, spec } : null;
|
|
} catch {
|
|
return null;
|
|
}
|
|
}
|
|
|
|
function armFromEnvironment(): Armed | null {
|
|
const env = typeof process === "undefined" ? undefined : process.env;
|
|
if (!env || env.NODE_ENV === "production") return null;
|
|
const controlPath = env[CRASH_POINT_CONTROL_ENV]?.trim();
|
|
if (controlPath) {
|
|
process.stderr.write(`${CRASH_POINT_LOG_PREFIX} runtime arming from ${controlPath} pid=${process.pid}\n`);
|
|
// The spec is a placeholder; each hit reads the real one from the control file.
|
|
return { spec: { name: "" as CrashPointName, nth: 1 }, hits: 0, mode: "kill",
|
|
receiptPath: env[CRASH_POINT_RECEIPT_ENV]?.trim() || null, gatePath: null, controlPath, token: null };
|
|
}
|
|
const spec = parseCrashPointSpec(env[CRASH_POINT_ENV]);
|
|
if (!spec) return null;
|
|
process.stderr.write(`${CRASH_POINT_LOG_PREFIX} armed ${spec.name}:${spec.nth} pid=${process.pid}\n`);
|
|
return {
|
|
spec, hits: 0, mode: "kill",
|
|
receiptPath: env[CRASH_POINT_RECEIPT_ENV]?.trim() || null,
|
|
gatePath: env[CRASH_POINT_GATE_ENV]?.trim() || null,
|
|
};
|
|
}
|
|
|
|
let armed: Armed | null = armFromEnvironment();
|
|
|
|
/**
|
|
* Marks a durable write boundary. Disarmed (always in production) it returns
|
|
* immediately. Armed for this name, the nth hit kills the process with SIGKILL
|
|
* (or throws `CrashPointReached` under `armCrashPointForTest`).
|
|
*/
|
|
export function crashPoint(name: CrashPointName): void {
|
|
if (armed === null) return;
|
|
if (armed.controlPath) {
|
|
if (!readControl(armed)) return;
|
|
}
|
|
if (armed.spec.name !== name) return;
|
|
reach(armed);
|
|
}
|
|
|
|
/** Runtime arming: loads the control file's spec; a new token resets the count. */
|
|
function readControl(current: Armed): boolean {
|
|
let text: string;
|
|
try { text = readFileSync(current.controlPath!, "utf8"); } catch { return false; }
|
|
const control = parseCrashPointControl(text);
|
|
if (!control) return false;
|
|
if (control.token !== current.token) {
|
|
current.token = control.token;
|
|
current.spec = control.spec;
|
|
current.hits = 0;
|
|
}
|
|
return true;
|
|
}
|
|
|
|
function reach(current: Armed): void {
|
|
if (current.gatePath !== null) {
|
|
if (!existsSync(current.gatePath)) return;
|
|
current.gatePath = null;
|
|
}
|
|
current.hits += 1;
|
|
if (current.hits !== current.spec.nth) return;
|
|
if (current.mode === "throw") throw new CrashPointReached(current.spec.name, current.hits);
|
|
const receipt: CrashPointReceipt = {
|
|
point: current.spec.name, hit: current.hits, pid: process.pid, at: new Date().toISOString(),
|
|
...(current.token ? { token: current.token } : {}),
|
|
};
|
|
if (current.receiptPath) {
|
|
try { appendFileSync(current.receiptPath, `${JSON.stringify(receipt)}\n`); } catch { /* the log line below still records it */ }
|
|
}
|
|
try { process.stderr.write(`${CRASH_POINT_LOG_PREFIX} SIGKILL ${JSON.stringify(receipt)}\n`); } catch { /* ignore */ }
|
|
process.kill(process.pid, "SIGKILL");
|
|
}
|
|
|
|
/**
|
|
* Arms one point inside a test process: the nth hit throws `CrashPointReached`.
|
|
* Returns a disarm function; call it in `afterEach`. Replaces any armed point.
|
|
*/
|
|
export function armCrashPointForTest(spec: string | CrashPointSpec): () => void {
|
|
// One parser for both forms, so an object with a bad name or count is refused
|
|
// instead of arming a point that can never fire.
|
|
const parsed = parseCrashPointSpec(typeof spec === "string" ? spec : `${spec.name}:${spec.nth}`);
|
|
if (!parsed) throw new Error("armCrashPointForTest needs a crash point name");
|
|
const previous = armed;
|
|
const current: Armed = { spec: parsed, hits: 0, mode: "throw", receiptPath: null, gatePath: null };
|
|
armed = current;
|
|
return () => { if (armed === current) armed = previous; };
|
|
}
|
|
|
|
/** The armed point and its hit count so far, or null. For tests and diagnostics. */
|
|
export function armedCrashPoint(): { name: CrashPointName; nth: number; hits: number } | null {
|
|
return armed && armed.spec.name ? { name: armed.spec.name, nth: armed.spec.nth, hits: armed.hits } : null;
|
|
}
|