From f7c85bca1893c0132e843dd2fe0e57d54d149d2a Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 9 Nov 2026 09:20:09 -0600 Subject: [PATCH] test(runner-e2e): delete each runner E2E instance when it ends and expire kept instances after 6 hours (#20246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The restart chaos harness boots a full temporary Paperclip instance per wave: embedded Postgres, workspaces and caches, often 5–13 GB > - At wave end it deleted that instance only when the wave passed with no harness error and no `--keep` > - Almost every chaos wave has at least one failing task, so almost every instance stayed on disk > - Over Nov 6–9, 359 `paperclip-runner-e2e-*` directories filled the machine's disk (ENOSPC), which stopped a running storm wave > - This pull request deletes the instance at the end of every wave, smoke and baseline run unless it is explicitly kept or its processes could not be stopped, and expires kept instances after 6 hours > - The benefit is that chaos waves can run back to back without filling the disk ## Linked Issues or Issue Description **What happened?** `main.ts` kept the temporary root whenever `keep || anyFailure || harnessError`. A failing task is the normal output of a chaos wave, so instances accumulated until the disk was full. The codex K13 storm w-20261109T113925-s9403 died after about a minute with every task cut off. **Steps to reproduce** 1. Run any wave in which at least one task fails. 2. The `paperclip-runner-e2e-*` directory under the OS temp dir stays behind, with no expiry. **Expected behavior** The instance is removed when the wave ends. Evidence is already in `results/`. Only an explicit `--keep`, or a stop failure that may leave processes running, keeps it, and kept instances expire. **Actual behavior** Every failing wave leaks its instance permanently. ## What Changed - New shared `tests/runner-e2e/instance-retention.ts`: - `releaseInstance` deletes the root unless `--keep` was passed or the processes could not be stopped. In those cases it writes a `.paperclip-runner-e2e-kept.json` marker (owner, reason, time) and sets the directory to 0700. An optional `retain` list keeps small evidence files and removes everything else. - `sweepExpiredKeptInstances` removes `paperclip-runner-e2e-*` directories that carry the marker, are older than 6 hours, and have no live `postmaster.pid` anywhere inside them, and that no live process command line references. Each root is made removable first, and one failed delete does not stop the sweep. Unmarked directories are never touched, so existing directories from older runs stay until someone decides on them. `recovery_state` markers never expire. - `createRunnerE2ETemporaryRoot` runs the sweep, so every caller gets it. - Callers: - A wave whose Daytona cleanup is still pending (environment DELETE blocked, sandboxes still running after cleanup, or cleanup errors) keeps its instance as `recovery_state`, because its database owns that cleanup. - The wave releases its instance before writing the ledger and summary, so a failed report write cannot skip cleanup. - Restart chaos `main.ts` (crash-point suite included) releases the instance in its final cleanup, which also runs on failure and interrupts. The crash-point task-count check moved above supervisor creation so that error no longer leaks a fresh root. - `ttft-baseline.ts` releases after stopping the supervisor, including on SIGINT and SIGTERM. - The rolling-deployment and controller-replacement smokes now delete their roots, keeping only the evidence report, logs and screenshots. If cleanup is unconfirmed, the root is kept with a marker. - `launch.ts` marks its unconfirmed-remote-cleanup root as `recovery_state`, which never expires, and its `PAPERCLIP_RUNNER_E2E_KEEP_FAILED_PRIVATE` root as `keep`. Its existing deletion path is unchanged, and the file gets one line shorter. - README: the instance lifecycle and `--keep`. ## Verification - `npx vitest run --config tests/runner-e2e/vitest.config.ts`: 328 files, 6915 passed. The new retention tests cover: - deleting a finished instance, and retaining only listed evidence; - keeping it with a marker and mode 0700; - sweeping only marked, expired instances whose Postgres isn't running, while sparing fresh, live, recovery-state, unmarked and unrelated directories; - a stale `postmaster.pid`; - sweeping on `createRunnerE2ETemporaryRoot`. The create-sweep test failed before `createRunnerE2ETemporaryRoot` ran the sweep. - `npx vitest run --config tests/restart-chaos/vitest.config.ts`: 221 passed. - `node scripts/typecheck-test-projects.mjs tests/runner-e2e` and `tests/restart-chaos`: clean. - No wave or smoke was run, because the disk is still nearly full. ## Risks - A failed wave's instance is no longer available for post-mortem DB inspection unless the wave ran with `--keep`. Evidence bundles in `results/` are unchanged. - The sweep relies on `postmaster.pid`. An instance whose Postgres was started outside the harness, without that file, could be removed after 6 hours. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## 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 --- tests/restart-chaos/README.md | 10 ++ tests/restart-chaos/main.ts | 27 +++-- tests/restart-chaos/ttft-baseline.ts | 7 +- .../controller-replacement-smoke.ts | 8 +- tests/runner-e2e/instance-retention.test.ts | 112 ++++++++++++++++++ tests/runner-e2e/instance-retention.ts | 89 ++++++++++++++ tests/runner-e2e/launch.ts | 7 +- tests/runner-e2e/rolling-deployment-smoke.ts | 8 +- tests/runner-e2e/server-config.ts | 3 + 9 files changed, 254 insertions(+), 17 deletions(-) create mode 100644 tests/runner-e2e/instance-retention.test.ts create mode 100644 tests/runner-e2e/instance-retention.ts diff --git a/tests/restart-chaos/README.md b/tests/restart-chaos/README.md index 295fd115f5c..25d1f3ae704 100644 --- a/tests/restart-chaos/README.md +++ b/tests/restart-chaos/README.md @@ -26,6 +26,16 @@ Options: `--triggers time,comment,retry,queued,tool,result` (fault timing, below default 3000), `--poll-ms` (2000–5000), `--keep` (keep the temporary instance), `--k4-hold-max-seconds` (raise the seeded K4 hold upper end from 300 s, up to the 600 s outage cap). +Each wave's temporary instance (embedded Postgres, workspaces, often 5–13 GB) is +deleted when the wave ends, pass or fail, including after an interrupt. Evidence +is already copied into `results/`. Use `--keep` to inspect the instance itself. +A kept instance, or one whose processes could not be stopped, gets a +`.paperclip-runner-e2e-kept.json` marker; creating the next runner E2E or chaos instance removes marked instances older +than 6 hours whose Postgres is no longer running and that no live process still +references. An instance whose Daytona cleanup is still pending keeps its database +as the owner of that cleanup and never expires. Unmarked directories are never +removed. + `--rc06-masked` keeps the company budget amount but disables its hard stop on the isolated Chaos Lab company, so one stuck accounting row (RC-06) cannot wedge every later task. Masked tasks are marked `rc06-masked` in the ledger and diff --git a/tests/restart-chaos/main.ts b/tests/restart-chaos/main.ts index ffaa6943207..9cd7061d9fb 100644 --- a/tests/restart-chaos/main.ts +++ b/tests/restart-chaos/main.ts @@ -8,7 +8,7 @@ */ import { execFileSync, spawnSync } from "node:child_process"; import { existsSync, readFileSync, writeFileSync } from "node:fs"; -import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { setTimeout as delay } from "node:timers/promises"; @@ -19,6 +19,7 @@ import { PLUGIN_BUILD_STAMP, pluginBuildState } from "./plugin-build.js"; import { DaytonaWaveSweeper, leaksForTask, unattributedLeaks, type DaytonaSweepResult } from "./daytona-sweep.js"; import { knownSecretValues, redactKnownSecrets, writeEvidenceBundle } from "./evidence.js"; import { gradeTask } from "./grader.js"; +import { releaseInstance } from "../runner-e2e/instance-retention.js"; import { checkPlanInvariants, summarizeViolations, type InvariantCheck } from "./invariants.js"; import { appendLedger, ledgerLine, writeSummary, type LedgerLine } from "./ledger.js"; import { collectWaveSpeed, speedMarkdown } from "./wave-speed.js"; @@ -170,6 +171,10 @@ async function main(): Promise { // Baseline only after the harness's own build steps (preflight rewrites the runner's dist). startFingerprint = worktreeFingerprint(repositoryRoot, guardedOutputs); + const crashPoint = plan.replayOf ? null : plan.options.crashPoint ?? null; + const crashPoints = crashPoint ? crashPoint.split(",") : []; + const batchSize = crashPoints.length ? Math.max(1, Math.floor(plan.tasks.length / crashPoints.length)) : 0; + if (crashPoint && plan.tasks.length !== batchSize * crashPoints.length) throw new Error(`--tasks must be a multiple of the ${crashPoints.length} crash point(s)`); const supervisor = await ChaosSupervisor.create({ repositoryRoot, waveDir, timeline, ...(plan.options.upgradeFrom ? { upgradeFrom: plan.options.upgradeFrom } : {}) }); // Crash-point suite: one booted server, one point at a time. Each point gets @@ -178,9 +183,6 @@ async function main(): Promise { // checked when the batch rests, as violations new since the point was armed. // A replay fires the recorded faults (the crash point's K1 restart included) // at their offsets; re-arming the point could miss its nth hit with fewer tasks. - const crashPoint = plan.replayOf ? null : plan.options.crashPoint ?? null; - const crashPoints = crashPoint ? crashPoint.split(",") : []; - const batchSize = crashPoints.length ? Math.max(1, Math.floor(plan.tasks.length / crashPoints.length)) : 0; type PointResult = { point: string; token: string; taskIds: string[]; armedAt: number; restedAt: number | null; fault: FaultReceipt | null; invariants: InvariantCheck | null }; const pointResults: Array }> = []; @@ -188,7 +190,6 @@ async function main(): Promise { let databaseUrl: string | null = null; const violationKey = (violation: { rule: string; kind: string; recordId: string }) => `${violation.rule}:${violation.kind}:${violation.recordId}`; if (crashPoint) { - if (plan.tasks.length !== batchSize * crashPoints.length) throw new Error(`--tasks must be a multiple of the ${crashPoints.length} crash point(s)`); supervisor.crashPointHoldMs = plan.options.crashPointHoldMs ?? 0; supervisor.useCrashPointControl({ controlPath: path.join(supervisor.paths.temporaryRoot, "crash-point.control"), receiptPath: path.join(waveDir, "crash-point-receipts.jsonl") }); @@ -208,6 +209,8 @@ async function main(): Promise { const faultWork: Promise[] = []; const lines: LedgerLine[] = []; let anyFailure = false; + let stopFailed = false; + let remoteCleanupPending = false; let speedSection: string | null = null; let harnessError: unknown; process.on("uncaughtException", error => { @@ -528,34 +531,40 @@ async function main(): Promise { + `${sandboxSweep.heldLeases.length} held leases${sandboxSweep.error ? `, error ${sandboxSweep.error}` : ""}; ` + `environment deleted: ${cleanup.environmentDeleted}${cleanup.environmentDeleteBlocked ? " (still blocked by a pending sandbox cleanup)" : ""}; still running after cleanup: ${cleanup.remainingSandboxes.join(", ") || "none"}`); // A sandbox neither archived nor destroyed after cleanup is a leak for a person to act on. - if (cleanup.remainingSandboxes.length) anyFailure = true; + if (cleanup.remainingSandboxes.length) { anyFailure = true; remoteCleanupPending = true; } // A delete still blocked by pending sandbox cleanup after the settle window // is a leak, not a harness error: fail the wave and keep the instance, whose // database owns that cleanup work. if (cleanup.environmentDeleteBlocked) { anyFailure = true; + remoteCleanupPending = true; timeline.add("daytona_environment_delete_blocked", { detail: cleanup.environmentDeleteBlocked }); } + if (cleanup.errors.length) remoteCleanupPending = true; if (cleanup.errors.length) harnessError ??= new Error(`Daytona cleanup incomplete: ${cleanup.errors.join("; ")}`); - await writeFile(path.join(waveDir, "sandbox-sweep.json"), `${JSON.stringify({ sweep: sandboxSweep, cleanup }, null, 2)}\n`, { mode: 0o600 }); + await writeFile(path.join(waveDir, "sandbox-sweep.json"), `${JSON.stringify({ sweep: sandboxSweep, cleanup }, null, 2)}\n`, { mode: 0o600 }) + .catch(error => timeline.add("sandbox_sweep_write_failed", { error: String(error) })); } catch (error) { harnessError ??= error; + remoteCleanupPending = true; timeline.add("daytona_cleanup_failed", { error: String(error) }); } } finally { try { await supervisor.stop(); } catch (error) { + stopFailed = true; // Incomplete cleanup is a harness failure; keep the instance for inspection. harnessError ??= error; timeline.add("supervisor_stop_failed", { error: String(error) }); } } + const retention = await releaseInstance(supervisor.paths.temporaryRoot, { keep, stopFailed, remoteCleanupPending, owner: plan.waveId }) + .catch(error => { timeline.add("instance_cleanup_failed", { error: String(error) }); return null; }); + if (retention?.kept) timeline.add("instance_kept", { temporaryRoot: supervisor.paths.temporaryRoot, reason: retention.reason }); if (lines.length) { await appendLedger(resultsRoot, lines); const summary = await writeSummary(resultsRoot, plan.waveId); await writeFile(path.join(waveDir, "SUMMARY.md"), speedSection ? `${summary}\n${speedSection}` : summary, { mode: 0o600 }); console.info(`\n${summary}`); } - if (keep || anyFailure || harnessError) timeline.add("instance_kept", { temporaryRoot: supervisor.paths.temporaryRoot }); - else await rm(supervisor.paths.temporaryRoot, { recursive: true, force: true }).catch(() => undefined); } console.info(`Wave ${plan.waveId}: results ${waveDir}`); for (const line of lines.filter(candidate => !candidate.pass)) console.info(` FAIL ${line.taskId} ${line.signature} → ${line.evidence ?? "no evidence"}\n replay: ${replayCommand(plan.waveId, line.taskId)}`); diff --git a/tests/restart-chaos/ttft-baseline.ts b/tests/restart-chaos/ttft-baseline.ts index 893f18616c4..df422a72702 100644 --- a/tests/restart-chaos/ttft-baseline.ts +++ b/tests/restart-chaos/ttft-baseline.ts @@ -19,6 +19,7 @@ import { mkdir, readFile, writeFile } from "node:fs/promises"; import path from "node:path"; import { parseArgs } from "node:util"; import { setTimeout as delay } from "node:timers/promises"; +import { releaseInstance } from "../runner-e2e/instance-retention.js"; import { ChaosApi } from "./api.js"; import { CREDENTIAL_FOR_ADAPTER } from "./config.js"; import { ACTIVE_RUN_STATUSES } from "./grader.js"; @@ -280,9 +281,11 @@ async function main() { // One shutdown path for normal completion and for SIGINT/SIGTERM: the server // runs in a detached process group and the embedded Postgres has no exit hook. let stopping: Promise | null = null; - const shutdown = () => (stopping ??= supervisor.stop().catch(error => { + const shutdown = () => (stopping ??= supervisor.stop().then(() => false, error => { process.stderr.write(`supervisor stop: ${String(error)}\n`); - })); + return true; + }).then(stopFailed => releaseInstance(supervisor.paths.temporaryRoot, { keep: false, stopFailed, owner: "ttft-baseline" })) + .then(() => undefined, error => { process.stderr.write(`instance cleanup: ${String(error)}\n`); })); const onSignal = (signal: NodeJS.Signals) => { timeline.add("baseline_interrupted", { signal }); void shutdown().finally(() => process.exit(signal === "SIGINT" ? 130 : 143)); diff --git a/tests/runner-e2e/controller-replacement-smoke.ts b/tests/runner-e2e/controller-replacement-smoke.ts index 6f957cdcc4d..7743ae258ee 100644 --- a/tests/runner-e2e/controller-replacement-smoke.ts +++ b/tests/runner-e2e/controller-replacement-smoke.ts @@ -17,6 +17,7 @@ import { paperclipConfigSchema } from "../../packages/shared/src/config-schema.j import { configuredControllerOutage, validControllerOutageReceipt, type ControllerOutage } from "./controller-outage.js"; import { runnerE2EServerControlPaths } from "./harness-env.js"; import { reserveRunnerE2EServerPort } from "./ports.js"; +import { releaseInstance } from "./instance-retention.js"; import { createRunnerE2ETemporaryRoot, prepareRunnerE2EServerConfig } from "./server-config.js"; import { createRunnerE2EServerStopper } from "./server-stop.js"; import { cleanupControllerSmokeDatabase } from "./controller-smoke-cleanup.js"; @@ -196,7 +197,12 @@ try { report.cleanupConfirmed = cleanupErrors.length === 0; if (cleanupErrors.length) { report.cleanupErrors = cleanupErrors; report.passed = false; } await new Promise(resolve => log.end(resolve)); - await writeFile(reportPath, `${JSON.stringify(report, null, 2)}\n`, { mode: 0o600 }); + try { + await writeFile(reportPath, `${JSON.stringify(report, null, 2)}\n`, { mode: 0o600 }); + } finally { + await releaseInstance(temporaryRoot, { keep: false, stopFailed: !report.cleanupConfirmed, owner: "controller-replacement-smoke", + retain: entry => entry === path.basename(reportPath) || entry.endsWith(".log") }).catch(error => console.warn(`Temporary root cleanup failed at ${temporaryRoot}: ${String(error)}`)); + } console.info(JSON.stringify({ passed: report.passed, evidence: reportPath, cleanupConfirmed: report.cleanupConfirmed })); } if (failure) throw failure; diff --git a/tests/runner-e2e/instance-retention.test.ts b/tests/runner-e2e/instance-retention.test.ts new file mode 100644 index 00000000000..4c587adce9d --- /dev/null +++ b/tests/runner-e2e/instance-retention.test.ts @@ -0,0 +1,112 @@ +import { existsSync } from "node:fs"; +import { chmod, mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { KEPT_INSTANCE_MARKER, KEPT_INSTANCE_MAX_AGE_MS, keepInstance, releaseInstance, sweepExpiredKeptInstances } from "./instance-retention.js"; +import { createRunnerE2ETemporaryRoot } from "./server-config.js"; + +const roots: string[] = []; +afterEach(async () => { await Promise.all(roots.splice(0).map(root => rm(root, { recursive: true, force: true }))); }); + +async function parent() { + const root = await mkdtemp(path.join(os.tmpdir(), "runner-e2e-retention-")); + roots.push(root); + return root; +} +async function instance(under: string, name: string) { + const root = path.join(under, `paperclip-runner-e2e-${name}`); + await mkdir(path.join(root, "home", "instances", "default", "db"), { recursive: true }); + return root; +} + +describe("instance retention", () => { + it("deletes a finished instance unless --keep or a failed stop asks to keep it", async () => { + const under = await parent(); + const finished = await instance(under, "done"); + expect(await releaseInstance(finished, { keep: false, stopFailed: false, owner: "w1" })).toEqual({ kept: false }); + expect(existsSync(finished)).toBe(false); + const kept = await instance(under, "kept"); + expect(await releaseInstance(kept, { keep: true, stopFailed: false, owner: "w2" }, 1_000)).toEqual({ kept: true, reason: "keep" }); + expect(existsSync(path.join(kept, KEPT_INSTANCE_MARKER))).toBe(true); + expect((await stat(kept)).mode & 0o777).toBe(0o700); + const evidence = await instance(under, "evidence"); + await writeFile(path.join(evidence, "report.json"), "{}"); + expect(await releaseInstance(evidence, { keep: false, stopFailed: false, owner: "smoke", retain: entry => entry === "report.json" })).toEqual({ kept: false }); + expect(existsSync(path.join(evidence, "report.json"))).toBe(true); + expect(existsSync(path.join(evidence, "home"))).toBe(false); + const remote = await instance(under, "remote"); + expect(await releaseInstance(remote, { keep: false, stopFailed: false, remoteCleanupPending: true, owner: "w4" })) + .toEqual({ kept: true, reason: "recovery_state" }); + const stuck = await instance(under, "stuck"); + expect(await releaseInstance(stuck, { keep: false, stopFailed: true, owner: "w3" }, 1_000)).toEqual({ kept: true, reason: "stop_failed" }); + }); + + it("sweeps only marked kept instances past their expiry whose Postgres is not running", async () => { + const under = await parent(); + const now = 10 * KEPT_INSTANCE_MAX_AGE_MS; + const old = now - KEPT_INSTANCE_MAX_AGE_MS - 1; + const expired = await instance(under, "expired"); + await keepInstance(expired, { owner: "w1", reason: "keep" }, old); + const fresh = await instance(under, "fresh"); + await keepInstance(fresh, { owner: "w2", reason: "keep" }, now - 1_000); + const live = await instance(under, "live"); + await keepInstance(live, { owner: "w3", reason: "keep" }, old); + await mkdir(path.join(live, "a", "home"), { recursive: true }); + await writeFile(path.join(live, "a", "home", "postmaster.pid"), `${process.pid}\n/data\n`); + const recovery = await instance(under, "recovery"); + await keepInstance(recovery, { owner: "launch", reason: "recovery_state" }, old); + const unmarked = await instance(under, "unmarked"); + const unrelated = path.join(under, "something-else"); + await mkdir(unrelated); + await writeFile(path.join(unrelated, KEPT_INSTANCE_MARKER), JSON.stringify({ keptAt: 0 })); + + const runner = await instance(under, "runner"); + await keepInstance(runner, { owner: "w4", reason: "stop_failed" }, old); + + expect(await sweepExpiredKeptInstances(under, now, KEPT_INSTANCE_MAX_AGE_MS, () => [`node ${runner}/provider-bin/runnerd`])).toEqual([expired]); + expect(existsSync(expired)).toBe(false); + for (const survivor of [fresh, live, recovery, unmarked, unrelated, runner]) expect(existsSync(survivor)).toBe(true); + }); + + it("releases a finished instance whose immutable runtime bundles are read-only", async () => { + const under = await parent(); + const finished = await instance(under, "bundles"); + const bundle = path.join(finished, "home", "instances", "default", "runtime-context-assets", "bundles", "sha256-abc"); + await mkdir(bundle, { recursive: true }); + await writeFile(path.join(bundle, "AGENTS.md"), "x"); + await chmod(bundle, 0o555); + await chmod(path.dirname(bundle), 0o555); + expect(await releaseInstance(finished, { keep: false, stopFailed: false, owner: "w1" })).toEqual({ kept: false }); + expect(existsSync(finished)).toBe(false); + }); + + it("sweeps an expired instance that holds read-only directories", async () => { + const under = await parent(); + const locked = await instance(under, "locked"); + await keepInstance(locked, { owner: "w1", reason: "keep" }, 0); + const readOnly = path.join(locked, "home", "cache"); + await mkdir(readOnly, { recursive: true }); + await writeFile(path.join(readOnly, "entry"), "x"); + await chmod(readOnly, 0o500); + expect(await sweepExpiredKeptInstances(under, KEPT_INSTANCE_MAX_AGE_MS + 1)).toEqual([locked]); + expect(existsSync(locked)).toBe(false); + }); + + it("treats a stale postmaster.pid whose process is gone as not running", async () => { + const under = await parent(); + const stale = await instance(under, "stale"); + await keepInstance(stale, { owner: "w1", reason: "stop_failed" }, 0); + await writeFile(path.join(stale, "home", "instances", "default", "db", "postmaster.pid"), "2147483646\n/data\n"); + expect(await sweepExpiredKeptInstances(under, KEPT_INSTANCE_MAX_AGE_MS + 1)).toEqual([stale]); + }); + + it("sweeps expired kept instances whenever a caller creates a new temporary root", async () => { + const under = await parent(); + const expired = await instance(under, "expired"); + await keepInstance(expired, { owner: "w1", reason: "keep" }, Date.now() - KEPT_INSTANCE_MAX_AGE_MS - 1); + const created = await createRunnerE2ETemporaryRoot(under); + expect(existsSync(expired)).toBe(false); + expect(path.basename(created).startsWith("paperclip-runner-e2e-")).toBe(true); + }); +}); diff --git a/tests/runner-e2e/instance-retention.ts b/tests/runner-e2e/instance-retention.ts new file mode 100644 index 00000000000..1dd37b5ab76 --- /dev/null +++ b/tests/runner-e2e/instance-retention.ts @@ -0,0 +1,89 @@ +import { execFileSync } from "node:child_process"; +import { chmod, readdir, readFile, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { makeDirectoryTreeRemovable } from "./directory-cleanup.js"; + +export const KEPT_INSTANCE_MARKER = ".paperclip-runner-e2e-kept.json"; +export const KEPT_INSTANCE_MAX_AGE_MS = 6 * 60 * 60_000; +export const RUNNER_E2E_INSTANCE_PREFIX = "paperclip-runner-e2e-"; +const POSTMASTER_SEARCH_DEPTH = 6; + +export type KeepReason = "keep" | "stop_failed" | "recovery_state"; + +export async function keepInstance(root: string, input: { owner: string; reason: KeepReason }, now = Date.now()): Promise { + await chmod(root, 0o700); + await writeFile(path.join(root, KEPT_INSTANCE_MARKER), `${JSON.stringify({ owner: input.owner, reason: input.reason, keptAt: now })}\n`, { mode: 0o600 }); +} + +export async function releaseInstance(root: string, input: { keep: boolean; stopFailed: boolean; owner: string; + remoteCleanupPending?: boolean; retain?: (entry: string) => boolean }, now = Date.now()): Promise<{ kept: false } | { kept: true; reason: KeepReason }> { + const reason: KeepReason | null = input.remoteCleanupPending ? "recovery_state" : input.stopFailed ? "stop_failed" : input.keep ? "keep" : null; + if (!reason) { + await makeDirectoryTreeRemovable(root).catch(() => undefined); + if (!input.retain) { + await rm(root, { recursive: true, force: true }); + return { kept: false }; + } + for (const entry of await readdir(root)) if (!input.retain(entry)) await rm(path.join(root, entry), { recursive: true, force: true }); + return { kept: false }; + } + await keepInstance(root, { owner: input.owner, reason }, now); + return { kept: true, reason }; +} + +function processAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === "EPERM"; + } +} + +async function postgresRunning(directory: string, depth = POSTMASTER_SEARCH_DEPTH): Promise { + const entries = await readdir(directory, { withFileTypes: true }).catch(() => []); + for (const entry of entries) { + if (entry.isFile() && entry.name === "postmaster.pid") { + const pid = Number((await readFile(path.join(directory, entry.name), "utf8").catch(() => "")).split("\n")[0]); + if (Number.isInteger(pid) && pid > 0 && processAlive(pid)) return true; + } + } + if (depth === 0) return false; + for (const entry of entries) { + if (entry.isDirectory() && !entry.isSymbolicLink() && await postgresRunning(path.join(directory, entry.name), depth - 1)) return true; + } + return false; +} + +function liveProcessCommands(): string[] { + try { + return execFileSync("ps", ["-axww", "-o", "command="], { encoding: "utf8", maxBuffer: 64 * 1024 * 1024 }).split("\n"); + } catch { + return []; + } +} + +export async function sweepExpiredKeptInstances(parent = os.tmpdir(), now = Date.now(), maxAgeMs = KEPT_INSTANCE_MAX_AGE_MS, + processCommands: () => readonly string[] = liveProcessCommands): Promise { + const removed: string[] = []; + let commands: readonly string[] | null = null; + for (const entry of await readdir(parent, { withFileTypes: true }).catch(() => [])) { + if (!entry.isDirectory() || !entry.name.startsWith(RUNNER_E2E_INSTANCE_PREFIX)) continue; + const root = path.join(parent, entry.name); + const marker = await readFile(path.join(root, KEPT_INSTANCE_MARKER), "utf8") + .then(text => JSON.parse(text) as { keptAt?: unknown; reason?: unknown }).catch(() => null); + if (typeof marker?.keptAt !== "number" || marker.reason === "recovery_state" || now - marker.keptAt <= maxAgeMs) continue; + if (await postgresRunning(root)) continue; + commands ??= processCommands(); + if (commands.some(command => command.includes(root))) continue; + try { + await makeDirectoryTreeRemovable(root).catch(() => undefined); + await rm(root, { recursive: true, force: true }); + removed.push(root); + } catch { + continue; + } + } + return removed; +} diff --git a/tests/runner-e2e/launch.ts b/tests/runner-e2e/launch.ts index 5ac2e9049ad..6c11cb6ebac 100644 --- a/tests/runner-e2e/launch.ts +++ b/tests/runner-e2e/launch.ts @@ -10,7 +10,7 @@ import { runnerE2EPlaywrightCaseTimeoutMs, runnerE2EPlaywrightInvocation, runner import { verifyInstalledDaytonaPlugin } from "./installed-daytona-plugin.js"; import { assertInstalledCliSelection, assertInstalledStartupOnly, verifyInstalledCli } from "./installed-cli.js"; import { createProcessTreeOwner, stopOwnedProcessTree } from "./process-tree-owner.js"; -import { createRunnerE2ETemporaryRoot } from "./server-config.js"; +import { createRunnerE2ETemporaryRoot, keepInstance } from "./server-config.js"; import { createExternalControllerDatabase } from "./external-controller-database.js"; import { createHash, randomBytes } from "node:crypto"; import { prepareCodexCiSandbox, requiresCodexCiSandbox } from "./codex-ci-sandbox.js"; @@ -25,7 +25,6 @@ import os from "node:os"; import path from "node:path"; import { access, - chmod, cp, lstat, mkdir, @@ -915,7 +914,7 @@ async function runAttempt(input: { if (preserveRecovery || cleanupError) { // Remote allocation cleanup is journaled in this instance's database. // Deleting it after the controller exits would strand uncertain creates. - await chmod(temporaryRoot, 0o700); + await keepInstance(temporaryRoot, { owner: "runner-e2e-launch", reason: "recovery_state" }); cleanupError = new Error(`Preserving private recovery state after unconfirmed cleanup: ${temporaryRoot}`); } else if (shouldKeepFailedDiagnostics({ enabled: process.env.PAPERCLIP_RUNNER_E2E_KEEP_FAILED_PRIVATE === "1", @@ -924,7 +923,7 @@ async function runAttempt(input: { })) { // Explicit diagnosis only. Confirmed resource cleanup stays confirmed; // keep private traces for investigation without publishing provider data. - await chmod(temporaryRoot, 0o700); + await keepInstance(temporaryRoot, { owner: "runner-e2e-launch", reason: "keep" }); console.warn(`Retained private failed-case diagnostics: ${temporaryRoot}`); } else if ( temporaryRoot.startsWith(`${temporaryParent}${path.sep}paperclip-runner-e2e-`) diff --git a/tests/runner-e2e/rolling-deployment-smoke.ts b/tests/runner-e2e/rolling-deployment-smoke.ts index ee57d473ca6..bc637733806 100644 --- a/tests/runner-e2e/rolling-deployment-smoke.ts +++ b/tests/runner-e2e/rolling-deployment-smoke.ts @@ -12,6 +12,7 @@ import { setTimeout as delay } from "node:timers/promises"; import { startEmbeddedPostgresTestDatabase } from "../../packages/db/src/index.js"; import { createExternalControllerDatabase } from "./external-controller-database.js"; import { reserveRunnerE2EServerPort } from "./ports.js"; +import { releaseInstance } from "./instance-retention.js"; import { createRunnerE2ETemporaryRoot } from "./server-config.js"; import { openRollingDeploymentBrowser } from "./rolling-deployment-browser.js"; import { startRollingDeploymentProcess } from "./rolling-deployment-process.js"; @@ -305,7 +306,12 @@ try { report.cleanupConfirmed = cleanupErrors.length === 0; if (cleanupErrors.length) report.cleanupErrors = cleanupErrors; report.passed = !report.error && checks.length > 0 && checks.every(item => item.passed) && report.cleanupConfirmed; - await saveReport(); + try { + await saveReport(); + } finally { + await releaseInstance(root, { keep: false, stopFailed: !report.cleanupConfirmed, owner: "rolling-deployment-smoke", retain: entry => entry === path.basename(reportPath) || /\.(log|png)$/.test(entry) }) + .catch(error => console.warn(`Temporary root cleanup failed at ${root}: ${String(error)}`)); + } console.info(JSON.stringify({ passed: report.passed, checks, evidence: reportPath, cleanupConfirmed: report.cleanupConfirmed })); } if (!report.passed) process.exitCode = 1; diff --git a/tests/runner-e2e/server-config.ts b/tests/runner-e2e/server-config.ts index 3d9a42c0552..9275251f1d9 100644 --- a/tests/runner-e2e/server-config.ts +++ b/tests/runner-e2e/server-config.ts @@ -2,10 +2,13 @@ import { mkdir, mkdtemp, realpath, readFile, writeFile } from "node:fs/promises" import os from "node:os"; import path from "node:path"; import { paperclipConfigSchema } from "../../packages/shared/src/config-schema.js"; +import { sweepExpiredKeptInstances } from "./instance-retention.js"; +export { keepInstance } from "./instance-retention.js"; import { reserveRunnerE2EDatabasePort } from "./ports.js"; /** Runtime instructions and native filesystem grants must use the same path. */ export async function createRunnerE2ETemporaryRoot(parent = os.tmpdir()) { + await sweepExpiredKeptInstances(parent).catch(() => []); return await realpath(await mkdtemp(path.join(parent, "paperclip-runner-e2e-"))); }