HOMELAB-2682: test(sandbox-init): stop temp-dir leaks #888

Merged

pi-landing merged 1 commit from plane/HOMELAB-2682-sandbox-init-tmpfix into live 2026-11-07 06:40:23 +00:00

Conversation 2 Commits 1 Files changed 8 +200 -26

pi-landing commented 2026-11-07 06:31:20 +00:00

Collaborator

Copy link

What

The sandbox-init suites built real trees under $TMPDIR and never removed them. This PR makes every maker hand its directory to a new tracked helper (test/temp-dirs.mjs) and removes the tracked dirs from a top-level node:test after hook in each suite file.

Test code only — no production file changed.

Measured (mgmt VM, node v22.19.0, run-tests.sh)

/tmp inventory when this was measured:

family	count
/tmp/sandbox-init-fixture-*	5,829
/tmp/sandbox-init-agent-*	1,809

One full suite run before the fix (measured twice, same result):

	before run	after run	delta
sandbox-init-fixture-*	5,829	5,873	+44
all sandbox-init-*	8,442	8,528	+86

Per-family delta for that run: fixture +44, agent +9, slice +6, origin +5, e2e +6, username +4, hostile +2, driver/idempotent/prune/restart/empty/tiny/good/escape/shquote +1 each.

After the fix:

run	fixture delta	all sandbox-init-* delta
full suite, run 1	+0	+0
full suite, run 2	+0	+0
full suite, run 3	+0	+0
full suite with a deliberately failing test	+0	+0

