diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ecf24b..93172af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -293,6 +293,31 @@ actually cutting it, and this file starts there. covers `git ls-files '*.sh'` and fails naming the strays otherwise, so a future sweep that quietly narrows is red rather than green over nothing. +- **`cast` no longer leaves a full repo clone in the temp dir on every run** + (#117) — `resolveCheckout()` mkdtemps an `infra-checkout-` directory and + `git clone`s the infra repo into it, and nothing ever removed it. Every + `cast apply`, `diff`, or `capture` invoked *without* `--path` — the normal + way to run all three — left a shallow clone behind permanently. This is a + runtime leak, not a test one: #117 was filed as test-suite hygiene and + explicitly scoped the runtime out ("`cast` itself does not leak"), but the + box that found it was also holding 602 `infra-checkout-*` directories, + 73 MB of real `.git` trees, from the same day. The leak fires on the + failure path too, since the directory is created before the clone runs. + Ephemeral checkouts are now registered and removed on process exit, which + is the lifetime that fits: the tree has to outlive `resolveCheckout`'s + return — every caller reads it — so a `finally` would delete the checkout + out from under the command that asked for it. A `--path` checkout is the + operator's own working tree and is never registered. + +- **The test suite reaps its temp directories** (#117) — 68 `mkdtempSync` + call sites across 21 files, zero cleanups, accumulating ~6700 directories + and 189 MB per machine-day, some holding age keys. All 68 now go through a + single `tmp()` helper (`test/helpers/tmp.ts`) that allocates inside a + per-run root, which vitest's `globalSetup` teardown removes wholesale. A + class-guard test (`test/tmp-guard.test.ts`) fails if `mkdtempSync` appears + anywhere under `test/` outside those helpers, so the next raw call is + caught at review rather than after a day of accumulation. + ## 0.1.1 — 2026-07-19 ### Fixed diff --git a/src/resolve.ts b/src/resolve.ts index da2c059..9a7c738 100644 --- a/src/resolve.ts +++ b/src/resolve.ts @@ -1,5 +1,5 @@ import { execFileSync } from "node:child_process"; -import { existsSync, mkdtempSync, readFileSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import type { Desired } from "./diff.js"; @@ -162,6 +162,42 @@ export function refusesPathInProd(opts: { return opts.path !== undefined && opts.env === "prod"; } +// Checkouts this process cloned, to be removed when it exits. +const ephemeralCheckouts: string[] = []; +let reaperArmed = false; + +// A checkout resolved WITHOUT `--path` is ours: we made the directory, we cloned +// into it, and nothing outside this process refers to it. It has to outlive +// resolveCheckout's return — every caller reads the tree afterwards — so the +// lifetime that actually fits is the process, not the call. Hence an exit hook +// rather than a `finally`, which would delete the checkout out from under the +// command that just asked for it. +// +// A `--path` checkout is the operator's own working tree and is never registered +// here; deleting that would be catastrophic and is the reason this wraps the +// mkdtemp result specifically, not the function's return value. +// +// Without this, every `cast apply` / `diff` / `capture` run without `--path` +// left a full shallow clone in the temp dir forever (#117). +function reapOnExit(dir: string): string { + ephemeralCheckouts.push(dir); + if (!reaperArmed) { + reaperArmed = true; + process.once("exit", () => { + for (const d of ephemeralCheckouts) { + // Best-effort: failing to clean up must never change a command's exit + // status. The work is already done by the time we get here. + try { + rmSync(d, { recursive: true, force: true }); + } catch { + // ignore + } + } + }); + } + return dir; +} + export function resolveCheckout( orgRepo: string, opts: { env: string; path?: string }, @@ -170,7 +206,7 @@ export function resolveCheckout( throw new Error(PATH_IN_PROD_REFUSAL); } if (opts.path) return opts.path; - const dir = mkdtempSync(join(tmpdir(), "infra-checkout-")); + const dir = reapOnExit(mkdtempSync(join(tmpdir(), "infra-checkout-"))); const auth = resolveGitAuth(); try { execFileSync( diff --git a/test/backup-cli.test.ts b/test/backup-cli.test.ts index c730b96..11ebf88 100644 --- a/test/backup-cli.test.ts +++ b/test/backup-cli.test.ts @@ -1,10 +1,10 @@ import { execFileSync, spawn } from "node:child_process"; -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { mkdirSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeAll, describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; // Backup schedules, end to end: manifest -> `cast diff` -> what it prints and // what it exits with. The unit tests prove each half (coolify.ts parses the @@ -21,7 +21,7 @@ let recipient: string; let keyFile: string; beforeAll(() => { - const dir = mkdtempSync(join(tmpdir(), "cast-age-")); + const dir = tmp("cast-age-"); keyFile = join(dir, "age.key"); execFileSync("age-keygen", ["-o", keyFile], { stdio: "pipe" }); recipient = execFileSync("age-keygen", ["-y", keyFile], { @@ -110,11 +110,11 @@ environments: `; function fixture(url: string) { - const checkout = mkdtempSync(join(tmpdir(), "cast-co-")); + const checkout = tmp("cast-co-"); mkdirSync(join(checkout, ".infra", "env"), { recursive: true }); writeFileSync(join(checkout, ".infra", "manifest.yaml"), MANIFEST); - const state = mkdtempSync(join(tmpdir(), "cast-state-")); + const state = tmp("cast-state-"); mkdirSync(join(state, "secrets")); writeFileSync( join(state, ".coolify.env"), diff --git a/test/capture-cli.test.ts b/test/capture-cli.test.ts index 787d227..2440863 100644 --- a/test/capture-cli.test.ts +++ b/test/capture-cli.test.ts @@ -1,17 +1,11 @@ import { execFileSync, spawn } from "node:child_process"; -import { - existsSync, - mkdirSync, - mkdtempSync, - readFileSync, - writeFileSync, -} from "node:fs"; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeAll, describe, expect, it } from "vitest"; import { decryptSecrets, encryptSecrets } from "../src/secrets.js"; +import { tmp } from "./helpers/tmp.js"; // End-to-end: the real CLI, a real age identity, a stub Coolify holding real // live values. The point is the store that comes out the other side — it is @@ -34,7 +28,7 @@ let keyFile: string; let recipient: string; beforeAll(() => { - const dir = mkdtempSync(join(tmpdir(), "cast-age-")); + const dir = tmp("cast-age-"); keyFile = join(dir, "age-staging.key"); execFileSync("age-keygen", ["-o", keyFile], { stdio: "pipe" }); const pub = execFileSync("age-keygen", ["-y", keyFile], { encoding: "utf8" }); @@ -110,7 +104,7 @@ function fixture( url: string, opts: { template?: string; manifest?: string } = {}, ) { - const checkout = mkdtempSync(join(tmpdir(), "cast-co-")); + const checkout = tmp("cast-co-"); mkdirSync(join(checkout, ".infra", "env"), { recursive: true }); writeFileSync( join(checkout, ".infra", "manifest.yaml"), @@ -121,7 +115,7 @@ function fixture( opts.template ?? TEMPLATE, ); - const state = mkdtempSync(join(tmpdir(), "cast-state-")); + const state = tmp("cast-state-"); mkdirSync(join(state, "secrets")); writeFileSync( join(state, ".coolify.env"), diff --git a/test/capture.test.ts b/test/capture.test.ts index 1d18b8d..93b0620 100644 --- a/test/capture.test.ts +++ b/test/capture.test.ts @@ -1,5 +1,4 @@ -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { mkdirSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { @@ -14,6 +13,7 @@ import { resolveGeneratedSources, } from "../src/capture.js"; import { requiredSecrets } from "../src/resolve.js"; +import { tmp } from "./helpers/tmp.js"; const CTX = { orgRepo: "heavy-duty/incubator", @@ -241,7 +241,7 @@ describe("renderCapturePlan", () => { // the manifest's own templates, read by the same parser apply uses. describe("requiredSecrets", () => { function checkout(manifest: string, templates: Record) { - const dir = mkdtempSync(join(tmpdir(), "cast-cap-")); + const dir = tmp("cast-cap-"); mkdirSync(join(dir, ".infra", "env"), { recursive: true }); writeFileSync(join(dir, ".infra", "manifest.yaml"), manifest); for (const [name, body] of Object.entries(templates)) { diff --git a/test/cli.test.ts b/test/cli.test.ts index bdc99b5..9d09e4a 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -1,10 +1,10 @@ import { spawn } from "node:child_process"; -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { mkdirSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; // Spawned ASYNCHRONOUSLY, and that is load-bearing: the stub Coolify below // runs in THIS process, so a blocking execFileSync would hold the event loop @@ -74,7 +74,7 @@ function stateWith(opts: { named?: Record; boundInstance?: string; }): string { - const dir = mkdtempSync(join(tmpdir(), "cast-cli-")); + const dir = tmp("cast-cli-"); if (opts.default) writeFileSync(join(dir, ".coolify.env"), opts.default); if (opts.named) { mkdirSync(join(dir, ".coolify")); diff --git a/test/config.test.ts b/test/config.test.ts index 20ee672..fa47dfd 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -1,5 +1,4 @@ -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { mkdirSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { @@ -8,13 +7,14 @@ import { knownInstances, loadInstance, } from "../src/config.js"; +import { tmp } from "./helpers/tmp.js"; // A state dir with a default .coolify.env and any number of named instances. function stateDir( named: Record = {}, defaultEnv?: string, ): string { - const dir = mkdtempSync(join(tmpdir(), "cast-state-")); + const dir = tmp("cast-state-"); if (defaultEnv !== undefined) { writeFileSync(join(dir, ".coolify.env"), defaultEnv); } diff --git a/test/destroy.test.ts b/test/destroy.test.ts index e931a82..4813093 100644 --- a/test/destroy.test.ts +++ b/test/destroy.test.ts @@ -1,8 +1,7 @@ import { spawn } from "node:child_process"; -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { mkdirSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { @@ -14,6 +13,7 @@ import { readBackupState, renderDestroyPlan, } from "../src/destroy.js"; +import { tmp } from "./helpers/tmp.js"; // The refusals ARE the product. `destroy` is the only verb in cast that removes // something a manifest declared, and the difference between it and a hand @@ -460,11 +460,11 @@ function fixture( url: string, opts: { destroyAllowed?: boolean | undefined; readOnly?: boolean } = {}, ) { - const checkout = mkdtempSync(join(tmpdir(), "cast-co-")); + const checkout = tmp("cast-co-"); mkdirSync(join(checkout, ".infra"), { recursive: true }); writeFileSync(join(checkout, ".infra", "manifest.yaml"), MANIFEST); - const state = mkdtempSync(join(tmpdir(), "cast-state-")); + const state = tmp("cast-state-"); writeFileSync( join(state, ".coolify.env"), [ diff --git a/test/draft-cli.test.ts b/test/draft-cli.test.ts index 6906217..dbf6083 100644 --- a/test/draft-cli.test.ts +++ b/test/draft-cli.test.ts @@ -2,20 +2,19 @@ import { execFileSync, spawn } from "node:child_process"; import { existsSync, mkdirSync, - mkdtempSync, readFileSync, readdirSync, writeFileSync, } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeAll, describe, expect, it } from "vitest"; import { loadBindings } from "../src/bindings.js"; import { GENERATED_PLACEHOLDER } from "../src/capture.js"; import { loadManifest } from "../src/manifest.js"; import { decryptSecrets } from "../src/secrets.js"; +import { tmp } from "./helpers/tmp.js"; // `cast inventory --emit-draft` against a stub shaped like the box that made it // necessary: a Coolify nobody declared, holding our stack under names someone @@ -215,7 +214,7 @@ let KEY_FILE = ""; let RECIPIENT = ""; beforeAll(() => { - const dir = mkdtempSync(join(tmpdir(), "cast-age-")); + const dir = tmp("cast-age-"); KEY_FILE = join(dir, "key.txt"); execFileSync("age-keygen", ["-o", KEY_FILE], { stdio: "ignore" }); RECIPIENT = execFileSync("age-keygen", ["-y", KEY_FILE], { @@ -224,7 +223,7 @@ beforeAll(() => { }); function fixture(url: string, opts: { recipient?: string } = {}) { - const state = mkdtempSync(join(tmpdir(), "cast-state-")); + const state = tmp("cast-state-"); writeFileSync( join(state, ".coolify.env"), `COOLIFY_BASE_URL="${url}"\nCOOLIFY_ACCESS_TOKEN="t"\n`, @@ -242,7 +241,7 @@ function fixture(url: string, opts: { recipient?: string } = {}) { "", ].join("\n"), ); - const out = join(mkdtempSync(join(tmpdir(), "cast-out-")), "draft"); + const out = join(tmp("cast-out-"), "draft"); return { state, out }; } diff --git a/test/draft.test.ts b/test/draft.test.ts index 2e468f4..db450f7 100644 --- a/test/draft.test.ts +++ b/test/draft.test.ts @@ -1,5 +1,4 @@ -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { mkdirSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { GENERATED_PLACEHOLDER } from "../src/capture.js"; @@ -14,6 +13,7 @@ import { } from "../src/draft.js"; import { templateKeys, templateRefs } from "../src/envtemplate.js"; import { loadManifest } from "../src/manifest.js"; +import { tmp } from "./helpers/tmp.js"; const ctx = { env: "prod", @@ -240,7 +240,7 @@ describe("planDraft — the emitted shape", () => { expect(manifest?.content).toContain("`apply` does not read this file"); expect(manifest?.content).toContain("box-b"); - const dir = mkdtempSync(join(tmpdir(), "cast-draft-")); + const dir = tmp("cast-draft-"); const path = join(dir, "manifest.yaml"); writeFileSync(path, manifest?.content ?? ""); const loaded = loadManifest(path); @@ -285,7 +285,7 @@ describe("planDraft — the emitted shape", () => { }); const plan = planDraft([p], ctx); const manifest = plan.files.find((f) => f.path.endsWith("manifest.yaml")); - const dir = mkdtempSync(join(tmpdir(), "cast-draft-")); + const dir = tmp("cast-draft-"); const path = join(dir, "manifest.yaml"); writeFileSync(path, manifest?.content ?? ""); const build = @@ -325,7 +325,7 @@ describe("planDraft — the emitted shape", () => { }); const plan = planDraft([p], ctx); const manifest = plan.files.find((f) => f.path.endsWith("manifest.yaml")); - const dir = mkdtempSync(join(tmpdir(), "cast-draft-")); + const dir = tmp("cast-draft-"); const path = join(dir, "manifest.yaml"); writeFileSync(path, manifest?.content ?? ""); // The whole point: it loads (does not throw), and simply carries no `static`. @@ -374,7 +374,7 @@ describe("planDraft — the emitted shape", () => { // `static: true` would be exactly the fabrication UNCAPTURED.md exists to // prevent. const manifest = plan.files.find((f) => f.path.endsWith("manifest.yaml")); - const dir = mkdtempSync(join(tmpdir(), "cast-draft-")); + const dir = tmp("cast-draft-"); const path = join(dir, "manifest.yaml"); writeFileSync(path, manifest?.content ?? ""); const build = @@ -598,7 +598,7 @@ describe("backup schedules — read and drafted, not hand-waved (#75)", () => { }); const loadedDb = (plan: ReturnType) => { const manifest = plan.files.find((f) => f.path.endsWith("manifest.yaml")); - const dir = mkdtempSync(join(tmpdir(), "cast-draft-")); + const dir = tmp("cast-draft-"); const path = join(dir, "manifest.yaml"); writeFileSync(path, manifest?.content ?? ""); return loadManifest(path).environments.prod.databases?.[ @@ -690,7 +690,7 @@ describe("service hostnames — read and drafted via the per-service GET (#83)", }); const loadedSvc = (plan: ReturnType) => { const manifest = plan.files.find((f) => f.path.endsWith("manifest.yaml")); - const dir = mkdtempSync(join(tmpdir(), "cast-draft-")); + const dir = tmp("cast-draft-"); const path = join(dir, "manifest.yaml"); writeFileSync(path, manifest?.content ?? ""); return loadManifest(path).environments.prod.services?.["Incubator Umami"]; @@ -739,20 +739,20 @@ describe("service hostnames — read and drafted via the per-service GET (#83)", describe("the emit refusals — adoption is one-way", () => { it("refuses a target directory that is not empty", () => { - const dir = mkdtempSync(join(tmpdir(), "cast-draft-")); + const dir = tmp("cast-draft-"); writeFileSync(join(dir, "README.md"), "a repo lives here\n"); expect(() => assertEmptyTarget(dir)).toThrow(/is not empty/); expect(() => assertEmptyTarget(dir)).toThrow(/Adoption is one-way/); }); it("allows a directory that does not exist yet, and an empty one", () => { - const dir = mkdtempSync(join(tmpdir(), "cast-draft-")); + const dir = tmp("cast-draft-"); expect(() => assertEmptyTarget(dir)).not.toThrow(); expect(() => assertEmptyTarget(join(dir, "new"))).not.toThrow(); }); it("refuses to write a manifest over one that already exists", () => { - const dir = mkdtempSync(join(tmpdir(), "cast-draft-")); + const dir = tmp("cast-draft-"); mkdirSync(join(dir, ".infra"), { recursive: true }); const path = join(dir, ".infra", "manifest.yaml"); writeFileSync(path, "project: incubator\n"); diff --git a/test/fleet-cli.test.ts b/test/fleet-cli.test.ts index 2f68941..7d5203a 100644 --- a/test/fleet-cli.test.ts +++ b/test/fleet-cli.test.ts @@ -1,10 +1,10 @@ import { execFileSync, spawn } from "node:child_process"; -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { mkdirSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeAll, describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; // `cast diff --all` / `cast apply --all` (#26), end to end against a stub // Coolify carrying three projects. @@ -26,7 +26,7 @@ let recipient: string; let keyFile: string; beforeAll(() => { - const dir = mkdtempSync(join(tmpdir(), "cast-age-")); + const dir = tmp("cast-age-"); keyFile = join(dir, "age.key"); execFileSync("age-keygen", ["-o", keyFile], { stdio: "pipe" }); recipient = execFileSync("age-keygen", ["-y", keyFile], { @@ -134,7 +134,7 @@ function fixture( url: string, opts: { registry?: string[]; registryEnv?: string; refIn?: string } = {}, ) { - const root = mkdtempSync(join(tmpdir(), "cast-fleet-")); + const root = tmp("cast-fleet-"); for (const repo of REPOS) { const dir = join(root, "repos", "heavy-duty", `${repo}.git`); mkdirSync(join(dir, ".infra"), { recursive: true }); diff --git a/test/github-app-cli.test.ts b/test/github-app-cli.test.ts index dd8bb75..6f8d86d 100644 --- a/test/github-app-cli.test.ts +++ b/test/github-app-cli.test.ts @@ -1,10 +1,10 @@ import { execFileSync, spawn } from "node:child_process"; -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { mkdirSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeAll, describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; // When does `apply` need a GitHub App at all? (#103, found live in the // 2026-07-19 release drill.) @@ -26,7 +26,7 @@ let recipient: string; let keyFile: string; beforeAll(() => { - const dir = mkdtempSync(join(tmpdir(), "cast-age-")); + const dir = tmp("cast-age-"); keyFile = join(dir, "age.key"); execFileSync("age-keygen", ["-o", keyFile], { stdio: "pipe" }); recipient = execFileSync("age-keygen", ["-y", keyFile], { @@ -112,11 +112,11 @@ environments: // The state file the issue is about: no github_apps entry for this repo at all. function fixture(url: string, manifest: string) { - const checkout = mkdtempSync(join(tmpdir(), "cast-co-")); + const checkout = tmp("cast-co-"); mkdirSync(join(checkout, ".infra", "env"), { recursive: true }); writeFileSync(join(checkout, ".infra", "manifest.yaml"), manifest); - const state = mkdtempSync(join(tmpdir(), "cast-state-")); + const state = tmp("cast-state-"); mkdirSync(join(state, "secrets")); writeFileSync( join(state, ".coolify.env"), diff --git a/test/greenfield-cli.test.ts b/test/greenfield-cli.test.ts index b395fbe..d5afbc2 100644 --- a/test/greenfield-cli.test.ts +++ b/test/greenfield-cli.test.ts @@ -1,10 +1,10 @@ import { execFileSync, spawn } from "node:child_process"; -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { mkdirSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeAll, describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; // The greenfield manifest-first bootstrap (#104), end to end: fresh box, // registered project, a manifest that declares databases only and refs no @@ -23,7 +23,7 @@ let recipient: string; let keyFile: string; beforeAll(() => { - const dir = mkdtempSync(join(tmpdir(), "cast-age-")); + const dir = tmp("cast-age-"); keyFile = join(dir, "age.key"); execFileSync("age-keygen", ["-o", keyFile], { stdio: "pipe" }); recipient = execFileSync("age-keygen", ["-y", keyFile], { @@ -100,7 +100,7 @@ function fixture( manifest: ZERO_REFS_MANIFEST, }, ) { - const checkout = mkdtempSync(join(tmpdir(), "cast-co-")); + const checkout = tmp("cast-co-"); mkdirSync(join(checkout, ".infra", "env"), { recursive: true }); writeFileSync(join(checkout, ".infra", "manifest.yaml"), opts.manifest); writeFileSync( @@ -108,7 +108,7 @@ function fixture( "API_KEY=${API_KEY}\n", ); - const state = mkdtempSync(join(tmpdir(), "cast-state-")); + const state = tmp("cast-state-"); mkdirSync(join(state, "secrets")); writeFileSync( join(state, ".coolify.env"), @@ -148,7 +148,7 @@ function run( const { CAST_AGE_KEY_FILE_STAGING: _dropped, ...inherited } = process.env; const env = opts.withKey ? { ...inherited, CAST_AGE_KEY_FILE_STAGING: keyFile } - : { ...inherited, HOME: mkdtempSync(join(tmpdir(), "cast-home-")) }; + : { ...inherited, HOME: tmp("cast-home-") }; const child = spawn("node", ["dist/cli.js", "diff", ...args], { stdio: ["pipe", "pipe", "pipe"], env, diff --git a/test/helpers/global-setup.ts b/test/helpers/global-setup.ts new file mode 100644 index 0000000..5048db9 --- /dev/null +++ b/test/helpers/global-setup.ts @@ -0,0 +1,39 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +// The env var that tells `tmp()` (test/helpers/tmp.ts) where to allocate. +export const RUN_ROOT_ENV = "CAST_TEST_TMP_ROOT"; + +// One directory per `vitest run`, holding every temp dir the suite allocates. +// +// This is what actually reaps the suite's temp dirs (#117). The obvious design — +// each worker cleaning up after itself in a `process.once("exit")` hook — does +// NOT work under vitest, and it fails silently, which is worse: vitest recycles +// its pool workers by killing them, so `exit` handlers registered inside a test +// file never run. Measured, not assumed: a probe test that wrote a file from an +// `exit` hook produced no file, and a full suite run with per-worker exit hooks +// still left 750 directories behind. +// +// globalSetup's teardown runs in vitest's MAIN process, after every worker has +// finished, and vitest awaits it. That makes it the only hook in the run with +// both of the properties this needs: it is guaranteed to execute, and it sees +// the whole run rather than one worker's slice of it. +// +// Collapsing the whole run into a single root is what makes that teardown one +// `rmSync` instead of a list to keep in sync across processes — the workers do +// not have to report anything back, because the parent already knows the one +// path that contains everything. +export function setup(): () => void { + const root = mkdtempSync(join(tmpdir(), "cast-testrun-")); + process.env[RUN_ROOT_ENV] = root; + + return function teardown(): void { + // Best-effort: a failure to clean up must not turn a green run red. + try { + rmSync(root, { recursive: true, force: true }); + } catch { + // ignore + } + }; +} diff --git a/test/helpers/tmp.ts b/test/helpers/tmp.ts new file mode 100644 index 0000000..8f3b158 --- /dev/null +++ b/test/helpers/tmp.ts @@ -0,0 +1,54 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { RUN_ROOT_ENV } from "./global-setup.js"; + +// Every temp dir this process has handed out, in creation order. +const created: string[] = []; + +// Where to allocate. Under `vitest run` this is the per-run root that +// global-setup.ts created and will remove wholesale when the run ends; the +// fallback keeps `tmp()` usable if a file is ever executed outside that config. +function base(): string { + return process.env[RUN_ROOT_ENV] ?? tmpdir(); +} + +// Belt-and-braces reaper for the fallback case only. +// +// It is deliberately NOT the primary mechanism. Vitest recycles its pool workers +// by killing them, so an `exit` handler registered from a test file does not run +// — verified with a probe test, and by a full suite run that still leaked 750 +// directories with this hook in place. The real cleanup is global-setup.ts's +// teardown, which runs in the main process where an exit IS orderly. This hook +// only earns its keep when `tmp()` is called with no run root set, where nothing +// else would ever remove the directory. +let armed = false; + +function arm(): void { + if (armed) return; + armed = true; + process.once("exit", () => { + for (const dir of created) { + try { + rmSync(dir, { recursive: true, force: true }); + } catch { + // ignore + } + } + created.length = 0; + }); +} + +/** + * Create a temp dir and register it for cleanup. Drop-in replacement for + * `mkdtempSync(join(tmpdir(), prefix))` — the prefix survives as the directory's + * basename, so paths stay as greppable as they were. + * + * @param prefix e.g. `"cast-home-"` — mkdtemp appends six random characters. + */ +export function tmp(prefix: string): string { + arm(); + const dir = mkdtempSync(join(base(), prefix)); + created.push(dir); + return dir; +} diff --git a/test/install-sh.test.ts b/test/install-sh.test.ts index 54731e3..239cedb 100644 --- a/test/install-sh.test.ts +++ b/test/install-sh.test.ts @@ -4,16 +4,15 @@ import { copyFileSync, existsSync, mkdirSync, - mkdtempSync, readFileSync, readlinkSync, realpathSync, writeFileSync, } from "node:fs"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { promisify } from "node:util"; import { describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; const run = promisify(execFile); @@ -59,7 +58,7 @@ type Sandbox = { }; function sandbox(): Sandbox { - const root = mkdtempSync(join(tmpdir(), "cast-install-")); + const root = tmp("cast-install-"); const stubs = join(root, "stubs"); const home = join(root, "home"); const dest = join(root, "cast-home"); diff --git a/test/layout-cli.test.ts b/test/layout-cli.test.ts index 35b84cc..22e1c6b 100644 --- a/test/layout-cli.test.ts +++ b/test/layout-cli.test.ts @@ -4,15 +4,14 @@ import { copyFileSync, existsSync, mkdirSync, - mkdtempSync, readFileSync, realpathSync, writeFileSync, } from "node:fs"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { promisify } from "node:util"; import { describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; const run = promisify(execFile); @@ -50,7 +49,7 @@ type Sandbox = { }; async function installedSandbox(versions: string[]): Promise { - const root = mkdtempSync(join(tmpdir(), "cast-layout-")); + const root = tmp("cast-layout-"); const stubs = join(root, "stubs"); const home = join(root, "home"); const dest = join(root, "cast-home"); diff --git a/test/placement-cli.test.ts b/test/placement-cli.test.ts index 15f44af..1d5d1c9 100644 --- a/test/placement-cli.test.ts +++ b/test/placement-cli.test.ts @@ -1,10 +1,10 @@ import { execFileSync, spawn } from "node:child_process"; -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { mkdirSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeAll, describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; // Placement, end to end: `environments.yaml` -> `cast diff` -> what it prints // and what it exits with. The unit tests prove each half (bindings resolve a @@ -20,7 +20,7 @@ let recipient: string; let keyFile: string; beforeAll(() => { - const dir = mkdtempSync(join(tmpdir(), "cast-age-")); + const dir = tmp("cast-age-"); keyFile = join(dir, "age.key"); execFileSync("age-keygen", ["-o", keyFile], { stdio: "pipe" }); recipient = execFileSync("age-keygen", ["-y", keyFile], { @@ -103,11 +103,11 @@ environments: // `declared` is the destination_uuid the state file names for this project — // undefined means the state file says nothing, which is every state file today. function fixture(url: string, declared?: string) { - const checkout = mkdtempSync(join(tmpdir(), "cast-co-")); + const checkout = tmp("cast-co-"); mkdirSync(join(checkout, ".infra", "env"), { recursive: true }); writeFileSync(join(checkout, ".infra", "manifest.yaml"), MANIFEST); - const state = mkdtempSync(join(tmpdir(), "cast-state-")); + const state = tmp("cast-state-"); mkdirSync(join(state, "secrets")); writeFileSync( join(state, ".coolify.env"), diff --git a/test/read-side-cli.test.ts b/test/read-side-cli.test.ts index 6f7facd..63ff1f7 100644 --- a/test/read-side-cli.test.ts +++ b/test/read-side-cli.test.ts @@ -1,10 +1,10 @@ import { execFileSync, spawn } from "node:child_process"; -import { existsSync, mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeAll, describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; // The read side, end to end, against a stub Coolify shaped like a box nobody // declared: its project is `Incubator` (capital I), its environment is @@ -31,7 +31,7 @@ const LIVE_ENV = { let recipient: string; beforeAll(() => { - const dir = mkdtempSync(join(tmpdir(), "cast-age-")); + const dir = tmp("cast-age-"); const keyFile = join(dir, "age.key"); execFileSync("age-keygen", ["-o", keyFile], { stdio: "pipe" }); recipient = execFileSync("age-keygen", ["-y", keyFile], { @@ -103,14 +103,14 @@ ADMIN_EMAIL=\${ADMIN_EMAIL} `; function fixture(url: string) { - const checkout = mkdtempSync(join(tmpdir(), "cast-co-")); + const checkout = tmp("cast-co-"); mkdirSync(join(checkout, ".infra", "env"), { recursive: true }); writeFileSync(join(checkout, ".infra", "manifest.yaml"), MANIFEST); writeFileSync( join(checkout, ".infra", "env", "core.staging.env.template"), TEMPLATE, ); - const state = mkdtempSync(join(tmpdir(), "cast-state-")); + const state = tmp("cast-state-"); mkdirSync(join(state, "secrets")); writeFileSync( join(state, ".coolify.env"), diff --git a/test/release.test.ts b/test/release.test.ts index 368d352..7161ef6 100644 --- a/test/release.test.ts +++ b/test/release.test.ts @@ -4,16 +4,15 @@ import { cpSync, existsSync, mkdirSync, - mkdtempSync, readFileSync, readlinkSync, realpathSync, writeFileSync, } from "node:fs"; -import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; // The release flow (#96), proven offline. Two surfaces: the changelog-section // extraction release.yml publishes (.github/scripts/release-notes.sh), and @@ -84,7 +83,7 @@ Intro prose that belongs to no section. `; describe("release-notes.sh", () => { - const work = mkdtempSync(join(tmpdir(), "cast-relnotes-")); + const work = tmp("cast-relnotes-"); const fix = join(work, "CHANGELOG.md"); writeFileSync(fix, FIXTURE); const notes = (ver: string, file = fix) => run("bash", [NOTES, ver, file]); @@ -277,7 +276,7 @@ function disarmedBecause(version: string, changelog: string): string | null { } describe("the changelog is armed for the next entry (rig#66)", () => { - const work = mkdtempSync(join(tmpdir(), "cast-arming-")); + const work = tmp("cast-arming-"); const dated = (v: string) => `## ${v} — 2026-07-19`; const body = "\n\n- **An entry** — prose.\n"; const armed = `# Changelog\n\n## Unreleased${body}\n${dated("0.2.0")}${body}`; @@ -859,7 +858,7 @@ describe("release.yml", () => { // test opts into the stub build — so any release-channel install that // touches npm fails its assertion by failing the install. -const STUB = mkdtempSync(join(tmpdir(), "cast-stub-")); +const STUB = tmp("cast-stub-"); writeFileSync( join(STUB, "curl"), `#!/usr/bin/env bash @@ -943,7 +942,7 @@ async function runInstall( env: Record, opts: { preexistingDest?: boolean } = {}, ): Promise { - const work = mkdtempSync(join(tmpdir(), "cast-inst-")); + const work = tmp("cast-inst-"); const dest = join(work, "dest"); const bin = join(work, "bin"); const curlLog = join(work, "curl.log"); @@ -976,7 +975,7 @@ async function runInstall( } describe("install.sh — the three channels", () => { - const work = mkdtempSync(join(tmpdir(), "cast-tarballs-")); + const work = tmp("cast-tarballs-"); const asset = makeTarball(work, "9.9.9"); const mainSrc = makeTarball(work, "main"); const brokenAsset = makeTarball(join(work, "broken"), "9.9.9", { diff --git a/test/reserved.test.ts b/test/reserved.test.ts index 799de8b..4ef0f10 100644 --- a/test/reserved.test.ts +++ b/test/reserved.test.ts @@ -1,5 +1,4 @@ -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { mkdirSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { classify } from "../src/capture.js"; @@ -12,6 +11,7 @@ import { requiredSecrets, } from "../src/resolve.js"; import { SMOKE_KEEP_KEY, SMOKE_PROBE_KEY } from "../src/smoke.js"; +import { tmp } from "./helpers/tmp.js"; // #50. Coolify injects SOURCE_COMMIT and the COOLIFY_* family itself, and SKIPS // its own injection of a name the resource already carries a var of @@ -58,7 +58,7 @@ describe("the rule", () => { // --- resolve / apply: REFUSE --------------------------------------------------- function checkout(template: string, envName = "staging"): string { - const dir = mkdtempSync(join(tmpdir(), "infra-reserved-")); + const dir = tmp("infra-reserved-"); mkdirSync(join(dir, ".infra", "env"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), diff --git a/test/resolve.test.ts b/test/resolve.test.ts index ebe90dc..ed9b0a4 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -1,6 +1,8 @@ -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { execFileSync } from "node:child_process"; +import { existsSync, mkdirSync, readdirSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { pathToFileURL } from "node:url"; import { describe, expect, it, vi } from "vitest"; import { computeDiff } from "../src/diff.js"; import { DERIVED_UNRESOLVED } from "../src/envtemplate.js"; @@ -12,6 +14,7 @@ import { resolveCheckout, resolveGitAuth, } from "../src/resolve.js"; +import { tmp } from "./helpers/tmp.js"; describe("resolveCheckout", () => { it("hard-refuses --path with prod", () => { @@ -27,6 +30,57 @@ describe("resolveCheckout", () => { }), ).toBe("/tmp/x"); }); + + // #117: the ephemeral checkout used to survive the process that made it, so + // every `cast apply`/`diff`/`capture` without --path left a full clone behind. + // This has to run in a real child process — the reaper is an exit hook, and + // the thing under test is precisely what happens when the process ends. + // + // A stub `git` on PATH makes it hermetic and fast: the clone fails, which is + // the *harder* case, since the directory is created before the clone runs and + // the failure path rethrows. If the dir is gone after a failed clone, the + // registration happens early enough to cover the successful one too. + it("removes the ephemeral checkout when the process exits", () => { + const bin = tmp("cast-fakebin-"); + writeFileSync(join(bin, "git"), "#!/bin/sh\nexit 1\n", { mode: 0o755 }); + + // Other suites (and other machines) have their own checkouts lying around; + // only the one this child allocates is ours to assert on. + const preexisting = new Set( + readdirSync(tmpdir()).filter((d) => d.startsWith("infra-checkout-")), + ); + + const script = ` + const { resolveCheckout } = await import(${JSON.stringify( + pathToFileURL(join(process.cwd(), "dist/resolve.js")).href, + )}); + try { resolveCheckout("acme/widget", { env: "dev" }); } catch {} + // Report what was allocated, then let the process exit normally. + const fs = await import("node:fs"); + const os = await import("node:os"); + console.log(JSON.stringify( + fs.readdirSync(os.tmpdir()).filter((d) => d.startsWith("infra-checkout-")), + )); + `; + + const out = execFileSync( + process.execPath, + ["--input-type=module", "-e", script], + { + env: { ...process.env, PATH: `${bin}:${process.env.PATH}` }, + encoding: "utf8", + }, + ); + + const allocated: string[] = JSON.parse( + out.trim().split("\n").pop() ?? "[]", + ); + const mine = allocated.filter((d) => !preexisting.has(d)); + // It must have allocated exactly one — otherwise this test proves nothing. + expect(mine).toHaveLength(1); + // ...and that one must be gone now that the child has exited. + expect(existsSync(join(tmpdir(), mine[0]))).toBe(false); + }); }); describe("resolveGitAuth", () => { @@ -119,7 +173,7 @@ describe("cloneFailureMessage", () => { describe("desiredFromManifest", () => { it("maps manifest + templates to Desired[] with resolved env", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra", "env"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -171,7 +225,7 @@ environments: expect(desired[0].fields).not.toHaveProperty("start_command"); }); it("emits is_static:false when static:false is explicitly declared (a guard against a UI flip)", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -190,7 +244,7 @@ environments: }); // #63: the static-site build settings a workspace monorepo needs. it("emits is_static:true and the three commands for a non-compose app that declares them", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -226,7 +280,7 @@ environments: // /databases/{uuid}/backups), and the side channel is what made a `backup:` // block added to an existing database silently do nothing (#51). it("puts a database backup block in fields, so it is diffed like any other", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -249,7 +303,7 @@ environments: }); }); it("leaves `backup` out of fields entirely when none is declared", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -271,7 +325,7 @@ environments: expect("backup" in desired[0].fields).toBe(false); }); it("emits a service's service_domains into fields, canonicalized (cast#72)", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -297,7 +351,7 @@ environments: }); }); it("is clean for a service whose live per-container hostnames match (cast#72, no perpetual update)", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -331,7 +385,7 @@ environments: expect(report.clean).toBe(true); }); it("diffs a service whose declared hostname is missing live (apply will set it)", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -370,7 +424,7 @@ environments: ]); }); it("a service with no service_domains carries only its type", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -387,7 +441,7 @@ environments: expect(desired[0].fields).toEqual({ type: "plausible" }); }); it("resolves a dockercompose app to docker_compose_location/docker_compose_domains and no port/healthcheck/domains keys", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra", "env"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -436,7 +490,7 @@ environments: expect(desired[0].fields).not.toHaveProperty("start_command"); }); it('warns that apply cannot enable "Include Source Commit in Build" on a dockercompose app (unsettable via the Coolify 4.1.2 API)', () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -466,7 +520,7 @@ environments: ); }); it("does not warn about the source-commit toggle for a non-dockercompose app (the build arg is a compose concern)", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -486,7 +540,7 @@ environments: warn.mockRestore(); }); it("throws when the env is missing from the manifest", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -505,7 +559,7 @@ describe("derived resource refs (#60)", () => { ref = "${resource:postgres.url}", dbBlock = " databases:\n postgres: { type: postgresql }\n", ): string => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra", "env"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), @@ -580,7 +634,7 @@ describe("derived domain refs (#66)", () => { // Assemble a manifest from an applications block plus one env template. The // env_template line is appended to whichever app comes last in `apps`. const write = (apps: string, tmpl: string): string => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + const dir = tmp("infra-co-"); mkdirSync(join(dir, ".infra", "env"), { recursive: true }); writeFileSync( join(dir, ".infra", "manifest.yaml"), diff --git a/test/secrets.test.ts b/test/secrets.test.ts index da908f5..3f19fe8 100644 --- a/test/secrets.test.ts +++ b/test/secrets.test.ts @@ -1,19 +1,13 @@ import { execFileSync, spawn } from "node:child_process"; -import { - closeSync, - mkdirSync, - mkdtempSync, - openSync, - writeFileSync, -} from "node:fs"; -import { tmpdir } from "node:os"; +import { closeSync, mkdirSync, openSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { decryptSecrets, keyFileFor, secretsFileFor } from "../src/secrets.js"; +import { tmp } from "./helpers/tmp.js"; // A key file and a store encrypted to it, for the decrypt tests. function ageFixture(): { keyFile: string; enc: string } { - const dir = mkdtempSync(join(tmpdir(), "infra-age-")); + const dir = tmp("infra-age-"); const keyFile = join(dir, "key.txt"); execFileSync("age-keygen", ["-o", keyFile]); const recipient = execFileSync("age-keygen", ["-y", keyFile], { @@ -70,7 +64,7 @@ describe("secretsFileFor", () => { describe("decryptSecrets identity caching", () => { it("a read-once pipe key survives two decrypts — the --all loop shape", () => { const { keyFile, enc } = ageFixture(); - const dir = mkdtempSync(join(tmpdir(), "infra-fifo-")); + const dir = tmp("infra-fifo-"); const fifo = join(dir, "key.fifo"); execFileSync("mkfifo", [fifo]); // One writer, one serving of the key: exactly what a process substitution @@ -124,7 +118,7 @@ describe("keyFileFor", () => { // Isolate $HOME: a standing age-drill-b.key on the dev machine must not // turn the refusal into a hit (os.homedir() reads $HOME on POSIX). const home = process.env.HOME; - process.env.HOME = mkdtempSync(join(tmpdir(), "cast-home-")); + process.env.HOME = tmp("cast-home-"); try { expect(() => keyFileFor("drill-b")).toThrow( /no age key for drill-b.*CAST_AGE_KEY_FILE_DRILL_B.*age-drill-b\.key/s, @@ -135,7 +129,7 @@ describe("keyFileFor", () => { }); it("falls back to a standing key on disk when one exists", () => { const home = process.env.HOME; - const dir = mkdtempSync(join(tmpdir(), "cast-home-")); + const dir = tmp("cast-home-"); const cfg = join(dir, ".config", "cast"); mkdirSync(cfg, { recursive: true }); writeFileSync(join(cfg, "age-staging.key"), "AGE-SECRET-KEY-1\n"); diff --git a/test/smoke-cli.test.ts b/test/smoke-cli.test.ts index abcc068..79fca9d 100644 --- a/test/smoke-cli.test.ts +++ b/test/smoke-cli.test.ts @@ -1,10 +1,10 @@ import { spawn } from "node:child_process"; -import { mkdtempSync, writeFileSync } from "node:fs"; +import { writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; // `cast smoke`, end to end, against the instance shape #29 is actually about. // @@ -167,7 +167,7 @@ function state( "acme/client-site": "core", }, ): string { - const dir = mkdtempSync(join(tmpdir(), "cast-smoke-")); + const dir = tmp("cast-smoke-"); writeFileSync( join(dir, ".coolify.env"), `COOLIFY_BASE_URL="${url}"\nCOOLIFY_ACCESS_TOKEN="t"\n`, diff --git a/test/sweep-cli.test.ts b/test/sweep-cli.test.ts index abd004e..826c7f8 100644 --- a/test/sweep-cli.test.ts +++ b/test/sweep-cli.test.ts @@ -1,10 +1,10 @@ import { spawn } from "node:child_process"; -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { mkdirSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; -import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { tmp } from "./helpers/tmp.js"; // The sweep, against a stub shaped like the box that made it necessary. // @@ -91,10 +91,10 @@ environments: `; function fixture(url: string) { - const checkout = mkdtempSync(join(tmpdir(), "cast-co-")); + const checkout = tmp("cast-co-"); mkdirSync(join(checkout, ".infra", "env"), { recursive: true }); writeFileSync(join(checkout, ".infra", "manifest.yaml"), MANIFEST); - const state = mkdtempSync(join(tmpdir(), "cast-state-")); + const state = tmp("cast-state-"); mkdirSync(join(state, "secrets")); writeFileSync( join(state, ".coolify.env"), diff --git a/test/tmp-guard.test.ts b/test/tmp-guard.test.ts new file mode 100644 index 0000000..a667a3c --- /dev/null +++ b/test/tmp-guard.test.ts @@ -0,0 +1,52 @@ +import { readFileSync, readdirSync, statSync } from "node:fs"; +import { join, relative } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const TEST_DIR = fileURLToPath(new URL(".", import.meta.url)); + +// The only files allowed to call the raw API: the allocator and the per-run +// root that global setup reaps. Everything else goes through `tmp()`. +const HELPER = "helpers/tmp.ts"; +const ALLOWED = new Set([HELPER, "helpers/global-setup.ts"]); + +// Assembled at runtime rather than written out, so THIS file is not itself a +// hit. Exempting the guard by path instead would punch a permanent hole in the +// very check it performs. +const NEEDLE = ["mkdtemp", "Sync"].join(""); + +function walk(dir: string, acc: string[] = []): string[] { + for (const entry of readdirSync(dir)) { + if (entry === "node_modules") continue; + const full = join(dir, entry); + if (statSync(full).isDirectory()) walk(full, acc); + else acc.push(full); + } + return acc; +} + +// The class guard the issue asks for (#117): the suite leaked 6731 temp dirs +// because cleanup was opt-in and 68 sites opted out. Making `tmp()` the only +// way to allocate is what keeps the next site clean by default — and this test +// is what stops the next raw call surviving review, the same shape as box#112's +// eof_guard_sweep. It is a source-text check on purpose: it fails at the point +// the habit returns, not after a machine-day of accumulation. +describe("temp dir allocation", () => { + it(`uses the tmp() helper everywhere — no raw ${NEEDLE} under test/`, () => { + const offenders = walk(TEST_DIR) + .filter((f) => /\.(ts|js|mjs|sh)$/.test(f)) + .filter((f) => !ALLOWED.has(relative(TEST_DIR, f))) + .filter((f) => readFileSync(f, "utf8").includes(NEEDLE)) + .map((f) => relative(TEST_DIR, f)) + .sort(); + + expect(offenders).toEqual([]); + }); + + it("keeps the helper as the single allocation point", () => { + const helper = readFileSync(join(TEST_DIR, HELPER), "utf8"); + expect(helper).toContain(NEEDLE); + // and it must actually reap + expect(helper).toContain("rmSync"); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 808ec69..91ae773 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,2 +1,8 @@ import { defineConfig } from "vitest/config"; -export default defineConfig({ test: { include: ["test/**/*.test.ts"] } }); +export default defineConfig({ + test: { + include: ["test/**/*.test.ts"], + // Creates the per-run temp root and removes it when the run ends (#117). + globalSetup: ["./test/helpers/global-setup.ts"], + }, +});