perf(tests): stop leaking a temp dir per test, and measure where the suite's CPU goes

[max-sixty]

Measured where the test suite's CPU actually goes, fixed what was doing real extra work, and added task profile-tests so the measurement is repeatable.

Baseline

cargo nextest run --features shell-integration-tests on an 18-core M-series machine: 4,570 tests, ~95s wall, 988 CPU-seconds (332 user + 655 sys).

Two thirds is kernel time, so the cost is process creation and filesystem churn rather than computation. The integration binary is 86% of summed self-time: 2,184 tests at ~0.6s each, each spawning wt (a 65 MB debug binary, ~11ms CPU per spawn against 2.6ms for a trivial process) and git against a fresh fixture copy. It sits in a broad middle, not a few outliers: 73% of self-time is in tests taking 0.25 to 2.0s.

One leaked temp directory per test

isolated_test_cwd() held a TempDir in a LazyLock. Statics don't run destructors at process exit and nextest runs one process per test, so every test leaked an empty directory into the system temp root. Measured with TMPDIR pointed at a fresh directory: 704 per integration-suite run. This machine had accumulated 454,907 entries, 353,268 of them empty strays older than a day.

Stale entries are cheap to ignore but expensive to enumerate, and git::recover::recover_from_path reads every ancestor directory of a deleted CWD:

temp root	test_recover_from_path_returns_none_for_unrelated_path
454k entries	14.2s (34.4s on a quieter run)
empty	0.27s

One fixed directory replaces it. Leaks per run: 704 to 0, verified across full suite runs, and the directory is still empty after ~9,000 test executions.

I also checked whether a crowded temp root slows ordinary temp operations. It does not: create, populate and delete of a fixture-sized tree ran at 21ms/iter in a 454k-entry parent against 40 to 60ms in an empty one. The leak's cost is concentrated entirely in code that enumerates.

Fixture temp dirs no longer sit in the shared temp root

The fixtures created their temp directories directly in the system temp dir, among however many entries the machine had put there. test_temp_root() ($TMPDIR/wt) roots them one level in, and test_tempdir() replaces TempDir::new() across every TestRepo constructor, the mock-command helper, the temp_home fixture and the recovery tests. That test again: 14.2s to 0.06s.

Two constraints worth knowing, both found by trying the more aggressive version first:

• A cache-dir root fails. ~/Library/Caches is under /Users/, which a conditional includeIf "gitdir:/Users/" matches, so 16 picker tests fail their commits under commit.gpgsign — they drive git through Repository::run_command, the production API with no isolated config. step_promote above was not a one-off: the suite was hermetic against host git config only because macOS puts $TMPDIR outside /Users/. That is the hole the next section closes — at the layer that covers in-process git, not by choosing where temp files live.
• The root's name is load-bearing. A unix socket path cannot exceed sun_path, 104 bytes on macOS. The canonicalized per-user $TMPDIR is 56, and test_copy_ignored_skips_non_regular_files binds a listener 89 bytes in. worktrunk-tests overflowed by one byte; wt costs 3 of the 14 spare.

The ~240 tests that call tempfile directly still use the system temp dir. They are transient and a clean run leaks nothing, so converting them is tidiness rather than a fix.

A test that passed by accident of TMPDIR location

step_promote::test_promote_bare_repo_with_worktrees drove git through bare Cmd::new("git") instead of configure_git_env, so the host's config applied. A conditional includeIf "gitdir:/Users/" enabling commit.gpgsign fails its commit, but only when TMPDIR sits inside the matched tree. macOS puts TMPDIR under /var/folders, so it passed; pointing TMPDIR anywhere under the home directory failed it.

The suite was not actually isolated from the developer's git config

Chasing the temp-root change turned up a real hole. TestRepo exposes the production Repository type, and Repository::run_command builds a plain Cmd::new("git") with no GIT_CONFIG_GLOBAL, so it inherits the test process's own environment. Separately, a bare wt_command() had no GIT_CONFIG_GLOBAL at all and fell through to ~/.gitconfig. 280 Repository::at/current/discover constructions across 31 test files and 156 direct run_command* calls in test code sat on that path.

Signing was only the symptom that surfaced. Confirmed leaking from a real developer config: commit.gpgsign, core.fsmonitor (spawns a daemon per fixture repo), worktree.guessremote (changes git worktree add, directly under test), help.autocorrect = prompt (a mistyped git command blocks). Structurally: core.hooksPath, credential.helper, filter.* clean/smudge and diff.external all execute arbitrary programs; url.*.insteadOf rewrites remotes; merge.conflictstyle, diff.context, rebase.autostash, fetch.prune, push.default all change what the code under test observes. That set is unbounded, which is why hardening each fixture's local config was rejected — a denylist can't cover it, and local config cannot unset an inherited [include] or credential.helper at all.

