mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(heartbeat): sweep orphaned run scratch directories (#19671)
## 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>
This commit is contained in:
1 parent
0b73c805e8
commit
29e05f7e06
5 files changed
+386
No files matched your search
@@ -0,0 +1,129 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { agents, companies, createDb, heartbeatRunEvents, heartbeatRuns } from "@paperclipai/db";
|
||||
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
||||
|
||||
vi.mock("../telemetry.ts", () => ({ getTelemetryClient: () => ({ track: vi.fn() }) }));
|
||||
vi.mock("../middleware/logger.js", () => ({
|
||||
logger: {
|
||||
child: vi.fn(function child(this: unknown) {
|
||||
return this;
|
||||
}),
|
||||
trace: vi.fn(),
|
||||
debug: vi.fn(),
|
||||
info: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
fatal: vi.fn(),
|
||||
},
|
||||
httpLogger: vi.fn(),
|
||||
}));
|
||||
|
||||
import { heartbeatService } from "../services/heartbeat.ts";
|
||||
import { HEARTBEAT_RUN_SCRATCH_MARKER } from "../services/run-scratch.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
describeEmbeddedPostgres("heartbeat recovery sweeps orphaned run scratch", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
let originalTmpdir: string | undefined;
|
||||
let scratchRoot = "";
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-run-scratch-recovery-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
originalTmpdir = process.env.TMPDIR;
|
||||
scratchRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-scratch-root-"));
|
||||
process.env.TMPDIR = scratchRoot;
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
await db.delete(heartbeatRunEvents);
|
||||
await db.delete(heartbeatRuns);
|
||||
await db.delete(agents);
|
||||
await db.delete(companies);
|
||||
await fs.rm(scratchRoot, { recursive: true, force: true });
|
||||
await fs.mkdir(scratchRoot, { recursive: true });
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
if (originalTmpdir === undefined) delete process.env.TMPDIR;
|
||||
else process.env.TMPDIR = originalTmpdir;
|
||||
await fs.rm(scratchRoot, { recursive: true, force: true });
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
async function seedRun(status: string) {
|
||||
const companyId = randomUUID();
|
||||
const agentId = randomUUID();
|
||||
const runId = randomUUID();
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await db.insert(agents).values({
|
||||
id: agentId,
|
||||
companyId,
|
||||
name: "Coder",
|
||||
role: "engineer",
|
||||
status: "idle",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
});
|
||||
await db.insert(heartbeatRuns).values({
|
||||
id: runId,
|
||||
companyId,
|
||||
agentId,
|
||||
invocationSource: "on_demand",
|
||||
status,
|
||||
nextEventSeq: 1,
|
||||
});
|
||||
return { companyId, agentId, runId };
|
||||
}
|
||||
|
||||
async function leaveScratch(run: { companyId: string; agentId: string; runId: string }, runIdInMarker = run.runId) {
|
||||
const dir = await fs.mkdtemp(path.join(scratchRoot, `paperclip-run-x-${run.runId.slice(0, 12)}-`));
|
||||
await fs.writeFile(
|
||||
path.join(dir, HEARTBEAT_RUN_SCRATCH_MARKER),
|
||||
JSON.stringify({
|
||||
version: 1,
|
||||
companyId: run.companyId,
|
||||
agentId: run.agentId,
|
||||
runId: runIdInMarker,
|
||||
issueId: null,
|
||||
issueIdentifier: null,
|
||||
createdAt: new Date().toISOString(),
|
||||
}),
|
||||
);
|
||||
return dir;
|
||||
}
|
||||
|
||||
const exists = (p: string) => fs.stat(p).then(() => true, () => false);
|
||||
|
||||
it("removes a terminal run's leftover directory and keeps others, even beside a malformed marker", async () => {
|
||||
const terminal = await seedRun("failed");
|
||||
const queued = await seedRun("queued");
|
||||
const terminalDir = await leaveScratch(terminal);
|
||||
const queuedDir = await leaveScratch(queued);
|
||||
const malformedDir = await leaveScratch(terminal, "not-a-uuid");
|
||||
const unknownRunDir = await leaveScratch(terminal, randomUUID());
|
||||
const unmarkedDir = await fs.mkdtemp(path.join(scratchRoot, "paperclip-run-x-unmarked-"));
|
||||
|
||||
await heartbeatService(db).reapOrphanedRuns();
|
||||
|
||||
expect(await exists(terminalDir)).toBe(false);
|
||||
expect(await exists(queuedDir)).toBe(true);
|
||||
expect(await exists(malformedDir)).toBe(true);
|
||||
expect(await exists(unknownRunDir)).toBe(true);
|
||||
expect(await exists(unmarkedDir)).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user