mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 08:53:19 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - One server process prepares native runs, renews every controller lease on timers, and serves every request > - On a run path, the server hashed the 31 MB runner binary with `readFileSync` (more than once per run), hashed provider packs and harness backups synchronously, and ran `tar` with `execFileSync` (timeouts up to 60 s, one with no timeout) > - On a slow disk, each of these calls freezes the whole process. A 103 s stall on a hosted instance lapsed every lease. The last event before that stall was immediately before a synchronous read and hash of the runner binary (see #20694 and #20709) > - This pull request moves those reads, hashes and archive commands off the event loop. It computes the runner binary digest once per boot > - The benefit is that a slow disk delays only the run that waits for it. Turns also start faster, because the runner binary is no longer re-read for every run ## Linked Issues or Issue Description Refs #20694, #20709 **What happened?** On a hosted instance, the server process stopped for 103 s without a restart. Every controller lease lapsed, and two tasks needed a manual Retry. The last run event before the gap was immediately before `matchesControllerRunnerArtifact`, which does `createHash("sha256").update(readFileSync(controllerRunnerBinary))`. Four workspace restores were running at the same time, and file I/O was very slow (a 43 s archive hash, 20 s snapshot decrypts). The trigger is not proven. But each synchronous call below blocks the event loop for as long as the disk is slow. **Expected behavior** File hashing and archive commands on a run path must not block the event loop. A slow disk must delay only the run that waits for it. **Steps to reproduce** 1. Make a remote runner binary slow to read (the test uses a FIFO whose writer waits), or make `tar` slow (the test puts a `tar` wrapper that sleeps first on `PATH`). 2. Call `resolveRemotePaperclipRunnerBinary`, `stageRemoteRunnerDirectory`, or `syncRemoteRunnerDirectoryOut` while a 10 ms timer runs. 3. Before this change, the timer stops for the whole slow step (814 ms, 1,764 ms and 3,379 ms). After this change, it keeps firing. ## What Changed - `server/src/services/native-runtime/off-loop-file-work.ts` (new): - `sha256FileStreamed` - `cachedFileSha256`: a streamed digest, reused while the file's device, inode, size, mtime and ctime do not change - `binaryTargetFromHeader`: reads 64 KiB; reads the rest of the file only when the header cannot answer, so the result equals `runnerBinaryTarget(readFileSync(path))` - `sha256DirectoryTreeStreamed` - `execFileBuffered`: `execFile` with stdin input, `maxBuffer`, `timeout` and `env`. It rejects on a non-zero exit, a timeout or a `maxBuffer` overflow, like `execFileSync`. - `native-session-executor.ts`: - `matchesControllerRunnerArtifact` uses `cachedFileSha256`. Each boot reads the binary once, not once or twice for every run. - The remote Claude daemon target is read from the binary header. - The bundled remote runner binary is checked by `verifiedBundledRemoteRunnerBinary`: the same checks and errors, with a cached digest. - Provider-pack upload verification uses the new `verifyRemoteProviderPackManifest`. It runs the same checks, in the same order, with the same errors. One generator drives both the sync and the streamed readers. - Harness backup build and verify on run paths use streamed digests (`buildNativeHarnessBackupManifestStreamed`, `verifyNativeHarnessBackupStreamed`). The digest format does not change. - `stageRemoteRunnerDirectory` (3 calls), `assertSafeRemoteCheckpointArchive` (3 calls) and the checkpoint extract use `execFileBuffered`, with the same arguments, timeouts and buffer limits. `stageRemoteRunnerFile` uses `readFile`. - `packages/paperclip-runner/src/live/runner-binary.ts`: the default daemon lookup (`resolvePackagedRunnerBinary`, used by the local and remote resolvers) classifies the daemon with `runnerBinaryFileTarget`. It reads the 64 KiB header, not the whole file. - `packages/paperclip-runner/src/live/bundled-remote-provider-pack.ts`: the release-manifest checks now live in one helper, `bundledRemoteRunnerRelease`. The sync `bundledRemoteRunnerBinary` and the server's streamed check both use it. - `native-codex-runner.ts`: - `resolveRemotePaperclipRunnerBinary` reads only the header. - The local `runnerDigest` uses `cachedFileSha256`. - New `verifiedBundledRemoteRunnerBinary`. **Ordering and atomicity.** I checked each converted call for code that relied on the sync call being atomic with respect to other runs: - **Checkpoint copy-back.** It extracts into a private `mkdtemp` directory. The replace sequence (`existsSync`, `rename`, `rename`, `rm`) stays synchronous, so no other run can see a half-installed checkpoint. This function already awaited a remote command before the extract, so this change adds no new interleaving with the target. - **Harness backup verify then restore.** The verify was sync, but the restore after it was already async. The window between them existed before. One owner serializes the turns of a session, so no other turn publishes a backup for that session while a turn verifies it. - **Staging and provider-pack hashing.** These read build-owned, immutable trees, or the private staging copies of a run. ## Verification - Red/green: `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/off-loop-file-work.test.ts` - With `native-session-executor.ts` and `native-codex-runner.ts` from `origin/master`: 3 failed. The longest timer gaps were 814 ms (runner binary target read), 1,764 ms (directory staging tar) and 3,379 ms (checkpoint list, size check and extract). - With this change: 6 passed. The gaps stay under 400 ms. The same file also checks the digest cache, the header fallback, and the `execFileSync`-like errors (exit code, timeout kill, `ERR_CHILD_PROCESS_STDIO_MAXBUFFER`). - `native-session-executor.test.ts`: - The streamed provider-pack verifier returns the same manifest as the sync reader, and rejects a changed asset tree with the same error. - The streamed harness backup manifest equals the sync one. - The streamed verify agrees with the sync verify, for a valid backup and for a corrupt one. - Run together: `native-session-executor`, `off-loop-file-work`, `native-codex-runner`, `setup-readiness`, `antigravity-connection-probe` (1,653 passed). `run-completion.test.ts` passed all 153 tests. Its `afterAll` embedded-Postgres cleanup timed out locally, which is not related to this change; CI runs it. - `pnpm --filter @paperclipai/server exec tsc --noEmit` passes. ## Risks - **Turn start.** This change adds no wait before `turn.start`; it removes work. The first run of a boot streams the runner binary once. Later runs reuse the digest after one `stat`. - **Cache staleness.** The cache keys on device, inode, size, mtime and ctime. An in-place rewrite changes mtime and ctime, and a replaced file changes its inode, so a changed binary is hashed again. - **stderr.** `execFileSync` wrote a child's stderr to the server's stderr. `execFileBuffered` captures it. Tar warnings no longer appear in server logs; errors and exit codes are unchanged. - **Persisted state.** No persisted shape changes. Old saved state and new code (and the reverse) read the same manifests and digests. - **Not converted in this PR:** small metadata syscalls (`existsSync`, `lstatSync`, `readdirSync`, small JSON reads and writes, renames) in the native state machines. Some of these rely on running synchronously. Also not converted: the sync harness backup verify inside the sync restart-authority checks, and the sync stamp verify in `native-harness-backup-storage.ts`. Both are listed for a follow-up. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude Code with tool use, shell, 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 (code comments carry the contract) - [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>
135 lines
5.2 KiB
TypeScript
135 lines
5.2 KiB
TypeScript
import { execFile } from "node:child_process";
|
|
import { createHash } from "node:crypto";
|
|
import { createReadStream } from "node:fs";
|
|
import { open, readdir, readlink, stat } from "node:fs/promises";
|
|
import { join } from "node:path";
|
|
|
|
// File hashing and archive commands on a run path must not block the event
|
|
// loop: on a slow disk one synchronous read or `execFileSync` freezes every
|
|
// lease renewal and every request in the process. These helpers keep the
|
|
// result and the error shape of the synchronous calls they replace.
|
|
|
|
/** `sha256:<hex>` of a file, streamed. */
|
|
export async function sha256FileStreamed(path: string, signal?: AbortSignal): Promise<string> {
|
|
const hash = createHash("sha256");
|
|
for await (const chunk of createReadStream(path, { signal })) hash.update(chunk as Buffer);
|
|
return `sha256:${hash.digest("hex")}`;
|
|
}
|
|
|
|
type CachedDigest = { key: string; digest: Promise<string> };
|
|
const digestCache = new Map<string, CachedDigest>();
|
|
const DIGEST_CACHE_LIMIT = 64;
|
|
|
|
/**
|
|
* `sha256:<hex>` of an immutable artifact such as the runner binary. The digest
|
|
* is reused while the file's device, inode, size, modification and change times
|
|
* are unchanged, so every run of a boot reads it at most once.
|
|
*/
|
|
export async function cachedFileSha256(path: string): Promise<string> {
|
|
const metadata = await stat(path, { bigint: true });
|
|
const key = [metadata.dev, metadata.ino, metadata.size, metadata.mtimeNs, metadata.ctimeNs].join(":");
|
|
const cached = digestCache.get(path);
|
|
if (cached?.key === key) return cached.digest;
|
|
const digest = sha256FileStreamed(path);
|
|
digestCache.set(path, { key, digest });
|
|
if (digestCache.size > DIGEST_CACHE_LIMIT) digestCache.delete(digestCache.keys().next().value!);
|
|
digest.catch(() => {
|
|
if (digestCache.get(path)?.digest === digest) digestCache.delete(path);
|
|
});
|
|
return digest;
|
|
}
|
|
|
|
/** Test seam: forget every cached digest. */
|
|
export function clearCachedFileSha256(): void {
|
|
digestCache.clear();
|
|
}
|
|
|
|
const HEADER_BYTES = 64 * 1024;
|
|
|
|
/**
|
|
* The executable target of a native binary from its header. Reads the rest of
|
|
* the file only when the header alone cannot answer (a PE header past 64 KiB),
|
|
* so the result always equals `classify(readFileSync(path))`.
|
|
*/
|
|
export async function binaryTargetFromHeader(
|
|
path: string,
|
|
classify: (bytes: Buffer) => string | null,
|
|
): Promise<string | null> {
|
|
const handle = await open(path, "r");
|
|
try {
|
|
const header = Buffer.alloc(HEADER_BYTES);
|
|
let length = 0;
|
|
let ended = false;
|
|
while (length < HEADER_BYTES) {
|
|
const { bytesRead } = await handle.read(header, length, HEADER_BYTES - length, null);
|
|
if (bytesRead === 0) { ended = true; break; }
|
|
length += bytesRead;
|
|
}
|
|
const target = classify(header.subarray(0, length));
|
|
if (target !== null || ended) return target;
|
|
return classify(Buffer.concat([header.subarray(0, length), await handle.readFile()]));
|
|
} finally {
|
|
await handle.close();
|
|
}
|
|
}
|
|
|
|
/** `sha256:<hex>` of a directory tree, in the format of `sha256DirectoryTree`. */
|
|
export async function sha256DirectoryTreeStreamed(
|
|
root: string,
|
|
unsupported: (relativePath: string) => Error,
|
|
): Promise<string> {
|
|
const hash = createHash("sha256");
|
|
const visit = async (directory: string, prefix = ""): Promise<void> => {
|
|
const entries = (await readdir(directory, { withFileTypes: true })).sort(
|
|
(left, right) => left.name.localeCompare(right.name),
|
|
);
|
|
for (const entry of entries) {
|
|
const relativePath = prefix ? `${prefix}/${entry.name}` : entry.name;
|
|
const absolutePath = join(directory, entry.name);
|
|
if (entry.isDirectory()) {
|
|
hash.update(`directory\0${relativePath}\n`);
|
|
await visit(absolutePath, relativePath);
|
|
} else if (entry.isFile()) {
|
|
hash.update(`file\0${relativePath}\0${await sha256FileStreamed(absolutePath)}\n`);
|
|
} else if (entry.isSymbolicLink()) {
|
|
hash.update(`symlink\0${relativePath}\0${await readlink(absolutePath)}\n`);
|
|
} else {
|
|
throw unsupported(relativePath);
|
|
}
|
|
}
|
|
};
|
|
await visit(root);
|
|
return `sha256:${hash.digest("hex")}`;
|
|
}
|
|
|
|
export type FileCommandOptions = {
|
|
input?: Buffer;
|
|
maxBuffer: number;
|
|
timeout?: number;
|
|
env?: NodeJS.ProcessEnv;
|
|
};
|
|
|
|
/**
|
|
* Runs a command like `execFileSync(file, args, { encoding: "buffer", ... })`,
|
|
* without blocking the event loop: it resolves with stdout and rejects when the
|
|
* command exits non-zero, is killed by the timeout, or exceeds `maxBuffer`.
|
|
*/
|
|
export function execFileBuffered(file: string, args: readonly string[], options: FileCommandOptions): Promise<Buffer> {
|
|
return new Promise((resolveOutput, rejectOutput) => {
|
|
const child = execFile(file, [...args], {
|
|
encoding: "buffer",
|
|
maxBuffer: options.maxBuffer,
|
|
...(options.timeout === undefined ? {} : { timeout: options.timeout }),
|
|
...(options.env ? { env: options.env } : {}),
|
|
}, (error, stdout) => {
|
|
if (error) rejectOutput(error);
|
|
else resolveOutput(stdout);
|
|
});
|
|
// A command can exit before it reads all of its input; its exit status,
|
|
// not the broken pipe, decides the result (as with execFileSync).
|
|
child.stdin?.on("error", () => undefined);
|
|
if (options.input) child.stdin?.end(options.input);
|
|
else child.stdin?.end();
|
|
});
|
|
}
|