The floor is one constant, shell_exec::HERMETIC_TEST_GIT_ENV — the deny pair pointing GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM at a path that does not exist, plus the settings the suite needs through GIT_CONFIG_COUNT / GIT_CONFIG_KEY_n / GIT_CONFIG_VALUE_n — applied to every child at its spawn site. There is no git-config file anywhere in the repo. For the spawn sites the harness owns (git_test_env, configure_git_cmd, isolate_subprocess_env for wt children, pty_env_vars for env_cleared PTY children) that's ordinary per-child env. For the git that production code spawns while a test drives it in-process, the test never holds the command — so the harness flips an atomic latch (shell_exec::enable_hermetic_test_env, called from the fixture constructors), and Cmd, the choke point every production spawn passes through, applies the floor to each child while the latch is set. Setting env on a child is safe; it was setting the test process's own env that wasn't (set_var races the other test threads), and the latch dissolves the need for it — no unsafe, no pre-main constructor, no cargo [env]. Because the latch lives in the binary, every runner agrees by construction: cargo test, nextest, cargo llvm-cov, cargo bench, an IDE, a debugger, a directly executed target/debug/deps/integration-*. .config/nextest.toml was tested and rejected, and is now ruled out standing: nextest 0.9.132 has no [env] key, and a $NEXTEST_ENV setup script would miss the three non-nextest runners CI uses (cargo llvm-cov, the Nix cargo test derivation, cargo bench). A runner-specific knob doesn't fail loudly when another runner misses it — it yields a different result, usually in the coverage job whose numbers gate a merge. tests/CLAUDE.md → One Result Per Test, Whatever Runs It records the rule; .config/nextest.toml points at it from the place someone would be tempted.

Acceptance test, since the suite passed before only by accident of $TMPDIR sitting outside /Users/: with test_temp_root() temporarily pointed under $HOME so a conditional includeIf "gitdir:/Users/" fires, the picker tests go from 20 of 38 failing to 38 of 38 passing.

The cost: the latch is a test-serving switch compiled into shell_exec — one static, one relaxed load per spawn, marked TODO(hermetic-env) with the structural alternative (threading an explicit env value through Repository). cargo run -- <cmd> is untouched: nothing in production latches it, so a developer's own invocations keep their aliases, credential helper, and identity. The one production git spawn that bypasses Cmd (the fsmonitor daemon launch) re-applies the floor by hand. (Two earlier shapes were tried and replaced: cargo's [env], which taxed every cargo run and vanished whenever a test binary ran outside cargo, and a pre-main constructor crate, which every test target had to link and which put the floor on developers' cargo bench-adjacent runs too.)

In-process tests were reading the developer's worktrunk config too

config_path()'s third priority is the real ~/.config/worktrunk/config.toml, and a lib-crate test cannot set the second for itself — set_var is unsafe and this crate forbids unsafe. So a test reaching priority 3 got the developer's own config. A panic! build of the guard named the live callers immediately: git::repository::tests::prewarm_* read it on every run, and set_skip_shell_integration_prompt / set_skip_commit_generation_prompt reach the same resolver to write.

Priority 3 is now absent under #[cfg(test)], which is compiled out of the real binary — so unlike the git floor above, this one costs cargo run nothing. It returns None rather than panicking like approvals_path(): that guards a mutation target where silent absence would let a test believe it saved something, whereas this is a lookup whose absent state is already handled — require_config_path() turns it into an error, so a write still fails loudly while a best-effort read preloads nothing.

The guard covers lib-crate tests only; src/commands/ and src/output/ link the lib in non-test mode. Nothing there exercises the fall-through today (968 bin-crate tests create no config under a scratch $HOME), so that is a requirement on new tests, recorded in tests/CLAUDE.md. system_config_path() stays unguarded on purpose — machine-wide file, and config::deprecation's PendingDefault rules need the lookup.

Merging main's parallel fix

#3620 attacked the same in-process hole from the other side, writing LOCAL_TEST_CONFIG into each fixture's own .git/config. The two compose rather than compete and both are kept: the floor denies the host's config to every git, and the local config supplies what a hermetic in-process git still needs — an identity, which the floor deliberately withholds because it has per-command homes already (git_test_env, LOCAL_TEST_CONFIG) and useConfigOnly fails loudly if a path misses both. main's structure (TestConfigPaths, TestRepo::bare) is kept as-is.

