fix(tests): remove remaining temp-dir leaks from test suites

[kyle-sexton]

Closes #5223

Summary

Test suites kept leaking into TMPDIR after the earlier fixes for #5223. This branch finishes that work:

• scripts/test-tmp-cleanup-baseline.txt is empty, and scripts/check-test-tmp-cleanup.sh --check passes with it.
• Every suite that writes AGENTS.md fixtures, every *.test.sh, and the Python and Node suites that call mkdtemp were run with an isolated TMPDIR. Each leaves it empty after the fixes below.
• pytest base-directory retention is handled by the earlier commit on this branch (fix(ci): stop pytest tmp_path directories accumulating).
• The plan/ fixture directories are attributed. The tmp.*/plan plus *.manifest directories came from the old clean-batch.sh default, PLAN_DIR="$(mktemp -d)", which clean-batch.test.sh exercised on every run. feat(repo-hygiene): durable batch plan location and dry-run path listing #5580 moved the default plan location under CLAUDE_PLUGIN_DATA, and clean-batch.test.sh now leaves TMPDIR empty. The AGENTS.md/CLAUDE.md/.git fixture directories came from the instruction-placement suites fixed earlier on this branch.

Fix

The static trap check cannot see these, so they were found by running each suite with an isolated TMPDIR and listing what remained:

• clean-build.test.sh, clean-caches.test.sh, audit-fleet.test.sh: default dry-run manifests and plan files come from mktemp in TMPDIR; the suites now point TMPDIR inside their cleaned directory.
• actionlint-check.test.sh: four telemetry files were recreated by the asynchronous sink after the suite removed them; they now live under the cleaned WORK directory.
• hook-utils.test.sh: run17b made a new plugin data directory per call; it now makes it under WORK.
• cutover-check.sh: the ACTION_MAP file leaked when the release-map parse failed, before the EXIT trap was set.
• automode-entry-diff.sh: the --oracle scratch directory was never removed; an EXIT trap now removes it.
• main-module.test.js, cli-entry.test.js (ai-briefing), detect-recoverable-bootstrap.test.js, extract-anchor-frames.test.js, check-watch-outcomes.test.js: fixtures are now removed after the test.
• run-watch-envelope.test.js, run-watch-degradation.test.js, mixed-source-storage-invariant.test.js: run-watch intentionally keeps its temp directories, so the tests point TMPDIR into a directory they remove.

Out of scope: scripts/check-silent-revert.test.sh mk_repo committing into the caller's repository. #4472 is closed and is not closed or reopened here.

Not changed, not ours: surface.test.sh leaves Chromium's own org.chromium.Chromium.* profile directory through playwright-cli, and fails on an existing js array error at line 218 unrelated to this change. check-usage-limit-reset.py keeps a deliberate tzdata cache under TMPDIR.

Verification

• bash scripts/check-test-tmp-cleanup.sh --check: pass, 535 suites, empty baseline.
• bash scripts/validate-plugins.sh: pass.
• check-changelog-parity.sh --check, --check-order and --check-bump origin/main: pass after a patch bump and a changelog entry for each of the 15 touched plugins.
• git merge-tree --write-tree origin/main HEAD: no conflicts.
• Isolated TMPDIR run over all 535 *.test.sh, the 40 Node test files that use temp directories, and the Python suites with mkdtemp: every fixed suite leaves it empty and still passes.
• acquire.test.js "lists files in the real temp directory" times out on this host because /tmp holds about 11,000 entries from these same leaks; it does not write to TMPDIR.

Related

• Test suites and the pr-linkage validator leak files into /tmp (2,238 scratch files, 1,835 fixture repos) #5223
• worktree-create tests leak fixture worktrees into the real worktree root #4472 (worktree-create tests leaked worktrees; closed, untouched)
• feat(repo-hygiene): durable batch plan location and dry-run path listing #5580 (durable clean-batch.sh plan location)

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-30T22:37:24.947734Z	06a9330	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 1m 26s —— View job

---

Security review

Reviewed the full diff (gh pr diff 5633, 81 files) against origin/main per /review:security-review, head SHA 06a9330b50b73911bfd8daa8573d895f826e7d70.

