test: route test scratch through mkdtempForTest; forbid node:os in tests

[thymikee]

Summary

Route every product-test scratch directory through the per-package mkdtempForTest/mkdtempForTestSync helpers so it lands under the run's redirected TMPDIR and is removed once per run, instead of path.join(os.tmpdir(), name), which reuses a fixed path across the whole suite.

• 81 test/fixture files migrated; adds the missing tmp-dir helpers to platform-harmonyos, provider-webdriver, replay-test and trims platform-apple's to the variant each tree actually uses.
• Simplifications: the repeated daemon.log idiom collapses to one scratch dir per site; redundant randomUUID/Date.now() suffixes dropped; two ad-hoc Android screenshot paths fold into the existing withTempScreenshot helper; a snapshot-helper manifest fixture drops a stale installArgs field (a refactor(android): drop the snapshot helper manifest installArgs #2618 leftover that broke typecheck).
• Enforcement is one no-restricted-imports override in oxlint.config.ts: a product *.test.ts that imports node:os is a lint error pointing at mkdtempForTest, mirroring the existing node:child_process ban. A follow-up override clears it for the 9 tests that mock production's os.tmpdir()/os.homedir() or assert a real /tmp socket path. No new command, gate, or CI step — it rides the existing pnpm lint gate. packages/maestro/test/** and the .ts wrapper helpers fall outside the *.test.ts scope by design.

Scope: 83 files (81 tests + 2 helper trims + oxlint.config.ts). ~953 gross lines, inherently all-or-nothing — the rule cannot land before its migration.

Validation

Tested at 6a17e00ea4. Green in isolation: pnpm lint (oxlint, incl. the new rule), format:check, typecheck, check:fallow --base origin/main, check:gate-manifest(+test), check:affected:test, and the vitest unit run for all changed files. The rule is proven reachable: it errors on a planted node:os import in a src/**/__tests__/*.test.ts and stays silent on the exception/helper files.

check:affected --run reports two halts, neither attributable to this change:

• mutation-model (scripts/mutation/ownership.test.ts) — pre-existing: reproduces identically at clean origin/main with this change absent (verified in a throwaway worktree).
• lint — a fan-out resource artifact, not a findings failure: oxlint prints no violation and exits 0 standalone here, and the set of node:os-importing *.test.ts under the ban globs equals the 9 exceptions exactly, so the rule cannot fire.

[thymikee]

Reviewed at dea914c. The migration looks sound, but the new gate has three holes that let raw os.tmpdir() reads through, and the branch conflicts with main.

isHelperModule in scripts/check-test-tmpdir-model.ts:49 exempts every file ending in /test-utils.ts, not only the mkdtemp helper. packages/platform-web/src/__tests__/test-utils.ts:126-127 reads os.tmpdir() for runtimeHomeDir and socketDir, and the gate never sees it, while the same code in src/__tests__/test-utils/web-managed-*-browser.ts has to be allowlisted by hand. Can the exemption be limited to the helper itself, either by exact path like RAW_TMPDIR_ALLOWLIST (with platform-web's test-utils.ts added for its socket-length reason), or by skipping only calls inside functions named mkdtempForTest/mkdtempForTestSync?

The scan at scripts/check-test-tmpdir-model.ts:97 matches by identifier name, so import { tmpdir as t } from 'node:os'; t(), os['tmpdir']() and const t = os.tmpdir; t() all pass. Can it resolve the local bindings from the import specifiers (aliases and destructuring included) and flag computed ['tmpdir'] access? A model test with an aliased import and a computed-access probe would prove it.

The allowlist at scripts/check-test-tmpdir-model.ts:36 covers whole files. A new, unrelated path.join(os.tmpdir(), 'x') added later to cli-diff.test.ts or runner-xctestrun.test.ts would pass, though each file has only one justified read today. Can the allowlist be per call, with a count per file or a marker comment on the justified line?

The "allowlist holds" test in scripts/check-test-tmpdir-model.test.ts:37 copies the six paths and checks membership only. It does not check that those files still contain a raw read, and findRawTmpdirViolations never runs against a fixture tree, so the exemption rule has no test. Could it assert that each allowlisted file still has a raw read, and add a fixture case with a non-helper read in a test-utils.ts?

A few cleanup leftovers: packages/replay-test/src/internal/__tests__/session-test-artifacts.test.ts:14 keeps a comment about a function that was removed, several files keep empty import lines (snapshot.test.ts:4, request-router-lock-policy.test.ts:7, runner-transport.test.ts:4, adapter.test.ts:5, screenshot-crop.test.ts:15), and runner-transport.test.ts:58 and screenshot-crop.test.ts:93 still add pid/Date.now/random suffixes inside an already-unique directory, which the PR body says were dropped. Please remove them or adjust the PR body.

On design: this adds the 11th and 12th copy of the two-line mkdtemp helper, and the gate then finds helpers by file name. Would exempting only functions named mkdtempForTest/mkdtempForTestSync be smaller, so the file-name rule and the helper-module predicate can go away? One shared helper that per-package rootDir builds can import would remove the copies, but that needs a decision on the package test surface first.

The branch conflicts with main. #2618 already removed the snapshot-helper installArgs, and main also changed packages/platform-android/src/__tests__/app-deployment.test.ts since the merge base. A rebase should drop the now-empty installArgs hunk.

The 5 reported checks pass, and the new gate runs in the tooling job.

Next step: rebase, then close the three gate holes with model tests that fail without each fix.

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	4.57 MB	4.57 MB	0 B
Package (unpacked)	4.56 MB	4.56 MB	0 B
Package (download)	1.36 MB	1.36 MB	0 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	22.1 ms	20.9 ms	-1.2 ms
CLI --help	61.8 ms	56.6 ms	-5.1 ms

[thymikee]

Reviewed at 6a17e00, as a follow-up to the review at dea914c. Banning the node:os import closes the aliased and computed-access routes, but two gaps still let a raw os.tmpdir() scratch dir back in.

The enforced glob at oxlint.config.ts#L152 matches only *.test.ts. *.fixtures.ts files and helper files under __tests__/ or test-utils/ are outside it, and those are the files where scratch-dir helpers live. For example, packages/platform-web/src/__tests__/test-utils.ts still imports node:os and calls os.tmpdir() twice, and nothing flags or lists it. Can the override also cover *.fixtures.ts and the helper files under __tests__/ and test-utils/, other than the tmp-dir helper itself?

The override for the 9 exempt tests at oxlint.config.ts#L174 clears no-restricted-imports for the whole file, not only the one justified read each comment describes. A new, unrelated os.tmpdir() scratch dir in one of those files would pass silently. Can the exemption be scoped to the call, for example with oxlint-disable-next-line on the justified line?

A small leftover: session-close-shutdown.fixtures.ts#L3 imports node:os and re-exports it as sessionCloseShutdownFixture.os, but none of its five consumers read .os. It can go.

All checks passed at 6a17e00 and there were no conflicts. The newer head 853f612 rebases onto main and adapts one fixture to an upstream mock; it does not change the lint config or the files named above, so these points still apply. Next: widen the glob and scope the exemptions to the justified calls.

[thymikee]

Addressed in 853f612904. Rebased onto latest origin/main (#2625/#2600/#2601/#2602/#2633); GitHub reports CLEAN/MERGEABLE. The now-empty installArgs hunk is gone (#2618 landed), and app-deployment.test.ts rebased without conflict.

Gate reworked per your design note. The bespoke scripts/check-test-tmpdir* model is gone, so the three holes it had (whole-test-utils.ts exemption, identifier-name matching, whole-file allowlist) no longer exist. Enforcement is now one no-restricted-imports override banning node:os in src/**/*.test.ts and packages/*/src/**/*.test.ts, mirroring the node:child_process ban and riding the existing pnpm lint gate — no new command, model, allowlist file, or CI step.

• The alias/computed/held-reference probes are closed by construction: any os.tmpdir() use requires importing node:os, and the ban is on the import, so import { tmpdir as t }, os['tmpdir'](), and const t = os.tmpdir are all caught at the specifier. A planted-import check confirms it fires, and the set of node:os-importing *.test.ts under the globs equals the 9 exceptions exactly.
• Scope is *.test.ts, so .ts helpers fall outside the ban. That is deliberate (each helper is the sanctioned place to touch os.tmpdir()), but it is a real judgment call: packages/platform-web/src/__tests__/test-utils.ts (socketDir/runtimeHomeDir) is not checked, consistent with the other wrapper helpers but not "guarded." Say the word and I will widen the glob to **/__tests__/**/*.ts and clear the ~15 wrapper files individually so an unrelated read inside a helper is flagged too.

Cleanup leftovers fixed. Removed the node:os-removal blank-line artifacts in snapshot.test.ts, request-router-lock-policy.test.ts, runner-transport.test.ts, adapter.test.ts, screenshot-crop.test.ts; removed the orphan comment in session-test-artifacts.test.ts; and replaced the redundant pid/Date.now/random suffixes in runner-transport.test.ts and screenshot-crop.test.ts with fixed names (tunnel.json, screenshot.png) since the mkdtempForTest dir is already unique.

On the 11th/12th helper copy — agreed it is duplication, but a single shared root test-surface helper importable by each package rootDir build is a package test-surface decision (your call), so I left it out of scope; happy to do it as a follow-up.

Validation at 853f612904: pnpm lint (incl. the new rule), format:check, full typecheck, fallow --base origin/main, gate-manifest(+test), affected:test, and the unit run for every changed file are green. check:affected --run halts on two things, neither from this change: mutation-model (reproduces at clean origin/main with this change absent) and a no-findings lint halt (oxlint exits 0 standalone; ban scope provably equals the exceptions).

[thymikee]

Checked at 853f612. Yes, please widen the ban to the helper files: *.fixtures.ts and the .ts files under __tests__/ and test-utils/, with only the tmp-dir helpers exempt. packages/platform-web/src/__tests__/test-utils.ts can keep its two socket-path reads behind a line-level oxlint-disable-next-line that states the reason.

The other two points from the 6a17e00 review still apply to this head. The nine exemptions at oxlint.config.ts#L171-L179 clear the rule for whole files, so a new unrelated os.tmpdir() in one of them would pass. Can each become a line-level disable on the justified read? And session-close-shutdown.fixtures.ts#L3 still imports node:os for an .os export that no consumer reads.

The shared helper for all packages is fine as a follow-up. All checks pass and there are no conflicts. Next: widen the glob, scope the exemptions to lines, and drop the unused import.