Suite result: run-tests.sh: OK — 64/64 passed (floor 60), 0 failed (was 60/60; the floor is untouched — the +4 is the three new tests in temp-dirs.test.mjs plus one more no-test file that node's default discovery counts, exactly as test/fixtures.mjs already was).

Negative controls (proving the measurement is not vacuous)

• Reintroducing a bare mkdtempSync in git-slice.test.mjs fails the guard: git-slice.test.mjs calls mkdtempSync directly — use makeTempDir().
• Removing the hook from render.test.mjs alone leaks again: +42 dirs from that one file.

How cleanup works

test/temp-dirs.mjs:

• makeTempDir(prefix) — mkdtemp under $TMPDIR, recorded.
• trackTempDir(dir) — record a caller-owned dir.
• cleanupTempDirs() — rm -rf the tracked dirs, idempotent, tolerates an already-removed dir (force), reports anything it could not remove instead of leaking silently.

Each suite file calls after(cleanupTempDirs) once. A top-level after hook runs after every test in the file including after failures (verified on node 22 with a failing test and on the full suite with a deliberately failing test — +0 both times). Nothing here imports node:test: fixtures.mjs is also imported by the LD1.3 proof harness, which is not a test file.

Cleanup is scoped strictly to tracked dirs — no globbing, no prefix sweep — so a directory this suite did not create is never touched (tested).

Files changed

file	change
agent-sandbox/pi-image/sandbox-init/test/temp-dirs.mjs	new — tracked-dir helper
agent-sandbox/pi-image/sandbox-init/test/temp-dirs.test.mjs	new — helper behaviour + "no bare mkdtempSync in any suite file" source guard + untracked-dir-survives case
agent-sandbox/pi-image/sandbox-init/test/fixtures.mjs	buildFixturePackage default baseDir → makeTempDir("sandbox-init-fixture-")
agent-sandbox/pi-image/sandbox-init/test/render.test.mjs	scratch() → makeTempDir, after(cleanupTempDirs)
agent-sandbox/pi-image/sandbox-init/test/security.test.mjs	scratch() → makeTempDir, after(cleanupTempDirs)
agent-sandbox/pi-image/sandbox-init/test/sandbox-init.e2e.test.mjs	sandbox() root → makeTempDir, after(cleanupTempDirs)
agent-sandbox/pi-image/sandbox-init/test/git-slice.test.mjs	makeOrigin()/scratch() → makeTempDir, after(cleanupTempDirs)
agent-sandbox/pi-image/sandbox-init/test/run-tests-guard.test.mjs	three synthetic suite dirs → makeTempDir, after(cleanupTempDirs)

for file in ...; git diff --name-only — every path is under agent-sandbox/pi-image/sandbox-init/test/. No lib/, no sandbox-init.mjs, no manifest, no workflow.

Notes for the reviewer

• The image copies the whole directory (COPY sandbox-init/ /opt/sandbox-init/), so the new helper ships with the suite; CI's in-image run-tests.sh step exercises it.
• Same pattern as the already-landed pi-extensions fix (PR #119, forgejo-fake-openbao-* / redact-test-*): tracked created dirs + the test framework's own teardown hook.
• Pre-existing /tmp debris is not deleted by this change: cleanup is scoped to dirs this process created. Removing the ~7.6k stale dirs on the mgmt VM is a separate, deliberate action.

## What The sandbox-init suites built real trees under `$TMPDIR` and never removed them. This PR makes every maker hand its directory to a new tracked helper (`test/temp-dirs.mjs`) and removes the tracked dirs from a top-level `node:test` `after` hook in each suite file. Test code only — no production file changed. ## Measured (mgmt VM, node v22.19.0, `run-tests.sh`) `/tmp` inventory when this was measured: | family | count | |---|---| | `/tmp/sandbox-init-fixture-*` | 5,829 | | `/tmp/sandbox-init-agent-*` | 1,809 | One full suite run **before** the fix (measured twice, same result): | | before run | after run | delta | |---|---|---|---| | `sandbox-init-fixture-*` | 5,829 | 5,873 | **+44** | | all `sandbox-init-*` | 8,442 | 8,528 | **+86** | Per-family delta for that run: `fixture` +44, `agent` +9, `slice` +6, `origin` +5, `e2e` +6, `username` +4, `hostile` +2, `driver`/`idempotent`/`prune`/`restart`/`empty`/`tiny`/`good`/`escape`/`shquote` +1 each. After the fix: | run | fixture delta | all `sandbox-init-*` delta | |---|---|---| | full suite, run 1 | +0 | +0 | | full suite, run 2 | +0 | +0 | | full suite, run 3 | +0 | +0 | | full suite with a deliberately failing test | +0 | +0 | Suite result: `run-tests.sh: OK — 64/64 passed (floor 60), 0 failed` (was 60/60; the floor is untouched — the +4 is the three new tests in `temp-dirs.test.mjs` plus one more no-test file that node's default discovery counts, exactly as `test/fixtures.mjs` already was). ## Negative controls (proving the measurement is not vacuous) - Reintroducing a bare `mkdtempSync` in `git-slice.test.mjs` fails the guard: `git-slice.test.mjs calls mkdtempSync directly — use makeTempDir()`. - Removing the hook from `render.test.mjs` alone leaks again: `+42` dirs from that one file. ## How cleanup works `test/temp-dirs.mjs`: - `makeTempDir(prefix)` — `mkdtemp` under `$TMPDIR`, recorded. - `trackTempDir(dir)` — record a caller-owned dir. - `cleanupTempDirs()` — `rm -rf` the tracked dirs, idempotent, tolerates an already-removed dir (`force`), reports anything it could not remove instead of leaking silently. Each suite file calls `after(cleanupTempDirs)` once. A top-level `after` hook runs after every test in the file **including after failures** (verified on node 22 with a failing test and on the full suite with a deliberately failing test — `+0` both times). Nothing here imports `node:test`: `fixtures.mjs` is also imported by the LD1.3 proof harness, which is not a test file. Cleanup is scoped strictly to tracked dirs — no globbing, no prefix sweep — so a directory this suite did not create is never touched (tested). ## Files changed | file | change | |---|---| | `agent-sandbox/pi-image/sandbox-init/test/temp-dirs.mjs` | **new** — tracked-dir helper | | `agent-sandbox/pi-image/sandbox-init/test/temp-dirs.test.mjs` | **new** — helper behaviour + "no bare `mkdtempSync` in any suite file" source guard + untracked-dir-survives case | | `agent-sandbox/pi-image/sandbox-init/test/fixtures.mjs` | `buildFixturePackage` default `baseDir` → `makeTempDir("sandbox-init-fixture-")` | | `agent-sandbox/pi-image/sandbox-init/test/render.test.mjs` | `scratch()` → `makeTempDir`, `after(cleanupTempDirs)` | | `agent-sandbox/pi-image/sandbox-init/test/security.test.mjs` | `scratch()` → `makeTempDir`, `after(cleanupTempDirs)` | | `agent-sandbox/pi-image/sandbox-init/test/sandbox-init.e2e.test.mjs` | `sandbox()` root → `makeTempDir`, `after(cleanupTempDirs)` | | `agent-sandbox/pi-image/sandbox-init/test/git-slice.test.mjs` | `makeOrigin()`/`scratch()` → `makeTempDir`, `after(cleanupTempDirs)` | | `agent-sandbox/pi-image/sandbox-init/test/run-tests-guard.test.mjs` | three synthetic suite dirs → `makeTempDir`, `after(cleanupTempDirs)` | `for file in ...; git diff --name-only` — every path is under `agent-sandbox/pi-image/sandbox-init/test/`. No `lib/`, no `sandbox-init.mjs`, no manifest, no workflow. ## Notes for the reviewer - The image copies the whole directory (`COPY sandbox-init/ /opt/sandbox-init/`), so the new helper ships with the suite; CI's in-image `run-tests.sh` step exercises it. - Same pattern as the already-landed pi-extensions fix (PR #119, `forgejo-fake-openbao-*` / `redact-test-*`): tracked created dirs + the test framework's own teardown hook. - Pre-existing `/tmp` debris is *not* deleted by this change: cleanup is scoped to dirs this process created. Removing the ~7.6k stale dirs on the mgmt VM is a separate, deliberate action.

pi-landing added 1 commit 2026-11-07 06:31:20 +00:00

HOMELAB-2682: test(sandbox-init): fixtures clean their temp dirs ... b1adb7fe75

The sandbox-init suites build real trees under $TMPDIR and never removed
them. Measured on the mgmt VM before this change: 5,829
`/tmp/sandbox-init-fixture-*` and 1,809 `/tmp/sandbox-init-agent-*`, and a
single full `run-tests.sh` run added another 44 fixture dirs and ~86
sandbox-init-* dirs in total (fixture, agent, origin, slice, empty, tiny,
good, e2e, ...).

Every maker now hands its directory to test/temp-dirs.mjs, and each suite
file removes what it made from a top-level node:test `after` hook:

  makeTempDir(prefix)   mkdtemp under $TMPDIR + track
  trackTempDir(dir)     track a caller-owned dir
  cleanupTempDirs()     rm -rf tracked dirs only; idempotent; tolerates
                        already-removed dirs

Makers covered: test/fixtures.mjs (the buildFixturePackage default baseDir),
render.test.mjs and security.test.mjs scratch(), sandbox-init.e2e.test.mjs
sandbox(), git-slice.test.mjs makeOrigin()/scratch(), and
run-tests-guard.test.mjs's three synthetic suite dirs.

The hook runs on the failure path too (a top-level `after` runs after every
test in the file, including failures), cleanup is scoped strictly to tracked
dirs — no prefix sweep, so nothing this suite did not create is touched — and
temp-dirs.test.mjs guards both the helper's behaviour and a source scan that
refuses a return to bare `mkdtempSync` in any suite file.

Measured after: +0 dirs on three consecutive full suite runs and +0 on a run
with a deliberately failing test. Test code only; no production file changes.

ld-reviewer commented 2026-11-07 06:39:33 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head b1adb7fe75: APPROVE

model: deepseek-flash
session: 01a1b4ba-3efa-702c-a118-4393a70e5546

Blockers

None.

Exposure

git objects of Homelab/infra-core at 0cf5da36..b1adb7fe only; no PR comments, census, or ticket thread read; my only write is this verdict posted as ld-reviewer (the suite was additionally run once from a git archive copy under /tmp, removed afterwards — nothing written to the repo, any PR, or Plane).

Verdict of record — ld-reviewer — head b1adb7fe756ed4dee6cf04f6a5a6c6cb2bf2b714: APPROVE model: deepseek-flash session: 01a1b4ba-3efa-702c-a118-4393a70e5546 ## Blockers None. ### Exposure git objects of Homelab/infra-core at 0cf5da36..b1adb7fe only; no PR comments, census, or ticket thread read; my only write is this verdict posted as ld-reviewer (the suite was additionally run once from a `git archive` copy under /tmp, removed afterwards — nothing written to the repo, any PR, or Plane).

ld-reviewer commented 2026-11-07 06:39:44 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head b1adb7fe75: APPROVE

model: deepseek-flash
session: 01a1b4ba-3f26-702c-a118-4395d4af7644

Blockers

None.

Exposure

git objects of Homelab/infra-core at 0cf5da36..b1adb7fe only; no PR comments, census, or ticket thread read; my only write is this verdict posted as ld-reviewer.

Verdict of record — ld-reviewer — head b1adb7fe756ed4dee6cf04f6a5a6c6cb2bf2b714: APPROVE model: deepseek-flash session: 01a1b4ba-3f26-702c-a118-4395d4af7644 ## Blockers None. ### Exposure git objects of Homelab/infra-core at 0cf5da36..b1adb7fe only; no PR comments, census, or ticket thread read; my only write is this verdict posted as ld-reviewer.

pi-landing merged commit 4dde88e1f0 into live 2026-11-07 06:40:23 +00:00

pi-landing referenced this pull request from a commit 2026-11-07 06:40:25 +00:00

Merge pull request 'HOMELAB-2682: test(sandbox-init): stop temp-dir leaks' (#888) from plane/HOMELAB-2682-sandbox-init-tmpfix into live

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

operator-review

Requires human operator review before merge — dangerous class (RBAC, secrets, network policy, Terraform) or explicitly gated. Never auto-merge.

No labels

operator-review

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Homelab/infra-core!888

No description provided.