Two docstrings were true on each side and false together: test_gitconfig_path restated the gitconfig inline, and LOCAL_TEST_CONFIG said in-process git reads the developer's ~/.gitconfig, which is what the floor prevents. test_gitconfig_path also held its TempDir in a LazyLock, the leak this PR removes. Both are moot now: the function is gone with the files.

Why there is no gitconfig file

The isolation went through two file-based shapes before this one, and neither earned its keep. Denial never needed a file, because GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM are the denial; the file existed only to set things, and GIT_CONFIG_COUNT is git's environment spelling of -c. Deleting both files also deletes the [include] that kept them from drifting, the per-process write that broke test (windows) on a shared path, and the config-path argument threaded through 22 call sites.

The floor that remains is the deny pair plus two settings. user.useConfigOnly is a backstop: denial alone leaves git guessing an identity from the OS username and hostname rather than failing, which is the one way a hermetic suite could still author a commit as the developer. Nothing exercises it, and that is the reason to keep it. rerere.enabled = false is set rather than left unset, so the suite's rerere state cannot depend on what a fixture happens to carry. commit.gpgsign, advice.mergeConflict and advice.resolveConflict are gone: denial leaves git on its own default for the first, and tests/fixtures/template-repo/gitconfig carries the other two into the fixtures that need them.