Scope: every file changed is test-only tmp-dir cleanup (added trap 'rm -rf ...' EXIT, narrowing mktemp/mktemp -d calls into an already-cleaned $TMP_ROOT/$WORK/$SCRATCH, or redirecting TMPDIR for the duration of a test), plus mechanical plugin.json version bumps and CHANGELOG.md entries, one .github/workflows/ci.yml line adding -o tmp_path_retention_policy=none to the pytest invocation, and a .gitignore-adjacent baseline file (scripts/test-tmp-cleanup-baseline.txt) being emptied.

No production/runtime code paths change — only test harnesses and their own scratch-directory handling.

Findings: none. Specifically checked and found no issues in:

• New trap ... EXIT cleanup handlers (e.g. automode-entry-diff.sh#L327, cutover-check.sh#L232-233) — all operate on paths produced by mktemp/mktemp -d, no unvalidated/attacker-influenced paths reach rm -rf.
• Test env mutation of process.env.TMPDIR in the run-watch-* and mixed-source-storage-invariant Vitest suites — save/restore is scoped to beforeEach/afterEach in the same file, not exposed beyond the test process.
• No new command construction from untrusted input, no credential/token handling changes, no authorization or access-control logic touched.
• No CLAUDE.md/AGENTS.md/rules/skill/agent-body files are touched, so the instruction-surface-deletion check does not apply.
• GitHub Actions change (ci.yml) is a benign pytest flag addition with no trigger, permissions, or expression-injection surface — deferred to zizmor's lane regardless.

No action needed from a security standpoint.

[claude]

Claude finished @kyle-sexton's task in 2m 52s —— View job

---

Code review complete

• Gather context (PR body, diff, changed files)
• Run skip gate checks (not draft, not trivial, no prior review on this head — proceeded)
• Review changed test/script files for correctness
• Post findings

Summary

Reviewed the full diff (gh pr diff 5633) against the CI code-review criteria (correctness, maintainability, CLAUDE.md alignment). This PR is a large but mechanical sweep: every *.test.sh/*.test.js change follows one of a small number of patterns — introduce a TMP_ROOT/SCRATCH dir with an EXIT trap, redirect mktemp/mktemp -d calls into it, and drop the now-redundant manual rm -rf at the end of the suite. Plus per-plugin plugin.json/CHANGELOG.md version bumps.

I traced the non-trivial cases specifically for correctness, since a botched cleanup fix can silently stop testing what it used to test, or leave a trap that clobbers state on an early exit:

• plugins/instruction-placement/scripts/precompute.test.sh and verify-load.test.sh: confirmed $repo/$empty/$near are all sourced from build_fixture(), which now nests under $SCRATCH, so the deleted manual rm -rf lines are safe — nothing now escapes cleanup.
• plugins/instruction-placement/skills/migrate/scripts/cutover-check.sh (L233): the added rm -f "$ACTION_MAP" sits in the early-exit branch taken before the script's own trap ... EXIT is installed (L253), so without this line the temp file would leak on that specific failure path. Correct, targeted fix.
• plugins/claude-config/skills/audit-permission-state/scripts/automode-entry-diff.sh (L327): verified this is the only trap in the file, so the new trap 'rm -rf "$scratch"' EXIT doesn't clobber an existing one.
• plugins/knowledge/.../run-watch-envelope.test.js, run-watch-degradation.test.js, mixed-source-storage-invariant.test.js: process.env.TMPDIR is set in beforeEach and restored in afterEach (including the undefined-vs-delete distinction), and run-watch runs in-process rather than via spawn, so os.tmpdir() picks up the mutated env correctly — no cross-file leakage since vitest isolates files into separate workers.
• plugins/work-items/tools/work-item-tracker/conformance/bindings/jira.test.sh / local-markdown.test.sh vs linear.test.sh: only linear.test.sh exports TMPDIR="$TMP_ROOT" into the stubbed subshell. This isn't an inconsistency — run-conformance.sh itself already cleans up its own mktemp dirs (BINDING_DIR via its own trap, SKEW_ROOT via explicit rm -rf) regardless of $TMPDIR, so jira/local-markdown don't need the redirect; linear's binding apparently does write something that isn't otherwise cleaned.
• scripts/test-tmp-cleanup-baseline.txt: the removed entries line up 1:1 with the suites that gained a trap/redirect in this diff, consistent with the PR's claim of an empty baseline.
• plugins/testing/scripts/gen-hook-filters.test.sh (L34): the trap is installed one line before the initial cp "$HOOKS" "$backup" runs, so there's a theoretical zero-width window where an interrupt between those two lines would restore an empty $backup over $HOOKS. Not a real risk in practice (adjacent statements, no I/O in between) and not something a careful reviewer would block on — noting it only because I looked, not as a finding.
• .github/workflows/ci.yml: -o tmp_path_retention_policy=none is a real pytest option (6.1+, values all/failed/none), correctly wired to stop tmp_path fixtures accumulating under $TMPDIR/pytest-of-<user>; the code-metrics README documents the same flag.

No correctness or maintainability issues rose to the high-signal bar (nothing a linter/formatter would miss that would actually block this PR). No inline comments posted since there are no findings to anchor.

Author-claimed verification (test runs, check-test-tmp-cleanup.sh --check, validate-plugins.sh, changelog-parity checks) is described in the PR body but not independently re-run here — network/deep fetch access in this review environment is restricted to gh CLI, so I reviewed via gh pr diff rather than executing the suites myself.
· branch

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 06a9330b50

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 1m 3s —— View job

---

Code review complete

• Gather context (PR body, diff, changed files)
• Run skip gate checks
• Review changed files for correctness/maintainability
• Post findings

Summary

This lane already reviewed the full 81-file diff at 06a9330 (both the code-review and security-review passes above) and found nothing to flag. The only change since then is the single commit 11c4dc5 — docs(code-metrics): pass the retention flag in the coverage command, a one-line README fix:

-python3 -m coverage run -m pytest -q && python3 -m coverage json
+python3 -m coverage run -m pytest -q -o tmp_path_retention_policy=none && python3 -m coverage json

This addresses the Codex P2 comment on plugins/code-metrics/README.md:140 (visible above, under "Review comments"): the prose two lines above already told readers to add -o tmp_path_retention_policy=none, but the copy-pasteable coverage command below it didn't carry the flag, so a contributor following the doc verbatim would still accumulate tmp_path directories under $TMPDIR. This commit adds the flag to that command, closing the gap. Verified by reading plugins/code-metrics/README.md:135-144 directly — the prose and the runnable command are now consistent.

No other file changed between 06a9330 and 11c4dc5 (git diff 06a9330..11c4dc5 --stat shows only this one line in one file).

Findings: none. No inline comments posted.

Note: this is a docs-only fix to an already version-bumped plugin (the code-metrics patch bump and changelog entry landed in an earlier commit on this branch, 100c842); I did not re-run check-changelog-parity.sh myself and am not asserting a new bump is required here, only noting it for the author's awareness.
· branch

[claude]

Claude finished @kyle-sexton's task in 1m 28s —— View job

---

Security review

• Gather context (PR body, diff since the previous review, changed files)
• Run skip gate checks (PR open; new head since the last review at 06a9330, so re-reviewed; security-relevant surface: none)
• Review the full diff (gh pr diff 5633, 81 files) against origin/main, head SHA 11c4dc5d4782660a9f9c3c2f039e81c02f3a16a2
• Post findings

Scope: everything already covered by the prior review at 06a9330 (test-only trap ... EXIT cleanup, mktemp/mktemp -d narrowed into an already-cleaned TMP_ROOT/WORK/SCRATCH/TMPDIR, mechanical plugin.json version bumps and CHANGELOG.md entries, and the .github/workflows/ci.yml pytest flag), plus one new commit on top: 11c4dc5 docs(code-metrics): pass the retention flag in the coverage command, which adds -o tmp_path_retention_policy=none to the documented coverage command in plugins/code-metrics/README.md#L140, addressing Codex's P2 documentation-consistency comment. No production/runtime code paths change anywhere in this PR — only test harnesses, their own scratch-directory handling, plugin metadata, and documentation.

Findings: none.

• The new commit is a one-line markdown edit inside a fenced shell example; it has no security-relevant surface (no code execution path, no credential/token handling, no trust-boundary change).
• Re-checked the rest of the diff against the same criteria as the prior pass: every trap ... EXIT / mktemp narrowing operates on paths the script itself produced via mktemp/mktemp -d, so no unvalidated or attacker-influenced path reaches rm -rf; no new command construction from untrusted input; no authorization/access-control logic touched; no CLAUDE.md/AGENTS.md/rules/skill/agent-body files are touched, so the instruction-surface-deletion lens does not apply.
• .github/workflows/ci.yml line is an unchanged-from-last-review benign pytest flag addition with no trigger/permissions/expression-injection surface — deferred to zizmor's lane per this skill's scope.

No action needed from a security standpoint.
· branch