mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Run preparation publishes skill revisions into a shared runtime skill cache. A file lock serializes each publication. A lock whose holder provably exited is released at once. > - That check used `controllerProcessExited`, a PID proof. It fails closed whenever the recorded process space (host, kernel boot, PID namespace) differs from this server's. > - Every Railway deploy starts the server in a new process space. A lock left by a server that a deploy stopped mid-publish can never be released. Each later preparation of that skill waits 60 s and fails with "Runtime skill cache publisher is busy … remove it", until an operator deletes the file. > - The rest of recovery already uses one shared prover (`proveOwnerGone`). Across process spaces it accepts a replaced platform deployment on the same exclusive volume, or a stopped or fenced server boot. > - This pull request makes the skill cache lock use that prover. It was the last caller of the separate PID-only proof. > - The benefit is one rule for "the controller from a previous deployment is provably gone", and no manual file removal after a deploy. ## Linked Issues or Issue Description **What happened?** A census of staging failures after deploys showed that `controllerProcessExited` always fails closed after a deploy, because the process-space hash changes. On `master`, the only remaining caller is the runtime skill cache lock check. The restart chaos harness restarts in the same process space, so the PID proof succeeded there and hid the gap. I did not find this failure in the staging run census. It is a latent dead end with the same cause as the census class. **Expected behavior** A lock whose holder ran in a deployment that the platform replaced on the same exclusive volume, or whose server boot stopped or was fenced, is released at once. Any other holder is still waited for. **Steps to reproduce** 1. Run the server on Railway with a volume. 2. Redeploy while a skill revision is being published (the lock file exists). 3. Prepare a run that needs the same skill revision on the new deployment. 4. Observe a 60 s wait and `Runtime skill cache publisher is busy; retry preparation.` on every attempt. **Paperclip version or commit** `master` at `c444da7d05` (the new test fails there). ## What Changed - `server/src/services/runtime-skill-cache.ts`: `runtimeSkillCacheLockHolderExited` builds a local owner stamp from the recorded controller (boot ID, PID, process space, start time, and the Railway deployment and volume when recorded). It returns true only for the `gone` verdict of `proveOwnerGone`. - Same-space PID rules do not change. A dead PID is gone, a live PID with no start time is unprovable, and this server's own boot is alive. - `server/src/__tests__/runtime-skill-cache.test.ts`: a new test for a holder in another process space whose deployment the platform replaced. - The proof is bounded by the lock wait (5 s, never past the deadline). A timed-out or failed proof keeps the lock held. A proof still running is joined by later waits on the same holder, so a stalled read adds at most one database query per holder. - `server/src/services/controller-process-identity.ts`: removes `controllerProcessExited`, which has no caller left; its test assertions now check `sameProcessSpace`. ## Verification - `npx vitest run src/__tests__/runtime-skill-cache.test.ts src/services/controller-process-space-identity.test.ts` (from `server/`): 31 passed. - New: "releases a lock whose holder ran in a deployment the platform replaced". It fails on `master` (`expected false to be true`) and passes with this change. - The same test keeps the lock held for: the same deployment ID, another volume, a non-exclusive volume, another service, and no recorded deployment. - The existing test "never treats age, another host, or a legacy holder as proof that a lock holder exited" still passes. Without a deployment or controller lease proof, another host stays held. - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` selected 2504 of 3294 test files (the change reaches `owner-proof.ts` importers) and hit my 30 min local limit with no failure seen. Required CI supplies the broad coverage. - Live proof: not done (tests only). ## Risks - Across process spaces, a held lock can now trigger a read of `controller_instances` for the holder's boot. This happens at most once per second, and only while a lock is held. Uncontended preparation does no extra work. - The prover may fence a boot row whose lease lapsed past its grace, as it already does for every other recorded owner. - No persisted shape changes. Older lock files without a `controller` field stay held, as before. > 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 - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use and code execution. ## 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 <noreply@anthropic.com>
217 lines
9.7 KiB
TypeScript
217 lines
9.7 KiB
TypeScript
import { createHash, randomUUID } from "node:crypto";
|
|
import { execFileSync } from "node:child_process";
|
|
import { readFileSync, readlinkSync } from "node:fs";
|
|
import { hostname } from "node:os";
|
|
import { readProcessStartedAt } from "./hot-restart.js";
|
|
import { currentWorkspaceWorker, type WorkspaceWorker } from "./native-runtime/native-workspace-platform-proof.js";
|
|
|
|
/**
|
|
* Identity of the server process that started a provider side effect, saved
|
|
* with its durable record. After a restart, recovery uses it to prove that the
|
|
* starter has exited, so the side effect cannot still be in flight there.
|
|
*/
|
|
export type ControllerDeployment = { worker: WorkspaceWorker; exclusiveVolume: boolean; volumeId: string | null };
|
|
export type ControllerProcess = {
|
|
bootId: string; pid: number; hostHash: string; processStartedAt: string | null; deployment?: ControllerDeployment;
|
|
};
|
|
|
|
export function currentControllerDeployment(env: NodeJS.ProcessEnv = process.env): ControllerDeployment | null {
|
|
const worker = currentWorkspaceWorker(env);
|
|
if (!worker) return null;
|
|
const volumeId = env.RAILWAY_VOLUME_ID?.trim() || null;
|
|
return { worker, exclusiveVolume: Boolean(volumeId && env.RAILWAY_VOLUME_MOUNT_PATH?.trim()), volumeId };
|
|
}
|
|
|
|
function deploymentField(): { deployment?: ControllerDeployment } {
|
|
const deployment = currentControllerDeployment();
|
|
return deployment ? { deployment } : {};
|
|
}
|
|
|
|
function readDeployment(value: unknown): ControllerDeployment | null {
|
|
if (!value || typeof value !== "object" || Array.isArray(value)) return null;
|
|
const record = value as Record<string, unknown>;
|
|
const worker = record.worker as Record<string, unknown> | null | undefined;
|
|
if (!worker || typeof worker !== "object" || worker.provider !== "railway") return null;
|
|
const ids = [worker.projectId, worker.environmentId, worker.serviceId, worker.deploymentId];
|
|
if (ids.some(id => typeof id !== "string" || id.length === 0)) return null;
|
|
return { worker: { provider: "railway", projectId: ids[0] as string, environmentId: ids[1] as string,
|
|
serviceId: ids[2] as string, deploymentId: ids[3] as string },
|
|
exclusiveVolume: record.exclusiveVolume === true,
|
|
volumeId: typeof record.volumeId === "string" ? record.volumeId : null };
|
|
}
|
|
|
|
/**
|
|
* The one boot id of this server process. Native coordinator claims
|
|
* (`native_run_finalizations.controller_boot_id`), legacy controller leases
|
|
* (`heartbeat_runs.controller_boot_id`), sandbox creation intents and teardown
|
|
* claims all stamp this value, so "owned by this process" means the same thing
|
|
* in every table. Records written by an older release carry that release's
|
|
* random per-module ids; those never equal this value, exactly as a record
|
|
* from any earlier boot never did.
|
|
*/
|
|
const bootId = randomUUID();
|
|
export const UNKNOWN_PROCESS_SPACE = "unknown";
|
|
|
|
/**
|
|
* Names the space in which PIDs are comparable: one kernel boot and, on Linux,
|
|
* one PID namespace. Containers that share a hostname but not a PID namespace
|
|
* get different values. When the space cannot be read, no record is judged.
|
|
* A redeploy onto another host or namespace also never matches; such records
|
|
* keep their original deadlines.
|
|
*/
|
|
function processSpaceHash(): string {
|
|
try {
|
|
const parts = [hostname()];
|
|
if (process.platform === "linux") {
|
|
parts.push(readFileSync("/proc/sys/kernel/random/boot_id", "utf8").trim(), readlinkSync("/proc/self/ns/pid"));
|
|
} else if (process.platform === "darwin") {
|
|
// One bounded, read-only kernel identity observation per module. Rounded
|
|
// boot-time estimates can collide across hosts and cannot authorize PID
|
|
// comparisons. Old weak hashes simply stop matching this stronger proof.
|
|
const bootSession = execFileSync("/usr/sbin/sysctl", ["-n", "kern.bootsessionuuid"], {
|
|
encoding: "utf8", timeout: 1_500, maxBuffer: 4_096, windowsHide: true,
|
|
}).trim().toLowerCase();
|
|
if (!/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/.test(bootSession)) return UNKNOWN_PROCESS_SPACE;
|
|
parts.push(bootSession);
|
|
} else {
|
|
return UNKNOWN_PROCESS_SPACE;
|
|
}
|
|
if (parts.some(part => !part)) return UNKNOWN_PROCESS_SPACE;
|
|
return createHash("sha256").update(parts.join("\0")).digest("hex").slice(0, 32);
|
|
} catch {
|
|
return UNKNOWN_PROCESS_SPACE;
|
|
}
|
|
}
|
|
|
|
const hostHash = processSpaceHash();
|
|
|
|
/** OS-observed start time of any PID (epoch ms), or null when it cannot be read. */
|
|
export async function observedProcessStartedAt(pid: number): Promise<number | null> {
|
|
try {
|
|
const startedAt = Date.parse(await readProcessStartedAt(pid) ?? "");
|
|
return Number.isFinite(startedAt) ? startedAt : null;
|
|
} catch {
|
|
return null;
|
|
}
|
|
}
|
|
|
|
// This process's own start time never changes, so it is read from the OS once.
|
|
// On macOS the read runs `ps -o lstart`; uncached, it ran on every native
|
|
// claim, preparation check and lease renewal. A failed read is not cached, so
|
|
// a transient failure is retried by the next caller.
|
|
let ownStartedAt: Promise<number | null> | undefined;
|
|
/** The start time from the last resolved read, for `currentControllerProcessStamp`. */
|
|
let resolvedProcessStartedAt: string | null = null;
|
|
|
|
/** OS-observed start time of this server process (epoch ms), read at most once
|
|
* successfully per process. Null when the OS cannot report it. */
|
|
export function currentProcessStartedAtMs(): Promise<number | null> {
|
|
if (!ownStartedAt) {
|
|
const read = observedProcessStartedAt(process.pid);
|
|
ownStartedAt = read;
|
|
void read.then(value => {
|
|
if (value === null && ownStartedAt === read) ownStartedAt = undefined;
|
|
// Any resolved read also feeds the no-I/O stamp.
|
|
if (value !== null) resolvedProcessStartedAt = new Date(value).toISOString();
|
|
});
|
|
}
|
|
return ownStartedAt;
|
|
}
|
|
|
|
/** Test seam: forget the cached start time so a test can stub the OS read. */
|
|
export function resetCurrentProcessStartedAtForTests(): void {
|
|
ownStartedAt = undefined;
|
|
}
|
|
|
|
export function currentControllerBootId(): string {
|
|
return bootId;
|
|
}
|
|
|
|
const stampListeners = new Set<() => void>();
|
|
/** Called (synchronously, without I/O) whenever this process stamps ownership of a record. */
|
|
export function onControllerStamp(listener: () => void): () => void {
|
|
stampListeners.add(listener);
|
|
return () => { stampListeners.delete(listener); };
|
|
}
|
|
function notifyStamp() {
|
|
for (const listener of stampListeners) {
|
|
try { listener(); } catch { /* a listener never blocks a stamp */ }
|
|
}
|
|
}
|
|
|
|
export async function currentControllerProcess(): Promise<ControllerProcess> {
|
|
const startedAt = await currentProcessStartedAtMs();
|
|
const iso = startedAt === null ? null : new Date(startedAt).toISOString();
|
|
resolvedProcessStartedAt = iso;
|
|
notifyStamp();
|
|
return { bootId, pid: process.pid, hostHash, processStartedAt: iso, ...deploymentField() };
|
|
}
|
|
|
|
/**
|
|
* This process's identity without any I/O, for stamps written on the turn
|
|
* path. The start time is present only once an earlier call resolved it. A
|
|
* stamp without one still proves exit when its PID is gone in this PID space;
|
|
* a live PID then stays ambiguous, which fails closed.
|
|
*/
|
|
export function currentControllerProcessStamp(): ControllerProcess {
|
|
notifyStamp();
|
|
return { bootId, pid: process.pid, hostHash, processStartedAt: resolvedProcessStartedAt, ...deploymentField() };
|
|
}
|
|
|
|
export function readControllerProcess(value: unknown): ControllerProcess | null {
|
|
if (!value || typeof value !== "object" || Array.isArray(value)) return null;
|
|
const record = value as Record<string, unknown>;
|
|
if (typeof record.bootId !== "string" || typeof record.hostHash !== "string" ||
|
|
typeof record.pid !== "number" || !Number.isInteger(record.pid) || record.pid <= 0) return null;
|
|
const deployment = readDeployment(record.deployment);
|
|
return { bootId: record.bootId, pid: record.pid, hostHash: record.hostHash,
|
|
processStartedAt: typeof record.processStartedAt === "string" ? record.processStartedAt : null,
|
|
...(deployment ? { deployment } : {}) };
|
|
}
|
|
|
|
/**
|
|
* True when some process owns the PID in this PID space. EPERM means the
|
|
* process exists but belongs to another user, so it counts as alive.
|
|
*/
|
|
export function processExists(pid: number): boolean {
|
|
try {
|
|
process.kill(pid, 0);
|
|
return true;
|
|
} catch (error) {
|
|
return (error as NodeJS.ErrnoException | undefined)?.code === "EPERM";
|
|
}
|
|
}
|
|
|
|
/** Process start times closer than this may be the same process. */
|
|
export const PROCESS_START_PRECISION_MS = 1_000;
|
|
|
|
/**
|
|
* True only when two start times for one PID prove different processes.
|
|
* `ps -o lstart` on macOS and BSD has whole-second precision, while a recorded
|
|
* start can come from a millisecond clock, so a sub-second gap is ambiguous
|
|
* and fails closed (same process). A missing or unparsable time proves nothing.
|
|
*/
|
|
export function processStartsDefinitelyDiffer(
|
|
recorded: Date | string | null | undefined,
|
|
observed: Date | string | null | undefined,
|
|
): boolean {
|
|
const left = recorded instanceof Date ? recorded.getTime() : Date.parse(recorded ?? "");
|
|
const right = observed instanceof Date ? observed.getTime() : Date.parse(observed ?? "");
|
|
return Number.isFinite(left) && Number.isFinite(right) && Math.abs(left - right) >= PROCESS_START_PRECISION_MS;
|
|
}
|
|
|
|
/** This process's PID-space hash. An unreadable space is stored as
|
|
* "unknown", which `sameProcessSpace` never accepts. */
|
|
export function currentProcessSpaceHash(): string {
|
|
return hostHash;
|
|
}
|
|
|
|
/**
|
|
* Whether a PID recorded under `recorded` can be judged in this process.
|
|
* True only for the same readable PID space. A different or unreadable space
|
|
* is never proof: a dead PID here says nothing about a process elsewhere.
|
|
*/
|
|
export function sameProcessSpace(recorded: string | null | undefined): boolean {
|
|
return typeof recorded === "string" && hostHash !== UNKNOWN_PROCESS_SPACE && recorded === hostHash;
|
|
}
|