Once the latch made denial universal, the redundant copies went too: git_test_env no longer restates the deny pair per command (the floor is denial's only writer, so its value is uniform across every transport), LOCAL_TEST_CONFIG dropped its commit.gpgsign (denial guarantees the default), the platform-dependent NULL_DEVICE constant is deleted, and the .env.GIT_CONFIG_GLOBAL snapshot redaction is gone — the recorded value is one cross-platform constant, so there is nothing volatile to redact.

I removed rerere.enabled first, on a local measurement that was wrong, and CI failed on all three platforms. The standard fixture is built once into target/debug/wt-test-fixtures/ and copied per test, and that cached copy held an rr-cache directory left by a rebase run while the floor still enabled rerere. Git turns rerere on by itself whenever rr-cache exists, so every local test kept the behavior the change had just removed, while CI built the fixture fresh and lost it. tests/CLAUDE.md now records the trap: clear the fixture cache before trusting a local measurement of a git-config change.

Why the floor can't live in a fixture: every other test variable is set on a child — git_test_env on a git command, configure_cli_command on a wt subprocess. In-process git is not a child the test configures: Repository::run_command is production code building a plain Cmd::new("git"), and the test never holds that command, so there is no place to set env on it — while setting the test process's own environment is the one thing a test can't do safely (set_var races the other test threads). The identity did move to the fixtures and the per-command env this way; the denial reaches production's children through the latch at Cmd, the choke point they all pass through.

Two things fall out of -c semantics, both pinned:

• It outranks a repository's own config, where a global file would yield to it. So init.defaultBranch cannot live in the floor: default_branch.rs sets that key in a repo to prove wt reads it, and an entry would silently win. The three harness git init calls that relied on the floor now name their branch, as the other two already did.
• A PTY child is env_cleared, so it inherits nothing and used to get the floor through the file. pty_env_vars copies the family across, pinned by pty_env_vars_carry_the_git_config_floor — the settings only quiet advice and refuse a guessed identity, so no PTY assertion would have caught their loss.

Four tests wrote their own gitconfig to get init.defaultBranch plus an identity; the harness supplies both, so those writes are gone too. Net 45 lines lighter, and no snapshot changed.

Measurement

task profile-tests builds first, then runs the suite under CPU accounting, printing every test over 1s with its duration (a perf nextest profile that lowers slow-timeout purely as a reporting threshold, with no terminate-after). It began with a scratch-TMPDIR leak check bolted on, which dragged a sun_path byte budget into the Taskfile; the profiling goal never needed it, so the task is now three lines and the leak-check recipe is one sentence in tests/CLAUDE.md. Method and how to read the numbers: tests/CLAUDE.md under Profiling the Suite.

Found, measured, not changed

The gate keeps a duplicate cargo artifact set. RUSTFLAGS='-D warnings' on the insta step is part of cargo's fingerprint, so it forks all 343 crates into a second artifact set: 261 CPU-seconds to prime plus a duplicate of target/debug/deps (target/debug here is 35 GB, in a 52 GB target/). I removed it and then put it back: the clippy step that would cover it runs on ubuntu only, while the cross-platform matrix runs wt hook pre-merge --yes insta, so this RUSTFLAGS is the only thing denying warnings on macOS and Windows. [lints.rust] warnings = "deny" would keep that coverage everywhere without forking the graph (verified locally: fails on a planted unused variable, recompiles only worktrunk and wt-perf, feature-check commands still pass), but it also makes plain local builds fail on warnings and newly exposes cargo msrv verify and the minimal-versions job. That is a workflow call. The duplication is a one-time cost per artifact set rather than per-edit, so it is second-order next to the ~600 CPU-seconds each suite run costs.

recover_from_path enumerates every ancestor up to /. At each ancestor it reads the directory and stats .git in every child. Bounding the child scan to the first existing ancestor would preserve both documented layouts (sibling and nested) and both of that module's regression tests, but it would break a custom worktree-path layout where the repo is a child of a higher ancestor, so it needs a decision about which layouts recovery must support.

Nothing in the gate is the biggest cost; concurrency is. The five [[pre-merge]] keys are one table, so they run concurrently, and each is a cargo command that wants the whole machine. They serialize on cargo's build-directory lock (Blocking waiting for file lock on build directory appears in every run) while test execution overlaps another step's build. The lockfile comment says it "must be first", which concurrent execution does not provide. Separately, agent worktrees run whole gates at once: during this work a second worktree ran its own wt hook pre-merge alongside mine, load average hit 154 on 18 cores, and the same suite took 147.7s instead of 92.9s.

Tried and rejected

[profile.dev] debug = "line-tables-only" first measured 14% less CPU, but per-spawn CPU was unchanged, which did not fit the proposed mechanism. Re-measuring both configurations on a quiet machine gave 602.6s (baseline) against 606.5s (line-tables). The original delta was contention from a sibling worktree running its own suite.

macOS Gatekeeper (syspolicyd) looked like a candidate at 230% CPU, but 300 spawns of the freshly built wt cost it 0s, and 0.2s after a relink, against 0.4s during a 10s idle baseline.

Verification

cargo run -- hook pre-merge --yes green, 4,611 tests passed, run against a freshly built fixture cache after the switch to the latch. The latch was verified to be the only source of the variables: the invoking shell carries no GIT_CONFIG_*, and the meta-tests (in_process_git_reads_only_the_hermetic_config asserting the origin of every resolved setting, pty_env_vars_carry_the_git_config_floor, and the isolate_subprocess_env scrub test asserting the floor is re-set after the scrub) pin each transport. Across earlier runs one hit a single intermittent PTY failure in shell_wrapper (exit 127) that passes 3/3 in isolation and whose code path never touches wt_command() or the shared cwd; a later run was green under heavier load than the one that failed.

This was written by Claude Code on behalf of Maximilian

[worktrunk-bot]

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

The core change reads well: swapping the LazyLock<TempDir> for one fixed empty dir cleanly kills the per-test leak, and because nothing writes into it the shared-across-processes access is race-safe (create_dir_all is idempotent). The step_promote fix to route git through configure_git_env matches the established pattern in bare_repository.rs, and dropping the now-unused Repository import is correct.

One portability note on task profile-tests inline. Nothing blocking.

[worktrunk-bot]

Reviewed the now-ready state end to end — the isolation story holds together and the docs are unusually thorough. The two changes since the draft pass read well: the #[cfg(test)] guard in config_path() returning None (write callers still fail loudly via require_config_path, best-effort reads preload nothing) is the right shape, and checking dev/test-gitconfig / dev/hermetic-gitconfig in as files — the former [include]-ing the latter so they can't drift — cleanly removes the per-process write race and the per-test temp-dir leak. Nothing blocking.

The one red check, affected tests (windows, advisory), is not this PR — it's cargo affected failing to decode git diff output as UTF-8, and the affected workflow itself concluded green (the job is continue-on-error). Safe to ignore.

Why the advisory-windows failure is tooling noise, not your change

The job errored with git diff stdout was not valid UTF-8: invalid utf-8 sequence of 1 bytes from index 118096 — cargo affected reads git diff over the affected range through String::from_utf8 and chokes on binary content. The repo carries non-UTF-8 fixtures (tests/fixtures/template-repo/_git/index, _git/objects/**); when a binary blob lands in the affected range, the decode fails.

Two things confirm it's not caused by this change:

• The earlier draft commit 6fcb8ea8 already carried the identical .cargo/config.toml GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM env change and passed this same advisory job — so the hermetic-config env isn't reaching cargo affected's git in a way that breaks it.
• Whether it fires depends on which files sit in the affected-vs-collect range (the run noted "16 commit(s) since collect"), so it's non-deterministic across runs rather than a property of this diff.

Required test (windows) / (linux) / (macos), lint, and code-coverage are all green.

[worktrunk-bot]

Reviewed the environment-vs-file refactor end to end. Moving the isolation floor into .cargo/config.toml env is a clean improvement — it's the one layer that reaches in-process Repository::run_command git, the GIT_CONFIG_COUNT vs. repo-config precedence reasoning is right, and the isolate_subprocess_env pass-through plus pty_env_vars copy are both pinned by their own tests. Keeping identity out of the floor (so cargo run can't author Test User commits) and gating config_path() to None under #[cfg(test)] are both the right shape. The docs are thorough.

One consistency note, not a breakage. Dropping init.defaultBranch = main from the floor and adding explicit -b main to every harness constructor is correct — but three git init calls elsewhere in the suite were left un-updated and now silently create master instead of main:

• repo.run_git_in(&sub_source, &["init"]) in test_remove_foreground_with_submodules (tests/integration_tests/remove.rs)
• repo.run_git_in(&sub_source, &["init"]) in test_remove_worktree_submodule_dirty_fails_closed (tests/integration_tests/remove.rs)
• the bare ["init", "--bare", <remote_dir>] in test_complete_excludes_remote_branches (tests/integration_tests/completion.rs)

None of the three break — the first two repos are opaque submodule sources and the third is an origin remote whose default HEAD is never consulted (branches arrive via explicit refspec pushes), so no assertion touches the branch name and the tests are green. But they drift from the invariant you just documented ("every git init in the harness names its branch"), and the two non-bare ones now emit a hint: Using 'master' … line on stderr. They're outside this PR's diff, so I can't inline-suggest; happy to push a one-line -b main fix to all three if you'd like, or you may prefer to leave the opaque sources as-is.

Nothing blocking.

[worktrunk-bot]

The floor trim reads cleanly — useConfigOnly is the right one to keep (it's the only setting with a job once the host config is denied), and the reasoning that git's own defaults already cover gpgsign / advice.* / rerere under the deny pair holds. The tests/CLAUDE.md rewrite explaining why the floor can't move into a fixture (in-process git reads the test process's own env, which a test can't set for itself) is a nice tightening.

One thing that slipped in with this commit, though: src/testing/.mod.rs.pending-snap is an insta pending-snapshot artifact that got committed by accident. It's not a snapshot you want tracked — its recorded new snapshot is "", which doesn't even match the inline snapshot you accepted (user.useconfigonly=true), so it's stale transient state left over from the cargo insta run, not review-worthy content.

The reason it wasn't caught is that .gitignore only has *.snap.pending (line 14), which is insta's external-snapshot pending format. Inline-snapshot pending files use the .pending-snap suffix instead (.mod.rs.pending-snap), which that pattern doesn't match — so nothing ignored it and git add picked it up.

Happy to push a one-commit fix: git rm src/testing/.mod.rs.pending-snap plus adding *.pending-snap to .gitignore so the inline-snapshot form is covered going forward. Just say the word (or grab it yourself — it's a two-line change).

Nothing else blocking.

[worktrunk-bot]

One correctness issue in the /usr/bin/time → shell-time switch: the time keyword in Task's interpreter (mvdan/sh) only measures wall time — it prints user/sys as a hardcoded 0m0.000s, with a // TODO: can we do these? in interp/runner.go right above the two lines that emit them. So the new time cargo nextest … produces:

real  1m23.456s
user  0m0.000s
sys   0m0.000s

That undercuts the task's whole premise. tests/CLAUDE.md still says wall time is unreliable here (sibling worktrees building/testing) and to track user/sys instead — but those are now exactly the two numbers that are always zero. GNU /usr/bin/time reported them including children; the shell keyword can't. So this trades a portability caveat for the loss of the only reliable metric the task documents.

Two defensible resolutions, depending on what you're after — flagging rather than pushing since it's your call:

• Keep GNU /usr/bin/time (restore the portability note), if user/sys accounting is the point.
• Keep the shell keyword but rewrite the docs to stop promising user/sys and to treat real as the (noisy) metric — which weakens the CPU-accounting rationale.

Nothing else in the incremental diff is a concern; the pending-snap removal and simplification read fine.

[worktrunk-bot]

mvdan/sh's time keyword doesn't include child user/sys — it hardcodes both to 0m0.000s (// TODO: can we do these? in interp/runner.go, just above the user/sys outf calls). Only real (wall) time is real here, so this comment's claim that user/sys "include every child process" is inverted — they're always zero.

[worktrunk-bot]

This line still tells the reader to track user/sys because wall time is unreliable — but after the switch to the shell time keyword, user/sys are always 0m0.000s (mvdan/sh doesn't collect child rusage). So the guidance now points at the two zeroed columns and leaves only the wall time it warns against. Whichever way you resolve the Taskfile side, this sentence needs to match.