mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 06:10:44 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Each heartbeat run gets a scratch directory in the system temp folder > - Each process attempt makes a new directory with a marker file > - Cleanup removes only the directory of the current attempt > - A restart kills the old attempt, so its directory stays forever > - One restart storm left 83 directories across 78 runs > - This pull request sweeps those directories when the run ends and at startup recovery > - The benefit is that temp space no longer fills with leftover directories ## Linked Issues or Issue Description **What happened?** Run scratch directories made before a restart are never removed. A restart storm left 83 `paperclip-run-*` directories across 78 runs. Each holds only `.paperclip-run-scratch.json`. **Steps to reproduce** 1. Start a run that makes a scratch directory. 2. Kill the server process and start it again. 3. Let the run finish. 4. List `$TMPDIR/paperclip-run-*`. The old directory is still there. **Expected behavior** The old directory is removed after the run ends, or at startup. ## What Changed - Add `sweepOrphanedRunScratch` in `server/src/services/run-scratch.ts`. - The sweep removes a directory only if it has a valid regular-file marker and its run is terminal in this instance database. It keeps a directory whose run it cannot see, because another instance can share the temp root. - The sweep keeps active runs, unmarked directories, symlinks, and directories outside `paperclip-run-*`. - The sweep checks each directory again before removal and bounds the scan (500, with a rotating start so kept entries do not starve later ones) and removals (100). A per-run sweep filters by directory name first. Marker run ids that are not UUIDs never reach the database query. - Call the sweep after the current attempt's cleanup in `heartbeat.ts`, for that run only. - Call the sweep in `reapOrphanedRuns` (startup recovery) with run status from the database. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/services/run-scratch.test.ts` - New tests cover: leftover directories of a terminal run are removed, an active run is kept, an unmarked directory is kept, a symlink is not followed, and removals are bounded. ## Risks - Low risk. The sweep acts only on marked directories of terminal runs. - A directory left by a previous attempt could hold a live grandchild process. The sweep does not check old process groups. The run is already terminal. ## Model Used - Claude, claude-opus-4-5 class model via 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 Opus 5.5 <noreply@anthropic.com>
237 lines
8.8 KiB
TypeScript
237 lines
8.8 KiB
TypeScript
import fs from "node:fs/promises";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { afterEach, describe, expect, it } from "vitest";
|
|
import {
|
|
HEARTBEAT_RUN_SCRATCH_MARKER,
|
|
buildHeartbeatRunScratchEnv,
|
|
cleanupHeartbeatRunScratch,
|
|
prepareHeartbeatRunScratch,
|
|
sweepOrphanedRunScratch,
|
|
type HeartbeatRunScratch,
|
|
type RunScratchSweepRunState,
|
|
} from "./run-scratch.js";
|
|
|
|
const cleanupDirs = new Set<string>();
|
|
|
|
async function trackScratch(scratch: HeartbeatRunScratch) {
|
|
cleanupDirs.add(scratch.dir);
|
|
return scratch;
|
|
}
|
|
|
|
afterEach(async () => {
|
|
await Promise.all(
|
|
Array.from(cleanupDirs, (dir) =>
|
|
fs.rm(dir, { recursive: true, force: true }).catch(() => undefined),
|
|
),
|
|
);
|
|
cleanupDirs.clear();
|
|
});
|
|
|
|
describe("heartbeat run scratch cleanup", () => {
|
|
it("removes only a marked run-owned scratch directory", async () => {
|
|
const scratch = await trackScratch(await prepareHeartbeatRunScratch({
|
|
companyId: "company-1",
|
|
agentId: "agent-1",
|
|
runId: "run-1",
|
|
issueId: "issue-1",
|
|
issueIdentifier: "PAP-13071",
|
|
now: new Date("2026-07-08T00:00:00.000Z"),
|
|
}));
|
|
await fs.writeFile(path.join(scratch.dir, "tool-cache.txt"), "cache");
|
|
|
|
const result = await cleanupHeartbeatRunScratch({ scratch });
|
|
|
|
expect(result).toEqual({ removed: true, dir: scratch.dir });
|
|
await expect(fs.stat(scratch.dir)).rejects.toMatchObject({ code: "ENOENT" });
|
|
});
|
|
|
|
it("preserves paperclip-named directories without the ownership marker", async () => {
|
|
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-run-unmarked-"));
|
|
cleanupDirs.add(dir);
|
|
const scratch: HeartbeatRunScratch = {
|
|
dir,
|
|
markerPath: path.join(dir, HEARTBEAT_RUN_SCRATCH_MARKER),
|
|
metadata: {
|
|
version: 1,
|
|
companyId: "company-1",
|
|
agentId: "agent-1",
|
|
runId: "run-1",
|
|
issueId: null,
|
|
issueIdentifier: null,
|
|
createdAt: new Date("2026-07-08T00:00:00.000Z").toISOString(),
|
|
},
|
|
};
|
|
|
|
const result = await cleanupHeartbeatRunScratch({ scratch });
|
|
|
|
expect(result).toEqual({ removed: false, dir, reason: "unmarked" });
|
|
await expect(fs.stat(dir)).resolves.toMatchObject({ isDirectory: expect.any(Function) });
|
|
});
|
|
|
|
it("preserves marked scratch when the marker owner does not match the run", async () => {
|
|
const scratch = await trackScratch(await prepareHeartbeatRunScratch({
|
|
companyId: "company-1",
|
|
agentId: "agent-1",
|
|
runId: "run-1",
|
|
}));
|
|
const mismatched = {
|
|
...scratch,
|
|
metadata: {
|
|
...scratch.metadata,
|
|
runId: "run-2",
|
|
},
|
|
};
|
|
|
|
const result = await cleanupHeartbeatRunScratch({ scratch: mismatched });
|
|
|
|
expect(result).toEqual({ removed: false, dir: scratch.dir, reason: "owner_mismatch" });
|
|
await expect(fs.stat(scratch.dir)).resolves.toMatchObject({ isDirectory: expect.any(Function) });
|
|
});
|
|
|
|
it("skips cleanup while the run process group is still alive", async () => {
|
|
const scratch = await trackScratch(await prepareHeartbeatRunScratch({
|
|
companyId: "company-1",
|
|
agentId: "agent-1",
|
|
runId: "run-1",
|
|
}));
|
|
|
|
const result = await cleanupHeartbeatRunScratch({
|
|
scratch,
|
|
processGroupId: 123,
|
|
isProcessGroupAlive: () => true,
|
|
});
|
|
|
|
expect(result).toEqual({ removed: false, dir: scratch.dir, reason: "process_group_alive" });
|
|
await expect(fs.stat(scratch.dir)).resolves.toMatchObject({ isDirectory: expect.any(Function) });
|
|
});
|
|
|
|
it("builds explicit scratch env without clobbering configured temp dirs", async () => {
|
|
const scratch = await trackScratch(await prepareHeartbeatRunScratch({
|
|
companyId: "company-1",
|
|
agentId: "agent-1",
|
|
runId: "run-1",
|
|
}));
|
|
|
|
const result = buildHeartbeatRunScratchEnv({ TMPDIR: "/custom/tmp" }, scratch);
|
|
|
|
expect(result.env.PAPERCLIP_RUN_SCRATCH_DIR).toBe(scratch.dir);
|
|
expect(result.env.PAPERCLIP_TASK_SCRATCH_DIR).toBe(scratch.dir);
|
|
expect(result.env.PAPERCLIP_SCRATCH_DIR).toBe(scratch.dir);
|
|
expect(result.env.PAPERCLIP_TMPDIR).toBe(scratch.dir);
|
|
expect(result.env.TMPDIR).toBeUndefined();
|
|
expect(result.env.TEMP).toBe(scratch.dir);
|
|
expect(result.env.TMP).toBe(scratch.dir);
|
|
expect(result.tempKeysApplied).toEqual(["TEMP", "TMP"]);
|
|
});
|
|
});
|
|
|
|
describe("orphaned run scratch sweep", () => {
|
|
async function makeRoot() {
|
|
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-sweep-test-"));
|
|
cleanupDirs.add(root);
|
|
return root;
|
|
}
|
|
async function leave(root: string, runId: string, createdAt = new Date().toISOString()) {
|
|
const dir = await fs.mkdtemp(path.join(root, `paperclip-run-x-${runId}-`));
|
|
await fs.writeFile(
|
|
path.join(dir, HEARTBEAT_RUN_SCRATCH_MARKER),
|
|
JSON.stringify({ version: 1, companyId: "c", agentId: "a", runId, issueId: null, issueIdentifier: null, createdAt }),
|
|
);
|
|
return dir;
|
|
}
|
|
const exists = (p: string) => fs.stat(p).then(() => true, () => false);
|
|
const states = (m: Record<string, RunScratchSweepRunState>) => async (ids: string[]) =>
|
|
new Map(ids.map((id) => [id, m[id] ?? "missing"] as const));
|
|
|
|
it("removes leftover attempt dirs of a terminal run, keeps active and unmarked dirs", async () => {
|
|
const root = await makeRoot();
|
|
const killedA = await leave(root, "run-done");
|
|
const killedB = await leave(root, "run-done");
|
|
const active = await leave(root, "run-live");
|
|
const unmarked = await fs.mkdtemp(path.join(root, "paperclip-run-x-nomarker-"));
|
|
const foreign = await fs.mkdtemp(path.join(root, "other-"));
|
|
|
|
const result = await sweepOrphanedRunScratch({
|
|
tmpRoot: root,
|
|
resolveRunStates: states({ "run-done": "terminal", "run-live": "active" }),
|
|
});
|
|
|
|
expect(result.removed.sort()).toEqual([killedA, killedB].sort());
|
|
expect(await exists(killedA)).toBe(false);
|
|
expect(await exists(killedB)).toBe(false);
|
|
expect(await exists(active)).toBe(true);
|
|
expect(await exists(unmarked)).toBe(true);
|
|
expect(await exists(foreign)).toBe(true);
|
|
});
|
|
|
|
it("limits a per-run sweep to that run", async () => {
|
|
const root = await makeRoot();
|
|
const a = await leave(root, "run-a");
|
|
const b = await leave(root, "run-b");
|
|
await sweepOrphanedRunScratch({
|
|
tmpRoot: root,
|
|
runId: "run-a",
|
|
resolveRunStates: states({ "run-a": "terminal", "run-b": "terminal" }),
|
|
});
|
|
expect(await exists(a)).toBe(false);
|
|
expect(await exists(b)).toBe(true);
|
|
});
|
|
|
|
it("keeps dirs whose run this instance cannot see, however old", async () => {
|
|
const root = await makeRoot();
|
|
const foreignRun = await leave(root, "run-other-instance", new Date(Date.now() - 48 * 3600_000).toISOString());
|
|
await sweepOrphanedRunScratch({ tmpRoot: root, resolveRunStates: states({}) });
|
|
expect(await exists(foreignRun)).toBe(true);
|
|
});
|
|
|
|
it("reaches orphans past the scan limit on repeated sweeps", async () => {
|
|
const root = await makeRoot();
|
|
const kept = [await leave(root, "run-keep1"), await leave(root, "run-keep2")];
|
|
const orphan = await leave(root, "run-zzorphan");
|
|
const resolveRunStates = states({ "run-keep1": "active", "run-keep2": "active", "run-zzorphan": "terminal" });
|
|
for (let i = 0; i < 3 && (await exists(orphan)); i += 1) {
|
|
await sweepOrphanedRunScratch({ tmpRoot: root, maxScan: 2, resolveRunStates });
|
|
}
|
|
expect(await exists(orphan)).toBe(false);
|
|
expect(await exists(kept[0]!)).toBe(true);
|
|
expect(await exists(kept[1]!)).toBe(true);
|
|
});
|
|
|
|
it("finds a run's dirs for a per-run sweep regardless of unrelated entries", async () => {
|
|
const root = await makeRoot();
|
|
for (let i = 0; i < 4; i += 1) await leave(root, `run-other${i}`);
|
|
const target = await leave(root, "run-target");
|
|
await sweepOrphanedRunScratch({
|
|
tmpRoot: root,
|
|
runId: "run-target",
|
|
maxScan: 1,
|
|
resolveRunStates: states({ "run-target": "terminal" }),
|
|
});
|
|
expect(await exists(target)).toBe(false);
|
|
});
|
|
|
|
it("does not follow symlinks and bounds removals per sweep", async () => {
|
|
const root = await makeRoot();
|
|
const outside = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-sweep-outside-"));
|
|
cleanupDirs.add(outside);
|
|
await fs.writeFile(
|
|
path.join(outside, HEARTBEAT_RUN_SCRATCH_MARKER),
|
|
JSON.stringify({ version: 1, companyId: "c", agentId: "a", runId: "run-t", createdAt: new Date().toISOString() }),
|
|
);
|
|
await fs.symlink(outside, path.join(root, "paperclip-run-x-link"));
|
|
const dirs = [await leave(root, "run-t"), await leave(root, "run-t"), await leave(root, "run-t")];
|
|
|
|
const result = await sweepOrphanedRunScratch({
|
|
tmpRoot: root,
|
|
maxRemove: 2,
|
|
resolveRunStates: states({ "run-t": "terminal" }),
|
|
});
|
|
|
|
expect(await exists(outside)).toBe(true);
|
|
expect(result.removed).toHaveLength(2);
|
|
expect(result.truncated).toBe(true);
|
|
expect((await Promise.all(dirs.map(exists))).filter(Boolean)).toHaveLength(1);
|
|
});
|
|
});
|