fix(runner): keep requests responsive during retired backup cleanup (#18619)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Remote native runs keep durable harness backups for continuation and
recovery.
> - Publishing a new backup retires earlier backup directories in the
server process.
> - The synchronous permission walk and recursive removal block
unrelated requests.
> - This pull request makes that cleanup asynchronous and keeps
publication waiting for completion.
> - The benefit is continued request progress while large retired
backups are removed.

## Linked Issues or Issue Description

**What happened?**

Publishing a backup with many small files blocks the Node event loop
during directory permission repair and removal. A local comparison with
20,000 retired files reproduced 1.37–1.95 second event-loop stalls. This
is a pause candidate. A short hosted profile has not established it as
the dominant source of sustained CPU use.

**Expected behavior**

Unrelated requests can progress during cleanup. Publication still waits
for complete removal, preserves rollback on a failed lease stamp, and
leaves live files and symlink targets unchanged.

**Steps to reproduce**

Run the benchmark in `doc/performance/native-backup-publication.md`. It
compiles the actual publication service from a pinned baseline and the
working tree, prepares immutable retired backups, and sends foreground
HTTP probes from a separate process.

**Paperclip version or commit**

Measured baseline: `774ddc72713d6aa7cb516691c71f1fc23bcfa5ff`. The
measured treatment source is committed in this branch. Current head:
`0a69cf2d43c2f9fcf032d96c1dceb56163bf21ba`. The runtime patch is
unchanged from the measured source; later commits only strengthen the
test and benchmark evidence paths.

**Deployment mode**

Local compiled service on macOS arm64 with Node v24.22.2. No hosted
rollout has occurred.

Related work: #18380 changed retention scheduling. #13676 included the
existing immutable-backup cleanup behavior. Neither change makes
publication cleanup asynchronous. The GitHub duplicate search also found
#11929, which concerns database dumps rather than native harness
backups. No matching open event-loop cleanup issue was found.

## What Changed

- Use asynchronous filesystem operations for retired-backup cleanup.
Visit directories serially and skip unnecessary metadata calls for file
entries.
- Await cleanup before publication proceeds, during rollback, after
successful rotation, and in executor finalization.
- Exercise immutable current and previous backups in the publication
test. Preserve the existing symlink-isolation and rollback tests. Add a
real separate-process HTTP probe that must complete while publication is
still active.
- Add a repeatable service benchmark and document its fixture, metrics,
and acceptance limits.

## Verification

Draft status: focused correctness tests, the local comparison, and
repository typecheck pass. The complete local test gate is incomplete.
The build passed. Complete CI, hosted acceptance, and final-head review
remain pending.

- Final focused Vitest run: 2 files, 4 tests passed in 649 ms with the
normal server configuration. The regression warms a separate Node
client, waits for its HTTP request during the stamp callback, queues the
response for the next event-loop turn, and then allows final
retired-tree removal to start. The async treatment serves that response
before publication completes. The original synchronous cleanup fails the
same final regression in a separate source fixture. That expected
control failure is retained.
- Syntax and `git diff --check` passed.
- `pnpm -r typecheck` passed on head `332c57b88c` in 152.18 seconds,
including the native Runner build. Validation uses Node v24.22.2 and the
repository-pinned pnpm v9.15.4 with isolated dependencies.
- `pnpm test:run` was intentionally stopped after 1,390.24 seconds (exit
143). All six chat shards passed, covering all 1,817 parameterized cases
across their exact partitions. The general server phase was advancing
but incomplete. The repository timing manifest estimates another roughly
two hours for the serial remainder, so validation moved to final-head
parallel CI. This is not a full local test pass. The command began on
`332c57b88c`; later changes only refine the regression and benchmark,
and the final regression has its own focused pass.
- Benchmark failure/metadata checks pass in a tiny isolated Git fixture:
an injected worker exit 17 is reported in 0.44 seconds without hanging,
and the result includes a staged source change when the unstaged diff is
empty.
- `pnpm build` passed in 41.80 seconds on `9f7e4e669b`. The final commit
only releases a pending test probe on failure; runtime and build sources
are unchanged. An injected client exit verifies that final test failure
path exits promptly in 0.97 seconds instead of hanging.
- Three alternating baseline/treatment pairs used fresh fixtures with
100 directories × 100 files in each of two retired backups. Each
publication removed 20,000 files. A separate process sent HTTP probes at
fixed 5 ms intervals, including overdue arrivals.
- Every run verified the replacement manifest, complete cleanup, and
unchanged symlink-target contents and mode. All 2,395 HTTP requests
completed without errors. The largest measured generator delay was 8.8
ms.

| Metric | Baseline median (range), ms | Treatment median (range), ms |
| --- | ---: | ---: |
| Foreground request p95 | 1,357.7 (1,287.4–2,014.0) | 1.8 (1.7–25.9) |
| Maximum event-loop delay | 1,439.7 (1,371.5–1,954.5) | 19.9
(17.4–40.7) |
| Publication completion | 1,429.1 (1,360.2–1,952.9) | 1,439.5
(897.9–1,649.6) |
| Process CPU during publication | 1,194.1 (1,041.8–1,225.5) | 2,423.1
(1,995.7–2,514.7) |

The treatment served 180–329 requests while publication was active. The
baseline served zero during that interval. Process CPU includes
foreground work that the baseline cannot serve until cleanup completes.
The result supports responsiveness, with a CPU tradeoff. It does not
establish lower total CPU or a hosted latency gain.

Reproduce with Node v24.22.2:

```sh
pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/native-harness-backup-cleanup.test.ts src/services/native-runtime/native-harness-backup-publication.test.ts
node server/scripts/benchmark-native-backup-publication.mjs --baseline-ref 774ddc7271 --output /tmp/native-backup-publication.json
```

The benchmark records the baseline SHA, treatment HEAD and source diff.
Raw local evidence remains outside the repository. A first small smoke
run hit a sandbox restriction on loopback listeners; the authorized
retry resolved it. The smoke also exposed load-generator interval drift.
Fixed-clock arrivals replaced that generator before the reported
measurements.

Pending gates: complete current-head CI, final-head Greptile review, and
the real run/checkpoint/recovery journey. The full local test run
remains explicitly incomplete. The four concrete review findings were
fixed. The prior head received 5/5; final-head review remains pending.

Local evidence remains outside Git:
`/private/tmp/backup-publication-measurement-20261102.json`,
`/private/tmp/native-backup-focused-final.log`,
`/private/tmp/native-backup-typecheck.log`,
`/private/tmp/native-backup-test.log`,
`/private/tmp/native-backup-test-interruption.json`,
`/private/tmp/native-backup-build.log`, and
`/private/tmp/native-backup-harness-check.json`. The harness-check
receipt points to its isolated metadata and worker-failure logs. The
final client-failure and baseline-control logs are
`/private/tmp/backup-benchmark-check-gprcfiiz/failed-client-cleanup.log`
and
`/private/tmp/backup-benchmark-check-gprcfiiz/final-probe-baseline-control.log`.
These local paths are retained for the author; the commands above let
reviewers reproduce the synthetic proof.

## Risks

- Asynchronous removal adds filesystem-pool work and used more process
CPU in the fixture. Check resource pressure as well as responsiveness
before rollout.
- Filesystem operations can now interleave with unrelated work. The
existing session owner still controls publication. Cleanup remains
awaited; no deletion is detached and no scheduler is added.
- Errors must still fail publication or restore the prior backup as
before. Corruption checks and destructive lease authorization are
unchanged.
- This synthetic macOS fixture does not represent every Linux volume or
backup shape. It does not qualify a deployed fix.
- Rollout acceptance should confirm the serving SHA, checkpoint
completion, continuation/recovery, error rate, and loop delay under
matched backup load. Revert if cleanup or recovery fails.

## Model Used

OpenAI Codex, GPT-6 family, with tool use and code execution. The exact
served model ID and context-window size were not exposed to this agent.

## 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

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip authored and GitHub committed 2026-11-02 17:00:42 +00:00
1 parent 8424a51018
commit 3f7811eef0
7 files changed
+364 -17

No files matched your search

@@ -4,7 +4,7 @@ import { join } from "node:path";
import { expect, it } from "vitest";
import { removeNativeHarnessBackup } from "./native-harness-backup-cleanup.js";
it("removes retired backups containing immutable skills without modifying live files or symlink targets", () => {
it("removes retired backups containing immutable skills without modifying live files or symlink targets", async () => {
const base = mkdtempSync(join(tmpdir(), "paperclip-backup-cleanup-"));
const retired = join(base, "previous");
const nested = join(retired, "claude", "skills", "first-task");
@@ -19,12 +19,12 @@ it("removes retired backups containing immutable skills without modifying live f
chmodSync(retired, 0o555);
chmodSync(live, 0o555);
try {
removeNativeHarnessBackup(retired);
await removeNativeHarnessBackup(retired);
expect(existsSync(retired)).toBe(false);
expect(readFileSync(join(live, "SKILL.md"), "utf8")).toBe("live instructions");
expect(lstatSync(live).mode & 0o777).toBe(0o555);
expect(lstatSync(join(live, "SKILL.md")).mode & 0o777).toBe(0o444);
expect(() => removeNativeHarnessBackup(retired)).not.toThrow();
await expect(removeNativeHarnessBackup(retired)).resolves.toBeUndefined();
} finally {
for (const directory of [retired, join(retired, "claude", "skills"), nested, live]) {
if (existsSync(directory)) chmodSync(directory, 0o700);
@@ -1,18 +1,23 @@
import { chmodSync, lstatSync, readdirSync, rmSync } from "node:fs";
import { chmod, lstat, readdir, rm } from "node:fs/promises";
import { join } from "node:path";
/** Remove a retired controller-owned backup, never a live harness directory. */
export function removeNativeHarnessBackup(root: string): void {
export async function removeNativeHarnessBackup(root: string): Promise<void> {
// Archives preserve immutable skill directory modes. Deleting their children
// requires write permission on those directories, even with rm's force flag.
// Only change directories in the backup being discarded. lstat deliberately
// avoids following provider-created symlinks into live or unrelated trees.
const prepareDirectory = (path: string): void => {
const metadata = lstatSync(path, { throwIfNoEntry: false });
const prepareDirectory = async (path: string): Promise<void> => {
const metadata = await lstat(path).catch((error: NodeJS.ErrnoException) => {
if (error.code === "ENOENT") return undefined;
throw error;
});
if (!metadata?.isDirectory()) return;
chmodSync(path, (metadata.mode & 0o777) | 0o700);
for (const name of readdirSync(path)) prepareDirectory(join(path, name));
await chmod(path, (metadata.mode & 0o777) | 0o700);
for (const entry of await readdir(path, { withFileTypes: true })) {
if (entry.isDirectory()) await prepareDirectory(join(path, entry.name));
}
};
prepareDirectory(root);
rmSync(root, { recursive: true, force: true });
await prepareDirectory(root);
await rm(root, { recursive: true, force: true });
}
@@ -1,4 +1,6 @@
import { promises as fs } from "node:fs";
import { existsSync, promises as fs } from "node:fs";
import { spawn } from "node:child_process";
import { createServer, type ServerResponse } from "node:http";
import os from "node:os";
import path from "node:path";
import { afterEach, describe, expect, it } from "vitest";
@@ -32,7 +34,17 @@ describe("native harness backup publication", () => {
});
it("publishes the completed replacement once and removes its predecessor", async () => {
const input = await fixture(); const changes: unknown[] = [];
unsubscribe = onNativeHarnessBackupChange(input.db, change => changes.push(change));
const previous = path.join(backupRoot, "previous");
for (const root of [input.current, previous]) {
const skill = path.join(root, "skills", "immutable");
await fs.mkdir(skill, { recursive: true });
await fs.writeFile(path.join(skill, "SKILL.md"), "saved instructions", { mode: 0o444 });
await fs.chmod(skill, 0o555);
}
unsubscribe = onNativeHarnessBackupChange(input.db, change => {
expect(existsSync(previous)).toBe(false);
changes.push(change);
});
await publishNativeHarnessBackup({ ...input, stamp: async current => {
expect(changes).toEqual([]);
expect(await fs.readFile(path.join(current, "manifest.json"), "utf8")).toBe("replacement");
@@ -41,4 +53,85 @@ describe("native harness backup publication", () => {
expect(await fs.readdir(backupRoot)).toEqual(["current"]);
expect(changes).toEqual([{ companyId: input.companyId, scopeDigest: input.scopeDigest }]);
});
it("serves an unrelated request while a retired backup is being removed", async () => {
const input = await fixture();
for (let index = 0; index < 40; index++) {
const directory = path.join(input.current, `skill-${index}`);
await fs.mkdir(directory);
await Promise.all(Array.from({ length: 50 }, (_, file) =>
fs.writeFile(path.join(directory, `file-${file}`), "retired instructions", { mode: 0o444 })));
await fs.chmod(directory, 0o555);
}
let publishing = false;
let probeReceived: (response: ServerResponse | null) => void;
const probeReady = new Promise<ServerResponse | null>(resolve => { probeReceived = resolve; });
const server = createServer((request, response) => {
if (request.url === "/probe") { probeReceived(response); return; }
response.end("ok");
});
await new Promise<void>((resolve, reject) => {
server.once("error", reject);
server.listen(0, "127.0.0.1", resolve);
});
const address = server.address();
if (!address || typeof address === "string") throw new Error("HTTP probe did not start");
const client = spawn(process.execPath, ["--input-type=module", "-e", `
import { Agent, get } from "node:http";
const agent = new Agent({ keepAlive: true });
const request = path => new Promise((resolve, reject) => {
get({ hostname: "127.0.0.1", port: Number(process.argv[1]), path, agent }, response => {
response.resume();
response.on("end", () => resolve(response.headers["x-publication-active"]));
}).on("error", reject);
});
await request("/");
process.send({ kind: "ready" });
process.once("message", async () => {
const active = await request("/probe");
process.send({ kind: "result", active }, () => { agent.destroy(); process.disconnect(); });
});
`, String(address.port)], { stdio: ["ignore", "ignore", "inherit", "ipc"] });
const message = (kind: string) => new Promise<{ active?: string }>((resolve, reject) => {
const timer = setTimeout(() => { cleanup(); reject(new Error(`HTTP probe ${kind} timed out`)); }, 10_000);
const received = (value: { kind?: string; active?: string }) => {
if (value.kind === kind) { cleanup(); resolve(value); }
};
const failed = () => { cleanup(); reject(new Error(`HTTP probe exited before ${kind}`)); };
const cleanup = () => {
clearTimeout(timer); client.off("message", received); client.off("exit", failed); client.off("error", failed);
};
client.on("message", received); client.once("exit", failed); client.once("error", failed);
});
let publication: Promise<void> | undefined;
try {
await message("ready");
const response = message("result");
publishing = true;
publication = publishNativeHarnessBackup({
...input,
stamp: async () => {
client.send({ kind: "probe" });
const response = await probeReady;
if (!response) return;
setImmediate(() => {
response.setHeader("x-publication-active", publishing ? "1" : "0");
response.end("ok");
});
},
}).finally(() => { publishing = false; });
const result = await response;
await publication;
expect(result.active).toBe("1");
expect(await fs.readdir(backupRoot)).toEqual(["current"]);
expect(await fs.readFile(path.join(input.current, "manifest.json"), "utf8")).toBe("replacement");
} finally {
probeReceived!(null);
try { await publication; }
finally {
client.kill();
server.closeAllConnections();
await new Promise<void>(resolve => server.close(() => resolve()));
}
}
}, 15_000);
});
@@ -9,7 +9,7 @@ export async function publishNativeHarnessBackup(input: {
}) {
const currentRoot = resolve(input.backupRoot, "current");
const previousRoot = resolve(input.backupRoot, "previous");
removeNativeHarnessBackup(previousRoot);
await removeNativeHarnessBackup(previousRoot);
let movedCurrent = false;
if (existsSync(currentRoot)) {
renameSync(currentRoot, previousRoot);
@@ -19,10 +19,10 @@ export async function publishNativeHarnessBackup(input: {
renameSync(input.pendingRoot, currentRoot);
await input.stamp?.(currentRoot);
} catch (error) {
if (existsSync(currentRoot)) removeNativeHarnessBackup(currentRoot);
if (existsSync(currentRoot)) await removeNativeHarnessBackup(currentRoot);
if (movedCurrent && existsSync(previousRoot) && !existsSync(currentRoot)) renameSync(previousRoot, currentRoot);
throw error;
}
removeNativeHarnessBackup(previousRoot);
await removeNativeHarnessBackup(previousRoot);
noteNativeHarnessBackupChange(input.db, { companyId: input.companyId, scopeDigest: input.scopeDigest });
}
@@ -13734,7 +13734,7 @@ async function createRunnerdBackendWithinSessionClaim(
} : undefined,
});
} finally {
removeNativeHarnessBackup(pendingRoot);
await removeNativeHarnessBackup(pendingRoot);
}
},
{