fix(server): clean heartbeat run scratch directories (#9234)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
38cca22b09
commit
f616b6746c
|
|
@ -122,6 +122,7 @@ export const DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE = [
|
|||
"- Use child issues for parallel or long delegated work instead of polling agents, sessions, or processes.",
|
||||
"- If woken by a human comment on a dependency-blocked issue, respond or triage the comment without treating the blocked deliverable work as unblocked.",
|
||||
"- Create child issues directly when you know what needs to be done; use issue-thread interactions when the board/user must choose suggested tasks, answer structured questions, or confirm a proposal.",
|
||||
"- Use `PAPERCLIP_SCRATCH_DIR` / `PAPERCLIP_RUN_SCRATCH_DIR` for temporary scratch files instead of ad hoc `/tmp` paths; Paperclip removes that run-owned directory after the run ends.",
|
||||
"- To ask for that input, create an interaction on the current issue with POST /api/issues/{issueId}/interactions using kind suggest_tasks, ask_user_questions, or request_confirmation. Use continuationPolicy wake_assignee when you need to resume after a response; for request_confirmation this resumes only after acceptance.",
|
||||
"- When you intentionally restart follow-up work on a completed assigned issue, include structured `resume: true` with the POST /api/issues/{issueId}/comments or PATCH /api/issues/{issueId} comment payload. Generic agent comments on closed issues are inert by default.",
|
||||
"- For plan approval, update the plan document first, then create request_confirmation targeting the latest plan revision with idempotencyKey confirmation:{issueId}:plan:{revisionId}. Wait for acceptance before creating implementation subtasks, and create a fresh confirmation after superseding board/user comments if approval is still needed.",
|
||||
|
|
|
|||
|
|
@ -135,6 +135,13 @@ import { buildPlanReviewContext } from "./plan-review-context.js";
|
|||
import { executionWorkspaceService, mergeExecutionWorkspaceConfig } from "./execution-workspaces.js";
|
||||
import { workspaceOperationService, type WorkspaceOperationRecorder } from "./workspace-operations.js";
|
||||
import { isProcessGroupAlive, terminateLocalService } from "./local-service-supervisor.js";
|
||||
import {
|
||||
HEARTBEAT_RUN_SCRATCH_MARKER,
|
||||
buildHeartbeatRunScratchEnv,
|
||||
cleanupHeartbeatRunScratch,
|
||||
prepareHeartbeatRunScratch,
|
||||
type HeartbeatRunScratch,
|
||||
} from "./run-scratch.js";
|
||||
import {
|
||||
buildExecutionWorkspaceAdapterConfig,
|
||||
gateProjectExecutionWorkspacePolicy,
|
||||
|
|
@ -10197,6 +10204,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
|
|||
}
|
||||
|
||||
activeRunExecutions.add(run.id);
|
||||
let runScratch: HeartbeatRunScratch | null = null;
|
||||
|
||||
try {
|
||||
const agent = await getAgent(run.agentId);
|
||||
|
|
@ -10702,7 +10710,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
|
|||
const runtimeSkillEntries = await companySkills.listRuntimeSkillEntries(agent.companyId, {
|
||||
versionSelections: skillVersionSelectionMap(runtimeSkillPreference.desiredSkillEntries),
|
||||
});
|
||||
let runtimeConfig = {
|
||||
let runtimeConfig: Record<string, unknown> = {
|
||||
...effectiveResolvedConfig,
|
||||
paperclipRuntimeSkills: runtimeSkillEntries,
|
||||
};
|
||||
|
|
@ -11204,6 +11212,47 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
|
|||
const workspaceRealization = realizationResult.workspaceRealization;
|
||||
const executionTarget = realizationResult.executionTarget;
|
||||
const remoteExecution = realizationResult.remoteExecution;
|
||||
if (!executionTarget || executionTarget.kind === "local") {
|
||||
try {
|
||||
runScratch = await prepareHeartbeatRunScratch({
|
||||
companyId: agent.companyId,
|
||||
agentId: agent.id,
|
||||
runId: run.id,
|
||||
issueId: issueRef?.id ?? null,
|
||||
issueIdentifier: issueRef?.identifier ?? null,
|
||||
});
|
||||
const existingRuntimeEnv = parseObject(runtimeConfig.env);
|
||||
const scratchEnv = buildHeartbeatRunScratchEnv(existingRuntimeEnv, runScratch);
|
||||
runtimeConfig = {
|
||||
...runtimeConfig,
|
||||
env: {
|
||||
...existingRuntimeEnv,
|
||||
...scratchEnv.env,
|
||||
},
|
||||
};
|
||||
context.paperclipScratch = {
|
||||
type: "heartbeat_run",
|
||||
dir: runScratch.dir,
|
||||
cleanupPolicy: "terminal_run",
|
||||
marker: HEARTBEAT_RUN_SCRATCH_MARKER,
|
||||
tempKeysApplied: scratchEnv.tempKeysApplied,
|
||||
};
|
||||
} catch (scratchPrepareError) {
|
||||
runScratch = null;
|
||||
delete context.paperclipScratch;
|
||||
logger.warn(
|
||||
{
|
||||
err: scratchPrepareError,
|
||||
runId: run.id,
|
||||
issueId,
|
||||
agentId: agent.id,
|
||||
},
|
||||
"failed to prepare heartbeat run scratch directory; continuing without scratch env",
|
||||
);
|
||||
}
|
||||
} else {
|
||||
delete context.paperclipScratch;
|
||||
}
|
||||
context.paperclipEnvironment = {
|
||||
id: selectedEnvironment.id,
|
||||
name: selectedEnvironment.name,
|
||||
|
|
@ -12458,6 +12507,58 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
|
|||
failureReason: latestRun?.error ?? undefined,
|
||||
});
|
||||
await releaseRuntimeServicesForRun(run.id).catch(() => undefined);
|
||||
if (runScratch && latestRun && isHeartbeatRunTerminalStatus(latestRun.status)) {
|
||||
const scratchForCleanup = runScratch;
|
||||
let scratchCleanup: Awaited<ReturnType<typeof cleanupHeartbeatRunScratch>> | null = null;
|
||||
try {
|
||||
scratchCleanup = await cleanupHeartbeatRunScratch({
|
||||
scratch: scratchForCleanup,
|
||||
processGroupId: latestRun.processGroupId,
|
||||
isProcessGroupAlive,
|
||||
});
|
||||
} catch (scratchCleanupError) {
|
||||
logger.warn(
|
||||
{
|
||||
err: scratchCleanupError,
|
||||
runId: run.id,
|
||||
scratchDir: scratchForCleanup.dir,
|
||||
},
|
||||
"failed to clean heartbeat run scratch directory",
|
||||
);
|
||||
await appendRunEvent(latestRun, await nextRunEventSeq(latestRun.id), {
|
||||
eventType: "error",
|
||||
stream: "system",
|
||||
level: "warn",
|
||||
message: "run scratch cleanup failed",
|
||||
payload: {
|
||||
dir: scratchForCleanup.dir,
|
||||
error: scratchCleanupError instanceof Error
|
||||
? scratchCleanupError.message
|
||||
: String(scratchCleanupError),
|
||||
},
|
||||
}).catch(() => undefined);
|
||||
}
|
||||
if (scratchCleanup) {
|
||||
await appendRunEvent(latestRun, await nextRunEventSeq(latestRun.id), {
|
||||
eventType: "lifecycle",
|
||||
stream: "system",
|
||||
level: scratchCleanup.removed ? "info" : "warn",
|
||||
message: scratchCleanup.removed
|
||||
? "run scratch cleaned"
|
||||
: `run scratch cleanup skipped: ${scratchCleanup.reason}`,
|
||||
payload: scratchCleanup,
|
||||
}).catch((scratchCleanupEventError) => {
|
||||
logger.warn(
|
||||
{
|
||||
err: scratchCleanupEventError,
|
||||
runId: run.id,
|
||||
scratchDir: scratchForCleanup.dir,
|
||||
},
|
||||
"failed to record heartbeat run scratch cleanup event",
|
||||
);
|
||||
});
|
||||
}
|
||||
}
|
||||
activeRunExecutions.delete(run.id);
|
||||
await startNextQueuedRunForAgent(run.agentId);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,125 @@
|
|||
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,
|
||||
type HeartbeatRunScratch,
|
||||
} 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"]);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,157 @@
|
|||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
export const HEARTBEAT_RUN_SCRATCH_MARKER = ".paperclip-run-scratch.json";
|
||||
|
||||
export interface HeartbeatRunScratchMetadata {
|
||||
version: 1;
|
||||
companyId: string;
|
||||
agentId: string;
|
||||
runId: string;
|
||||
issueId: string | null;
|
||||
issueIdentifier: string | null;
|
||||
createdAt: string;
|
||||
}
|
||||
|
||||
export interface HeartbeatRunScratch {
|
||||
dir: string;
|
||||
markerPath: string;
|
||||
metadata: HeartbeatRunScratchMetadata;
|
||||
}
|
||||
|
||||
export interface HeartbeatRunScratchEnvResult {
|
||||
env: Record<string, string>;
|
||||
tempKeysApplied: string[];
|
||||
}
|
||||
|
||||
export type HeartbeatRunScratchCleanupResult =
|
||||
| { removed: true; dir: string }
|
||||
| { removed: false; dir: string; reason: "missing" | "unmarked" | "owner_mismatch" | "process_group_alive" };
|
||||
|
||||
const TEMP_ENV_KEYS = ["TMPDIR", "TEMP", "TMP"] as const;
|
||||
const ISSUE_SEGMENT_MAX_CHARS = 32;
|
||||
|
||||
function sanitizePathSegment(value: string | null | undefined, fallback: string): string {
|
||||
const normalized = (value ?? "")
|
||||
.trim()
|
||||
.toLowerCase()
|
||||
.replace(/[^a-z0-9._-]+/g, "-")
|
||||
.replace(/-+/g, "-")
|
||||
.replace(/^-+|-+$/g, "")
|
||||
.slice(0, ISSUE_SEGMENT_MAX_CHARS)
|
||||
.replace(/[.-]+$/g, "");
|
||||
return normalized || fallback;
|
||||
}
|
||||
|
||||
function isPathInside(parent: string, child: string): boolean {
|
||||
const relative = path.relative(parent, child);
|
||||
return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
|
||||
}
|
||||
|
||||
async function readMarker(markerPath: string): Promise<HeartbeatRunScratchMetadata | null> {
|
||||
try {
|
||||
const parsed = JSON.parse(await fs.readFile(markerPath, "utf8")) as unknown;
|
||||
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return null;
|
||||
const rec = parsed as Record<string, unknown>;
|
||||
if (
|
||||
rec.version !== 1 ||
|
||||
typeof rec.companyId !== "string" ||
|
||||
typeof rec.agentId !== "string" ||
|
||||
typeof rec.runId !== "string" ||
|
||||
typeof rec.createdAt !== "string"
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
version: 1,
|
||||
companyId: rec.companyId,
|
||||
agentId: rec.agentId,
|
||||
runId: rec.runId,
|
||||
issueId: typeof rec.issueId === "string" ? rec.issueId : null,
|
||||
issueIdentifier: typeof rec.issueIdentifier === "string" ? rec.issueIdentifier : null,
|
||||
createdAt: rec.createdAt,
|
||||
};
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export async function prepareHeartbeatRunScratch(input: {
|
||||
companyId: string;
|
||||
agentId: string;
|
||||
runId: string;
|
||||
issueId?: string | null;
|
||||
issueIdentifier?: string | null;
|
||||
now?: Date;
|
||||
}): Promise<HeartbeatRunScratch> {
|
||||
const issueSegment = sanitizePathSegment(input.issueIdentifier, "unassigned");
|
||||
const runSegment = sanitizePathSegment(input.runId.slice(0, 12), "run");
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), `paperclip-run-${issueSegment}-${runSegment}-`));
|
||||
const markerPath = path.join(dir, HEARTBEAT_RUN_SCRATCH_MARKER);
|
||||
const metadata: HeartbeatRunScratchMetadata = {
|
||||
version: 1,
|
||||
companyId: input.companyId,
|
||||
agentId: input.agentId,
|
||||
runId: input.runId,
|
||||
issueId: input.issueId ?? null,
|
||||
issueIdentifier: input.issueIdentifier ?? null,
|
||||
createdAt: (input.now ?? new Date()).toISOString(),
|
||||
};
|
||||
await fs.writeFile(markerPath, `${JSON.stringify(metadata, null, 2)}\n`, { mode: 0o600 });
|
||||
return { dir, markerPath, metadata };
|
||||
}
|
||||
|
||||
export function buildHeartbeatRunScratchEnv(
|
||||
existingEnv: Record<string, unknown>,
|
||||
scratch: HeartbeatRunScratch,
|
||||
): HeartbeatRunScratchEnvResult {
|
||||
const env: Record<string, string> = {
|
||||
PAPERCLIP_RUN_SCRATCH_DIR: scratch.dir,
|
||||
PAPERCLIP_TASK_SCRATCH_DIR: scratch.dir,
|
||||
PAPERCLIP_SCRATCH_DIR: scratch.dir,
|
||||
PAPERCLIP_TMPDIR: scratch.dir,
|
||||
};
|
||||
const tempKeysApplied: string[] = [];
|
||||
for (const key of TEMP_ENV_KEYS) {
|
||||
const existing = existingEnv[key];
|
||||
if (typeof existing === "string" && existing.trim().length > 0) continue;
|
||||
env[key] = scratch.dir;
|
||||
tempKeysApplied.push(key);
|
||||
}
|
||||
return { env, tempKeysApplied };
|
||||
}
|
||||
|
||||
export async function cleanupHeartbeatRunScratch(input: {
|
||||
scratch: HeartbeatRunScratch;
|
||||
processGroupId?: number | null;
|
||||
isProcessGroupAlive?: (processGroupId: number | null | undefined) => boolean;
|
||||
}): Promise<HeartbeatRunScratchCleanupResult> {
|
||||
const tmpRoot = path.resolve(os.tmpdir());
|
||||
const dir = path.resolve(input.scratch.dir);
|
||||
if (!isPathInside(tmpRoot, dir) || !path.basename(dir).startsWith("paperclip-run-")) {
|
||||
return { removed: false, dir, reason: "unmarked" };
|
||||
}
|
||||
try {
|
||||
const stats = await fs.stat(dir);
|
||||
if (!stats.isDirectory()) return { removed: false, dir, reason: "missing" };
|
||||
} catch {
|
||||
return { removed: false, dir, reason: "missing" };
|
||||
}
|
||||
|
||||
const marker = await readMarker(path.join(dir, HEARTBEAT_RUN_SCRATCH_MARKER));
|
||||
if (!marker) return { removed: false, dir, reason: "unmarked" };
|
||||
if (
|
||||
marker.companyId !== input.scratch.metadata.companyId ||
|
||||
marker.agentId !== input.scratch.metadata.agentId ||
|
||||
marker.runId !== input.scratch.metadata.runId
|
||||
) {
|
||||
return { removed: false, dir, reason: "owner_mismatch" };
|
||||
}
|
||||
if (input.isProcessGroupAlive?.(input.processGroupId) === true) {
|
||||
return { removed: false, dir, reason: "process_group_alive" };
|
||||
}
|
||||
|
||||
await fs.rm(dir, { recursive: true, force: true });
|
||||
return { removed: true, dir };
|
||||
}
|
||||
Loading…
Reference in New Issue