mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
codex/remove-codex-cloud-specific-tests
84 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
00ab0ffa26 |
Canonicalize benchmark fixtures and variants (#3761)
Benchmark fixtures still encoded the benchmark that first needed each repository state, which left overlapping recipes and variants after the earlier harness consolidation. This change reduces the fixture catalog to two provenance-based bases: `Generated` builds an ordinary Git repository locally, while `Imported` copies the pinned `rust-lang/rust` corpus. Worktree, branch, and remote-ref populations remain parameters on `Generated`; prune candidates and backdrop are overlays that work with either base. The generated base deliberately combines heterogeneous worktree states, history-spread branches, and optional remote refs so ordinary list, completion, picker, first-output, alias, remove, and prune benchmarks can share it. Imported history-spread branches and clean base-tip worktrees carry their own commits, preserving the base populations without making them incidental prune candidates when overlays advance the default branch. The benchmark matrix now keeps single-factor contrasts: list scaling uses the 1- and 8-worktree endpoints; alias dispatch has a startup floor, two population endpoints, and one warm/cold variable-resolution pair; completion keeps one full-surface case; remove and prune vary cache or hook state only where the command exercises it. Historical recipes, redundant cache rows, and intermediate scaling points are removed. Manual setup paths live under `target/`, and the benchmark guide documents the resulting fixture and cache model. Tests: `cargo run -- hook pre-merge --yes` after merging current `main` (4,571 tests); targeted Criterion test-mode runs; `cargo test -p wt-perf`; benchmark check, clippy, formatting, and diff checks. > _This was written by Codex on behalf of max-sixty_ |
||
|
|
970976bd32 |
Consolidate benchmark recipes and cases (#3721)
Benchmark fixtures had accumulated around individual call sites, leaving the same repository shapes and command modes expressed several ways. This change makes repository state the organizing concept: benchmark groups select semantic `FixtureRecipe`s, share table-driven cases, and retain separate fixtures only when a controlled contrast, destructive precondition, or disproportionate setup cost requires one. The real-repository list benchmarks now share one pinned `rust-lang/rust` fixture with eight worktrees and fifty branches spread across history. The list matrix keeps default, branch, warm, and cold coverage without maintaining several “real” repository handles. Remove and prune cases share the same case machinery, while the destructive large-repository prune state remains separate. The scheduled workflow now converts Criterion estimates directly with `jq`, removing the one-off Python converter and its tests. The benchmark guide records the canonical-fixture principle and the remaining recipe-to-group mapping. Tests: `cargo run -- hook pre-merge --yes` (4,551 tests); `cargo bench --bench list large_repository -- --test`; `cargo test -p wt-perf`; benchmark check, clippy, formatting, and diff checks. > _This was written by Codex on behalf of max-sixty_. |
||
|
|
dd453304ed |
test: fold the mock stub into the wt binary (#3712)
`cargo test --test integration` neither built nor rebuilt `mock-stub`: a
target filter deselects the dummy test that pulled it in, so a fresh
tree panicked ("mock-stub binary not found") and a warm one could run a
stale stub. The chain that compensated — the separate helper package,
its dummy `builds.rs`, the `default-members` entry, nextest's
experimental `build-bins` setup script, `workspace_bin()` — existed only
because cargo-dist ships every `[[bin]]`, and carried a TODO to collapse
once that changed. dist 0.30.2 does support per-binary exclusion now
(`[dist.binaries]`, since 0.29.0; verified with `dist plan`), but the
TODO's plan has a hole it predates: `cargo install worktrunk` installs
every feature-satisfied `[[bin]]`, and dist config doesn't govern
crates.io installs.
So the mock commands are now the `wt` binary itself. `mock_commands`
links `wt` under the mock's name (`gh`, `glab`, …), and `main()`
dispatches to the ported playback (`testing::mock_stub`) when
`WORKTRUNK_TEST_MOCK_CONFIG_DIR` is set, argv[0] is a foreign name, and
the config dir holds `<argv0>.json` for it. The existence check keeps wt
under a foreign argv[0] *without* a config being wt — the
argv0-validation security test symlinks it as `wt;touch` and must reach
wt's own rejection. The shipped binary already compiles the whole
`testing` module unconditionally, so this adds no new category of test
code to it. Windows links `wt.exe` with `hard_link` (copy fallback for
cross-drive dirs); the debug binary is ~67 MB, so per-mock copies stay
the fallback.
Every runner is now safe by construction — cargo rebuilds a package's
own binaries whenever its integration tests build, so there is no
separate artifact to go missing or stale. Deleted: the helper package,
the setup script plus `experimental = ["setup-scripts"]`, the
`default-members` trick, `workspace_bin()`, and `wt_bin()`'s dead
compile-time branch (unit-test targets get neither the runtime variable
nor the `option_env!` value, so the runtime resolution is the one
mechanism).
Validated locally: the pre-merge gate's full suite passes (4542/4542;
its doc step also caught unescaped `argv[0]` intra-doc links in the new
comments, fixed and `cargo doc -Dwarnings` re-verified). With all stub
artifacts purged from `target/`, plain `cargo test --test integration`
on mock-dependent tests builds them and passes — the command that used
to hit the trap. Two integration tests pin the dispatch's argv[0] edges
(an empty argv[0], and a non-UTF8 one), alongside the existing
`wt;touch` carve-out test.
The first commit is the investigation that preceded the fix: it verified
the `wt` binary itself was never subject to the staleness the mock-stub
was, and documented that in `tests/CLAUDE.md`; the fix then narrows that
paragraph further, since the gap it scoped no longer exists.
A two-reviewer subagent round (one prosecuting the diff against the
failure modes documented in the repo's own mock history — the #401/#407
Windows era, #547, #654, #127, #2544, #2730, #2744 — the other
adversarial) then hardened the dispatch. The reserved-name guard is
case-insensitive, matching the config probe, which goes through a
filesystem that equates `WT.json` with `wt.json` on macOS and Windows;
`command_name()` reads `args_os` — `env::args()` panics on a non-Unicode
argument, and this runs inside `main()` on every invocation (caught by
the tend review) — and returns `None` for a degenerate argv[0] instead
of panicking; the `.exe` suffix is stripped explicitly rather than via
`file_stem`, so a dotted mock name (`python3.11`) resolves identically
on every platform; and `copy_mock_binary` is now private —
`MockConfig::write` writes `<name>.json` before linking and is the only
way to create a mock, so a link cannot exist without its config, and the
dispatch's missing-config fall-through can only mean "wt under a foreign
name" (the argv0-validation tests' `wt;touch`), never a half-configured
mock that silently runs real wt with the mocked tool's arguments. The
`Option<&str>` mock helpers whose `None` arm produced exactly such
configless links lost the arm (every caller passed `Some`), and 25
redundant standalone link calls went with it.
The review also surfaced the one remaining spawn-a-stale-binary path
outside the suite: `wt-perf timeline` resolved a sibling `wt` by path,
checked only existence, and told the user to build it manually — so
`cargo run -p wt-perf -- timeline` after a `src/` edit silently measured
stale code. It now builds `wt` first and takes the artifact path from
cargo's `--message-format=json` report rather than deriving a sibling
location, so target-dir and profile overrides can't divert the build
away from where it's resolved; a release wt-perf builds and measures a
release wt. The build runs before the timeline's wall-clock measurement
starts, cargo's progress streams on stderr, and stdout keeps the
`--chrome` JSON contract.
> _This was written by Claude Code on behalf of max-sixty_
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
|
aba7bad089 |
perf(completion): overlap the git reads a Tab press waits on (#3664)
Shell completion forks a whole `wt` process on every Tab press, with the user's finger still on the key, and it has nowhere to hide work: nothing paints progressively, no cache survives the process, and no shell caches a dynamic completion. It was doing every git read in sequence. Measured on `mixed-80-80-1400`, M4 Max, same binary before and after: | | before | after | | |---|---|---|---| | `wt switch <Tab>` | 45.7 ms | 27.4 ms | 1.66× | On this repo's own checkout, `wt switch <Tab>` goes 47.9 ms → 29.0 ms; `wt <Tab>` (subcommand names, no branch work) goes 22.2 ms → 12.8 ms. ## The benchmark first `benches/completion.rs` ran against fixtures carrying two remote-tracking refs and no worktrees — a shape nobody has. It never reached the completer's 100-candidate threshold, where every remote-only branch is discarded *after* being scanned, so the most expensive call on the path was measured at a size that hid it. It now runs one repo, on the `mixed` fixture `full` already uses: `mixed-80-80-1400` — 80 worktrees in four states, 80 more branches forked across 200 commits of history, and 1400 remote-tracking refs. That covers all three categories `BranchCompleter` distinguishes, puts both candidate slow calls (`for-each-ref refs/remotes/` and `worktree list --porcelain`) in one process at a size where each is expensive, and lands past the 100-entry threshold. `completion_switch/mixed` reports 23.1 ms; the fixture builds in 9.7 s. The remote-ref count is the one dimension `mixed` was missing, so it goes on `create_mixed_repo` as an optional third component of the handle. `full` passes 0 and is unaffected, so its tracked series has no discontinuity — and its own version of the same gap (`wt list` scans `refs/remotes/` too, at two refs) becomes a one-argument change if we want it. `completion_switch`'s own history does break, and knowingly: its two ids (`branches_only`, `with_worktrees`) collapse into one `mixed`, and `criterion-to-jsonl.py` keys the nightly gist rows by `<group>/<id>`, so those two series stop accumulating. Worth paying — they measured the fixture that hid this cost in the first place, so carrying them forward would mean maintaining a benchmark that structurally cannot see the thing it exists to watch. Worth calling out explicitly, since #3669 landed while this was open and states that it preserves all 41 existing Criterion IDs: this PR is the one place that number changes, deliberately, and 41 becomes 40. Deliberately not a variant matrix. Completion has no phases worth timing apart, and the calls within a wave overlap, so no split of the wall time says which one moved — localizing a regression is a trace's job, as `benches/CLAUDE.md` already says for `full`. ## The changes - **Call `Repository::prewarm` on this path.** It already exists for exactly this, filling the git-discovery, git-config, and user-config caches on three threads — but completion answers and returns long before `main` reaches it, so those reads were each a separate serial fork. - **Run the three scans behind `branches_for_completion` concurrently.** They are independent git calls the function only joins in memory; in sequence the user waits for their sum where they could wait for the max. - **Stop scanning `refs/remotes/` where a remote-only branch can never be offered** (`wt remove`, worktree-only arguments). The completer decides that once, up front, and it drives the scan as well as the filter. - **Return to the completion handler before `main` builds the rayon pool** — `available_parallelism() * 2` OS threads, spawned eagerly, on a path that reaches no rayon. `wt switch <Tab>` goes from six serial forks to five in two concurrent waves; `wt remove <Tab>` to four. ## A bug found on the way `branches_for_completion` groups remote refs through a `HashMap` and sorts by timestamp with a stable sort, so on tied timestamps the order is whatever that process's hashing produced — three consecutive completions print three different orders. Fixed by breaking the tie on name, in its own commit. ## Verification - Candidate sets byte-identical to the pre-change binary across bash/fish/zsh/nu at 64 completion points — including the 100-candidate threshold either side, `--create` suppression, and outside a git repo. - `cargo run -- hook pre-merge --yes` green on the merge commit, after merging current `main` in: 4491 tests, all lints. - `cargo bench --bench completion -- --test` green — worth running explicitly now that `run_and_check` asserts the measured subprocess succeeded, since the assert is new to this bench. Merged `main` twice while this was open. The second, #3669, consolidated the bench harness onto a shared `FixtureRepo` plus `wt_command` / `run_and_check` and touched all four bench files this branch changes; every conflict was resolved toward main's harness. Notably the completion bench now uses main's `run_completion`, which asserts the measured subprocess actually succeeded — the version here did not, so a `wt` that failed under benchmark would have been silently timed. ## Review feedback folded in The draft-stage review caught the `add_remote_refs` rationale overclaiming, and it was right. The comment said the round-robin makes the refs "name distinct objects", with the scan reading "one commit object per ref". It can only spread over the history it has: at 200 base commits, 1400 refs land 7 apiece on ~200 distinct commits, and git parses each object once and reuses it for the rest of the `for-each-ref`. So the scan pays ~200 parses plus a cheap iteration hit per ref, not 1400 parses. That gap is now stated rather than papered over. Closing it would need a history as deep as the ref count, and `BASE_COMMITS` is shared with `full` — deepening it would slow that fixture and shift the repo `full` has been measured on, to sharpen a per-object parse this bench isn't primarily about. The ref count is the dimension it exists to vary. (The review raised this against `long_lived_clone`, the parallel fixture since deleted; the same overclaim had survived into the shared-fixture version.) ## Not addressed Two things from the report that prompted this, both left alone deliberately: - The dominant per-fork tax on the reporter's machine is their `~/.gitconfig` symlink chain landing on NFS, re-resolved by every git child. That's local environment, not code — though it is exactly why overlapping the forks helps them more than the numbers above suggest. - `worktrunk.state.<branch>.*` markers accumulate without bound: on this checkout, 79 branches have markers and 72 of those branches no longer exist. Real, but a separate subsystem with its own design question about when a marker may be dropped. > _This was written by Claude Code on behalf of @max-sixty_ |
||
|
|
7fe28bf574 |
Consolidate benchmark fixtures and harness (#3669)
Benchmark targets had each reimplemented temporary-repository ownership, linked-worktree paths, subprocess isolation, and warm/cold loops. This adds a shared `FixtureRepo` and canonical command/cache helpers while leaving each scenario-specific workload and destructive lifecycle explicit. All 41 existing Criterion IDs, fixture dimensions, command arguments, and cache ordering are preserved. The completion benchmark now also fails when its measured subprocess fails instead of silently accepting the result. Tests: `cargo run -- hook pre-merge --yes`; `cargo test -p wt-perf`; `cargo check --benches -p worktrunk -p wt-perf`; representative Criterion test-mode runs for completion, list, first-output, remove, and prune. > _This was written by Claude Code on behalf of max_. |
||
|
|
6d09125b7b |
test: converge suite on semantic boundaries (#3663)
This follows the first test-simplification tranche by converging the remaining suite around distinct semantic and pragmatic contracts rather than raw case count. The branch removes false-confidence tests, invalid setup variants, repetitive snapshots, and expensive PTY overlap while strengthening the retained route, precondition, and interaction proofs. ## What changed - Replace obsolete CI-status integration mocks and blank snapshots with direct provider semantics, mixed-priority cases, and strict GitHub/GitLab route assertions. - Remove free-riding merge, push, remove, list, security, config, and switch cases whose setup never reached the named behavior; consolidate repetitive direct cases into labeled tables. - Reduce the switch picker from 42 PTYs to 19 distinct terminal contracts, using causal release gates for asynchronous loading and repaint behavior. - Add a cached main-only picker fixture, eliminating 138 unnecessary Git subprocesses across the retained PTYs, and integrate it with main's generated hermetic standard fixture. - Tighten test guidance around proving setup preconditions and mock invocation routes, and correct the comments-tab help text and generated mirrors. ## Reviewer map - `tests/integration_tests/ci_status.rs` and `src/commands/list/ci_status/`: provider semantics and route coverage. - `tests/integration_tests/switch_picker.rs`, `src/commands/picker/`, and `src/testing/`: retained PTY contracts, causal mocks, and fixture design. - `tests/integration_tests/config_show.rs`, `src/config/deprecation.rs`, `src/config/expansion.rs`, and worktree type/resolve tests: direct-boundary consolidation. - `tests/CLAUDE.md`: the testing rules extracted from the false-confidence cases found during the survey. The measured loop removed 98 tests, 65 snapshots, and 23 picker PTYs. Controlled warm Nextest execution improved from a 79.593-second mean to 72.574 seconds (8.8%), while comparable production-line coverage moved from 97.32% to 97.23%. The tracked PR diff is a net deletion of more than 5,700 lines. ## Validation - `cargo run -- hook pre-merge --yes` after syncing current `main`: 4,468 passed, one configured skip; docs, doctests, clippy, formatting, policy checks, and snapshots green. - `task coverage` on the completed change before the base sync: 4,465 passed, one configured skip; 97.23% comparable production-line coverage. - Three independent final audits found no remaining lost beliefs, fixture hazards, or safe PTY consolidations. > _This was written by Claude Code on behalf of max_. |
||
|
|
4554f50ce6 |
perf(tests): stop leaking a temp dir per test, and measure where the suite's CPU goes (#3604)
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` (16 bytes with its slash) overflowed by two;
`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_clear`ed 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 the
snapshot layer strips the gutter-prefixed `hint:` lines the other two
quieted (they vary across git versions), so nothing depends on
suppressing them at the source. The long-dead
`tests/fixtures/template-repo/` fixture went with 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_clear`ed**, so it inherits nothing and used to
get the floor through the file. It now gets the family from
`configure_pty_command`, the choke point every PTY spawn routes through,
and again from `pty_env_vars`, whose vector declares a PTY `wt` child's
complete environment. Each copy is pinned by its own test, because the
settings only quiet advice and refuse a guessed identity, so no PTY
assertion would catch 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 bash's
`time` keyword (task's own interpreter, mvdan/sh, parses `time` but
hardcodes `user`/`sys` to zero): CPU totals on the console, every
per-test duration in the default profile's `junit.xml`. It began three
sizes larger — a scratch-`TMPDIR` leak check that dragged a `sun_path`
byte budget into the Taskfile, a `/usr/bin/time` dependency GNU-less
Linux lacks, and a `perf` nextest profile whose console slow-listing
restated what junit already carries — and each piece fell to the same
question, whether the measuring goal needed it. 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.
## Review round
A full review of the branch (three finder lenses, findings adversarially
verified) landed one more commit:
- **The wrapper-suite PTY children never got the floor.**
`configure_pty_command` env-clears, skips the `Cmd` latch, and sets the
real `HOME`, and the shell-wrapper call sites layer only fixture paths
and an identity on top. Every git those ~89 tests ran therefore read the
developer's real `~/.gitconfig` (and lost the gpgsign shield when
`LOCAL_TEST_CONFIG` dropped it). The floor now rides that choke point,
pinned by `configure_pty_command_carries_the_git_config_floor`; every
raw `CommandBuilder` site was checked to route through it.
- **`task profile-tests` could never report CPU.** mvdan/sh's `time`
hardcodes `user`/`sys` to `0m0.000s`, so the numbers the docs said to
track were unproducible; now `bash -c 'time "$@"' bash ...`. Measured
post-fix: user 5m47s / sys 12m8s, a 67.7% kernel share, confirming the
two-thirds claim above; the integration mean measured ~1s and the docs
were corrected from ~0.6s.
- Smaller: two raw `Command::new("git")` asserts in `remove.rs` tests
now go through `configure_git_cmd`; the hermetic meta-test keys on
`--show-scope` scopes rather than git's origin-path spelling; the dead
`template-repo` fixture is deleted; three test `git init`s name `-b
main`; doc corrections (sun_path arithmetic, recover-walk attribution,
dead `[TEST_GIT_CONFIG]` remnants, volatile counts).
Final gate on the finished tree: green, 4,612 tests. Deferred with
rationale: ~985 snapshots carry stale env-block metadata (insta never
compares it; it churns on future re-records), `$TMPDIR/wt` is not
per-user on Linux (a root run poisons it for later users), and
`spawn_detached_exec_*` / `step tether` do not hand-apply the floor (no
in-process test reaches them today).
> _This was written by Claude Code on behalf of Maximilian_
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
a285ed308d |
refactor(tests): bring the mock-stub env vars under the WORKTRUNK_TEST_ prefix (#3621)
`MOCK_CONFIG_DIR` and `MOCK_CALL_LOG_DIR` were the only two worktrunk-invented environment variables without the `WORKTRUNK_` prefix. That is not just a naming inconsistency: `isolate_subprocess_env` scrubs the parent environment by prefix — `GIT_*` and `WORKTRUNK_*` — so an unprefixed name is the one thing a test child inherits from whoever ran the suite. Renaming them brings them under that scrub. - `MOCK_CONFIG_DIR` → `WORKTRUNK_TEST_MOCK_CONFIG_DIR` - `MOCK_CALL_LOG_DIR` → `WORKTRUNK_TEST_MOCK_CALL_LOG_DIR` `TEST` rather than a bare `WORKTRUNK_` because both are read only by `tests/helpers/mock-stub` — they are the protocol between the harness and its helper binary, and `wt` itself never reads either one. That matches the ~15 existing `WORKTRUNK_TEST_*` knobs. ## The snapshot half Mechanical but not a substitution: the `env:` block is byte-sorted by key, so the renamed entry moves position within it and a `sed` in place would leave it where the old name sorted. All 971 affected blocks were rewritten by dropping the old line, inserting the new one, and re-sorting — the aggregate diff is exactly one removed and one added line per file: ``` 971 files changed, 971 insertions(+), 971 deletions(-) - MOCK_CONFIG_DIR: "[MOCK_CONFIG_DIR]" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" ``` The redaction placeholder follows its neighbours' convention in `add_standard_env_redactions` (`WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIR` → `[TEST_NU_VENDOR_AUTOLOAD]`), so it now reads `[TEST_MOCK_CONFIG]` rather than repeating the full key. `MOCK_CALL_LOG_DIR` appears in no snapshot — its two call sites are `.output()` assertion tests — so it needs no redaction. ## Keeping it from regressing `tests/CLAUDE.md` gains the rule under "Where a new environment variable goes": name it `WORKTRUNK_TEST_*`, and the rule covers the harness↔helper-binary protocol, not just knobs `wt` itself reads. Without it the next helper-binary variable gets named `MOCK_*` again and the hermeticity hole reopens. ## Verification `cargo run -- hook pre-merge --yes` passes (exit 0) on the merged tree: 4607 tests including `--features shell-integration-tests`, `pre-commit run --all-files`, clippy, doctests. No pending snapshots. A repo-wide sweep finds no remaining unprefixed spelling. ## Merged main #3620 landed while this was in flight and regenerated several `for_each` snapshots that still carried the old key, so main is merged in here. It resolved with no conflicts, and the result is what you'd want rather than what git happened to produce: those blocks now carry #3620's new keys (`GIT_ALLOW_PROTOCOL`, `CLAUDE_CONFIG_DIR`, `WORKTRUNK_TEST_PARENT_SHELL`) *and* the renamed key, each in sorted position. The sweep and the gate above both ran after the merge. #3620 also rewrote the "Where a new environment variable goes" section this branch adds to — three layers became four. Both edits survived; the new naming paragraph follows the updated layer list. The two advisory `affected tests` checks are red for the same reason, and merging clears them: `cargo affected` errors on `git diff stdout was not valid UTF-8`, because the diff from this PR's base contains 16 binary files — the `tests/fixtures/standard/` git objects and index files that #3620 deleted. This branch's own commit contributes none. A PR based after #3620 won't see them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
203603909d |
test(picker): release --prs loading marker on parent exit, not a timer (#3532)
## Problem `test_switch_picker_prs_shows_loading_marker` intermittently times out on `test (windows)` — it flaked on #3531 (a docs-only change), which is what prompted this fix. The test asserts a *transient* frame: while a mocked, delayed `gh pr list` is in flight, the picker header shows `↳ Loading open PRs…` and the `#42` row has not yet streamed in. The mock held that loading state for a fixed time (originally 3s). On a loaded Windows runner, the PTY boot sequence — `boot_picker_pty`'s skim-ready wait plus its initial `wait_for_stable`, together with the async worktree-column churn (`·` placeholders resolving) — can consume that whole window. By the time the stabilization helper first polls for `Loading open PRs`, the mocked fetch has already returned and the rows have streamed in, so the marker is gone and the wait times out at `STABILIZE_TIMEOUT` (30s). ## Fix Per review feedback ([comment](https://github.com/max-sixty/worktrunk/pull/3532#issuecomment-5039146240)), tie the marker's lifetime to the picker's, with **no timing number at all** — the file's "poll, don't pick a time" rule applied to the producer side. `mock-stub` gains a `hold_until_parent_exit` response mode: the mocked `gh pr list` holds until `wt` exits instead of sleeping a fixed `delay_ms`. Detection needs no platform primitive (`getppid` / `OpenProcess`): - While `wt` lives, its `--prs` fetch thread runs the mock with a piped stdout and blocks reading that pipe to EOF, so as long as the mock neither writes a full response nor exits, the loading marker stays. - When the picker aborts, `wt` detaches the fetch thread (`drop(prs_handle)`) and exits, which closes the read end of the mock's stdout pipe. The mock's next write then fails with `BrokenPipe` (Rust ignores `SIGPIPE`, so the broken write surfaces as an `Err` rather than killing the process), and the mock exits. Polling for that write error is a **causal** parent-death signal — release is tied to `wt` exiting, not to a timer — and it behaves identically on Unix and Windows, so there's no platform-specific FFI to get right. The one-byte probes written into stdout are never observed (this mode's contract is that the parent aborts without parsing the response, and the fetch thread drains the pipe until process exit). A 60s poll cap bounds a stuck orphan if the pipe somehow never breaks. This removes the race the earlier 30s bump only widened: the marker is present for the entire window the poll could observe it, on any boot latency, and no orphaned sleeper is left behind — the mock exits the moment the picker does. ## Testing `cargo test --test integration --features shell-integration-tests test_switch_picker_prs_shows_loading_marker` passes in ~1.3s, confirming the capture-and-abort path stays fast and the mock releases promptly on `wt` exit. The sibling `--prs` tests (`test_switch_picker_prs_{github,gitlab}_list`, `..._rows_survive_alt_x_removal`) still pass through the refactored `mock_forge_env` helper. Reasoning is documented inline at the mock-stub call site and in `wait_for_parent_exit`. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
|
ae363c2b4c |
test: stop the suite reaching the network (#3596)
`ci_status::test_list_full_with_configured_platform_github` hit nextest's 180s timeout in an advisory CI job while the same 2586-test run passed everything else. Its unloaded runtime is 2.2s, and a `-vv` trace of the same fixture shows exactly one non-local subprocess: `git ls-remote --symref origin HEAD` against `https://bitbucket.org/test-owner/test-repo.git`, the URL the test sets on itself. `Repository::default_branch()` falls through to the wire when neither the worktrunk cache nor `origin/HEAD` resolves, which is sanctioned behavior, but nothing bounds it, so the test blocks for as long as the connect takes. Reproduced by pointing that remote at an unroutable address: 11s on macOS, and on Linux an unanswered SYN costs ~127s per address (`tcp_syn_retries=6`), with `bitbucket.org` publishing six. It isn't one test. Setting `GIT_TRACE` across the harness shows a single full-suite run spawning 180 `git-remote-https` processes: 63 to github.com, 42 to gitlab.com, 27 to dev.azure.com, 3 to bitbucket.org, and the rest to names that don't resolve. Every one is unbounded, on three OSes, on every PR. So the fix goes in the harness rather than in the one test. `GIT_ALLOW_PROTOCOL=file` joins `STATIC_TEST_ENV_VARS`, which covers every `wt` subprocess and every PTY env builder including the hand-rolled ones, and the two git-only builders set it as well for direct `git_command()` spawns that don't consult that table. Local paths and `file://` keep working; anything else fails in milliseconds with `transport 'https' not allowed`, and default-branch detection falls back to local inference, so output is unchanged. `GIT_TERMINAL_PROMPT` was never enough on its own: it only suppresses the credential prompt that the host's 401 triggers, by which point the request has already gone out. The rust-lang/rust benchmark fixture clone is the one git call that *should* reach upstream. It opts back in through `allow_network_transports`, which lives next to the constant so the deny and the opt-out can't drift apart. `switch_pr_self_hosted_tea_authed` recorded the resolver's `Could not resolve host` in its snapshot, which is the same defect class (asserted output depending on DNS). It now records the transport refusal. Also folded in: `build_worktree_config_bare_layout` in `src/git/repository/tests.rs` carried a nine-line hand-rolled copy of `configure_git_env`, so it now calls it. Not addressed here: nothing bounds that `ls-remote` for real users either, so an unreachable remote blocks `wt list --full` and `wt switch` for minutes on the first call per repo. That needs a decision about whether a timed-out detection should write the `worktrunk.default-branch` cache. > _This was written by Claude Code on behalf of max_ |
||
|
|
a5ca8a169f |
refactor(perf): build every wt-perf fixture branch through one primitive (#3584)
Follow-up to #3548, which consolidated the `wt-perf` prune fixtures but left the branch *builders* diverged. ## The refactor Three generators had each grown their own way to create a fixture branch. `add_history_spread_branches` pointed a ref at an old commit, `add_diverged_backdrop` hand-rolled ~45 lines of index plumbing, and `create_mixed_repo_at` used `checkout -b` plus a return-to-main for two of its four states. They are the same operation. A branch's state is entirely the pair **(fork point, own-commit count)**: zero commits at an old fork is "behind" and at the tip is "identical"; a positive count from the tip is "ahead" and from anywhere else is two-sided "diverged". `add_branch_with_commits` takes that pair, and the mixed fixture's four-state rotation collapses to a table: ```rust let (fork, commits) = match i % 4 { 0 => (checkpoint, 0), // behind 1 => (base_tip.as_str(), 1 + i % 3), // ahead 2 => (checkpoint, 1 + i % 3), // diverged _ => (base_tip.as_str(), 0), // identical to the tip }; ``` The checkout path is what actually mattered. `git checkout` of an old fork point on rust-lang/rust rewrites the whole tree (minutes per branch), which is why `add_diverged_backdrop` grew plumbing in the first place. Routing the mixed fixture through the same primitive removes the last checkout from fixture construction, so the main worktree is never touched and the fixture is no longer structurally barred from a rust-scale repo. The win is structural. Fixture build time is unchanged: `mixed-8-8` runs 3.25/3.28/3.71s after against 3.33/3.55/3.91s before, and `mixed-24-120` 14.9/15.7s against 15.5/23.5s. That is parity within noise, and the before-side spread swamps the difference. ## The guards These generators had nothing pinning their output. The benchmarks measure one wall-clock number, so a generator change that collapsed (say) "diverged" into "ahead" would keep every bench green while silently measuring a different repo. Three tests now assert the shapes directly, via `merge-base --is-ancestor` exit codes and porcelain status: - the `full` fixture's documented `index % 4` rotation of branch and worktree states - the same fixture's zero-dimension contract (`mixed-W-0` / `mixed-0-B`), which holds only because `0..0` never enters the loop that divides by `branches` - `add_diverged_backdrop`'s own wiring: every member unintegrated, fork depths fanning across history, worktrees dirty only via untracked scratch, and populations that don't follow each other Each was mutation-tested rather than trusted for being green. A clamped divisor gives `left: 1, right: 0`, and a fork spread collapsed onto the tip gives `fork depths must fan out across history: [0, 0, 0, 0]`. ## One thing to look at Writing the guards turned up a docstring overclaim in two places. `history_spread_shas` samples newest-first, so index 0 is the default branch's own tip. That makes `add_diverged_backdrop`'s "the default branch has advanced past every fork" and `add_history_spread_branches`'s "behind-only" both false as written. The first is true in its only caller, where `add_squash_merged` advances the default branch on the very next line. I corrected the docstrings rather than the sampling, and pinned the behavior with `assert_eq!(depths[0], 0, "the newest sample is main's own tip")`. Changing the sampling would shift the workload of every prune and `list` benchmark that uses the spread, moving Criterion's historical series with no rename to signal it, in exchange for one branch out of 24 to 120 gaining fork depth. Happy to flip it if you'd rather; it is documented and asserted either way. ## Verification `hook pre-merge` green (4542 tests), and `cargo bench --bench list full` runs end to end against the rebuilt fixture. Shape verified at benchmark scale rather than only at the test's `N = 8`: a real `mixed-24-120` holds the rotation through `br-0118`. The refactor is content-identical for the backdrop (same blob content, filenames, commit messages, plumbing sequence) and structurally identical for the mixed fixture (same fork points and commit counts, since `0..=(i%3)` and `1 + i%3` agree). > _This was written by Claude Code on behalf of max_ |
||
|
|
a0c1b662d2 |
perf(fsmonitor): resolve all daemons in one lsof spawn (#3581)
## Why `wt remove`'s end-of-command fsmonitor sweep forked one `lsof` per *machine-wide* `git fsmonitor--daemon` — and with `core.fsmonitor` enabled globally, a machine accumulates one daemon per repo ever touched (100+ is routine). So every `wt remove` paid ~100 process spawns. The cost is superlinear under load, not linear: a fork storm makes macOS Gatekeeper/XProtect assess each new image under contention, inflating per-spawn cost for everything else on the box, sweeps included. The `~50ms each` figure in the old docstring was measured *under that load* and read as a fixed per-call cost — so the sweep looked merely slow rather than self-amplifying. Two concurrent test suites driving hundreds of `wt remove` calls pinned all 18 cores (load average 142) with nothing hot in `htop`. This was diagnosed and measured before (#2814 added the sweep; #3401 wrote the cost into the docstring and explicitly changed no behavior) but never fixed. ## The fix Resolve every daemon in a single `lsof -a -p <pid1,pid2,…> -U -F pn` call and split its output on the `p<pid>` record boundary, handing each PID exactly the output the per-PID call produced — so classification and the whole data-safety contract in the module docs are unchanged. Verified end-to-end against a live 108-daemon machine with a PATH shim counting invocations: **108 spawns → 1**. Two details the batched path must preserve, each covered by a new test: - **Don't gate on `lsof`'s exit status.** It exits non-zero when *any* requested PID vanished mid-scan, while still printing full records for every survivor. Gating would drop the whole sweep whenever one daemon happened to exit — the common case on a busy machine. - **A PID with no record is dropped**, never synthesized as a socket-less entry. A socket-less entry reads as orphan class 1 and gets SIGTERM/SIGKILL — on a possibly-recycled PID. ## Also here - **Test consolidation.** Three integration fixtures built their branch set by spawning git per loop iteration. A new `TestRepo::create_branches` creates them in one `git update-ref --stdin`. The over-threshold completion test drops **10.5s → 1.4s** (~230 git spawns → 5); `switch_picker` and `statusline` branch loops get the same treatment. `mock-stub` gains an opt-in `MOCK_CALL_LOG_DIR` call log so a test can assert *spawn count*, not just response — deliberately outside the repo under test, since a log written inside the tree would dirty it mid-`wt merge`. - **Docs.** Dropped the "disable `core.fsmonitor` globally" workaround from the troubleshooting docs (canonical `skills/`, mirror regenerated). It stays enabled by choice, and the daemons were never the actual fseventsd CPU driver. - **CI hardening.** pre-commit.ci's `typos` hook maps the token `pn`→`on` and had auto-rewritten `lsof -F pn` to the broken `lsof -F on` (offset+name — no `p<pid>` records, so the parser reaps nothing). Pinned `pn` in `.typos.toml` next to the existing `PN`-from-`PNGs` entry that documents the same mapping, and added a test covering the empty-PID-set early return (flagged by `codecov/patch`). ## Testing New unit tests cover `daemons_from_batched_lsof` against real captured `lsof -F pn` output, a vanished-PID gap, and empty output; the rewritten integration test asserts exactly one `lsof` spawn with a comma-joined PID list. Full integration suite green (1960 passed), clippy and fmt clean. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
|
166e1190ba |
refactor(perf): canonicalize wt-perf prune-fixture layering + completion config (#3548)
## What Three behavior-preserving canonicalizations of the `wt-perf` benchmark fixtures, from a design pass on "why does prune need its own generator, and can the fixtures consolidate?" 1. **Share the prune-population layer.** `create_prune_repo_at` (synthetic) and `create_prune_real_repo_at` (rust-scale) both layered the same two populations onto their base repo — `add_diverged_backdrop` (the scanned-but-never-removed backdrop) + `add_squash_merged` (the removable candidates). Extracted into `add_prune_populations`, so the base repo (synthetic `create_repo_at` vs. rust clone) is now the *only* thing the two fixtures differ in, spelled out once instead of twice. 2. **Canonicalize `completion.rs` onto `RepoConfig::branches`.** Its two inline `RepoConfig` literals were exactly `RepoConfig::branches(50, 0)` (one with `worktrees` overridden), so they now use the canonical preset via struct-update instead of respelling all seven fields. 3. **Document the fixture → bench mapping.** `benches/CLAUDE.md` listed the `wt-perf setup` handles in one section and the bench groups in another with nothing connecting them, so a bench and its fixture (`full` and `mixed`) read as overlapping names for the same thing. Added a mapping table and the rule behind it. ## Why this is the whole safe consolidation The design question was whether prune could drop its bespoke generator for a "standard one with different params" — ideally one diverse repo serving prune *and* the other benches. The answer is that prune's generator is **already** a thin composition (`create_repo_at` + two additive layers), and it can't fold into the shared `mixed`/`full` fixture without breaking three prune-specific invariants: a known/cleanly-separated candidate count (the destructive `live` path re-creates exactly the consumed candidates), untracked-only dirt (the `git reset --mixed` index-heal depends on it — `mixed`'s staged-dirt states violate it), and a provably-unintegrated backdrop. And benchmark fixtures optimize for *attribution*, not realism: the uniform single-axis fixtures (`worktree_scaling`, etc.) exist precisely because the diverse `full` fixture can't localize a regression, so diversifying them would lose that. On the `full`/`mixed` naming specifically: they can't be canonicalized to one name, because fixtures and benches are **many-to-one** — `RepoConfig::typical` alone backs five bench groups (`skeleton`, `worktree_scaling`, `first_output`, `picker_preview`, `remove_e2e`), two of which sit in the same file measuring different things. `full`/`mixed` only looks 1:1 by coincidence. Renaming also isn't free: `.github/scripts/criterion-to-jsonl.py` keys each time-series row by the criterion output path (`<group>/<id>`) for the daily gist append, so a rename orphans that series. Documented rather than renamed. ## Proposed follow-up (not in this PR) The one remaining real duplication is two *implementations* of "two-sided-diverged population forked across history depth": `add_diverged_backdrop` (plumbing-based, scales to rust) vs. the diverged branch states inside `create_mixed_repo` (checkout-based, synthetic-only). They can't naively merge — the checkout path would make `prune-real` take minutes per branch — but a shared plumbing primitive could back both. That rewrites a carefully-tuned fixture feeding the `full` bench with **no shape-assertion test to catch drift**, so I'd land a fixture-shape test first (count refs by state + assert integration status). Happy to do it if wanted. ## Verification fmt `--check` clean; `clippy -p wt-perf --all-targets` and `clippy --bench completion` clean; `pre-commit` clean on the doc; `cargo test -p wt-perf` passes (incl. the prune-fixture tests `squash_merged_fixture_is_content_integrated` / `prune_fixture_state_classifies_lifecycle`, which exercise `create_prune_repo_at` → `add_prune_populations`); all benches build. Table contents verified against the code by grep. > _This was written by Claude Code on behalf of Maximilian Roos_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
a7c4a7a120 |
refactor(wt-perf): place fixtures under target/wt-perf, not the cache dir (#3547)
## What Move every `wt-perf` on-disk fixture out of the per-user cache dir and under the cargo target dir at `<target>/wt-perf/`, resolved by a renamed `wt_perf_fixture_dir()`: - `setup <config>` fixtures → `<target>/wt-perf/<config>` (was `~/.cache/wt-perf/<config>`). - The rust-lang/rust clone and `prune-real` fixtures → `<target>/wt-perf/bench-repos/` (was `~/.cache/wt-perf/bench-repos/`). - `wt_perf_cache_dir()` → `wt_perf_fixture_dir()`. The target dir is derived from the **running executable's own path** (it lives inside whichever dir cargo built into — `<target>/debug/wt-perf`, `<target>/release/deps/<bench>`), so it honors `CARGO_TARGET_DIR`, a config-file `build.target-dir`, and cargo-llvm-cov's `target/llvm-cov-target/` — none of which a bare `CARGO_TARGET_DIR` env read covers. Falls back to `<workspace>/target` if the binary isn't under a recognizable profile dir. The `etcetera` dependency is dropped. - The `WT_PERF_CACHE_DIR` env override (and the empty-value guard it required) is removed; the benchmarks workflow now caches the deterministic `target/wt-perf/bench-repos` path directly. - In-process throwaway fixtures (`create_repo`) are unchanged — they keep using `tempfile::TempDir`. This reverses the location decision from #3542, which had moved these into `~/.cache/wt-perf`. ## Why `target/` is the conventional home for build/test-generated artifacts — gitignored, reaped by `cargo clean`, and already where criterion writes (`target/criterion`). Keeping every wt-perf fixture there gives one predictable location under the repo that `cargo clean` fully resets. Deriving the target dir from the running binary (rather than assuming `<workspace>/target`) keeps that property intact even when the target dir is relocated. ## Tradeoff (accepted) `target/` is **not** shared across git worktrees (worktrees don't share it) and is wiped by `cargo clean`. So the ~15 GiB `prune-real` rust clone re-clones per worktree and after every `cargo clean` — the cost #3542 avoided by using the cache dir. This is deliberate and documented on `wt_perf_fixture_dir`; it's cheap for the synthetic `setup` fixtures, which rebuild in seconds. ## Testing - `cargo build -p wt-perf --all-targets`, `cargo test -p wt-perf` — clean. - New unit test `target_dir_from_exe_finds_cargo_target` covers the resolution logic (relocated `CARGO_TARGET_DIR`, bench binary under `release/deps/`, cargo-llvm-cov's nested target, closest-profile-wins, and the outside-any-target fallback). - `pre-commit run --all-files` (fmt, clippy, yaml, typos, cargo-lock, custom hooks) — clean. - Smoke-tested end-to-end: `setup branches-2` lands at `<worktree>/target/wt-perf/branches-2` and writes nothing to `~/.cache/`; a copy of the binary run from a fake `<dir>/debug/wt-perf` correctly resolves fixtures to `<dir>/wt-perf/`, proving a relocated target dir is honored. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
4121dc979c |
refactor(wt-perf): cache fixtures in the platform cache dir, not /tmp or target/ (#3542)
## What Move `wt-perf`'s benchmark/debug fixture repos out of `std::env::temp_dir()` and `target/bench-repos/` into the per-user cache dir, resolved by a new `wt_perf_cache_dir()`: - `$WT_PERF_CACHE_DIR` if set, else `<cache>/wt-perf` via `etcetera::choose_base_strategy().cache_dir()` — `~/.cache/wt-perf` (or `$XDG_CACHE_HOME/wt-perf`) on Linux **and** macOS. This matches worktrunk's own base-dir convention (`src/config/user/path.rs` resolves user config through the same XDG-on-macOS strategy). - `setup <config>` fixtures → `<cache>/<config>` (was `temp_dir()/wt-perf-<config>`). `--path` still overrides. - Real cloned fixtures (`prune-real`, the rust-lang/rust clone) → `<cache>/bench-repos/` (was `target/bench-repos/`). - In-process throwaway fixtures (`create_repo`) are unchanged — they keep using `tempfile::TempDir`. ## Why Both old homes were wrong for a *reused* fixture (these are given stable names, persist between runs, and are referenced from docs and `-C <path>` — that's a cache, not a temp file): - **A fixed name in a shared `/tmp` (Linux)** collides across users: `/tmp` is sticky (`1777`), so a second user's `setup` hits `remove_dir_all().unwrap()` on a dir they don't own → `EPERM` → panic. It's also a predictable-path/TOCTOU hazard, and `systemd-tmpfiles` reaps `/tmp` per-file at 10 days by `max(atime,mtime,ctime)` — on a `noatime` mount an actively-*read* fixture still loses cold `.git/objects/pack/*.pack` mid-use → silent git corruption. - **On macOS**, `temp_dir()` is already `/var/folders/.../T` (per-user, `0700`), so the docs' `/tmp/wt-perf-*` paths were simply wrong there. - **`target/`** is per-worktree (git worktrees don't share it) and wiped by `cargo clean`, so the ~15 GiB rust clone was re-cloned per worktree — worst for the most expensive fixture, in a worktree-heavy workflow. The cache dir is per-user (no collision, no TOCTOU), stable and discoverable (docs/`-C` references still work), shared across worktrees (clone once per machine), and survives `cargo clean` — matching sccache, cargo, rustup, Go, Bazel, and Hugging Face, which all place large reusable caches there rather than in `/tmp`. ## Migration notes - First bench/setup run after this rebuilds the cache under the new location (the old `target/bench-repos/` is no longer read). Existing fixtures can be dropped with `rm -rf target/bench-repos`. - `.github/workflows/benchmarks.yaml` sets `WT_PERF_CACHE_DIR` and caches `$WT_PERF_CACHE_DIR/bench-repos`, so the cached path and the path wt-perf writes stay pinned together (no drift if a runner sets `$XDG_CACHE_HOME`); key unchanged. - Docs (`benches/CLAUDE.md`, `src/commands/CLAUDE.md`) updated to the new paths, noting that `wt-perf setup` prints the exact path. ## Testing - `cargo build --workspace --all-targets`, `cargo clippy --workspace --all-targets --features shell-integration-tests -- -D warnings` — clean. - `cargo test -p wt-perf` — passes. - Smoke-tested default resolution (`~/.cache/wt-perf/…` on macOS), `$WT_PERF_CACHE_DIR` override, and the `prune-real --path` rejection (now names the resolved cache path). > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
c310767216 |
refactor(bench): canonicalize prune cold to probe-cold via bench_wt (#3437)
Prune's Criterion "cold" had two structural problems: it over-invalidated (deleting worktree indexes models a state users never see, and needed the `restore_worktree_indexes` pairing just to keep prune's clean-worktree gate from silently dropping candidates), and its per-iteration stdout assertions kept it off the canonical `bench_wt` runner. This PR fixes both. **Probe-cold is now the canonical prune cold state.** `invalidate_probe_caches` clears only `.git/wt/cache/`, the exact state a repo is in right after fetching new commits — probes re-run at real cost, git's own caches (indexes with stat data, commit graph, `worktrunk.default-branch`) stay warm. `bench_wt` picks its invalidation via a new `CacheState` enum (`Warm` / `Cold` / `ProbeCold`), and every prune dry-run arm is now a `bench_wt` one-liner. `restore_worktree_indexes` is demoted to a private fixture-healing helper. **Verification moved out of the timed loops.** Timed iterations assert only exit status, like every other bench. Fixture correctness is pinned once after setup: `verify_candidates` runs one dry-run that must list exactly the expected candidates (8 synthetic, 24 rust-scale, 0 on the live fixture's backdrop). A live run that removes nothing trips the next iteration's candidate re-creation (branch-name collision) instead of being silently timed. **The rust-scale cold cell is now repeatable.** `prune_e2e/dry_run_cold` is renamed to `dry_run_probe_cold` (the metric changed meaning, so the old gist series ends deliberately), and a new `prune_real_repo/dry_run_probe_cold` measures at rust scale what was previously a hand-run one-shot — feasible because probe-cold iterations cost ~0.6 s where full-cold costs ~1 min. Measured on a quiet machine, all with tight CIs: synthetic probe-cold 159 ms / warm 96 ms / live 704 ms; rust-scale warm 247 ms / probe-cold 630 ms. ## Testing All five bench arms run end-to-end with the fixture checks passing (synthetic groups plus the feature-gated rust-scale group against a freshly rebuilt 13 GiB fixture). `cargo test -p wt-perf`, clippy on both feature paths, and the rustdoc `-Dwarnings` gate all pass. Benchmarks don't run on PR CI; the daily workflow picks up the renamed series. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
350ea1f6a8 |
refactor(perf): make wt-perf setup persist-only and trim the CLI/lib surface (#3433)
Follow-up to #3403, applying the deferred `--persist` decision and two sibling subtractions found sweeping the perf tooling for the same shape (a vestigial option or dual path guarding nothing). - **`wt-perf setup` is persist-only.** The `--persist` flag and the interactive "Press Enter to clean up (or Ctrl+C to keep)" path are gone — it was the one blocking, stdin-reading path in an otherwise scriptable tool, and it guarded little: default-path fixtures land in `/tmp/wt-perf-<config>`, which the next `setup <config>` run already wipes and rebuilds. `prune-real` never offered cleanup, so setup's exit behavior is now uniform. - **`timeline --repo` deleted.** It only restated the traced repo: `run_timeline` resolves the repo from `-C`/cwd, `--repo` defaulted to exactly that, and the only documented use passed the identical path to both flags. `--cold` now invalidates the repo being traced — the only coherent target. - **`wt_perf` lib surface tightened.** Seven helpers plus `PruneFixtureState` had no callers outside `lib.rs` and are now private (`ensure_rust_repo`, `clone_rust_repo_at`, `history_spread_shas`, `add_diverged_backdrop`, `create_mixed_repo_at`, `create_prune_real_repo_at`, `prune_fixture_state`); the public surface now matches what the benches and CLI actually use. Doc references to them are plain code spans rather than intra-doc links — rustdoc's `private-intra-doc-links` lint rejects public docs linking private items even under `--document-private-items`. Checked and deliberately kept: `wt-perf trace` and `wt-perf invalidate` (distinct documented workflows), the `TraceResult`/`CacheReport` re-exports (public field types of `TraceEntry`/`Profile`), and the `create_repo`/`create_repo_at` two-layer fixture API (both layers used). Gate green: 4387 tests, clippy/fmt/doctests/rustdoc (`-Dwarnings`) clean. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
81031bdd2a | chore: bump MSRV and toolchain to 1.96 (#3428) | ||
|
|
18d2bef84b |
refactor(perf): canonicalize the benchmark / wt-perf system (#3403)
Implements the design proposal from this branch's first commit
(
|
||
|
|
5e58827a28 |
bench(prune): rust-scale prune fixture, criterion benches, and removal trace spans (#3401)
Makes `wt step prune` / `wt remove` staging performance measurable and reproducible. Users report prune taking many seconds while the synthetic benches scan in ~150 ms; this adds the observability and fixtures that reproduce the gap and attribute it. No `wt` runtime behavior changes — the `src/` diff is ~29 lines of trace spans. ## What a reviewer needs - **Trace spans** (`src/commands/step/prune.rs`, `src/commands/process.rs`, `src/git/fsmonitor.rs`): `prune-gather` / `prune-scan` / `prune-check:<ref>` / `prune-remove:<label>` plus `internal-sweep` around `wt remove`'s end-of-command janitor. A `prune-remove` span starts before the write-lock acquisition, so it reads as "how long this removal stalled the run" (documented in benches/CLAUDE.md). - **Fixtures** (`tests/helpers/wt-perf/src/lib.rs`, the bulk of the diff): `prune-M-U` — M squash-merged candidate pairs (content-integrated, the post-PR-squash shape) against a two-sided-diverged backdrop forked across history; `prune-real[-M-U]` — the same shape on a rust-lang/rust clone, default 12+24 → 36 linked worktrees. The real fixture is cache-managed under `target/bench-repos/` (~15 GiB, minutes to build): `ensure_prune_real_repo` classifies it Intact/Consumed/Broken on reuse, re-creates candidates consumed by a live prune in ~1 min (round derived from the surviving squash commits, no sidecar state), and heals invalidated worktree indexes. - **Benchmarks** (`benches/prune.rs`): criterion groups `prune_e2e` (dry-run cold/warm + live with per-iteration candidate re-creation) and `prune_real_repo` (warm dry-run only). The real group is gated behind a new `real-repo-benches` cargo feature so the nightly benchmarks workflow — plain `cargo bench` on a hosted runner — never builds a fixture bigger than the runner's disk and the actions cache cap. ## Measured (M-series Mac, in benches/CLAUDE.md) Live prune on the rust-scale fixture: **~12 s wall** — a 5.4 s stat-cold scan (36 `git status` at ~4.5 s each, absorbed by the rayon pool) plus 24 removals serialized under the scan write lock (~0.5–1.7 s per worktree candidate). Warm re-scan: 0.25–0.8 s. This reproduces the reported "prune takes seconds" experience and points optimization at status cost and removal overlap, not the integration probes. Rider fix picked up en route: `docs.anthropic.com/en/docs/build-with-claude/claude-code` now 404s (failing lychee on every push, including on main), so the llm-commits installation link and the `wt config plugins` install-hint now point at `code.claude.com/docs/en/setup`. Also fixes a latent fixture bug: `history_spread_shas` divided by its hardcoded 5000-commit cap, so on short synthetic histories every "history-spread" fork silently collapsed to the tip. ## Testing wt-perf unit tests cover the fixture lifecycle (state classification, derived repair rounds, squash content-integration); every `ensure_prune_real_repo` path (build, cached, repair, index-heal, foreign cwd) was exercised end-to-end locally, as were both criterion groups and the one-shot timelines. An integration test asserts the `wt remove -vv` trace surfaces the fsmonitor sweep (span plus daemon count), pinning the sweep's observability contract. The rust-scale numbers are I/O-bound and ambient-load-sensitive — documented as shape, not thresholds. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
e9bedf5b03 |
fix(bench): restore worktree indexes after cache invalidation (#3359)
## Summary Deleting a worktree's `.git/index` isn't a cold cache: git treats a missing index as empty, so `git status` reports every tracked file as a staged deletion — a different repo state, not a cold cache. That flips any clean-worktree gate a benchmarked command exercises (e.g. `wt step prune`'s removability check would silently drop every worktree candidate against such a worktree; verified empirically on a `timeline --cold` prune run). Adds `wt_perf::restore_worktree_indexes`, which `git reset -q`s every worktree back to a clean `git status` after `invalidate_caches_auto`, and documents the trap in `benches/CLAUDE.md`. `restore_worktree_indexes` has no callers on this branch yet — an upcoming prune-benchmark branch is its first consumer. ## Test plan - [x] `cargo build -p wt-perf` — clean, no warnings - [x] `cargo clippy -p wt-perf --all-targets` — clean - [x] `cargo run -- hook pre-merge --yes` — all pre-commit hooks, fmt, clippy, full nextest suite (4309 passed), doctests, and docs pass > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
|
28c937dbbf |
bench(list): consolidate synthetic wt list benches into one full group (#3335)
Consolidates the narrow synthetic `wt list` benchmarks into a single `full` group backed by one combined fixture, so one bench exercises the whole surface (many worktrees AND many branches in varied states) instead of several groups that each covered worktrees-only or branches-only in a uniform state. ## What changed - **New `full` group** (`benches/list.rs`) runs `create_mixed_repo(24, 120)` cold + warm: 24 worktrees and 120 branches in mixed clean/dirty/staged working trees and merged/ahead/diverged branch states, with branch divergence spread across history depth (the GH #461 cost driver). `full/warm` ≈ 147ms, `full/cold` ≈ 480ms locally. - **Folded in:** the old `full` (worktrees-only — a duplicate of `worktree_scaling` despite the name), `many_branches` (100 uniform branches), and the warm-only `rerun_warm` seed. No coverage dropped: `worktree_scaling` keeps the worktree-count scaling sweep, and the combined fixture's branch side subsumes `many_branches`. - **Kept standalone:** `divergent_branches` (the pure #461 deep-divergence stress) and `worktree_scaling` — both are cited in `src/main.rs` as the `rayon_thread_count` tuning anchors, and together they are the per-side regression trackers. - **Fixture extension** (`create_mixed_repo` in `tests/helpers/wt-perf`): added the deep-divergence shape it lacked — deeper base history (120 → 200 commits), checkpoints every 5, behind/diverged branches forking at points fanned across the full depth, and multi-commit diverged chains. Signature unchanged, so `wt-perf setup mixed-W-B` still works. - **Attribution:** a `full` wall time can't be decomposed by side (worktree- and branch-side git subprocesses overlap on the rayon pool), so `benches/CLAUDE.md` documents the `wt-perf timeline` `args.context` trace query (query #3) as the way to localize a regression. - `run_benchmark` now takes `cold_cache: bool` instead of `&BenchConfig`, decoupling it from the fixture type. Removed a stale `timeout_effect` doc reference (no such group exists). Benchmark-only change (plus the `wt-perf` test helper and `benches/CLAUDE.md`); no production code. All six `list` groups compile and run; clippy `-Dwarnings`, fmt, and the `wt-perf` tests are clean. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
f045e0f449 |
perf(list): cut warm-cache wt list re-run ~36% by killing redundant per-row git forks (#3334)
## What
A warm re-run of `wt list` (where `.git/wt/cache/` is already populated)
is throughput / critical-path bound on per-worktree and per-branch git
subprocesses. Two of worktrunk's three cache layers — `Arc<RepoCache>`
and the process-global path/merge-base/commit-tree `DashMap`s — live in
process memory and don't survive across invocations, so every re-run
re-forks work that's content-addressed or already in hand. The "~1 ms
recompute, not worth a disk file" assumption behind keeping merge-base
and commit→tree in memory is false at scale on macOS (10–25 ms per fork
× dozens of rows).
This eliminates three classes of those forks. On a 24-worktree /
120-branch mixed-state fixture, subprocess work drops **3629 ms → 1013
ms**; the criterion `rerun_warm` benchmark goes **212 → 135 ms median**
(−36%, p<0.05). What remains is the irreducible working-tree floor —
per-worktree `git status`, and `add -A`/`diff`/`write-tree` for dirty
worktrees — which depends on live working-tree state and genuinely can't
be cached.
## The three changes
1. **`%T` primes the commit→tree cache** (`diff.rs`). The pre-skeleton
`git log --no-walk` batch already resolves each commit for `%ct`; adding
`%T` rides along for free and primes `commit_tree`, so the per-row
`CommittedTreesMatch` / `WouldMergeAdd` tree lookups never fork `git
rev-parse <sha>^{tree}`.
2. **Persist merge-base** (`sha_cache.rs`, `diff.rs`). New
content-addressed `merge-base/` sha_cache kind (in-memory front over
disk back) plus a `sha1 == sha2` short-circuit. The `AheadBehind` orphan
check forked `git merge-base` per row even when ahead/behind counts were
already cache-warm; now it's a file read on re-runs.
3. **Prime worktree root/git-dir** (`mod.rs`, `collect/mod.rs`). `git
worktree list` already gives every worktree's path; seeding
`WORKTREE_ROOTS`/`GIT_DIRS` from it (git-dir read from each `.git`
entry, mirroring `prewarm_rev_parse`'s canonicalization) eliminates the
per-worktree `git rev-parse --show-toplevel` / `--git-dir` forks. Runs
post-skeleton — its values are consumed only by the worker pool, so it
stays off the skeleton critical path and is skipped under
`WORKTRUNK_SKELETON_ONLY` (measured: skeleton time unchanged at 24.4
ms).
All three are behavior-preserving: caches are content-addressed and
never stale, and any value that can't be derived cleanly falls back to
the original subprocess.
## Benchmark
Adds `cargo bench --bench list rerun_warm` (24 worktrees + 120 branches
in varied states: clean / unstaged / staged+untracked worktrees; merged
/ ahead / diverged / identical branches), a reusable
`wt_perf::create_mixed_repo` fixture, and a `wt-perf setup mixed-W-B`
config. The existing list benches are warm-capable but each cover
worktrees-only or branches-only in a single uniform state. A
`TODO(bench-full-combined)` marks consolidating these into one cold+warm
"full" scenario with per-feature trace attribution.
## Reviewer orientation
- Cache mechanics: `src/git/repository/sha_cache.rs` (new kind) and the
`# Caching` docstring in `src/git/repository/mod.rs` (updated to
document the in-memory-front-over-disk-back decision and correct the
stale cost premise).
- The list-collect path: `src/commands/list/collect/mod.rs` module
docstring (caching table + `%T` fork doc updated).
- Tests: cache priming, merge-base roundtrip (incl. orphan), and a
prime-vs-subprocess equivalence test that pins the derived git-dir
against the real `git rev-parse` across main + linked worktrees.
> _This was written by Claude Code on behalf of max_
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
c1c76b150d |
refactor(trace): decouple human trace.log from machine trace.jsonl (#3297)
At `-vv`, worktrunk's `trace.log` carried machine-parseable `[wt-trace] ts=… tid=… seq=… cmd="git status" dur_us=12300 ok=true` lines *alongside* a `$ git status [ctx]` start echo — so every command appeared twice in two inconsistent renderings, and the human file was cluttered with `key=value` machine fields. The root cause: `trace.log` was doing double duty as both the human artifact and the machine-parsed source, even though `trace.jsonl` (added in #3232) already carries the same fields losslessly and nothing read it as input. This decouples the two. `trace.log` + stderr become purely human; `trace.jsonl` becomes the sole machine format. ```text # BEFORE — two inconsistent renderings of one command [h] $ git rev-parse --show-toplevel [worktrunk.rev-parse-dedup] [h] [wt-trace] ts=11593 tid=8 seq=5 context=worktrunk.rev-parse-dedup cmd="git rev-parse --show-toplevel" dur_us=6101 ok=true # AFTER — start ($) and finish (✓) pair, one `cmd [ctx]` rendering, no clutter [h] $ git rev-parse --show-toplevel [worktrunk.rev-parse-dedup] [h] ✓ git rev-parse --show-toplevel [worktrunk.rev-parse-dedup] 6.1ms ``` Commands render `$ …` (start) → `✓`/`✗ … dur` (finish), in-process spans `◷ name dur`, milestones `· event` — the leading glyph names the line type at a glance. ## What a reviewer needs - **`src/trace/parse.rs`** — rewritten to parse `trace.jsonl` (one JSON object per line) via a `kind`-dispatch (`cmd_completed`/`cmd_errored`/`instant`/`span`), skipping non-records (`{"message":…}` logs, `$ cmd` echoes, non-JSON). Cleaner than inferring kind from which fields are present. - **`src/logging.rs`** — `format_wt_trace` now emits the human line; `style_stderr_line` bolds the command for `$`/`✓`/`✗` lines so a start and its finish read as a pair. The machine fields (`ts`/`tid`/`seq`) live only in `trace.jsonl` (via the separate JSON visitor). - **Consumers repointed at `trace.jsonl`**: `wt config state logs profile`, the `wt-perf` helper (`timeline` now runs `wt -vv` and reads the file, located via `git rev-parse --git-common-dir`), and the `diagnostic.md` profile section (while still inlining the human `trace.log`). - **Docs** — help text, the FAQ file inventory (added `trace.jsonl`), and the `[wt-trace]`-grammar descriptions across `emit.rs`/`log_files.rs`/`benches/CLAUDE.md` updated. One behavior note: `wt-perf timeline` now writes trace files to the repo's `.git/wt/logs/` (a side effect of `-vv`) where the old `RUST_LOG=debug` path didn't — expected for a `-vv`-based perf helper. ## Testing Well-covered: parser (14 unit tests), renderer + stderr styling (8), `logs profile`/`diagnostic`/`switch`/`completion` integration tests migrated to JSON fixtures, plus a `wt_target_dir` unit test for the wt-perf `-C` resolution. Verified `logs profile`, `wt-perf timeline` (text + `--chrome`), and `cache-check` end-to-end on a real `-vv` capture. An independent review pass found no correctness issues; its findings (a missed doc, two wt-perf robustness fixes) are folded in. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
bfdb22b889 |
fix(trace): don't flag stdin-driven commands as cache duplicates (#3296)
The performance profile's cache analysis keyed each command by `(command, context)`, assuming that pair fully determines its work. Commands that read stdin carry input the command string doesn't capture, so identical command lines collapsed into one bucket and were reported as wasteful re-runs. In a real capture the CACHE section claimed `sh -c … claude -p … (×11)` and `git patch-id --verbatim (×6)` were ~45s of "duplicate" waste — but each `claude -p` got a different prompt on stdin, and each `patch-id` a different diff. Distinct work, not a cache miss. This adds a `stdin=true` marker to the `[wt-trace]` grammar, set at every spawn site that feeds uncaptured stdin (`run`, `stream`, both `pipe_into` ends, the concurrent runner, pipeline steps, and the pre-spawn failure paths). `CacheReport::from_entries` then excludes stdin-reading commands from duplicate detection while still counting them in `total_commands`/`total_time`/`contexts`. I chose to *exclude* stdin commands rather than *hash stdin into the dedup key*: a `pipe_into` sink like `git patch-id` reads a live pipe with no buffer to hash, so exclusion covers both the buffer and pipe cases uniformly where hashing can't. The trade-off is that a genuinely-identical stdin re-run (same prompt twice) is no longer flagged — acceptable, since that doesn't occur for these commands. The wire field trails `ok`/`err` and is omitted when false, so existing trace records are byte-identical; `trace.jsonl` carries `stdin` on command records. **Testing:** new unit coverage for the exclusion (`cache_report_excludes_stdin_reading_commands`), the parse round-trip, and the `format_wt_trace` grammar lock; full pre-merge gate green (4274 tests). No end-to-end test asserts the marker on a real captured trace — the producer→consumer wiring is covered by construction and unit tests. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
4943ccdc4a | test(wt-perf): fix stale helper docs and guard degenerate spread count (#3235) | ||
|
|
9425a22bd0 |
Add wt config state logs profile performance profiler (#3184)
## `wt config state logs profile` A new subcommand that turns a `-vv` trace into a performance profile, answering the three questions in `benches/CLAUDE.md` without leaving the terminal: where time went, how parallel the run was, and where work was wasted. `-vv` already writes `[wt-trace]` records to `.git/wt/logs/trace.log`; this parses them and renders: - **BY COMMAND TYPE / SLOWEST CALLS** — subprocess time grouped by command shape (`git status`, `gh pr list`), plus the slowest individual jobs. - **parallelism / peak concurrency** — Σ(subprocess time) ÷ wall span, and the most subprocesses in flight at once. - **CACHE** — commands re-run within the same context (a cache miss that should have hit). - **KEY INTERVALS / PHASES** — for a `wt list` or picker capture, derived latencies (time to skeleton, time to first result) and a collect-milestone timeline. These come from the `worktrunk::trace::instant(…)` milestones, so they populate only for those commands; every other command still gets the sections above. `--format=json` serializes the same struct (durations as `*_us` integers) for scripting, so the text and JSON views can't drift. The `diagnostic.md` bug-report bundle now inlines a rendered profile beside the raw trace, so any `-vv` report involving slowness shows where time went at a glance. ### Navigating the diff - `src/trace/profile.rs` — the analysis (`Profile` / `CacheReport::from_entries`, pure data over `&[TraceEntry]`) and the text renderer; the `Serialize` impl is the single canonical JSON source. - `src/cli/config.rs`, `src/commands/config/state.rs` — CLI surface and `handle_logs_profile` (reads a path arg, `-` for stdin, or the default `.git/wt/logs/trace.log`). - `src/diagnostic.rs` — inlines the rendered profile into the bundle. - `tests/helpers/wt-perf/src/main.rs` — reuses the shared `CacheReport`. ### Testing Well covered: the renderer across full / minimal / collect-without-skeleton / truncation variants, the JSON shape, and the CLI end-to-end (path, stdin, default, missing-file, no-records, not-in-a-repo). The large text outputs are file snapshots, and an end-to-end `wt -vv list` test guards the milestone strings against drift. The diagnostic profile section is presence-checked (placeholdered in the snapshot, since its timing is non-deterministic) with the rendering logic unit-tested deterministically. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
81f9260a46 |
feat(switch): browse open PRs/MRs in the interactive picker (#3128)
Adds `wt switch --prs`: browse the repository's open PRs (GitHub) / MRs (GitLab) in the interactive picker, with a live CI column (same data as `wt list --full`), a `pr` preview tab showing the markdown-rendered description, and `#`-gutter-sigil filtering. Selection routes through the existing `pr:`/`mr:` fetch-and-switch path, so there's no new switch logic. |
||
|
|
958b9c188a |
refactor(picker): migrate to skim 4.8 (ratatui), drop vendored skim-tuikit (#3137)
Migrates the `wt switch` picker from skim 0.20.5 (tuikit backend) to
skim 4.8.0 (ratatui/crossterm), removes the `vendor/skim-tuikit/` patch
tree we carried against the old line, and makes the full test suite
green under the new backend.
## Why the vendor tree goes away
We vendored skim-tuikit for two patches: `alt-<digit>` key parsing and
an `Output::flush` `write_all` fix for dropped bytes under PTY pressure.
Both are moot in 4.8. Key parsing is native — `binds.rs` routes every
single character (digits included) through `KeyCode::Char` rather than a
hardcoded `alt-a..alt-z` arm list. The flush fix is subsumed by stdlib
`BufWriter::flush` in the ratatui backend, which loops over partial
writes correctly. So the migration carries zero vendor patches against
skim, and the build machinery that kept the vendored source alive
(Taskfile `vendor-diff`, flake `vendorSrc`/`extraDummyScript`) is
deleted too.
## The picker rewrite
skim 4.x is a different API. `WorktreeSkimItem::display()` now returns
`ratatui::text::Line` instead of an `AnsiString`. alt-r removal rides
skim's `reload(remove {})` token (the selected row's `output()`,
expanded into the reload command) instead of the old
`as_any().downcast_ref` path (which never worked across compilation
units) plus a signal file. Preview-tab switching and the progressive
list redraw are driven by `Skim::event_sender()` +
`Event::Render`/`Event::RunPreview`.
## The regression that motivated the bulk of this
skim 4.x renders on demand. There is no 100ms tuikit heartbeat
re-rendering the frame, so the progressive `wt switch` list rendered
blank while collection ran in the background — the original report. The
fix pokes `Event::Render` from the list-collect callbacks (throttled to
16ms). The same on-demand model surfaced four more regressions, all
fixed here and verified head-to-head against the 0.20 picker in a
terminal: preview-tab lag, current-row highlight, Shift-Tab back-cycle
(crossterm reports Shift-Tab under three distinct `KeyEvent` shapes, so
all three are bound), and alt-r removal (4.x `execute-silent` is
fire-and-forget and raced its reader).
## Help text and the test harness
Two things the integration suite caught that the unit tests did not (the
prior pass ran `--lib --bins` only):
- **Styled help.** skim 0.20 transitively pulled in
`clap/unstable-markdown`, which renders worktrunk's `///` doc-comments
as styled help. skim 4 with `default-features = false` drops it, so help
reverted to raw text (`[experimental]` → `\[experimental\]`). Restored
by depending on `unstable-markdown` explicitly, keeping help output
identical and the `test_help` / `test_step_alias` /
`test_docs_are_in_sync` snapshots passing unchanged.
- **PTY harness.** skim 4.x queries cursor position (`ESC[6n`) at
startup in partial-height mode and blocks in `select()` for the reply;
`portable_pty` is a bare PTY and never answered, so every
`switch_picker` test failed init with "Cursor position detection timed
out." The Unix harness now answers the query, mirroring the existing
ConPTY responder. skim 4.x also draws the list/preview separator one
column left, so the snapshot panel-split columns shifted to match. The
regenerated picker snapshots show two cosmetic changes: the match
counter no longer overlaps the preview tab header, and the HEAD column
shows the full short-SHA instead of a truncated one.
## Minimal-versions CI
The skim 4.8 dep tree pins tighter floors than our manifests declared,
so the nightly `minimal-versions` job needed updating. It now runs `-Z
direct-minimal-versions` — minimize only our own direct deps and let
transitive crates resolve normally — and the manifests raise each
under-specified floor to the minimum the workspace builds against
(largely mirroring skim 4.8's own requirements). Full `-Z
minimal-versions` would instead drag in skim's transitive TUI/image
stack (ansi-to-tui, ratatui's `instability` macro, color-eyre,
ratatui-image → image/avif), whose crates under-declare their floors and
don't compile at the picked versions; direct minimization confines the
check to floors we own, so no transitive pins are needed. Normal
resolution (the committed `Cargo.lock`) is unchanged. Full floor list
and the `signal-hook` libc-pin detail are in the `ci(min-versions)`
commit message.
## Reviewing
Start at `src/commands/picker/mod.rs` (the `run_skim` entry point, the
action keybinds, and `parse_reload_remove_token`), then
`progressive_handler.rs` (the render pokes) and `items.rs` (the
`Line`-based `display()`). Test-harness changes are in
`tests/common/pty.rs` (the `ESC[6n` responder) and
`tests/integration_tests/switch_picker.rs` (the panel-split columns).
The rest is the dependency swap, the vendor and build-machinery
deletions, and regenerated snapshots.
All 3980 tests pass (`cargo run -- hook pre-merge --yes`), including the
`shell-integration-tests` PTY suite that exercises the picker end-to-end
across the list, previews, scroll, create/remove, and accept flows. No
automated test drives a real terminal; the interactive surface was also
checked by hand against the 0.20 picker.
> _This was written by Claude Code on behalf of max_
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
330fa6ee4e |
chore(ci): weekly renovation 2026-05-31 (#2948)
## Summary Weekly CI renovation. MSRV/toolchain bump plus pin bumps where upstream has moved. - **MSRV: 1.94 → 1.95** (Rust 1.96.0 went stable 2026-05-28; policy is latest stable − 1) - **toolchain channel: 1.94.0 → 1.95.0** - `cargo-nextest`: 0.9.136 → 0.9.137 (MSRV 1.91, compatible with 1.95.0) - `worktrunk`: 0.53.0 → 0.55.0 (MSRV 1.94, compatible with 1.95.0) - `hustcer/setup-nu` nushell: 0.113.0 → 0.113.1 - `flake.lock`: `rust-overlay` bumped to `4a408e1` (ships `1.95.0.nix` and `1.96.0.nix`); refreshed in the follow-up commit after @max-sixty asked whether the agent could install Nix — it can, via the Determinate Systems installer. A separate skill PR will codify the Nix install step into the weekly workflow so future runs do this automatically. ## Already up to date - `cargo-insta`: 1.47.2, `cargo-llvm-cov`: 0.8.7, `cargo-msrv`: 0.19.3, `cargo-udeps`: 0.1.61, `lychee`: 0.24.2 - `taiki-e/install-action` tool: zola@0.22.1 - `taiki-e/install-action` action ref tracked by Dependabot (open in #2935: 2.79.11 → 2.79.13; latest is 2.80.0 — Dependabot will catch up next cycle) - Runner images: ubuntu-24.04, macos-15, windows-2022 ## Notes - windows-2022 remains pinned (actions/runner-images#12677 — windows-2025 lacks D: drive) --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
|
dee85bb55b | docs(test): document mock-stub stderr response type (#2971) | ||
|
|
de3ef902da | fix(bench): stop deleting packed-refs in invalidate_caches_auto (#2697) | ||
|
|
e10a1b7617 |
fix(bench): stabilize list-bench fixture and record thread-count findings (#2683)
## Summary `cargo bench --bench list` was failing ~30% of runs partway through fixture setup with errors like "fatal: unable to read tree", "invalid object ... for 'src/file_N.rs'", "unable to create temporary file", or "failed to insert into database". Root cause: git's auto-maintenance (`gc.auto` + `maintenance.auto`) fires during the 500-commit fixture-build loop and races the foreground `git add` / `git commit`. Partial-failure repos contained 5-6 pack files plus a `tmp_pack_*` — smoking gun for a detached pack/prune in flight. `create_repo_at` now silences both auto-maintenance knobs after `git init`, and runs one explicit `git gc` at the end after all commits, branches, and worktrees are in place. The result: 10/10 sequential `wt-perf setup typical-8` builds pass, and fixtures land in a packed shape (one packfile + commit-graph, zero loose objects) that matches day-N user repos rather than the loose-object cold-clone shape they had before. ## Thread-count follow-up to #2662 Using the stabilized benches to answer the question in #2662's "Follow-ups": should `rayon_thread_count` bump from 2× CPU now that we have one pool instead of two? Swept `RAYON_NUM_THREADS` across {0.5×, 1×, 1.5×, 2×, 3×, 4×, 6×} on an 18-CPU machine against `divergent_branches/warm` (branch-heavy) and `worktree_scaling/warm/8` (worktree-heavy). 3× CPU was at or within noise of the per-workload optimum on both; 2× trailed by 0–5% (divergent 259ms vs 257ms with overlapping CIs; worktree 86.6ms vs 82.4ms, ~5% gap). 4× regressed on branch-heavy workloads. Stayed at 2×: the win is small in absolute terms (≤ 5ms) and 2× has been validated in production on hardware we haven't benchmarked. Docstring on `rayon_thread_count` records the test and the reasoning so the next person who asks doesn't have to redo the experiment. ## Test plan - [x] `cargo run -- hook pre-merge --yes` — 3547 tests pass - [x] `wt-perf setup typical-8 --persist` × 10 sequential — no failures (pre-fix: ~30% failure rate) - [x] `cargo bench --bench list divergent_branches/warm` — completes, packed shape (1 pack, 0 loose) - [x] `cargo bench --bench list worktree_scaling/warm/8` — completes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
16c0f8a062 |
revert: drop wt-perf test layout workaround from #2604 (#2610)
PR #2604 moved timeline tests out of `tests/helpers/wt-perf/src/main.rs` into `src/lib.rs` and set `test = false` on the `[[bin]]` to dodge [cargo-affected#13](https://github.com/max-sixty/cargo-affected/issues/13) — the runner shim couldn't disambiguate `[lib]` and `[[bin]]` artifacts that both normalize to `wt_perf` when neither kind carries a marker. [cargo-affected#14](https://github.com/max-sixty/cargo-affected/pull/14) fixed that upstream by reading `NEXTEST_BINARY_ID` from the nextest env directly, so the shim now handles lib+bin collisions natively. The workaround is no longer needed. This restores the three files in `tests/helpers/wt-perf/` to their pre-#2604 state: tests live in `main.rs` again, the bin compiles its `#[cfg(test)]` block under nextest, and the `test = false` line is gone. CI installs `cargo-affected` from git without a pin (see `.github/workflows/ci.yaml:287`, `:359`), so the upstream fix is picked up automatically — no pin bump. ## Verification - `cargo build -p wt-perf` ✓ - `cargo test -p wt-perf` ✓ — both inline tests run from `main.rs` (`tests::cmd_failure_annotates_name`, `tests::renders_sorted_timeline_with_summary`), plus the `tests/builds.rs::builds` integration test. - The `affected tests` and `collect affected coverage` jobs in this PR's CI will exercise the actual fix end-to-end. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
e261e50761 |
fix(wt-perf): move timeline tests to lib + exclude bin from test compile (#2604)
## Problem The `collect affected coverage` job has been failing on every push to main since #2558 merged, with the same error shape: ``` FAIL [ 0.009s] (3487/3491) wt-perf::bin/wt-perf tests::cmd_failure_annotates_name cargo-affected runner-shim: failed to resolve binary_id for /home/runner/work/worktrunk/worktrunk/target/affected/build/debug/deps/wt_perf-01f04725fd165542: binary not found in map (path: /home/runner/work/worktrunk/worktrunk/target/affected/build/debug/deps/wt_perf-01f04725fd165542); target-name basename fallback ambiguous — 2 candidates share target name "wt_perf" ([wt-perf, wt-perf::bin/wt-perf]) and the marker probe matched 0 of them (none; candidates without markers: wt-perf::bin/wt-perf, wt-perf); re-run `cargo affected collect` ``` Root cause: the `[lib] name = "wt_perf"` and `[[bin]] name = "wt-perf"` artifacts both normalize to the same `wt_perf` basename. cargo-affected's runner shim disambiguates same-basename test binaries via a marker probe, but markers only fire for `kind = "test" | "bench" | "example"` — `lib` and `bin` get none. Once #2558 added inline tests under `bin/wt-perf`, both candidates appeared in cargo-affected's binary map without markers, so any hash drift between list and run aborted the shim with `basename fallback ambiguous`. Issue #2571 previously diagnosed this as transient, but it has continued to fail every run since (25295115776, 25301191157, 25303363793, 25309058661, [25336005330](https://github.com/max-sixty/worktrunk/actions/runs/25336005330)) with the same error shape — it is durable, not transient. ## Solution Two changes together fix it: - Move `render_timeline` (and its `describe`/`duration_of` helpers) plus the inline tests from `src/main.rs` into `src/lib.rs`. The bin no longer carries any `#[cfg(test)]` items. - Set `test = false` on the `[[bin]]` in `Cargo.toml` so cargo doesn't emit a test binary for it. This removes the bin from cargo-affected's binary map entirely, so even with hash drift the basename fallback finds a single candidate (the lib). The bin still gets compiled at `target/debug/wt-perf` because `tests/builds.rs` triggers it. ## Testing - `cargo build -p wt-perf` — produces the binary at `target/debug/wt-perf` and `wt-perf --help` works as before. - `cargo test -p wt-perf` — both timeline tests (`renders_sorted_timeline_with_summary`, `cmd_failure_annotates_name`) pass under the lib target; the bin target no longer ships a test binary. - `cargo clippy -p wt-perf --tests` — clean. This commit is identical to f168bdef on the orphaned `fix/ci-25289365418` branch (cherry-picked here onto a current main). --- Automated fix for [failed run](https://github.com/max-sixty/worktrunk/actions/runs/25336005330) Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
db1beb61b8 |
Add wt-perf timeline subcommand for trace capture & rendering (#2558)
## Summary Adds `wt-perf timeline` to standardize how we capture and read traces for one-off `wt` invocations (alias dispatch, `wt list`, etc.). Replaces the prior `RUST_LOG=debug wt … 2>&1 | wt-perf trace > trace.json` dance with a single command that runs `wt`, captures stderr, parses `[wt-trace]` records, and either pretty-prints a sorted text timeline or emits Chrome Trace Format JSON for Perfetto. The text mode is the path I expect to use most — it's a column-aligned table sorted by start time, with subprocess totals and an externally-measured wall (`spawn → wait`) so the unobserved prelude/epilogue is visible: ``` ts(ms) dur tid kind name 0.000 11µs 1 span init_logging 0.043 3.979ms 1 span prewarm 0.058 3.882ms 1 cmd git rev-parse --git-common-dir … HEAD [wt-perf-typical-1] … 3 subprocesses totaling 11.315ms (slowest: 3.882ms git rev-parse … HEAD) traced: 14.838ms (first → last [wt-trace] record) wall: 19.367ms (spawn → wait; +4.529ms untraced prelude/epilogue) ``` ## What's in the diff - **`tests/helpers/wt-perf/src/main.rs`** — new `Timeline` subcommand. Resolves `wt` as a sibling of `current_exe()` (Windows-aware via `EXE_SUFFIX`), optionally invalidates caches with `--cold [--repo PATH]`, runs `wt` with `RUST_LOG=debug`, captures stderr, and renders. Uses `tabwriter` for column alignment, `Duration`'s std `Debug` for compact unit display, and `insta` for inline snapshot tests. - **`tests/helpers/wt-perf/Cargo.toml`** — new deps: `tabwriter` (release), `insta` (dev). - **`CLAUDE.md`** — fixes stale `--skip` syntax in the Benchmarks section (Criterion takes a positional FILTER, per #2547), replaces the non-existent `bench_list_by_worktree_count` example, adds a Traces section. - **`benches/CLAUDE.md`** — leads "Generating traces" with `wt-perf timeline`; the manual stderr-redirect pipeline is now framed as the path for traces from a log already on disk. - **`src/trace/mod.rs`** module doc — same usage block update so library doc and CLAUDE.md don't drift. - **`wt-perf setup`** printed suggestion — hints at `wt-perf timeline` instead of the manual stderr redirect. `wt-perf trace` (stdin/file → Chrome JSON) and `wt-perf cache-check` are unchanged — they still serve the case where you have a captured log and want to convert or analyze it without re-running. ## Test plan - [x] `cargo run -- hook pre-merge --yes` (3432 tests, lints, doctests) passes locally - [x] `wt-perf timeline -- -C \$REPO stub` end-to-end on a typical-1 fixture - [x] `wt-perf timeline --cold --repo \$REPO -- -C \$REPO stub` invalidates and reruns - [x] `wt-perf timeline --chrome -- -C \$REPO stub` produces valid JSON (loaded into `python -m json.tool`) - [x] `cargo test -p wt-perf --bin wt-perf` — 2 inline insta snapshots pass - [ ] CI green 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
c710147c8c |
fix(bench): unbreak wt-perf invalidate; consolidate env-isolation helpers (#2540)
## Summary Two bugs in `wt_perf::invalidate_caches_auto`, plus consolidation of the env-isolation helpers shared between tests and benches. ## Bugs fixed **(1) Silent unset under inherited `GIT_DIR`.** `invalidate_caches_auto` shelled to `git config --unset worktrunk.default-branch` via `worktrunk::shell_exec::Cmd`. `Cmd::apply_common_settings` re-injects normalized inherited `GIT_*` path vars resolved against the wt-perf process's startup cwd (issue #1914 — correct for `wt`-from-git-alias where parent and child target the same repo). For wt-perf, which operates on an explicit, possibly-unrelated repo path, the override redirected the unset to the wrong git dir; the resulting non-zero exit was swallowed by `let _ =`, and the cache stayed put. Reproducer: ```bash cd /tmp && GIT_DIR=.git target/release/wt-perf invalidate /tmp/wt-prof-test # previously: silently leaves worktrunk.default-branch in place # now: actually unsets it ``` **(2) Linked worktrees never cleared.** `invalidate_caches_auto` resolved `<repo_path>/.git` naively. When `repo_path` is a *linked* worktree, `.git` is a file (gitdir-pointer), not a directory, so every `fs::remove_*` call no-op'd silently — `.git/wt/cache/`, `.git/index`, `packed-refs`, commit-graph all survived. Existing bench callers all pass the main worktree, so no current bench was affected, but the function's contract was broken for linked-worktree inputs. ## Fixes - wt-perf no longer routes through `worktrunk::shell_exec::Cmd`. All `git` subprocess work uses `std::process::Command` directly via a small `git_command()` helper that delegates to `worktrunk::testing::configure_git_cmd`. The inherited-`GIT_*` normalization (which was the root of bug 1) doesn't apply to plain `Command`. - `invalidate_caches_auto` resolves the git common dir from the filesystem — reads `.git` as dir-or-pointer, strips `worktrees/<name>` for linked worktrees — so the cache root is correct regardless of which worktree of a repo is named. - The `let _ =` on the unset is gone; non-success/non-5 exits print a one-line `eprintln!` so a future regression can't hide the same way. ## Consolidation Touched four near-duplicate env-isolation helpers (`configure_cli_command`, `configure_git_cmd`, `configure_git_env`, `wt_perf::isolate_cmd`). The shared core is now in `worktrunk::testing`: - `pub fn isolate_subprocess_env(cmd, user_config: Option<&Path>)` — strips host context (`GIT_*`, `WORKTRUNK_*`, `NO_COLOR`, `SHELL`, `PSModulePath`) and sets baseline `WORKTRUNK_*_PATH`. Used by `configure_cli_command` (test-side, layers on test determinism) and every bench (no extra layer — benches want realism). - `pub fn scrub_git_path_vars(cmd)` — env_remove the five `GIT_*` repo-discovery vars. Called defensively from `configure_git_cmd`/`configure_git_env` so test code that spawns `git` outside a `wt`-parent isn't vulnerable to the same redirect. - `wt_perf::isolate_cmd` deleted. Five bench files now import `worktrunk::testing::isolate_subprocess_env` directly. - Path strings unified: `/nonexistent/wt/{config,approvals}.toml` and `/etc/xdg/worktrunk/config.toml` (preserved as the real XDG path). 47 snapshots regenerated. ## Test plan - [x] `cargo clippy --all-targets --workspace` clean - [x] 1072 + 607 lib unit tests pass - [x] 1589 integration tests pass (vs. baseline's 3 pre-existing failures, which the snapshot regen also resolved) - [x] All 5 benches compile (`cargo bench --no-run`) - [x] Bug 1 (GIT_DIR redirect): unset succeeds - [x] Bug 2 (linked worktree): cache and config both cleared from any worktree path - [x] Vanilla path: still works 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
4601a9301b |
ci: add nightly feature-powerset check (#2442)
Adds a nightly workflow for feature-flag breakage, fixes the wt-perf
manifest leak that was masking the bug, and merges in the `features`
branch with the actual `src/progress.rs` fix that the new check exposes.
## Why
worktrunk v0.45.0 fails to build with `cargo install --locked
--no-default-features` because `src/progress.rs` used `crossterm`
unconditionally despite `crossterm` being gated behind the `cli`
feature. The lib code was broken on main but in-workspace `cargo check
--no-default-features --lib` passed silently.
The mask: `tests/helpers/wt-perf/Cargo.toml` had `worktrunk = { path =
"../../.." }` (default features on). Cargo's workspace resolver unifies
features across all members, so even when checking worktrunk with
`--no-default-features`, wt-perf's dep silently re-enabled `cli` in the
lib build, pulling `crossterm` back in. Setting `default-features =
false` plugs the leak — and with that change the existing
`feature-check` job in `ci.yaml` already catches this bug class on every
PR, before it can ship.
## What's added
- **`tests/helpers/wt-perf/Cargo.toml`** — `default-features = false` on
the worktrunk path dep, with a comment explaining the leak.
- **`.github/workflows/nightly.yaml`** — nightly `feature-powerset` job
(cargo-hack) plus `create-issue-on-nightly-failure`. Triggers:
`schedule: '37 5 * * *'` + `workflow_dispatch` only — no push/PR. Issue
creation is gated on `failure ∧ repo_owner == 'max-sixty' ∧ event_name
== 'schedule'` and uses `JasonEtco/create-an-issue@v2` with
`update_existing: true` for auto-dedup. Auth via `WORKTRUNK_BOT_TOKEN`
for consistent bot identity (per `.github/CLAUDE.md`) and so future
issue-triage automation can cascade. Coverage is partly redundant with
what `feature-check` in `ci.yaml` now catches, but the powerset is
broader (covers `git-wt` and `shell-integration-tests` combinations that
ci.yaml doesn't enumerate).
- **`.github/nightly-failure.md`** — minimal issue template (title, `ci`
label, link to failed run).
- **Merge of `origin/features`** — the `src/progress.rs` fix that this
PR's check would otherwise have flagged. Without it CI on this PR would
(correctly) fail.
## Verified locally
- `cargo check --lib --no-default-features` — passes
- `cargo hack check --feature-powerset --no-dev-deps` — all 20 subsets
pass
- `cargo test --lib --bins` — 1056 + 603 passing, no regressions
## Trade-off
The wt-perf fix relies on a convention — workspace members depending on
worktrunk must use `default-features = false` — that nothing enforces. A
future workspace member added without that flag would silently
re-introduce the leak. An out-of-workspace check (build a throwaway
crate that depends on the checkout) would be more defense-in-depth but
the brief opted for the simpler in-workspace job. A separate dispatched
investigation is looking at whether the helpers should be workspace
members at all.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
|
700beb0271 |
chore: bump MSRV from 1.93 to 1.94 (#2423)
Weekly maintenance per the [running-tend MSRV section](https://github.com/max-sixty/worktrunk/blob/main/.claude/skills/running-tend/SKILL.md#weekly-maintenance-msrv--toolchain): track latest stable − 1. Rust 1.95.0 was released on 2026-04-14, so MSRV moves from 1.93 to 1.94. The development toolchain is already at 1.94.0 (`rust-toolchain.toml`), so no toolchain or `flake.lock` changes are needed this week. ## Changes - `Cargo.toml`: `rust-version = "1.93"` → `"1.94"` - `tests/helpers/wt-perf/Cargo.toml`: `rust-version = "1.93"` → `"1.94"` ## Verification - `cargo check --all-targets --workspace` clean locally on rustc 1.94.0. - CI's `msrv` job is the source of truth for verifying the new MSRV builds. Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
|
83d6d076bb |
bench: invalidate wt caches per iteration in remove/TTFO benches (#2341)
## Summary `wt remove` populates `.git/wt/cache/` via `compute_integration_lazy` whenever `BranchDeletionMode` isn't force-delete (the CLI's `--force` is `force_worktree`, not `--force-delete`). The `remove_e2e/first_output` and `first_output/remove` benches used `b.iter` with no invalidation, so iter 1 was cold and iter 2+ hit the cache — reported timings reflected warm-cache cost, not the first-invocation TTFO users see. ## Changes - `benches/remove.rs::first_output` and `benches/time_to_first_output.rs::remove` switch to `iter_batched` + `invalidate_caches_auto`. - `wt_perf::invalidate_caches_auto` now also unsets `worktrunk.default-branch` (git config). That cache contributes ~17ms per iteration on a typical-8 repo (166ms with default-branch cached vs 183ms fully cold) — measurable but small enough that always clearing it is simpler than introducing a second mode. - `benches/CLAUDE.md` documents the rule, the full list of what `invalidate_caches_auto` clears (including what it deliberately preserves — `worktrunk.history`, `worktrunk.hints.*`, `worktrunk.state.*`, `.git/wt/logs/`, `.git/wt/trash/`), and which commands populate `.git/wt/cache/`. Audit found no other benches affected: `completion.rs` and `cow_copy.rs` don't touch the cache; `list.rs` already wires warm/cold via `BenchConfig`; `wt switch` and `wt list` under `WORKTRUNK_FIRST_OUTPUT=1` exit before the writers run; `remove.rs::no_hooks`/`with_hooks` short-circuit at `same_commit` because `recreate_worktree` rebuilds the branch at main's HEAD. > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
d2d38dd458 |
docs(perf): require --progressive in wt-perf trace pipeline and trim duplication (#2258)
Two TTY-gated trace events (`Skeleton rendered`, `First result received`) don't fire when `wt list` detects a non-TTY stdout, which every piped `wt-perf` invocation does. Every documented capture pipeline now passes `--progressive` so all seven instant events flow through `wt-perf trace` / `cache-check`. Fixes 10 sites across module docs, clap `after_long_help`, setup eprintln, and `benches/CLAUDE.md`. While in the area, I also consolidated some of the scattered examples: - Dropped the `# CLI Usage` block from `tests/helpers/wt-perf/src/lib.rs` — library docs shouldn't carry CLI examples, and they fully duplicate `wt-perf --help`. - Trimmed the `main.rs` module docstring from four example invocations to a one-liner pointing at `wt-perf --help`. - Dropped the "with benchmark repo" variant from `cache-check`'s `after_long_help`, since it's structurally identical to the first example with `-C <path>` prepended. - Replaced the stdin-hint and no-entries error in `wt-perf` (which repeated the full pipeline) with a pointer to `<subcommand> --help`. - Pruned `src/trace/chrome.rs`'s standalone usage example to defer to `crate::trace`. Verified end-to-end: `RUST_LOG=debug wt list --progressive 2>&1 | wt-perf trace` emits all seven expected instant events in the Chrome Trace JSON. > _This was written by Claude Code on behalf of Maximilian Roos_ --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
c3484b3b63 |
docs(perf): drop redundant grep wt-trace from wt-perf pipelines (#2257)
The trace parser already filters for the `[wt-trace]` marker internally
(`parse_lines` → `parse_line` uses `line.find("[wt-trace] ")?`), so the
upstream `grep` was redundant. Dropping it makes the documented
pipelines shorter and removes an inconsistency between callers that used
`grep wt-trace` vs `grep '\[wt-trace\]'`.
Changes span 12 sites across 6 files: module docs, `wt-perf`'s own CLI
help (trace/cache-check subcommands, setup eprintln, stdin hint,
empty-input error), `benches/CLAUDE.md`, and the `running-tend` skill.
Also fixed a stale reference in `src/trace/chrome.rs` to the old
`analyze-trace` binary name (now `cargo run -p wt-perf -- trace`).
Verified end-to-end: `RUST_LOG=debug wt list 2>&1 | wt-perf trace`
produces valid Chrome Trace JSON, and `| wt-perf cache-check` produces
the expected structured report.
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
|
e74abbd6a9 |
style(perf): rename total_extra_calls to extra_calls in cache-check JSON (#2254)
Disambiguates from `same_context_extra_calls` — the top-level `extra_calls` counts all extra calls regardless of context, while `same_context_extra_calls` is the subset within same context. Follow-up to #2253. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
37679ee9f6 |
refactor(perf): simplify wt-perf output, add wasted-time to cache-check (#2253)
cache-check previously emitted both structured JSON (stdout) and a human-readable report (stderr) with repetitive patterns like "max 2x/context, 1 extra" repeated dozens of times. JSON to stdout is the right default for a dev tool — pipe to `jq` for filtering. Changes: - **cache-check**: Remove redundant stderr report. Add time tracking (`extra_us`, `total_time_us`, `same_context_extra_us`) to the JSON and sort duplicates by wasted time instead of max count — surfaces the most impactful cache misses first. Delete unused `truncate()` helper. - **setup**: Condense verbose multi-line output (per-worktree listing, emoji, repeated labels) into a single summary line. - **invalidate**: Remove emoji from confirmation message. - **trace**: No changes — already clean JSON-only to stdout. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
2c602bbfd6 | chore: bump MSRV from 1.89 to 1.93 (#2125) | ||
|
|
d50bdfcde0 |
Cache remaining expensive tasks, unify wt list and picker (#2098)
Adds persistent SHA-keyed caching for `is_ancestor`, `has_added_changes`, and `branch_diff_stats` — the last three operations still running unconditionally in both `wt list` and the `wt switch` picker. With all five merge-base-dependent tasks now cached (joining `merge-tree-conflicts` and `merge-add-probe` from PR #2085), the `stale_branches` skip mechanism becomes redundant: the picker's "skip tasks on branches > 50 behind main" optimization was papering over slow first-run cost that the cache now handles eternally. ## What changed **New caches** (`src/git/repository/probe_cache.rs`): three new asymmetric SHA-keyed kinds — `is-ancestor`, `has-added-changes`, `diff-stats` — all using the same LRU-swept persistent file layout as the existing kinds. `branch_diff_stats` skips its cache when sparse checkout is active (path filters make the result environment-dependent). **Dead plumbing removed**: `EXPENSIVE_TASKS` constant, `CollectOptions::stale_branches` field, `skip_expensive_for_stale` parameter on `collect()`, the env-var gate in `list/mod.rs`, and two obsolete tests + snapshots. Both `wt list` and the picker now run the same task set. **`batch_ahead_behind` unconditional**: previously only the picker called it. Making it unconditional replaces N `rev-list --count` calls with one `for-each-ref`, a pure win for `wt list` too. **Branch-ref semantics unified across all tasks**: all task implementations now prefer the branch name over `ctx.branch_ref.commit_sha` when one is present. Previously, `IsAncestorTask`, `BranchDiffTask`, `MergeTreeConflictsTask`, and `CommittedTreesMatchTask` used the worktree's current HEAD sha, which during a rebase-in-progress is transiently at the replayed commit rather than the branch tip. This produced contradictory rows: `is_ancestor=true` + `1 ahead / 1 behind`. Now all tasks consistently report the branch's state, matching `git status` semantics. Addressed worktrunk-bot review feedback. **Caching docs updated**: the `## Caching` section in `collect/mod.rs` (from PR #2097) now lists the three tasks as cached rather than "cacheable but uncached". Also addressed worktrunk-bot PR overlap observation. **Benchmarks**: `invalidate_caches_auto()` now clears `.git/wt/cache/` so cold-cache benchmarks exercise cold state. The `warm_optimized` variant in `real_repo_many_branches` collapsed since it no longer measures anything different from `warm`. ## Testing New probe_cache unit tests cover roundtrip + tamper-based cache consultation for each new kind, plus a `clear_all_covers_all_kinds` regression test. Snapshot updates for `test_list_maximum_status_with_git_operation` and `test_list_json_with_git_operation` reflect the branch-ref fix — mid-rebase worktrees now show `✗` (real conflicts) and actual diff stats instead of the transient HEAD's empty state. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
|
c8db7795ed |
feat: structured JSON output for wt-perf cache-check (#1955)
`wt-perf cache-check` now outputs structured JSON to stdout (composable with `jq`) and keeps the human-readable summary on stderr. ```bash # Pipe to jq for specific queries RUST_LOG=debug wt list 2>&1 | grep wt-trace | wt-perf cache-check 2>/dev/null | jq '.same_context_duplicates[:3]' # Human report still visible by default (stderr) RUST_LOG=debug wt list 2>&1 | grep wt-trace | wt-perf cache-check > /dev/null ``` > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
5143b087c5 |
Simplify wt-perf truncate to plain ASCII slicing (#1953)
The `truncate` function in wt-perf processes git command strings, which are always ASCII. The UTF-8 char boundary walk (originally `floor_char_boundary`, then a manual loop) was unnecessary — plain byte slicing is safe and direct. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
4e14483b92 |
perf: cache resolve_preferring_branch to eliminate redundant ref verification (#1948)
Each integration check method (`is_ancestor`, `same_commit`,
`trees_match`, etc.) calls `resolve_preferring_branch` on both
arguments, running `git rev-parse --verify -q refs/heads/{ref}` each
time. With many worktrees, the same refs (especially "main") get
verified hundreds of times per `wt list` — 203 calls measured. The
result never changes during a command.
Adds a `DashMap<String, String>` to `RepoCache` (following the existing
pattern for `merge_base`, `ahead_behind`, `effective_remote_urls`).
First lookup runs git; subsequent lookups are cache hits. Drops verify
calls from 203 to 52 (one per distinct ref).
Also simplifies `wt-perf cache-check` to report all duplicated commands
generically instead of hardcoding knowledge of library internals, uses
multiline strings for output, and removes `cache_effectiveness` tests
(OnceCell regressions are compile-time errors; benchmarks catch perf
regressions better).
> _This was written by Claude Code on behalf of Maximilian Roos_
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
|
1ad18073db |
refactor: consolidate duplicated TestRepo across unit tests (#1944)
Four identical `TestRepo` structs existed across `src/config/` (in `test.rs`, `user/tests.rs`, `expansion.rs`, and `mod.rs`), and `configure_test_identity()` was duplicated between `src/git/recover.rs` and `src/commands/picker/summary.rs`. This extracts a shared `src/testutil.rs` module (gated with `#[cfg(test)]`) that the library-side tests import. The new `TestRepo::new()` uses `git init -b main` for determinism (the old copies used bare `git init` which depends on system config). A standalone `set_test_identity()` function handles the cases where tests manage their own temp directories and repo paths. Binary-side tests (`src/commands/`) can't access `#[cfg(test)]` library modules, so they keep their local helpers — this is a Rust crate boundary limitation, not oversight. Also adds cross-referencing doc comments to `wt_perf::isolate_cmd()` and `tests/common::configure_cli_command()` explaining why they're intentionally separate (benchmarks need minimal isolation for realistic timing; tests need full determinism for snapshots). > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com> |