126 Commits

Author	SHA1	Message	Date
Maximilian Roos	7aba380f0c	fix(remove): gate removal on the registration, not the repository (#3808) ... `wt remove --force` deleted a live worktree of this same repository, uncommitted work included, whenever that worktree had been moved onto another worktree's registered path. The guard added in #3785 asks which *repository* the occupant answers to: a linked worktree's git dir sits under `<common>/worktrees/`, the main worktree's *is* the common dir, anything else is someone else's. A sibling worktree moved onto the path satisfies that — its git dir sits under `<common>/worktrees/` like any worktree of this repo — so it passed, and the fast path renamed the directory into trash and handed the `rm -rf` to a detached process. It is not prunable either: its gitdir file points at a location that exists, so the `is_prunable` arm from the same PR doesn't catch it. Git's own validation is one level finer. `validate_worktree` requires the directory to point back at *this registration*, and refuses this removal with `--force`: ```console $ git -C repo worktree remove --force ../repo.feature fatal: validation failed, cannot remove working tree: '.../repo.feature' does not point back to '.git/worktrees/repo.feature' ``` <details> <summary>Reproducer, verified against a build of main</summary> The occupant has to be *moved* onto the path rather than created there — `git worktree add` refuses a registered path, which is what leaves a plain `mv` as the way this state arises. ```console $ git -C repo worktree add ../repo.feature -b feature $ git -C repo worktree add ../repo.bar -b bar $ rm -rf ../repo.feature && mv ../repo.bar ../repo.feature $ echo PRECIOUS > ../repo.feature/precious.txt $ wt remove --force --yes feature ◎ Removing feature worktree (--force) & branch in background (same commit as main, _) $ ls ../repo.feature ls: ../repo.feature: No such file or directory ``` </details> ## The fix The gate is now git's comparison at git's granularity: the directory's `.git` must name *this* registration, and that registration's `gitdir` file must name the directory back. Repository-level ownership stays as the weaker half of the conjunction — it is what rejects a `.git` file pointing at another repository — and the main worktree is the same test where there is no registration to point back at. `ensure_belongs_to_repo` becomes `ensure_holds_this_worktree`, since it no longer merely asks about repository membership. Resolution moves to `Repository::git_dir_at`, the fs-only resolver the `wt list` prewarm already used (`derive_worktree_git_dir`), generalized to answer for a directory rather than for a known worktree of this repo: its main-worktree branch returned `git_common_dir()` on trust, and now canonicalizes the `.git` it actually found. It also never walks up to a parent, which is what git reads too — `git rev-parse --git-dir` in an emptied worktree can resolve the *enclosing* repository. That settles a second thing the old docstring got wrong. It claimed the plan→rename window was "narrower than `ensure_clean`'s"; in fact `ensure_clean` re-runs `git status` while this gate answered from `GIT_DIRS`, memoized process-wide, so the second call was vacuous and the window — which contains the approval prompt and the `pre-remove` hook — was unguarded. `git_dir_at` reads the filesystem on every call, so the check at the rename now re-decides. The refusal was `Directory @ … is not this repository's worktree`, which is false in the sibling case: it *is* one of this repository's worktrees, just not the one registered there. Its hint didn't fit either — "move the directory aside, then run `git worktree prune`" is a repo-wide prune, and with a sibling moved aside *both* registrations are prunable, so following it clears both and leaves a live checkout that has stopped being a worktree: ```console $ mv ../repo.feature ../repo.aside && git worktree prune -v Removing worktrees/repo.feature: gitdir file points to non-existent location Removing worktrees/repo.other: gitdir file points to non-existent location $ git -C ../repo.aside status fatal: not a git repository: (null) ``` So the error carries where the occupant's own registration records it, and each case gets the remedy that fits. Moving it back to that path leaves prune with only the stale entry to clear: ```console $ wt remove --force --yes feature ✗ Directory @ ../repo.feature does not hold the worktree registered there ↳ Removing it could destroy the worktree registered @ ../repo.other; move the directory back there, then run git worktree prune ``` That path is read through `canonicalize_with_parents`, because a relative `gitdir` entry resolves against `<common>/worktrees/<id>` and would otherwise reach the hint with the `..` chain still in it — and plain canonicalization can't normalize a directory that no longer exists, which is the case the arm is reached for. Normalizing there also makes `crate::path::paths_match`, the crate's canonicalizing comparison over that same helper, the right test for the gate, so there is no second comparison beside it. The gate's fail-closed behavior now rests on that helper resolving `..` through the filesystem rather than collapsing it lexically — across a symlink the two readings name different directories — so `src/path.rs` records the constraint where a lexical rewrite would otherwise read as a tidy-up. The FAQ's "What can Worktrunk delete?" paragraph carried the same "a *different* repository" framing and is corrected. ## Scope Pre-existing, and 0.73.0 already narrowed it — 0.72.0 had no ownership check at all and deleted foreign clones too. The guard has two call sites (`prepare_worktree_removal` at planning, `stage_worktree_removal` at the rename), so this reaches `wt merge --remove`, `wt step prune`, and picker removal, not only `wt remove`. One incidental tightening: `wt remove <bare-repo-path>` previously passed the guard (a bare root's git dir *is* the common dir) and was stopped only by the dirty check, which `--force` skips. It now refuses at the guard. <details> <summary>One residual, left alone</summary> `git worktree repair <path>` after the `mv` produces a *double registration*: both `worktrees/repo.bar/gitdir` and `worktrees/repo.feature/gitdir` come to record the same path, and `git worktree list` reports two worktrees there. In that state the new gate accepts the removal — the occupant does point at the `feature` registration, and that registration does point back — while git refuses, because its path→worktree lookup happens to match the `bar` entry first. Closing it means knowing the registration id at the gate, or scanning every `worktrees/*/gitdir` for duplicate claims. Unchanged by this PR, and reachable only via `mv` followed by `repair`. </details> ## Testing Five new tests, each confirmed to fail with the line it covers reverted and to leave the others passing. Three drive the binary: - **the sibling case** — follows #3785's data-safety model: asserts the filesystem afterwards, not just the exit code, since removal stages by rename and deletes in a detached process. Snapshots the refusal, so the hint and the path it names are pinned. Fails with the pointer-back conjunct removed, while the foreign-repo test still passes without it — the two cover different halves. - **the re-check at the rename** — a `pre-remove` hook repoints the worktree's `.git` at a sibling's registration after planning has already cleared it, which is what makes the second gate's freshness observable. Fails when resolution routes back through the `GIT_DIRS`-cached `git_dir()`. - **a relative `gitdir` entry** — removal succeeds, and git reads the rewritten entry back, which is what makes it the form git itself writes. Rewriting the entry rather than setting `worktree.useRelativePaths` keeps the test independent of the git version that introduced the option. Two sit at the gate, where the CLI can't reach: - **both worktree shapes are accepted** — including the main worktree, whose git dir *is* the common dir. `wt remove` rejects the main worktree well upstream of this gate and a bare repository's worktrees are all linked, so nothing through the CLI would notice that arm inverting. - **the refusal names a normalized path** — asserted against `Diagnostic::render`, since the path is in the hint and `Display` carries only the title. Local gate green: 4607 tests, lints, doctests, rustdoc under `-Dwarnings`. > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-13 03:59:58 -07:00
Maximilian Roos	0cb3cc1ef7	test(remove): cover the reap guard's spared branch deterministically (#3805) ... The controlling-terminal guard's sparing branch had no CI coverage. `test_remove_reap_kills_process` branches on the suite's own terminal — none in CI, so CI only ever exercised the kill side, and the branch that protects an interactive process from `--reap` ran only on dev boxes. `test_remove_reap_spares_terminal_process` gives the child its own PTY instead of reading the suite's, so the spared branch runs everywhere. `portable_pty` makes the child a session leader with that PTY as its controlling terminal, which is exactly the property the guard reads. A spared process and an undiscovered one produce identical output ("No processes to reap"), so the test proves discovery sees the child first, then pins the guard's verdict in-process, then asserts the child survives removal. The reap phase prints before `handle_remove_output` runs the removal, so both tests now wait for the worktree to actually disappear — without it, a `wt` that gave up after the reap phase would still satisfy every output assertion. Both presence polls also move onto `wait_for`, the existing presence-poll helper, instead of hand-rolled deadline loops. `tests/CLAUDE.md` picks up two entries this work surfaced. The first is the slow-timeout triage signature: a kill at the 180s bound is a duration symptom with two causes, and the durations around it tell CPU starvation (a sibling worktree's build) from a blocked call inside the test. It records `threads-required` as starvation's only lever and why it stays unset. The second distinguishes a bounded poll on one identified `ErrorKind` from a retry, so `pin_test_binary` and `forward_with_etxtbsy_retry` don't read as violations of the No Retries doctrine — the opening sentence is narrowed to the re-run it actually bans. > _This was written by Claude Code on behalf of max-sixty_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-08-12 10:19:33 -07:00
Maximilian Roos	715af4cd72	fix(tests): pin the spawned wt binary against concurrent cargo uplifts (#3784) ... Two test-suite flakes fixed at the root, both dependencies on machine load and concurrent builds. **Concurrent-cargo spawn `NotFound`.** Cargo uplifts `target/debug/wt` by removing the path and recreating it, so a second `cargo` against the same target directory leaves the binary every test spawns absent for a fraction of a millisecond per rebuild — the one-off `NotFound` spawn failures that pass on re-run. `wt_bin()` now returns a hardlink pinned under `target/debug/wt-test-bin/<mtime>-<len>/`: the uplift unlinks only the uplifted name, so the pin keeps serving the observed binary through any number of concurrent rebuilds, at no disk cost beyond the `deps/` artifact whose inode it shares. `test_wt_spawns_are_pinned` keeps every spawn routed through it. Reproduced by re-creating the uplift every 200 ms alongside a full `cargo nextest run`: 302 of 4583 tests failed before this change (147 as direct `NotFound` spawn panics, five of them byte-identical to the original shell-wrapper report), 4585 of 4585 after — with an unrelated external cargo also rebuilding `wt` mid-validation, absorbed the same way. **`--reap` probe races.** `test_remove_reap_kills_process` predicted the reap guard's verdict with its own `lsof`/`ps` snapshot, and under load either probe's spawn can stall past the 5 s bound, whose fail-safe empty result flips the outcome — a prediction `wt` then contradicts, or `wt` reporting "No processes to reap" for a live child. The prediction now reads the session's controlling terminal directly (`/dev/tty` opens iff the session has one — the property the child inherits at spawn), the probe timeout is env-pinnable (`WORKTRUNK_TEST_PROBE_TIMEOUT_MS`, set to 60 s in the static test baseline; production keeps its 5 s bound), and the discovery poll uses the suite's 60 s presence-poll convention. Looped 15/15 green at load average ~60, where the previous shape failed 2/10. Not covered here, noted as follow-ups: benches still spawn `env!("CARGO_BIN_EXE_wt")` directly (same hazard, separate runner, outside the guard's scan), and the reap "spared" branch has no deterministic end-to-end test (needs a PTY-held child). > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-09 07:06:08 -07:00
Maximilian Roos	cf822f75cf	fix(worktree): guard a registered path that no longer holds its worktree (#3785) ... A registered worktree's path was trusted to still hold that worktree. Three defects followed, one of them destroying data, and the check that would have caught them — where it existed at all — was `Path::exists()`. ## `wt remove --force` deleted an unrelated repository A clone that came to sit at a stale registration's path was removed whole, including uncommitted work and, for a repo never pushed, the only copy of its objects. wt's own hint routed the user there: the dirty gate reads `git status` in that directory, reports the occupant's changes as this worktree's, and offers `--force` as the cure. ```console $ wt remove feature ✗ Cannot remove worktree: feature has uncommitted changes ?? precious.txt # ← the other repo's file ↳ ... to lose uncommitted changes, run wt remove --force feature ``` git refuses that same removal, `--force` included (`validation failed … is not a .git file`). Worktrunk's fast path renames the directory into trash rather than asking git to, so git's validation never ran. `ensure_belongs_to_repo` makes it, comparing the directory's git dir against this repository's: a linked worktree's sits under `<common>/worktrees/`, the main worktree's *is* the common dir, anything else answers to someone else. One comparison covers both worktree kinds and also rejects a `.git` file pointing at another repo, which git's shape test accepts. It runs at planning, ahead of the dirty gate, and again at the rename for callers that stage without planning. Now: ```console $ wt remove --force feature ✗ Directory @ ../repo.feature is not this repository's worktree ↳ Removing it could destroy unrelated data; move the directory aside, then run git worktree prune ``` ## A recreated worktree directory leaked git's exit 128 `wt switch`, `wt merge`, and `wt step push` walked into `git rev-parse --git-dir failed (exit 128)`. Two of them probed `Path::exists()` first, which a deleted-and-recreated directory passes; the third asked nothing. `worktree_is_unusable` is the union of both tests, because neither implies the other. `exists()` catches the absent directory; git's `prunable` catches the recreated one. `prunable` alone is *not* the wider test it looks like — git withholds the attribute from a **locked** worktree even when its directory is gone, since prunability is its pruning policy and a lock means "don't prune this": ```console $ git worktree list --porcelain # wt2 locked, all three directories removed worktree /tmp/ptest/wt1 prunable gitdir file points to non-existent location worktree /tmp/ptest/wt2 locked removable media # ← no prunable line worktree /tmp/ptest/wt3 prunable gitdir file points to non-existent location ``` A locked worktree on an unmounted volume is exactly what `prepare_worktree_removal`'s lock guard exists for, so a `prunable`-only test would read it as healthy. All three commands now give the message the merely-deleted case already gave. `wt remove` keeps `exists()`, deliberately: there it is the precondition for the cleanup path rather than a health test, since `prune_worktree_entry` unregisters via `git worktree remove`, which skips validation only while the directory is absent. No scoped git command clears the recreated case, so it reports and names the repo-wide `git worktree prune` that does. ## `wt switch docs/` missed a branch sitting right there Git's ref format forbids a trailing `/`, so the branch lookup never had a candidate — and shell completion produces exactly that spelling whenever a `docs` directory sits beside the branch. Selectors are normalized before resolution. ## Resolving selectors through one ladder The three fixes landed in three of the four places that assemble "expand shortcuts, try the branch, try the path, classify the failure" by hand. Each gated its path attempt on "did something rewrite this token?", answered by comparing an expansion's output against its input: | where | the comparison | |---|---| | `resolve_worktree` | `branch == name` | | `plan_switch` | `target.branch == branch` | | `target_worktree_at_path` | `target.filter(\|t\| *t == resolved)` | | `resolve_base_ref` | `resolved == base` | That is a fact the rewriting step knows, re-derived downstream from its output, and it is wrong in both directions. A shortcut can expand to the token it was given — `-` pointing at the branch you are already on — and string equality reads that as a literal, turning the path arm back on for a token nobody typed. Normalization breaks it the other way, which is why the trailing-separator fix needed threading through three call sites. `Selector` carries the fact instead: `expand_shortcut` reports whether it fired, `wt switch` reports its `pr:`/`mr:` dispatch and remote-prefix strip, and `names_a_path()` replaces all four comparisons. `resolve_selector` is the ladder, and `plan_switch` expands into it rather than re-implementing its phases. `names_a_path()` gates both path steps together — the worktree-by-path lookup and the directory verdict — which is what `wt switch --create` needs: the argument names a branch to create, so `branch_only()` takes the arm off at the producer rather than each consumer re-testing `create`. It also reaches the directory verdict, so `ResolvedWorktree` gains `NoWorktreeAtPath` and the four sites that called `path_selector_error` themselves stop re-deriving it. The docstring defending that laziness didn't survive checking — the function returns on `is_valid_branch_name` before touching the filesystem, so every ordinary branch name already short-circuited. | | before | after | |---|---|---| | `normalize_selector` call sites | 3 | 1 | | `path_selector_*` call sites | 4 | 2 | | "was it rewritten?" comparisons | 4 | 0 | ## Navigating the diff - `src/git/repository/mod.rs` — `Selector`, `normalize_selector`, the new `ResolvedWorktree` variant. - `src/git/repository/worktrees.rs` — `expand_shortcut`, `expand_selector`, `resolve_selector`, `usable_worktree_for_branch`. - `src/git/repository/working_tree.rs` — `ensure_belongs_to_repo`, the ownership check. - `src/git/remove.rs`, `src/commands/repository_ext.rs` — where it gates removal, and why before the dirty gate. - Call sites: `commands/worktree/switch.rs`, `commands/worktree/push.rs`, `commands/merge.rs`, `commands/remove.rs`, `git/repository/config.rs`. ## Size Comments and docstrings are the largest share: the ownership check and the four conditions behind the directory verdict all look like things to simplify away, so the reason each exists is recorded where it's enforced. | | + | − | |---|---:|---:| | Production code | 277 | 142 | | Comments & docstrings | 320 | 75 | | Tests | 325 | 8 | | Snapshots | 186 | 0 | | Docs | 6 | 0 | | **Total** | **1114** | **225** | ## Testing Seven new tests. The data-safety one drives the real binary and asserts the filesystem afterwards, not just the exit code — removal stages by rename and deletes in a detached process, so a passing exit would not have caught a staged-then-deleted tree. The others cover the recreated directory (switch and remove), the trailing separator, `--create` against a worktree registered at that path, and, at the unit boundary, the four states of `worktree_is_unusable` — healthy, absent, locked-and-absent, recreated — and the selector's path-ness, including the degenerate case string equality got wrong. Each new test was confirmed to fail with its fix reverted. One more covers an omitted merge target in a repo whose default branch can't be determined. `^` had a test for that error; the omitted-target route to the same message had none. The gap predates this branch — the closure is byte-identical to the one it replaces and codecov records those lines as missed at the base commit too — but relocating them into `resolve_target_selector` re-counted them as patch lines, which is what surfaced it. Local gate green: 4593 tests, lints, doctests, rustdoc under `-Dwarnings`. <details> <summary>Behavioral matrix, verified against a build</summary> ```console docs/ (trailing sep) ▲ Worktree for docs @ ../repo.docs detached by path ▲ Worktree for detached worktree @ ../repo.det leftover dir ✗ No worktree @ ../repo.leftover recreated dir ✗ Worktree directory missing for rec shortcut ^ ▲ Worktree for main @ ../repo remove leftover ✗ No worktree @ ../repo.leftover --base docs/ ✓ Created branch nf from docs foreign-repo remove ✗ Directory @ ../repo.frn is not this repository's worktree precious.txt survives ``` </details> <details> <summary>Also swept, and one thing left alone</summary> Three more instances of the same shape, fixed here: - `resolve_base_ref` was the fourth copy of the comparison, so `--base docs/` now resolves too. - `hint_for_repo` suggested `wt switch ^` after an existence probe a recreated directory passes, pointing at a worktree the switch then refuses. - The pre-switch hook's `target` var used the bare shortcut expander, so a hook saw `docs/` where the switch resolved `docs`. The identical unborn/stale default-branch block in `require_target_branch` and `require_target_ref` is extracted. The rest of that pair differs in its existence predicate, extra arms, and final error; sharing it would cost more in parameters than the duplication does. Left alone: `live_sibling_checkout` decides whether another worktree still holds a branch during removal, and also uses `exists()`. Switching it to `prunable` would make branch deletion *more* likely in a corner case where the detached path already answers the other way. That is a data-safety surface and a separate decision. </details> > _This was written by Claude Code on behalf of max-sixty_	2026-08-09 06:23:21 -07:00
Maximilian Roos	f3ea2fd598	fix(errors): report a directory holding no worktree as a directory (#3773) ... Passing a path that holds no worktree used to be reported as a missing branch: ``` $ wt remove /repo/.claude/worktrees/ghost ✗ No branch named /repo/.claude/worktrees/ghost ↳ To list branches, run wt list --branches --remotes ``` The argument is plainly a path; the error calls it a branch and sends the user to a listing it could never appear in. Now: ``` $ wt remove /repo/.claude/worktrees/ghost ✗ No worktree @ /repo/.claude/worktrees/ghost ↳ The directory exists but is not a worktree; to list worktrees, run wt list ``` #3767 fixed the hook that produced the leftover directory in #3753; this fixes the message a person gets when they type such a path themselves, which judewang named there as why it was hard to diagnose. Ref #3753. Two neighbouring commands had the same cause and are also fixed. `wt switch <path>` and `wt merge <path>` used to offer `wt switch --create <path>`, which fails with `fatal: … is not a valid branch name`; and `wt config state marker set --branch <path> foo` **succeeded silently**, storing state keyed by a path string. ## How the decision is made `Repository::path_selector_error` reports a directory only when all four conditions hold. Each is here because dropping it made `wt` assert something false, reproduced against a build: | Condition | Without it | |---|---| | Git could never accept the selector as a branch name | `wt step rebase HEAD~9` reports revision syntax as a path | | A directory is there | `wt remove docs` stops meaning the branch when `docs/` sits beside it | | No worktree of this repo is registered at it | a *detached* worktree — reachable by path alone — is reported as absent, contradicting the `wt list` in the hint | | It holds no git data | a sibling repo's live checkout, or a bare repository, is called a leftover only the user can delete | That last one is the data-safety case: the new variant's hint says only the user can delete the directory, so a false positive reads as an invitation to `rm -rf` a checkout with uncommitted work. Absence of git data is *established* rather than assumed — `symlink_metadata` with only `NotFound` counting, so an unreadable clone or a dangling `.git` symlink withholds the claim instead of earning it. The first condition is git's own ref-format rules, implemented in-process and pinned against `git check-ref-format` by a test. ## Navigating the diff - `src/git/repository/worktrees.rs` — `Repository::path_selector_error` and `holds_git_data`, with the rationale for each condition recorded where it's enforced. - `src/git/repository/branch.rs` — `is_valid_branch_name`, the ref-format predicate. - `src/git/error.rs` — the new `GitError::WorktreeNotFoundAtPath` variant. - `src/git/repository/config.rs` — `target_branch_at_path` → `target_worktree_at_path`, returning the worktree whole rather than collapsing "no worktree here" with "a detached worktree here". - Call sites: `commands/remove.rs`, `commands/worktree/switch.rs`, `commands/config/state.rs`. Reporting a detached *target* made `DetachedHead`'s fixed hint wrong: `git switch` acts on the tree it runs in, so `wt merge ../B` was telling the user to switch the tree they were standing in. The variant now carries the detached worktree when that isn't the current one, and the hint becomes `git -C ../B switch <branch>` — the same reason `OperationInProgress` carries `branch`. Set at the three sites where the detached worktree isn't the current one (`require_target_branch`, `require_selected_branch`, and `wt step promote`, which raises this about the main worktree); every other site passes `None` and renders as before. 140 of the 257 added production lines are comments and docstrings — three of the four conditions look like obvious things to simplify away, so the reason each exists is written down next to it. ## Testing Unit tests at the resolver boundary cover each condition and its complement, both `directory_exists` arms, the shortcut-expansion route (`wt merge -`), detached targets, revision syntax, unresolvable `.git` entries, and bare repositories. Two integration tests drive the real binary end-to-end against `../repo.leftover` — `wt remove` and `wt switch` — covering the relative-path spelling and each command's wiring, and the `switch` snapshot pins the thing that would regress silently: that no `--create` hint is offered for a name git would reject. The ref-format predicate is verified beyond its committed table: 2,791 generated names were run against real `git check-ref-format` with zero disagreements, and two review passes did the same over their own inputs. That sweep isn't committed — it needs Python and a live `git` — so the committed table is what guards against drift. Not exercised locally: the Windows-specific behaviours are reasoned, not run — `Path::new("foo.").is_dir()` succeeding against `foo`, and drive-relative `C:foo`. CI exercises the platform but not those inputs. <details> <summary>Known gaps this would ship with</summary> - `wt switch docs/` (trailing slash, branch `docs` has a worktree, `./docs` is a source directory) answers `No worktree @ docs/`. True, unhelpful. The real fix is stripping trailing separators before resolution — a resolution change, not a message change. On Windows it reads `No worktree @ …/docs`, because `path_slash`'s `to_slash_lossy` rebuilds the string from `Path::components()` there and drops the trailing separator — still true of the path, minus the character that explains it. The doc comment on `path_selector_error` now says so rather than promising a spelling guarantee that holds on one platform. - A worktree whose registration was pruned out from under it gets a safe-but-vague message. The useful answer is a distinct `git worktree repair`-or-delete diagnosis; that's new detection, and nobody has reported hitting it. - Under `-C`, a `./`-prefixed selector renders a visible `/./` (`wt -C repo remove ./ghost` → `repo/./ghost`). Cosmetic; tidying it would discard the trailing separator the `docs/` case needs. - `require_target_branch` and `require_target_ref` remain near-copies of one ladder, and this adds a line to each. Pre-existing; extracting it is a separate refactor. - `CommandEnv::require_branch` passes `worktree: None` unconditionally, and `CommandEnv::for_selector` builds an env whose `worktree_path` is the worktree the *argument* named — so a future caller combining the two would get the unqualified hint. Nothing reaches that combination today: `merge` and `squash` are the only `require_branch` callers and both build with `for_action`, and the one `for_selector` caller (`wt step commit --branch`) never calls it. Two pre-existing bugs surfaced while reviewing this, unrelated to the diff and reproducible by branch name: `wt remove` will trash a clone that now sits at a registered worktree's path, and a recreated worktree directory leaks a raw `git rev-parse --git-dir failed (exit 128)`. </details> > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-08 13:06:02 -07:00
Maximilian Roos	860fa02ecf	fix(output): don't panic when a --format=json consumer closes the pipe (#3746) ... `print_json` — the pretty-JSON printer behind `--format=json` — lived in `src/commands/list/mod.rs`, under the one command that happened to need it first, while `wt list statusline` and `wt config approvals list` reached across for it. It now lives in `src/output/json.rs`, re-exported as `crate::output::print_json`. Moving it surfaced thirty other call sites that had each open-coded the same two lines, and that those two lines were not equivalent everywhere. Four printed through anstream's `println!` (via `worktrunk::styling`), the other twenty-six through std's. anstream drops a `BrokenPipe` write error; std panics. So on main today: ```console $ wt config state get --format=json | head -3 { "ci_status": [ { thread 'main' panicked at library/std/src/io/stdio.rs:1165:9: failed printing to stdout: Broken pipe (os error 32) ``` while `wt config show --format=json | head -3` exits cleanly — same command family, different behavior, decided by which macro happened to be imported in that file. `EPIPE` depends on whether a reader is still attached, not on how much is written, so this is a race rather than a size threshold: an output that fits the pipe buffer usually lands before the consumer goes away, which is why it stayed hidden rather than why it is safe. A three-byte write panics just as readily once the reader has closed. All thirty-six call sites now go through `print_json`, and `print_json` prints through anstream, so no `--format=json` surface panics on a closed pipe. The command above now exits 0 with an empty stderr; also checked by hand on `wt list`, `wt step prune --dry-run`, `wt config show`, `wt step eval`, `wt config approvals list`, and `wt list statusline`. `wt switch --format=json` is the one surface that isn't a `print_json` caller: it emits its single result as one compact line, which is what a shell loop reads, so converting it would change a machine-readable format for no gain. It gets the same anstream printer instead, which is the part that matters here. `wt list statusline --format=json` had a third instance hiding in its schema-1 empty path, a bare `println!("[]")` on std's macro — on the surface that runs on every prompt redraw. It now serializes an empty array through `print_json`, which emits the same two bytes; `test_statusline_json_outside_worktree` already asserted them. A module-level `println` import would have been wrong there, since the text statusline's `println!` deliberately bypasses anstream so its pre-rendered ANSI survives a non-tty stdout. Pruned eight now-dead entries from `STDOUT_ALLOWED_PATHS` in `tests/integration_tests/output_system_guard.rs`. Centralizing these writes left seven allowlisted files with no direct `println!` at all, so their exemptions covered no code and the guard quietly stopped protecting exactly the files this branch touched; a stray `println!` in `merge.rs` or `remove.rs` fails the test again. (`alias.rs`'s entry was already dead.) Serialization is otherwise byte-identical: serde escapes control characters, so raw ANSI never reaches this output and anstream's stripping is a no-op on it. `src/commands/CLAUDE.md` gains one line under "Adding a CLI Command" stating the rule — stdout through anstream, `print_json` as the printer, switch's compact line as the one shape difference — so the next JSON surface doesn't open-code it again. Two `print_json` calls the diff rewrote were uncovered on the base commit, which pulled their misses into `codecov/patch` without any behavior changing. Rather than hand over a waiver, both now have tests: `wt remove --format=json` with no branch argument (the single-worktree path, distinct from the named removals already tested) and `wt step for-each --format=json` interrupted mid-loop, which still owes its consumer the results collected before the signal. Each was mutation-checked — deleting the line under test fails it — because a one-item array is what both the abort and completion paths produce, so a weaker assertion would pass either way. > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-05 23:03:14 -07:00
Maximilian Roos	5c42c5b7d5	feat: machine-readable approval state and branch-removal outcomes (#3710) ... Two of the five machine-readable-output requests NathanaelRea opened (#3696–#3700), reviewed as a set and implemented where the gap was real. ## `wt config approvals list --format=json` (#3698) The command already computed the four distinctions an orchestrator needs — no commands, approved, approval-required, and stale — read-only, without prompting or writing. It had no `--format` flag, so the only way to learn that a non-interactive run would stop for approval was to run the operation and catch `NotInteractive`, or to pass `--yes` and approve whatever was there. ```json { "state": "approval_required", "commands": [ {"phase": "post-start", "name": "dev", "template": "npm run dev", "approved": false}, {"phase": "pre-merge", "template": "cargo test", "approved": true} ], "stale": ["some removed command"] } ``` `state` is what a caller branches on. `stale` stays a separate list rather than a fourth `state`, because it co-occurs with all three — and those are the approvals `--yes` would silently re-approve after their command template changed, which is exactly what an orchestrator preserving the approval model needs to see. A flag on the existing read command rather than a new `status` verb, matching `wt config show`, `wt config state get`, `wt config state logs`, and `wt list`. Closes #3698. ## `branch_outcome` on removal (#3700, partly) `wt remove --format=json` reported the branch as one boolean, collapsing five internal outcomes into two values: | Internal outcome | `branch_deleted` was | |---|---| | `Deleted` | `true` | | `Deferred` — handed to a detached process, result never observed | `true` | | `NotAttempted` — no branch, or `--no-delete-branch` | `false` | | `Retained` — a sibling worktree has it checked out | `false` | | `Retained` — **the CAS refused; the ref moved under us** | `false` | The last row is the exact race #3700 asks for protection against. Worktrunk already deletes with `git update-ref -d <ref> <oid>` and already fails closed when the ref has moved — then reported it as the same `false` that means "you asked me not to". And `Deferred` reported `true` on intent. `branch_outcome` names it instead: `deleted`, `deferred`, `not_attempted`, `retained_unmerged`, `retained_checked_out`, `retained_raced`, `retained_failed`. A caller that sees `retained_raced` knows to re-read the ref and retry, which is what the guard detects it for. **This does not close #3700.** That issue asks for an *input* — a caller-supplied expected OID that makes `wt` fail closed against the orchestrator's own observation. This is an *output*. They land in the same place on the default path, because the integration check already refuses to delete unintegrated content, so the caller was never going to lose commits — they just couldn't classify the refusal. Where the gap is real is `--force-delete` / `-D`, which takes the early return in `delete_branch_if_safe` and runs `git branch -D` with no integration check and no CAS. If an `--expected-oid` flag lands, it has to gate that path. ## Notes - **Output-format break.** `branch_deleted` is replaced, not supplemented, on `wt remove --format=json` and on `wt step prune --format=json`'s live path. Per CLAUDE.md, output formatting is on the flexible side of the interface line; flagging it here so the release changelog picks it up. - **`wt step prune --dry-run` keeps `branch_deleted`.** A dry run predicts; it runs nothing to have an outcome. Different thing, different name, documented as such. - **`retained_raced` and `retained_checked_out` have no deterministic CLI trigger.** Both come from windows between `wt`'s own fresh read and the ref mutation, which no hook can be scheduled inside. They're covered at the unit level (`branch_fate_from_result_mapping`, `branch_fate_json_outcome_is_distinct_per_fate`, and `cas_rejects_delete_when_branch_advances` in `src/git/remove.rs`, which drives the race with a stale snapshot). The integration tests cover the two reachable contrasts: `retained_unmerged` via a `pre-remove` hook that commits, and `not_attempted` via `--no-delete-branch`. - **`print_json` lives under `src/commands/list/`** and now has a third caller from outside that module. Worth a more central home; not moved here. ## The other three Reviewed but not implemented: - **#3696** — already possible. `wt --config-set 'list.json-schema = 2' list --format=json` pins the schema per invocation above every config layer, as does `WORKTRUNK_LIST__JSON_SCHEMA`. Answered on the issue; what's left is a docs gap and making an out-of-range value fail rather than degrade in JSON mode. - **#3697** — the machine-readable error channel. A real gap and the one policy call in the set; not started. - **#3699** — aimed at `wt config state logs --format=json`, which is a directory listing reconstructed from paths, under a model that overwrites. The append-only run record it wants is `commands.jsonl`. ## Testing `cargo run -- hook pre-merge --yes` green: 4533 tests, clippy, fmt, doctests, rustdoc, docs sync. > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 11:17:27 -07:00
Maximilian Roos	bbd24fb8f3	refactor(remove): make one function own the pre-removal gate (#3694) ... Follow-up to #3690, which recorded the decision that removal's final dirty-worktree gate is answered by the fsmonitor daemon, exactly as `git worktree remove`'s own gate is. The decision went into the `src/git/remove.rs` module spec — but the default removal path never reads that module. Both paths ran the same three ordered steps before the worktree directory stopped existing: dirty-worktree gate, fsmonitor stop, rename into trash. They were two copies with near-identical comments. `execute_instant_removal_or_fallback` (`src/output/handlers.rs`, the default background path for `wt remove` and `wt merge --remove`) assembled them inline and never called `remove_worktree_with_cleanup`, so a reader working on the gate that issue #3646 was actually about would not find the reasoning written for it. `stage_worktree_removal` now owns all three steps and both paths call it. The gate becomes one decision instead of a sequence each caller re-assembles, and the ordering rationale — why the gate precedes the daemon stop, why the stop precedes the rename — lives on the function that implements it. The rename-and-prune step it previously named becomes the private `rename_into_trash`. No behavior change. ## Stale spec claims this surfaced - `stop_fsmonitor_daemon` documented three callers ("the library `remove_worktree_with_cleanup`, the foreground handler, and the background `spawn_background_removal`"). The foreground handler goes through `remove_worktree_with_cleanup`, so it was two; after this change it is one. - The module opened by calling itself "the canonical removal flow used by `wt remove`, `wt merge --remove`, and the TUI picker". `wt merge --remove` takes the background path, and so does `wt remove` unless `--foreground` is passed. ## Test Nothing pinned the final gate on the **default** path — the existing test covers `--foreground` only. Planning validates cleanliness before `pre-remove` runs, so a hook that dirties the worktree can only be caught by the gate immediately before the mutation. The test is now parameterized over both execution modes. Verified by mutation rather than by assuming the assertion binds: with the gate disabled, the background case reports `Removed feature-hook-dirties worktree & branch` and destroys the hook's file, which is the data-loss scenario #3646 described. Ref #3646 > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-01 20:45:30 -07:00
Maximilian Roos	1eded27943	Simplify forge, shell integration, and removal internals (#3662) ... This consolidates cross-cutting models that had accumulated parallel representations, while preserving the CLI and config interfaces. ## What changed - Forge identity now flows through one `ForgeKind`, with boundary-safe network-host classification shared by CI, remote references, and structured repository metadata. Branded SSH aliases such as `github-personal` remain outside provider dispatch and receive an actionable `forge.platform` diagnostic when forge data is requested. - Worktree removal now uses one owned target type and rechecks uncached worktree topology immediately before compare-and-swap branch deletion, retaining branches that gained a live or locked checkout. - Shell integration no longer implements the retired single-file directive writer. Stale wrappers receive repair guidance, execution fails closed, and child processes cannot inherit the retired sourceable-file capability. - Zsh and Git-version probes are shared, while dead mocks, redundant dependency edges, serializer detours, pass-through types, and duplicate tests are removed. The removal guard deliberately distinguishes live, stale-prunable, and locked registrations. The detached cleanup path performs the same record-aware check before deleting a branch ref. ## Testing `cargo run -- hook pre-merge --yes` passed after merging current `origin/main`: 4,489 tests, Clippy, formatting, lockfile checks, docs, doctests, and snapshot review. > _This was written by Claude Code on behalf of max_.	2026-07-30 02:10:07 -07:00
Maximilian Roos	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_.	2026-07-30 00:01:32 -07:00
Maximilian Roos	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>	2026-07-29 21:20:26 -07:00
Maximilian Roos	e9f69b2038	fix(remove): prune the named worktree's metadata, not the repository's (#3650) ... Every stale-metadata cleanup in the removal chain ran a bare `git worktree prune`. That command takes no path filter: it walks *every* entry in `.git/worktrees/` and unregisters each one whose directory it cannot find at that instant. A worktree that is merely absent right now — an unmounted volume, a dropped network mount, a half-finished `mv` — is indistinguishable from a deleted one, so removing worktree A also unregistered bystander B. B's committed work survives (refs and objects are shared) and its files stay on disk, but its admin dir does not: the index, `ORIG_HEAD`, the per-worktree reflog, `refs/worktree/*` and `refs/bisect/*`, and any in-progress rebase or merge go with it. Verified on git 2.55, in a scratch repo: ```console $ git worktree prune -v # no --expire; no grace period Removing worktrees/wtB: gitdir file points to non-existent location $ git -C wtB status # after the "volume" comes back fatal: not a git repository: (null) $ git worktree repair wtB error: unable to locate repository; .git file does not reference a repository ``` `git worktree repair` relinks an admin dir; it cannot recreate one. ## The change `Repository::prune_worktrees()` is replaced by `prune_worktree_entry(path)`, which runs `git worktree remove <path>`. Git skips its clean check when the directory is already gone, so that deletes exactly one `.git/worktrees/<id>`. All three call sites already had the target in hand: - `stage_worktree_removal` — the `wt remove` fast path, after the rename into the trash - `prepare_worktree_removal`'s missing-directory fallback - `wt step prune`'s stale-detached candidate No repo-wide prune remains anywhere in the codebase; a removal's blast radius now equals its intent. ## Locked worktrees A repo-wide prune silently skips a locked entry, whereas `git worktree remove` fails on one. Only the missing-directory fallback can reach a locked worktree — it resolves one before the lock guard below it — so that site skips the call explicitly, preserving the previous outcome. `pruned_from` stays unconditional there. It is what the sibling-checkout guard in Phase 4 tests, not a record of whether the metadata went; making it conditional would have quietly disabled a data-safety check. ## Tests Two integration tests cover both `wt remove` routes: remove one worktree while a sibling's directory is displaced, restore the sibling, assert it is still registered. Both fail against the old behavior with the bystander missing from `git worktree list` — confirmed by temporarily reverting the primitive to `git worktree prune` and re-running. `wt step prune`'s site is covered by the existing `test_prune_stale_detached_worktree`. `test_prune_skips_prunable_candidate_whose_preparation_fails` forced a preparation failure by shimming `git` to fail `worktree prune`; its own guard assertion (`"the shim never fired"`) caught that the shim had stopped firing. The shim now targets `worktree remove`. The test's subject — a failing preparation skips the candidate rather than failing the run — is unchanged. Local gate green: 4634 tests, lints, doctests. `RUSTDOCFLAGS="-Dwarnings" cargo doc` also checked separately, since a broken intra-doc link passes pre-commit but fails CI. ## Not done here `wt step prune` keeps stale-detached candidates on the write side of the scan lock. The stated rationale — that they race sibling prunes — no longer holds now that each names its own entry, so a `TODO(prune)` marks the move to the read-side fan-out as a separate concurrency change. --- ## Review response The `tend` reviewer verified the change and then held the merge for a human on deletion-surface policy rather than on any defect. It raised one inline comment, which does not hold up: > This inner `if wt.locked.is_none()` is unreachable-when-false: the early `return Err(WorktreeLocked)` at line 215 means `wt.locked` is always `None` by the time control reaches here. Line 215 is a comment added by this PR, not a `return`. The lock check is at line 232, inside the `else` arm, which runs only when the `if` above it is false. A locked worktree whose directory is missing takes the `if` branch and reaches the guard with `locked` still set, so the guard is live. The comment that misled it was genuinely ambiguous, and is reworded. ## Patch coverage The first push measured 69.23% of diff hit. Seven of the eight missed lines were avoidable: `prune_worktree_entry` had copied `remove_worktree`'s seven-line `GitError::Other` block to produce the `&str` that `run_command` takes, and that error closure never runs. Every caller's path comes from `list_worktrees`, which already parses git's porcelain as UTF-8, so it collapses to one line: ```rust let path_str = path.to_str().context("worktree path is not valid UTF-8")?; ``` One miss remains: the `tracing::debug!` inside `stage_worktree_removal`'s fail-soft arm, which fires only when the metadata prune fails after a successful rename. That line predates this change — only its message was reworded, which codecov counts as new. Restoring the old wording would make the message inaccurate (it now prunes one entry, not the repository's), so the wording stands and the line stays uncovered. ## Reconciled with #3647 `main` landed [#3647](https://github.com/max-sixty/worktrunk/pull/3647) while this PR sat in CI, fixing the same lock-safety issue from the other side: a top-of-arm `if wt.locked.is_some() { return Err(WorktreeLocked) }` placed *before* the missing-directory fallback, where this branch had kept the guard in the `else` arm and skipped the prune for a locked entry. The two auto-merge with **no textual conflict**, which is the hazard. The merged arm carried main's top guard *and* this branch's `if wt.locked.is_none()` skip, while dropping the `else`-arm guard the skip was paired with — leaving the inner check dead and its comment describing a route the merge had foreclosed, under a green CI. `main`'s placement is the better one and now owns lock handling alone, so the fallback calls `prune_worktree_entry(&wt.path)` unconditionally. What this PR contributes is unchanged and orthogonal to #3647: prune the named worktree's metadata, not the repository's. Caught by the reviewer's second pass, which read the merge tree rather than the branch. Gate re-run green on the reconciled tree: 4641 tests. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 22:41:51 -07:00
Worktrunk Bot	cc8aef3079	fix(remove): honor worktree lock when directory is missing (#3647) ... ## Problem In `prepare_worktree_removal` (`src/commands/repository_ext.rs`), the missing-directory fallback was tested before the lock guard. A worktree that was locked **and** whose directory was currently absent skipped `WorktreeLocked` entirely: its registration was pruned and it fell through to branch deletion — precisely the case `git worktree lock` exists for (removable media, a network mount, a dropped VPN). Reproduced against the current code: `wt remove <locked-worktree>` with the directory removed reported `✓ Pruned stale worktree & removed branch …`, silently bypassing the lock. ## Solution Hoist the lock check above the missing-directory check in the `Current`/`Path` arm, so the lock is honored regardless of whether the directory is present. `--force` still does not override the lock, matching `git worktree remove`. The duplicate lock check in the `else` branch is removed (it's now unreachable there). Blast radius is only the direct `wt remove` path: `wt step prune` already skips locked worktrees during candidate collection, and the `Branch` arm bails with "gained a worktree" before any deletion when a worktree entry (locked or not) exists. The FAQ's guarantee — *"Neither `git worktree remove` nor `wt remove` (even with `--force`) will delete [locked worktrees]"* — is left unchanged: it now describes the actual behavior. Fixing the code to match the documented guarantee is the stronger outcome than weakening the doc to describe a bypass. ## Testing Added `test_remove_locked_worktree_directory_missing` to `tests/integration_tests/remove.rs`: it locks a worktree, removes its directory externally, then asserts `wt remove` fails, the branch survives, and the stale registration is not pruned. The test fails on the pre-fix code (branch deleted, registration pruned) and passes after. The existing locked-worktree and pruned-missing-directory tests remain green, and `cargo clippy --bin wt` is clean. --- Closes #3645 — automated triage Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>	2026-07-28 20:36:18 -07:00
Maximilian Roos	c90c4b1b77	refactor(remove): report a removal's branch fate, not its plan's intent (#3637) ... Follow-up to #3633, which patched the most visible symptom (prune's summary counting candidate kinds). This lands the structural fix behind that class: the removal chain returned nothing from execution, so everything downstream reported the plan's intent as if it were the outcome. The type even said so — `RemoveResult` was produced by `prepare_worktree_removal` before anything ran, and the codebase couldn't agree on what it was (`RemovePlans` in remove.rs, `plan` in prune, `result` in the handler). ## The plan/outcome split `RemoveResult` → `RemovalPlan` (variant `RemovedWorktree` → `Worktree`), and `handle_remove_output` now returns a `BranchFate`: `Deleted` / `Retained` / `NotAttempted` observed synchronously, `Deferred` only for the detached legacy fallback whose outcome this process never sees. Consumers report the fate: - `wt remove --format=json` said `"branch_deleted": true` while stderr said `▲ Removed worktree but kept branch feature (not integrated)` — a pre-remove hook had advanced the branch, the SafeDelete re-check declined, and stdout reported the plan anyway. Now `false`. - `wt step prune`'s summary counts executed outcomes, so a deletion declined mid-run reads `✓ Pruned 1 worktree`, matching the per-item line above it. A declined *orphan* deletion (no worktree entry, nothing removed at all) now drops out of the removed list entirely instead of being counted as either lie. - The detached fallback emits the "kept branch" correction in the one case it can know (no CAS tail to append), instead of letting the progress message's "worktree & branch" stand uncorrected. The picker's `foreground: true, silent: true` contradiction collapses into one `RemovalExecution::{Foreground, Background(fallback), Silent}` axis, and the four hand-rolled capture-refs-then-CAS-delete sites share one `worktrunk::git::execute_branch_deletion` — the single spelling of "the plan said delete; do it now, against fresh refs". ## For the reviewer `src/output/handlers.rs` carries the bulk: each execution path constructs its fate where it observes the result, and rendering decisions (foreground propagates deletion errors, background warns, silent does neither) are unchanged. The merge with #3631's concurrent prune rework is the diff's riskiest region — the worker result channel widened from `Result<bool>` to `Result<Option<bool>>` so fates flow back through the done-channel and the collector stamps `candidate.deletes_branch` before the summary and JSON read it. Deliberately *not* done: reusing the planner from `wt merge`'s finish path (its divergences — clean-check strictness, preserve-vs-error on primary, merge-target — are intentional), and threading the fallback mode structurally (it just relocates an unreachable arm). Zero snapshot churn: the change is provably invisible wherever intent matched outcome. The divergence cases are pinned end-to-end by tests whose hook must be the branch tip to pass. The one deterministically unreachable arm is the detached-fallback warning (needs a cross-filesystem rename failure); expect `codecov/patch` to flag those few lines. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-28 15:49:33 -07:00
Worktrunk Bot	4af04c2260	fix(remove, prune, merge): target the named worktree, retain shared branches (#3533) ... Follow-up to the discussion on #3480: [@max-sixty asked](https://github.com/max-sixty/worktrunk/pull/3480#issuecomment-5039137116) to have `wt remove` honor an explicit path generally. #3607 has since landed the resolution half, `Repository::resolve_worktree`, which turns a path into a worktree. This is the removal half, and it is the part resolution cannot decide: which worktree to act on is a naming question, whether the branch may be deleted is not. ## Problem `wt remove` threw the resolved answer away. For any non-current worktree it re-targeted by branch name, and `prepare_worktree_removal` mapped that branch back to git's *first-listed* worktree. Two failures followed, both silent. **The wrong worktree is removed.** With `feature` checked out twice: ```console $ git worktree list …/dup [feature] …/first [feature] $ wt remove …/first ◎ Removing feature worktree & branch in background (same commit as main, _) ``` `…/dup` is gone. `…/first`, the one named, is still there. **The survivor is left broken.** The shared branch is deleted with it. Worktrunk deletes branches with `git update-ref -d`, git's compare-and-swap primitive, which unlike `git branch -d` does not refuse a ref that is checked out somewhere: ```console $ git worktree list …/first 0000000 [feature] $ git -C …/first rev-parse HEAD fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree. ``` `wt step prune` reached the same deletion unattended, and `wt merge` reached it with a freshly integrated branch, so nothing else declined. A branch gets a second worktree only through `git worktree add --force`; worktrunk never does it itself. ## Fix **Remove the worktree that was named.** `wt remove` drops non-current worktrees via `RemoveTarget::Path`. `wt step prune` does the same: its candidates already carry a path, and targeting a *stale* entry by branch name resolved to a live worktree that the same prune had just skipped as too young, then removed it. **Retain a branch another worktree holds.** One predicate, `live_sibling_checkout`, answers "would deleting this ref orphan a checkout?", and every path that can delete a branch asks it: `prepare_worktree_removal`'s worktree and pruned-branch-only arms (covering `wt remove`, `wt step prune`, and the picker, which already targeted by path) and `wt merge`'s finish. A hit forces `BranchDeletionMode::Keep`, the single chokepoint every deletion path honors, and names the surviving checkout: ```console $ wt remove …/dup ◎ Removing feature worktree in background ○ Branch feature retained; still checked out @ …/first ``` A sibling whose *directory* is already gone is stale metadata, not a checkout with anything to lose, so it does not retain: removing the last live checkout still deletes the branch. **`-D` is refused out loud.** Everywhere else `-D` is the override that wins, so one that cannot be honored warns rather than passing quietly: ```console $ wt remove …/dup -D ◎ Removing feature worktree in background ▲ Branch feature retained despite -D; still checked out @ …/first ``` The ordinary single-checkout case is unchanged, and a retained branch skips the integration check entirely rather than computing a verdict it would discard. #3480's duplicate-checkout hint now points at `wt remove <path>`, which this makes the safe answer. ## Testing Full gate green. New coverage, each case asserting the survivor still resolves `HEAD`, which is the corruption in question: - `remove`: by path, by name, refused `-D`, the pruned-directory fallback, and the mirror case where a stale sibling must *not* retain. - `step prune`: a stale entry whose branch is live in an age-skipped worktree. This test is what surfaced the wrong-worktree bug in prune. - `merge`: merging a branch that a `--force` duplicate also holds. ## Not addressed `wt step prune`'s summary counts candidates rather than outcomes, so a retained branch still reports `✓ Pruned 1 branch`. The per-item line above it already says the branch was retained. Fixing the count means threading removal outcomes back through prune's accounting, which is a separate change. > _This was written by Claude Code on behalf of Maximilian Roos_ --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Maximilian Roos <m@maxroos.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 09:57:34 -07:00
Maximilian Roos	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>	2026-07-26 21:44:57 -07:00
Maximilian Roos	14580de79c	feat(worktree): accept a worktree path wherever a branch is accepted (#3607) ... Follow-up to the [`wt remove <path>` discussion on #3480](https://github.com/max-sixty/worktrunk/pull/3480#issuecomment-5039137116), widened from that one command to the whole surface. #3480 has since landed and is merged in here — its duplicate-checkout warning composes with this: the warning names the shadowed worktrees, and a path is how you then address one. ## Audit Verified against the built binary. wt had three answers to "what does this token mean?": | Route | `@` `-` `^` | worktree path | `pr:N` | |---|---|---|---| | `wt switch` (`resolve_switch_target`) | yes | only if absolute or ≥2 components, and not `--create` | yes | | `wt remove` (`resolve_worktree_arg`) | yes | any token | no | | everything else (raw `worktree_for_branch`) | **no** | **no** | no | Same token, same cwd, two answers: ```console $ wt remove inner # ✓ Removed innerbranch worktree & branch $ wt switch inner # ✗ No branch named inner ``` And outside switch/remove the shortcuts didn't work at all — `wt step diff --branch @` was `✗ Branch @ has no worktree`, while `wt config state marker set --branch @` silently wrote state under the literal key `@`. Separately, wt prints paths as `~/…` but wouldn't accept that form back. ## Change One canonicalizer in the lib, `Repository::resolve_worktree`, absorbing the path fallback that lived in the bin crate's `resolve_worktree_arg` (now deleted). Resolution order is documented once, on that function: `@`, then `-`/`^`, then a branch with a worktree, then a path naming a registered worktree, then the branch alone. **Branch-first, everywhere.** A directory never shadows a branch that shares its name; a path answers only what a branch cannot — a detached worktree, or one of two checkouts of the same branch (#3480's case). The `looks_like_path` shape gate is gone, so a single-component path resolves like any other. Two shapes cover what callers need: `require_worktree` for commands that need a worktree to operate in, `require_selected_branch` for arguments that key by branch. The merge/rebase target validators fall through to the same path lookup, so a target can be named by the worktree it's checked out in. Routed through it: `switch` (including `--base`), `remove`, `step commit --branch`, `step diff --branch` and its target, `step copy-ignored --from`/`--to`, `step promote`, `step relocate`, `config state --branch` (9 sites), and `merge` / `step rebase` / `step squash` / `step push` targets. `resolve_input_path` — already documented as the one resolution point for user-supplied paths — now expands a leading `~`, so the tilde form worktrunk prints is a form it reads back. `~user` stays literal; wt doesn't reimplement that shell feature. ## Documentation A path is an alias, not a second addressing scheme, so it is stated once rather than on every argument: one paragraph in `wt switch`'s help and one sentence on the addressing line in `worktrunk.md`. Argument descriptions still read as branches. The two exceptions are the arguments whose descriptions are already catalogues of accepted forms — `wt switch`'s (`Branch, worktree path, shortcut, or PR/MR URL`) and `wt remove`'s, which has named the path since before this branch. The Worktree Model section of `CLAUDE.md` records which way to document it, so the next argument doesn't grow its own copy. ## Two silent no-ops fixed along the way - `wt step relocate <unmatched>` matched arguments against branch names by string equality, so a typo filtered everything out and the empty result rendered as `○ All worktrees are at expected paths` — a success message for work that never happened. Every way an argument can fail to land on a relocatable worktree now errors, including the detached and prunable cases the new path route makes reachable. - A selector matching nothing was reported as a branch without a worktree, hinting `wt switch <token>` — which creates a worktree only when the branch exists, so for a mistyped path it would just fail again. `WorktreeSelectorNotFound` now says `No branch or worktree named X`; a branch that genuinely exists without a checkout keeps the create hint. ## Testing Full gate green: 4596 tests, lints, docs sync, `--features shell-integration-tests` clippy. `codecov/patch` is 99.25% of diff hit against a 97.93% target. New coverage: - Unit: branch-and-path equivalence, branch-beats-same-named-directory, detached-by-path (and its `require_selected_branch` refusal), shortcuts never treated as paths, branch-only fallthrough, and the two distinct not-found errors. Plus `expand_tilde` round-tripping `format_path_for_display`. - Integration: `switch` by relative/single-component/absolute/tilde path, `--base` by path, `step diff --branch` by path and `@` (asserted equal to the by-branch output), `config state --branch` set via `@` and read via the worktree path, and both new relocate errors. `wt remove`'s resolution is unchanged — it already had this rule; it now shares the implementation. The 106-test `remove::` suite is untouched and green. - Integration: `wt step push <worktree-path>` (the `require_target_branch` half of the target fallback), and `wt step relocate` against a prunable worktree. One diff line is unhit: `expand_tilde`'s fallback when `home_dir()` returns `None`, which has no deterministic trigger. The `@`-resolution backstop in `resolve_worktree` is untested for the same reason — no CLI route reaches it — so it kept its original `match` arm rather than being re-indented into the diff. ## Left out `wt config state default-branch set` and `previous-branch set` take a branch name as a *value to store* rather than a selector, so they still take it literally. > _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 5 (1M context) <noreply@anthropic.com>	2026-07-26 09:10:46 -07:00
Maximilian Roos	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>	2026-07-24 17:27:36 -07:00
Maximilian Roos	edcfe5fe0c	fix: honour -C in the statusline and in every supplied path (#3567) ... `-C` sets the repository discovery path but never chdir's the process, so anything reading `std::env::current_dir()` silently ignores it. Two places did. ## `wt list statusline` described the wrong worktree `wt -C /path/to/other/worktree list statusline` printed the statusline for the *cwd's* worktree, and from a directory outside any repo it printed nothing at all. Both formats now resolve their worktree through `Repository::current_worktree()`, which anchors to the discovery path. That accessor already exists for this bug class (its docstring cites #2625), so this is routing the statusline onto the canonical path rather than adding a new one. Details worth knowing while reading the diff: - `--format=json` was mostly right already. It picked its worktree via `current_worktree()` and only its "are we in a worktree" gate read the cwd, so it misbehaved just when the process cwd sat outside a repo (printed `[]`). That gate moves to `current_worktree()` and the `cwd` binding is gone. - `git_status_segments` now takes a `WorkingTree` instead of `(&Repository, &Path)`. That also settles a split inside it: the worktree lookup used the cwd while the no-worktree fallback used `repo.current_worktree()`. - `--format=claude-code` is unchanged where it matters. The directory Claude Code sends on stdin still wins, since that names the session's directory rather than wt's. With no stdin it falls back to the same worktree as the other formats, so it honours `-C` too. Four integration tests. Three run from the main worktree with `-C` pointed at a `feature` worktree, one per format; the existing statusline tests all set the child's cwd, which is the route that already worked. The fourth pins the claude-code split: when stdin names a directory in a *different* repository, the worktree comes from stdin while `list_worktrees` comes from wt's discovery path, so nothing matches and only the branch is reported. That fallback previously had no test, which is what `codecov/patch` caught on the first push. Discovering the `Repository` from the stdin directory instead would remove that split, but it would miss the `prewarm` cache (keyed on the base path) that this prompt-redraw hot path depends on, so the split stays and is now asserted rather than incidental. ## Supplied paths resolved from the shell, not from `-C` `wt switch ../repo.feature` and `wt remove ../repo.feature` joined the argument onto the process cwd. With `-C` in play that split the invocation in two — wt chose the repository, the shell chose the path — and the combination resolved to nothing, so `switch` reported "No branch named ../repo.feature". `--config`, `WORKTRUNK_CONFIG_PATH`, `WORKTRUNK_SYSTEM_CONFIG_PATH`, and the trace file of `wt config state logs profile` name a file the same way and had the same gap: `wt -C repo --config mycfg.toml config show` reported "Config file not found" for a file sitting in `repo/`. Git resolves the paths in its command line against its own `-C`, because it chdir's there before doing anything else. wt's `-C` sets a discovery path instead, so `resolve_input_path` joins onto that — one resolution point for all five, so they cannot drift apart. It returns the path untouched when `-C` was not given, rather than joining `.` onto it: the process cwd already resolves those, and the join would surface as a stray `./` in output. `Path::join` leaves an absolute path alone, which drops an `is_absolute` branch and, in `resolve.rs`, a fallible `current_dir()` call. Three neighbours are deliberately outside the rule, and the docstring says why: - Paths wt derives itself — a worktree root from `git worktree list`, an XDG config directory — arrive absolute. - `WORKTRUNK_PROJECT_CONFIG_PATH` resolves from the worktree root, its documented base, because the project config is per-worktree. - `WORKTRUNK_BIN` and `XDG_CONFIG_DIRS` are never opened by wt: the first is expanded later by the generated shell wrapper, and the XDG spec already requires the second to be absolute. Four integration tests, one per surface, each running from *outside* the repo with `-C` pointed into it — the neighbouring tests run from a cwd inside the repo, which is the route that works under either rule. All four fail against the old code: `switch` with "No branch named ../repo.feature-detached", `remove` by leaving the worktree standing, and the two config paths by not finding their file. The `set_base_path` docstring picks up the rule, so the next reader has one place to find it: `-C` is the base for the two things git takes from its chdir — the repository discovered, and every path the user names — while the process cwd stays the answer to where the user's shell physically is. ## Noticed, not touched `run_json` and `git_status_segments` duplicate the worktree-lookup-and-populate sequence (`list_worktrees` → find by root → `is_home` → `build_worktree_item` → `populate_item` with identical `CollectOptions`). Worth collapsing separately. > _This was written by Claude Code on behalf of max_	2026-07-24 15:57:23 -07:00
Maximilian Roos	e02453aa5f	feat(list): demote branch-worktree mismatch from red flag to dim info (#3419) ... Implements option 3 ("demote the presentation") from the design proposal in #3415, for #3389: worktrees created by agent harnesses at their own conventional paths (`.claude/worktrees/…`) made every `wt list` row carry a red `⚑` at merge-conflict severity. The list predicate, JSON fields, and Path-column behavior are unchanged; only the presentation changes: - `⚑` renders dim yellow (was red) in `wt list`, the picker, and the `--help` legend: below the full-yellow actionable states (prunable, locked), above plain dim. - Priority within the status slot becomes `✘ > ⤴ > ⤵ > ⊟ > ⊞ > ⚑ > /`: the yellow actionable states (prunable, locked) now outrank the informational glyph instead of being masked by it. The JSON schema-1 `state` field follows the same priority; schema-2 booleans are independent and unaffected. - The inline notice on `wt switch` / `wt remove` / `wt merge` / `wt step prune` is gone entirely: the `wt list` glyph is the state's one surface. The `expected_path` plumbing is deleted end to end, so switching to an existing worktree no longer expands the `worktree-path` template at all (it only ran to feed the notice). Before/after on a repo with agent worktrees: ``` + claude/frosty-kilby-92c7d3 ⚑_ (red ⚑, reads as an error) + claude/frosty-kilby-92c7d3 ⚑_ (dim-yellow ⚑, reads as a note) ``` Option 1 from the design doc (narrowing the predicate so only genuine collisions flag at all) can land separately on top of this. Well-tested: the pre-merge gate passes (4390 tests), snapshots reviewed line-by-line (glyph color and priority changes in list/help output; the notice lines removed from switch/remove output), the removed-notice tests renamed to pin the silence, and clippy is clean including `--features shell-integration-tests`. Verified live: `wt list` shows the dim-yellow flag; `wt switch`/`wt remove` print nothing about the mismatch. Thanks to @dmsmidt for the report in #3389. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-12 15:27:17 -07:00
Maximilian Roos	57b303fda2	fix(nix): provide lsof to the flake's test sandbox (#3410) ... The nix-flake job has failed on main since #3396: `test_remove_reap_kills_process` discovers its child via `lsof -d cwd`, but the flake's `worktrunk-tests` check didn't provide `lsof`, so `processes_under` fails to spawn it, returns an empty list forever, and the test's 5s discovery poll panics with `child <pid> never discovered`. Failing main run: https://github.com/max-sixty/worktrunk/actions/runs/29066326142. Three changes: - `flake.nix`: add `pkgs.lsof` to the test check's inputs, the same pattern as `git`/`python3`/`procps`. `ps` (procps) already works in the sandbox, so /proc-based process inspection is available there and the test should be genuinely exercised rather than excluded. - `.github/workflows/nightly.yaml`: add `flake.nix`, `flake.lock`, and `nix/**` to the trigger paths (push and PR gate). A flake change couldn't previously trigger the nix-flake job that validates it; this PR relies on the fixed trigger for its own validation. - `tests/integration_tests/remove.rs`: point the discovery-timeout panic message at lsof, so the next environment missing it doesn't need re-diagnosing. Validation: nix isn't available locally, so the proof is this PR's own nightly run going green on the nix-flake job. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-09 23:29:40 -07:00
Maximilian Roos	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>	2026-07-09 21:35:28 -07:00
Worktrunk Bot	f593438e7c	feat(remove): add experimental --reap to kill worktree processes (#3396) ... Implements the design agreed in #3365: an opt-in, experimental flag on `wt remove` that reaps processes left running in the worktree. ## The name `--reap` — it's the term the whole thread and the issue title already use, it's concise (matching worktrunk's flag style), and the user-facing messages read naturally (`◎ Reaping 2 processes under feature worktree`). ## What it does ```console $ wt remove --reap feature ◎ Reaping 2 processes under feature worktree ┃ 51234 node ┃ 51240 esbuild ✓ Reaped 2 processes ◎ Removing feature worktree & branch in background (same commit as main, _) ``` Processes are discovered by working directory (`lsof -d cwd`): any process whose cwd is at or under the worktree path. Termination reuses the existing `SIGTERM`→wait→`SIGKILL` escalation (`escalate_terminate`, shared with the fsmonitor sweep). ## Data-safety posture Killing a process the user didn't mean to kill — a terminal editor with unsaved buffers — is exactly the silent loss-of-work the project refuses without consent, so two guards keep `--reap` conservative: - **Controlling-terminal exclusion.** A process holding a controlling terminal (an interactive shell, or `vim`/`nvim`/`emacs -nw`) is never reaped (`ps -o tty=`). Only detached processes — the dev servers and watchers this issue is about — remain candidates. This also spares the shell `wt remove` was run from. - **Self-exclusion.** The current `wt` process is never a candidate. The flag itself is the explicit opt-in; the list is printed before signalling for transparency. ## Scope / limitations (matching the #3365 discussion) - **Under-inclusive by design.** cwd discovery misses a daemon that forked and `chdir`'d away, or one that reparented to `init` — they no longer report a cwd under the path. Those are what [`wt step tether`](https://worktrunk.dev/step/#wt-step-tether) is built to reap (whole process group). `--reap` and `tether` cover different gaps and are complementary, not substitutes — the docs say so. - **Ordering.** Reaping runs before the worktree directory is staged/renamed (cwd matching needs the directory in place), so it's independent of foreground/background removal, trash-vs-delete, and `--force`. - **Unix only.** Windows has no cheap per-process cwd; `--reap` is rejected there with a clear error. ## Tests - Pure parsers for `lsof`/`ps` output (`parse_lsof_cwd`, `parse_ps_tty`). - End-to-end against the real `lsof`/`ps`: spawns a child with a cwd under a tempdir, asserts `processes_under` discovers it, asserts the controlling-terminal guard keeps-or-drops it in agreement with the child's *actual* TTY state (so the test is host-independent — CI has no TTY, a dev box does), then reaps it and confirms `SIGTERM`. - CLI snapshot (`test_remove_reap_no_processes`) covering the no-candidates path, deterministic whether or not `lsof` is installed on the runner. Help text, `docs/content/remove.md`, and the skill reference mirror are regenerated and in sync. One thing worth a maintainer's eye: I chose to print-then-signal rather than add an interactive confirm/`--dry-run` in this first cut — the opt-in flag + TTY exclusion + printed list felt like enough for an experimental flag, and a confirm step is easy to layer on if you'd prefer it. Closes #3365. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>	2026-07-09 15:13:46 -07:00
Maximilian Roos	9be99ee21b	test: consolidate absence-window sleeps behind a shared SLEEP_FOR_ABSENCE_CHECK (#3206) ... Absence assertions (verifying something did NOT happen) need a fixed window rather than polling, since there's no event to wait for. That window was expressed three inconsistent ways across the test suite: a per-file `const SLEEP_FOR_ABSENCE_CHECK` duplicated in `user_hooks.rs` and `post_start_commands.rs`, and bare `Duration::from_millis(500)` literals scattered through `switch.rs`, `step_alias.rs`, `remove.rs`, `bare_repository.rs`, and `merge.rs` — none greppable as a group, and `merge.rs` sitting below the documented 500ms floor at 200ms. This promotes `SLEEP_FOR_ABSENCE_CHECK` to `src/testing/mod.rs`, next to the presence helper `wait_for_file`, re-exported via `tests/common`, and routes all 15 absence sleeps through it. The constant's doc comment states the discriminator that decides the tool: a *presence* assertion polls (and returns the instant the event lands); an *absence* assertion has no event to wait for, so it holds a bounded window. The three sleeps left as literals are genuinely different roles — bash startup, signal sequencing between two SIGINTs, and PTY output sequencing — not absence checks. It also rewrites the `tests/CLAUDE.md` timing section around that polarity discriminator (split into Presence / Absence subsections), with the absence example using the constant, plus two traps: pairing one sleep with both a presence and an absence assertion (the presence half goes flaky), and structural absence — when the event is gated on a condition the test never sets up, drop the window and poll the positive precondition instead. This is the consistency-and-guidance follow-up to the watchdog de-flaking in #3187: the marker convention makes legitimate absence sleeps self-labeling, which is what lets a reviewer (or the nightly sweep) tell them apart from a flaky fixed sleep before a presence assertion. ## Testing No behavior change — test-infrastructure and docs only. Each of the 15 conversions was verified to be followed by a negative assertion (adversarial review, 20/20 confirmed); the full `pre-merge` gate passes (4170 tests). `merge.rs`'s window widened 200ms → 500ms, which only makes its absence check more conservative. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-24 10:41:57 -07:00
Maximilian Roos	5eb17d441b	refactor: delete dead code paths left by completed cut-overs (#2931) ... Three independent dead-code removals, each a parallel implementation left behind by a completed cut-over and kept alive only by tests (or nothing). ~610 net lines removed, no behavior change (JSON output byte-identical, full test suite passes). - git: drop the by-ref integration API (resolve_ref + ref-taking probes); push.rs uses is_ancestor_by_sha. - list: drop the ListItem serde surface and pre-rendered display-string layer; JSON flows only through JsonItem. - worktree: drop the dead OperationMode enum and the now-constant params of resolve_worktree_arg.	2026-05-28 13:09:39 -07:00
Maximilian Roos	43e1592503	fix: data-safety fixes across the worktree merge/remove/prune lifecycle (#2870) ... Five data-safety / correctness fixes in the worktree-lifecycle paths, plus one rebase-integration follow-up. Each closes a real TOCTOU or scoping gap and carries integration tests; together they form a dependency chain through `output/handlers.rs` and `step/prune.rs`. - **`merge` / `remove` — revalidate cleanliness after `pre-remove` hooks.** `wt merge` / `wt remove` snapshotted the clean check and branch-integration decision during planning, then ran `pre-remove` hooks before cleanup. A hook (or concurrent process) could dirty the worktree or advance the branch in between; the stale decision then trashed the directory or ran `git branch -D`. Cleanliness and integration are now recomputed from a fresh ref snapshot after hooks run. - **`remove` — recompute branch safety in the background path.** The background removal path also acted on pre-hook planning state, and synthesized `git worktree remove --force` whenever `.gitmodules` existed. It now re-runs `ensure_clean`, recomputes integration via `delete_branch_if_safe`, and fails closed for submodule worktrees on the fallback path instead of forcing. - **`prune` — serialize the rename-failure fallback.** When the fast-path rename failed, prune spawned a detached `git branch -d` that ran after the lock guard dropped, racing a live integration-check reader on `.git/config` (#2801's residual gap). It now runs synchronously under the write guard for non-current worktrees. - **`prune` — scope hook approval to the removal set.** Approval was built from every linked worktree (including ones prune would skip) plus an always-on primary `post-switch`, so a non-interactive prune could abort on an unrelated unapproved hook. Approval is now built only for the worktrees prune will actually remove. - **`prune` — prune stale *detached* worktree metadata.** A detached worktree whose directory was deleted outside Worktrunk (`branch == None`) was silently skipped and lingered in `git worktree list` forever; it is now pruned via an idempotent `git worktree prune`. The final commit drops a struct field — removed by the first commit — from a test constructor that landed in `picker/mod.rs` after this chain's base (#2845), which the cherry-pick's auto-merge did not catch. > _This was written by Claude Code on behalf of Maximilian Roos_ --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-05-24 16:12:09 -07:00
Maximilian Roos	1716ecab37	fix(hooks): resolve all hook config from the invoking worktree (#2873) ... Worktrunk resolved each hook's `.config/wt.toml` from a different worktree depending on the hook — `post-merge` from the merge target, `post-switch` from the destination, `pre-remove`/`post-remove` from each removed worktree, `wt step prune` from each prunable worktree, and `wt switch --create` from the base ref's *committed* config via `git show`. That last one is the bug behind #2856 and #2818: an uncommitted or branch-local `.config/wt.toml` silently failed to fire creation hooks, and `wt config show` (which reads the working tree) disagreed with what actually ran. This replaces all of it with one rule: **every hook resolves its commands from the `.config/wt.toml` of the worktree `wt` ran in** — the invoking worktree, read from its working tree, the same file `wt config show` displays. ## Behavior changes - `wt switch --create` / `pr:` / `mr:` creation hooks read the invoking worktree's config, so an uncommitted `.config/wt.toml` fires them; the base ref's or PR's committed config is no longer consulted. - `post-merge` runs the feature worktree's config, not the merge target's. - `post-switch` into an existing worktree uses the source, not the destination. - `wt remove <other-branch>` and `wt step prune` use the invoking worktree's config, not each removed worktree's. In the common case — a committed, repo-wide `.config/wt.toml` — these are identical; they diverge only when a branch carries its own working-tree edits. ## For reviewers The module docstring in `src/commands/hooks.rs` is the spec — its per-hook config-source table collapsed to one rule. The change is concentrated in five approval gates that now call `repo.load_project_config()` once instead of `Repository::at(<other-worktree>)`: `merge::approve_merge_plan`, `main.rs`'s `approve_remove`, `step::prune::approve_prune_hooks`, `picker::approved_removal_plan`, and `worktree::switch`. The `switch_hook_project_config` helper and the `base_ref_for_create` / `project_config_at_ref` `git show` machinery are deleted. The *anchor* — the worktree a hook runs in, the executor's plan-lookup key — is unchanged; only the config *source* unifies. The frozen `ApprovedHookPlan` still closes the approval-boundary TOCTOU. ## Testing Hook config-resolution tests across `switch`, `merge`, `remove`, and `step_prune` were rewritten to assert the new rule, each also checking that the non-invoking worktree's config is ignored. `test_post_merge_hook_from_rebased_in_config_does_not_run` is the TOCTOU regression: a `post-merge` that enters the invoking worktree's config only via the rebase, after the gate froze the plan, must not run. Ref #2856, #2818. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-21 18:56:14 -07:00
Maximilian Roos	61f4efc1e4	refactor(cli): consolidate --no-verify deprecation into one path (#2868) ... The deprecated `--no-verify` flag was declared as a clap arg five times — across `SwitchArgs`, `RemoveArgs`, `CommitArgs`, `SquashArgs`, and `MergeArgs` — under two field names, resolved through two helpers, with the deprecation-warning string hand-duplicated in two places. This collapses the four bool-resolving commands (`switch`, `remove`, `step commit`, `step squash`) onto a single flattened `HookFlags` args struct with one `resolve()` method, and routes every `--no-verify` deprecation warning — merge's included — through one emitter, so the warning text exists in exactly one place. `wt merge` keeps its own flags: its hooks flag is genuinely tri-state (`Option<bool>`, so config `[merge] verify` still applies) with a positive `--verify` override, which `HookFlags`'s `SetFalse`/default-true shape cannot express. It shares only the warning emitter — forcing it into the struct would need a special case. Behavior is unchanged: `--no-verify` still works as a deprecated alias and emits the same warning under the same conditions. One visible `--help` change, a direct consequence of the shared struct: `--no-hooks` for `step commit`/`step squash` moves under the `Automation` heading, matching where `switch`/`remove`/`merge` already place it. Doc mirrors regenerated. > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>	2026-05-21 18:41:20 -07:00
Maximilian Roos	84fb34c11a	fix(hooks): structurally close the approval-boundary TOCTOU class (#2806) ... ## Why Project-defined hook commands (`pre-*`/`post-*`) are arbitrary code shipped in a repo the user may have just cloned. They were selected from `.config/wt.toml` **twice**: once at the approval gate to build the prompt, and again at execution when `register`/`execute_hook` re-read `load_project_config()`. Between the two reads, the operation itself mutates state — a merge moves the target ref, an auto-rebase rewrites the feature config, a removal scrubs the worktree, `git worktree add` materializes a `--create` worktree — so the second read could select a command the user never approved. On a fresh `git clone && wt <op>` that is remote code execution. On `main` the post-merge path was entirely unpinned; the others used point-fix config snapshots that the executor could still re-resolve around. ## Approach The gate selects the command set exactly once and freezes it into an immutable, type-state `ApprovedHookPlan` (new `src/commands/hook_plan.rs`). Covered executors consume only that value via `execute_planned_hook` / `register_planned` and hold no `ProjectConfig`/`Repository` for selection, so re-derivation is a compile error, not a review invariant. Rendering stays deferred (post-`*` hooks legitimately need post-operation context like the merge commit) but consumes the frozen `CommandConfig` list, never config. Covered (gate and execution separated by a state mutation): `pre-merge`, `post-merge`, `pre-remove`, `post-remove`, `post-switch`, `pre-start`, `post-start`. Deliberately not covered — they have no gate→exec mutation window and share the gate's cached `Repository`: `pre-commit`, `post-commit`, `pre-switch`, `wt hook <type>`, aliases. This scope boundary is documented in the `commands::hooks` module spec. Clean cutover: the point-fix snapshot apparatus is deleted (`RemoveResult::removed_project_config`, `register_with_project_config`, `collect_remove_hook_commands`, `collect_merge_commands`, `removal_hooks_approved`, `approve_or_skip_with_config`) — no parallel path, no compatibility flag. ## Reviewer orientation - `src/commands/hook_plan.rs` — the whole model: `HookPlanBuilder` (sole config→commands point), type-state `HookPlan` → `ApprovedHookPlan` (constructible only via `approve`/`approve_readonly`/`empty`), `lookup`/`render_planned`. Start here. - `merge.rs` / `main.rs` / `step/prune.rs` / `worktree/switch.rs` / `picker/mod.rs` — the five gates that build a plan. - `output/handlers.rs` / `worktree/finish.rs` — the executors that consume it. - `commands::hooks` module doc — the canonical "which `.config/wt.toml` a hook reads" spec, rewritten for the plan model including why the uncovered set is safe (shared never-invalidated config cache). Behavior parity is preserved: an empty plan (`--no-hooks`, declined, or no project config) runs no project hooks; the merge approval prompt is unchanged (still lists pre-commit/post-commit); the picker's read-only gate drops only unapproved project pipelines (strictly better than the old all-or-nothing verify boolean). The empty-plan fast path returns before any `Approvals` load or project-id resolution, so a malformed `approvals.toml` no longer aborts a command with nothing to authorize, and `wt merge --no-hooks` no longer parses the destination config. The removal data-safety re-validation, the Ctrl-C signal policy, and source-scoped filtering are untouched. ## Testing `cargo run -- hook pre-merge --yes` green (3751 tests), clippy + pre-commit clean. New regression tests: `test_post_merge_hook_from_merged_feature_config_does_not_run` (the TOCTOU itself, causally bounded), `test_remove_no_project_hooks_ignores_malformed_approvals`, `test_merge_no_hooks_ignores_malformed_destination_config`, plus `hook_plan` unit tests (frozen lookup, read-only filter, source-group ordering). Reviewed across eight structurally-distinct passes (adversarial, generalization, evidential, subtraction, metric, classification, holistic) plus a Codex review whose two P2 findings are fixed and locked with the malformed-config tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-19 12:12:59 -07:00
Maximilian Roos	f68626dd03	fix: six independent single-file data-safety and correctness fixes (#2784) ... ## Summary Six independent single-file fixes found during a static review pass. Each touches its own file (the only shared file is `tests/integration_tests/switch.rs`, by the two PR-remote fixes), so they're grouped here as low-risk, self-contained wins. **Data safety** - **`relocate`: refuse `--clobber` when the backup path already exists** — `--clobber` could overwrite a pre-existing backup, destroying the only copy of prior state. Now bails if the backup path exists. - **`remove`: re-check cleanliness before synthesized-force submodule removal** — the submodule-removal path synthesized a `--force` after the initial cleanliness check; a worktree dirtied in the interim could lose work. It now re-runs `ensure_clean` immediately before the forced removal (time-of-check/time-of-use). **Correctness** - **`switch`: probe base worktree via a fresh `Repository` after `worktree add`** — the post-create base lookup read a stale cached worktree list; it now re-discovers via `Repository::at(...)`, matching the caching contract. - **`pr`: derive owner/repo from the forge remote, not the primary remote** — PR/MR lookups used the primary remote's owner/repo even when the forge remote differed, producing wrong-host or wrong-repo lookups. Now parsed from the forge remote. - **`pr`: validate empty Azure PR source branch at the provider boundary** — an empty Azure source branch slipped past unvalidated; now rejected at the provider boundary with a clear error. **Defense in depth** - **`switch`: guard branch operands with `--`** — `git worktree add` / `branch --unset-upstream` invocations now place options first and terminate with `--` so a branch name starting with `-` can't be interpreted as a flag. ## Test plan - [x] `cargo check` clean on this branch off `main` - [x] Per-fix regression / integration tests added; affected snapshots accepted - [ ] CI green 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-05-17 15:08:50 -07:00
Maximilian Roos	23d6552471	feat(hooks): post-remove reads the removed worktree's config (snapshot before removal) (#2736) ... `pre-remove` already reads the worktree-being-removed's `.config/wt.toml`, but `post-remove` read the *post-removal* working directory's config — the primary worktree for `wt remove` / `wt step prune`, the merge destination for `wt merge`. So a `post-remove` defined in a feature branch's branch-local config never fired, and a `[pre-remove]`/`[post-remove]` pair in the same `.config/wt.toml` didn't read the same file. The only reason for the asymmetry: the worktree ceases to exist between the two hooks. This snapshots the removed worktree's `ProjectConfig` before deletion (stashed on `RemoveResult::RemovedWorktree::removed_project_config`) and threads it to the `post-remove` executor. The rule the `commands::hooks` spec docstring now states up front: **every hook is anchored to one worktree — the one it's *about* — and reads that worktree's `.config/wt.toml`; when the worktree isn't on disk at read time, read a snapshot.** Two hooks need that: `post-remove` (worktree just deleted) and `wt switch --create`'s post-switch hooks at approval time (worktree not created yet — reads the base ref via `git show`). Same role, two physical sources; the spec's new "Snapshot paths" section frames them together, and `pre-remove`/`post-remove` collapse into one table row. ## What changed - **`RemoveResult::RemovedWorktree`** gains `removed_project_config: Option<Box<ProjectConfig>>` (boxed — `ProjectConfig` is ~600 bytes and would otherwise dominate the variant; clippy `large_enum_variant`). `prepare_worktree_removal` and `finish_after_merge` populate it before any deletion. A parse failure there is *not* propagated: the executor's own `load_project_config()` re-reads from disk and surfaces the parse error with full chain, and `pre-remove` aborts before `post-remove` fires, so the `None` is invisible. (Routing the parse error through `prepare_worktree_removal` would land it in `main.rs`'s `record_error`, which renders only the top message — a pre-existing display gap orthogonal to this work.) - **`output::handlers::spawn_hooks_after_remove`** registers `post-remove` via the new `HookAnnouncer::register_with_project_config` using that snapshot. `post-switch` after a removal keeps loading from `ctx.repo` (the post-removal cwd — the worktree it's *about*). - **`commands::picker::do_removal`** (the TUI in-place removal path) was the last hook-execution site reading the primary's config — `pre-remove` was rooted at `main_path`, `post-remove` likewise. Now `pre-remove` is rooted at the worktree being removed and `post-remove` uses the snapshot, matching `wt remove`. - **`collect_remove_hook_commands`** (the shared approval helper for `wt remove` / `wt merge` / `wt step prune`) collects `post-remove` per-worktree (same loop as `pre-remove`); `post-switch` keeps coming from `primary_repo`. - **`prepare_background_pipelines`** absorbs its single-use loading shim: it now takes a required `Option<&ProjectConfig>`, and `HookAnnouncer::register` (loads from `ctx.repo`) / `register_with_project_config` (explicit snapshot) are the two ergonomic wrappers — mirroring the approval side's `approve_or_skip` / `approve_or_skip_with_config`. - **Spec**: `commands::hooks` module docstring rewritten — "anchor worktree" framing, the `pre-`/`post-remove` row merge, the "Snapshot paths" section. ## Tests `test_post_remove_hook_reads_removed_worktree_config` (remove.rs) — the mirror of the existing `test_pre_remove_hook_reads_removed_worktree_config`: a `post-remove` in the removed worktree's branch-local `.config/wt.toml` fires correctly *after* the worktree is gone. Verified to fail without the snapshot wiring. Existing pre-remove / merge-teardown / prune / approval-prompt / picker suites all pass; `wt hook pre-merge --yes` (full suite + lints + doctests) is green. ## Follow-up (not in this PR) `commands::picker::do_removal` is still a parallel reimplementation of the `wt remove` pipeline (pre-remove → git removal → post-remove), ~80 lines that shadow `handle_remove_output`'s flow plus its own `PostRemoveContext` construction. This PR makes it follow the same config rule; routing it through `handle_remove_output` (which has a quiet/background mode) would delete the duplication outright — a separate, larger refactor with its own picker-test burden. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-11 19:02:47 -07:00
Maximilian Roos	c67803e8dd	docs+test(hooks): tighten "(else the primary)" wording; cover the no-config approval path (#2727) ... Two small consolidations on top of #2717 (which dropped the residual fallback in the shared `collect_remove_hook_commands` approval helper, finishing what #2714 started): **Spec tightening.** The `commands::hooks` "Which `.config/wt.toml` a hook reads" table row for `post-merge` / `post-remove` / `post-switch` after a removal previously read "the destination — for `wt merge`, the target branch's worktree (else the primary)". Once the fallback rule was gone, "(else the primary)" could be misread as a config fallback. Re-phrased it as the path choice for the destination — the merge target's worktree if one exists, otherwise the primary — explicitly noted "not a config fallback". **Regression test.** Added `test_remove_no_fallback_to_primary_pre_remove`: a worktree with no `.config/wt.toml` plus a `pre-remove` in the primary. Before #2717, `wt remove` without `--yes` aborted with "needs approval" because the helper surfaced the primary's hook and tests have no TTY; after #2717 the prompt is empty and removal succeeds. #2717's commit message argued the malformed-config tests covered the helper end-to-end, but no test covered the no-config (empty-prompt) path specifically — this fills that gap, so a future re-introduction of the fallback would surface immediately. Verified twice: stashed `project_config.rs` to its pre-#2717 state → test fails with exactly the symptom (`pre-remove: echo ran > ...` shown in the prompt, `Cannot prompt for approval in non-interactive environment`). Restored → test passes. `cargo run -- hook pre-merge --yes` is green. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-11 16:10:58 -07:00
Maximilian Roos	1dbbe84b8d	refactor(hooks): drop primary-worktree config fallback; surface parse errors (#2714) ... ## Summary Each worktree's `.config/wt.toml` now stands alone. When the worktree a hook acts on has no `.config/wt.toml`, no project hooks run — the primary worktree's config is no longer consulted as a fallback. A present-but-malformed config aborts the operation with the parse error instead of silently using a different one. ## Why The fallback (#2690, extended to `wt switch` in #2703) was an internal detail meant to keep worktrees branched off *before* a hook was added still firing the hook. But it made which file each hook actually reads ambiguous — Codex flagged this in review of #2703 (P3): a destination worktree with a malformed `.config/wt.toml` silently ran the *primary's* hooks instead of surfacing the parse error. Removing the fallback gives a cleaner model: each worktree owns its config. Users who want a hook to apply project-wide commit it on the default branch — new worktrees branched off that branch inherit it. Worktrees branched off *before* the commit no longer fire the hook silently, which was the leaky part. ## Behavior change A `pre-remove` hook added on `main` no longer fires when removing a feature worktree that predates the commit adding it. To restore that behavior, branch the feature off `main` again (or commit the hook on the feature branch itself). ## What changed - `Repository::project_config_at_ref` now returns `anyhow::Result<Option<ProjectConfig>>` — `Ok(None)` for refs with no `.config/wt.toml`, `Err` for present-but-malformed. - Dropped the fallback from `commands::project_config::collect_remove_hook_commands` (the shared helper introduced in #2709) and from `output::handlers::execute_pre_remove_hooks_if_needed`, `worktree::switch::{switch_hook_project_config, hook_repo_for_worktree, spawn_switch_background_hooks}`, and `worktree::finish::finish_after_merge`. - Updated the `commands::hooks` spec docstring — table rows lose the fallback sub-clauses; trailing paragraph drops the fallback handling. ## Tests - Unit test `project_config_at_ref_reads_committed_config` now asserts `Err` on malformed committed config (was `None`). - Added `test_switch_existing_aborts_on_malformed_destination_config` and `test_remove_aborts_on_malformed_worktree_config` for the end-to-end abort path. - Updated three tests that relied on the fallback to commit configs on the relevant branches instead: `test_switch_pr_hooks_see_pr_vars` (commits on `pr-source`), `test_merge_teardown_hooks_read_destination_worktree_config` → `..._read_acting_worktree_config` (feature carries pre-remove), `test_approval_prompt_remove_decline` (commits before creating the worktree to remove). Snapshot regenerated for the last. Co-authored-by: Claude <noreply@anthropic.com>	2026-05-11 12:09:18 -07:00
Maximilian Roos	78b3a95872	fix(hooks): resolve project config from the worktree the hook acts on (#2690) ... A hook resolves its project config from `ctx.repo.load_project_config()`, so whichever worktree that `Repository` is rooted at decides which `.config/wt.toml` it reads. Several call sites rooted it at the wrong worktree — most visibly `pre-remove`, which read the **primary** worktree's config rather than the worktree being removed, so a `pre-remove` you add on a feature branch never fired when removing that branch's worktree until the change landed on the default branch. This makes the rule uniform: **`ctx.repo` is rooted at the worktree the hook acts on** — and adds a spec docstring for it. ### What changed - **`pre-remove`** (`output/handlers.rs` `execute_pre_remove_hooks_if_needed`) now resolves config from the worktree being removed (it's still on disk at pre-remove time), falling back to the post-removal working directory's config (the primary worktree for `wt remove`, the destination for `wt merge`) if that worktree has no `.config/wt.toml` — so a project-wide `pre-remove` still applies to worktrees branched before it was added. The approval is split to mirror this: `main.rs` approves `pre-remove` per removed worktree against that worktree's config (`post-remove`/`post-switch` against the primary's), and `merge.rs` `collect_merge_commands` resolves `pre-remove` the same way the executor does — which closes the `TODO(pre-remove-context)` in `main.rs`. - **`post-merge`** (`worktree/finish.rs`) now resolves config from the target worktree (where it runs), not the feature worktree. `collect_merge_commands` splits the approval accordingly (feature config for `pre-commit`/`post-commit`/`pre-merge`; destination config for `post-merge`/`post-remove`/`post-switch`), which also fixes a pre-existing `wt merge` approval/execution mismatch on `post-remove`/`post-switch`. - **`wt step commit --branch <b>`** (`commands/context.rs` `CommandEnv::for_branch`) now roots the `Repository` at `<b>`'s worktree rather than the cwd, so `pre-commit`/`post-commit` for it read `<b>`'s config. As a side effect this fixes the diffstat suffix on that command's "Generating commit message…" line, which was empty because it was computed from the cwd worktree's (unstaged) index — see the updated `step_commit_branch_flag` snapshot. - **Spec**: `commands::hooks` module docstring now has a "Which `.config/wt.toml` a hook reads" section with the per-hook mapping and the construction site for each; `CommandContext.repo`, `Repository::project_config_path`, and `execute_hook` / `prepare_background_pipelines` point at it. The one exception, documented in the spec: `wt switch`'s `pre-start`/`post-start`/`post-switch` still read the invoking worktree's config — the new worktree doesn't exist yet when those hooks are approved, and for `wt switch --create` it's a fresh checkout of that worktree's HEAD anyway, so the config matches. ### Testing Three integration tests where the only worktree carrying a `.config/wt.toml` is the one the hook acts on, each verified to fail before the change: `test_pre_remove_hook_reads_removed_worktree_config` (remove.rs), `test_merge_teardown_hooks_read_destination_worktree_config` (merge.rs — covers `post-merge` and the `pre-remove` destination fallback), and the diffstat-suffix change in `step_commit_branch_flag` (merge.rs). Existing hook/merge/remove/step suites pass; `wt hook pre-merge --yes` (full suite + lints) is green. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-11 00:39:58 -07:00
Worktrunk Bot	1d97bb7a20	feat(config): add [remove] delete-branch config option (#2589) ... ## Summary Adds a `[remove]` config section that lets users default `wt remove` to keeping branches, requested in #2587. Setting `delete-branch = false` is equivalent to passing `--no-delete-branch` every time; CLI flags still override the config. ```toml [remove] delete-branch = false # never delete branches when removing worktrees ``` Implementation mirrors the existing `[merge]` pattern: new `RemoveConfig` section, a `[projects."<id>".remove]` override path, and a tri-state CLI flag pair (`--delete-branch` / `--no-delete-branch`) so users can override config either direction. ## Test plan - [x] Unit tests for `RemoveConfig` parse/default/merge + project override (`src/config/user/tests.rs`) - [x] Integration test: `[remove] delete-branch = false` keeps the branch on `wt remove` - [x] Integration test: explicit `--delete-branch` overrides the config - [x] All existing `wt remove` tests still pass (83 tests) - [x] `cargo test --lib --bins` and `cargo clippy --tests --all-features` clean - [x] Help/docs snapshots regenerated (`test_docs_are_in_sync`) Closes #2587. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>	2026-05-04 11:26:56 -07:00
Maximilian Roos	d66faea86b	fix(prune): OR over local + upstream in step prune integration check (#2515) ... ## Summary `wt step prune` was the lone remaining caller of `Repository::effective_integration_target` — a "pick one ref" helper whose own docstring warned it was unsafe for safety-critical checks. In the diverged case it returned `origin/main`, then handed it to `integration_reason`, which couldn't resolve an upstream of an already-remote ref and ended up checking only that single side. Branches merged into local `main` while `origin/main` had unique commits never got pruned — the same diverged-case false negative #2513 fixed for `wt list` and #2507 fixed for `wt remove` / `wt merge`. This change passes the local default branch directly to `integration_reason`, letting it OR over local + upstream internally like `wt remove` and `wt merge` already do. The actual deletion gate inside `prepare_worktree_removal` was already correct, so this just stops the outer filter from silently dropping legitimately-integrated candidates. `effective_integration_target`, `integration_target`, and their `RepoCache` fields are now dead and deleted. ## Test plan - [x] New regression `test_prune_locally_merged_when_upstream_diverged` (mirrors `test_remove_merged_locally_when_upstream_diverged`). Verified to fail on the pre-fix code path with `No merged worktrees to remove`, passes on the fix. - [x] Symmetric `test_prune_squash_merged_on_remote_when_local_diverged` covers the merged-on-remote direction; pins forward-compatibility. - [x] `cargo run -- hook pre-merge --yes` — 3411/3411 pass, no snapshots to review. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-04-30 19:57:57 -07:00
Maximilian Roos	5a12324dd3	fix(integration): OR over local + upstream for integration detection (#2507) ... ## Summary `Repository::integration_reason` picked a single target via `effective_integration_target` and checked only that, silently dropping half of history in the diverged case. After `wt merge` updates local `main` (while it's still diverged from `origin/main`), the upstream-only check would report the just-merged branch as unmerged and skip removal. The symmetric hole existed in `wt remove`: a branch merged into local `main` on a diverged `main` was misreported. ## Behavior change `integration_reason(branch, target)` now considers both local and upstream: - No upstream / same commit: check local only. - Local strictly behind upstream: check upstream (superset). - Upstream strictly behind local: check local (superset). - **Diverged: check local, then upstream — integrated if either matches.** The local/upstream ancestor checks run git directly with ref names (`merge-base --is-ancestor`) rather than going through cached `is_ancestor`/`rev_parse_commit`. The local target may have just been updated mid-command (e.g. `wt merge`'s ref update), so cached SHAs would be stale. `effective_integration_target` stays as a display helper for `wt list`'s status column — the docstring is updated to make clear it's not for safety-critical checks. ## Test plan Two new regression tests cover the symmetric cases: - `test_merge_removes_branch_when_local_main_diverged_from_upstream` — `wt merge` after the user's reported scenario (remote-only commit + local-only commit on `main` create divergence). Verified failing without the fix; passing with it. - `test_remove_merged_locally_when_upstream_diverged` — `wt remove` for a branch merged into local `main` while `origin/main` has diverged. Verified failing without the fix; passing with it. One existing snapshot (`merge_auto_commit_combines_post_commit_post_remove_post_switch_post_merge`) was capturing the buggy `Branch unmerged` output for an in-process scenario where the SHA cache went stale across the merge ref-update; updated to the correct integration message. All 3405 tests pass; all lints clean. > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-04-30 16:26:48 -07:00
Worktrunk Bot	5f126d21fe	docs(pages): add static command output to sections dominated by GIFs (#2405) ... ## Summary Adds static command-output blocks to the docs pages dominated by GIFs (addresses #2403). The blocks are **driven from insta snapshots** so they stay in lockstep with what `wt` actually prints — one source flows to all three surfaces: - terminal `wt <cmd> --help` (plain text, gutter-formatted) - `docs/content/*.md` (colorized `{% terminal(cmd="...") %}` shortcode) - `skills/worktrunk/reference/*.md` (plain `$ cmd\noutput\n` block) ## What changed - **Six scripted snapshot tests** produce realistic output (`cargo nextest run`, `flyctl scale count 0`, LLM-generated commit messages): - `test_docs_merge_pre_merge_hook` — `wt merge` with pre-merge hook - `test_docs_step_commit_llm` — `wt step commit` with LLM - `test_docs_step_squash_llm` — three-commit squash with LLM - `test_docs_merge_squash_llm` — `wt merge` (squash + LLM + merge) for `llm-commits.md` - `test_docs_remove_pre_remove_hook` — `wt remove` with pre-remove hook - `test_docs_hook_pre_merge` — `wt hook pre-merge` direct invocation - **Sync pipeline extension** in `tests/integration_tests/readme_sync.rs`: - New write-back pass `sync_cli_mod_example_bodies` fills the ```console``` body in each `<!-- wt <cmd> (docs-example) -->` placeholder in `src/cli/mod.rs` from the registered snapshot. Runs before `--help-page` reads the file. - `COMMAND_PLACEHOLDER_PATTERN` extended to match three forms (```bash```, `{{ terminal() }}` self-closing, `{% terminal %} body {% end %}`). This **fixes a pre-existing bug** in `docs/content/list.md` where the HTML-mode expansion was silently broken. - Stripped trailing `|||` corruption that arises when blank lines in snapshot bodies are interpreted as empty commands by `convert_dollar_console_to_terminal`. - **Docs page migration**: - `src/cli/mod.rs` — replaced four hand-written ```console``` blocks (merge, step, remove, hook) with `<!-- wt <cmd> (docs-example) -->` markers. - `docs/content/llm-commits.md` — replaced three hand-crafted HTML blocks with `<!-- ⚠️ AUTO-GENERATED-HTML from X.snap -->` markers. - **Refactor follow-up** in a separate commit: - `BADGE_EXPERIMENTAL_HTML`, `SUBDOC_MARKER_PREFIX`, `DEMO_MARKER_PREFIX` hoisted to `worktrunk::docs` so producer and consumer stay in lockstep. - `normalize_clap_help_fences()` consolidates the `text→` + `console→bash` replacement pair shared by `--help-md` and `help_reference_inner`. - **CLAUDE.md note** documenting the `.gitattributes` `linguist-generated=false` exemption requirement when adding skill-only files (carryover from #2409). ## Test plan - [x] `cargo test --test integration readme_sync` — all 13 tests pass, idempotent - [x] `cargo test --test integration test_help` — help snapshots updated - [x] `cargo run -- {merge,remove,step,hook} --help` — clean gutter formatting, no visible HTML comments - [x] `cargo run -- hook pre-merge --yes` — full project gate green (3355 tests) - [ ] Visual check on dev site (maintainer — terminal shortcodes now render with colors via ANSI→HTML; dark/light variants both expected to look like the GIFs they replace) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Maximilian Roos <m@maxroos.com>	2026-04-24 16:40:21 -07:00
Maximilian Roos	37fb27bc97	Deprecate table form for pre-* hooks (#2135) ... ## Summary Multi-entry table form for pre-* hooks currently runs serially, while the same form for post-* hooks runs concurrently. The parser produces `HookStep::Concurrent` either way, but the foreground executor flattens steps and runs them serially. To unify the semantics — table form will run concurrently for all hook types in a future version — this deprecates the current form for pre-* hooks and auto-migrates it to pipeline form, which is explicitly serial. ## Implementation Follows the existing deprecation recipe in `src/config/deprecation.rs`: - **Detection** and **migration** for top-level hooks (user/project config) and per-project overrides (`[projects."id".pre-*]`). - **Warning**: matches the terse `{old} → {new}` pattern of existing deprecations (`[merge] no-ff → ff`, `post-create → pre-start`, etc.). - **Auto-migration** at load time: table form rewrites to pipeline of inline tables so current behavior (serial) is preserved until users run `wt config update`. ## Docs Replaces the transitional "concurrent for post-*, sequential for pre-*" framing with a neutral three-form description (string / table / pipeline), plus a note recommending pipeline form for pre-* hooks to avoid the upcoming behavior change. ## Tests - `snapshot_migrate_pre_hook_table_form` — TOML migration diff - `test_config_show_displays_pre_hook_table_form_deprecation` — full user-facing `wt config show` output, covering the "Project config" label and multi-hook list form - Unit tests for detection/migration of top-level and per-project variants - Existing integration test fixtures migrated to canonical pipeline form > _This was written by Claude Code on behalf of max-sixty_ Co-authored-by: Claude <noreply@anthropic.com>	2026-04-12 15:55:28 -07:00
Maximilian Roos	b174658e29	Split directive file into CD (raw path) and EXEC (shell) files (#2118) ... The shell wrapper previously used a single `WORKTRUNK_DIRECTIVE_FILE` where wt wrote shell commands (`cd '/path'`, arbitrary `--execute` payloads). This meant the cd path went through shell parsing — any content wt wrote was sourced as shell. This splits the protocol into two files with different trust levels: - **`WORKTRUNK_DIRECTIVE_CD_FILE`** — raw path, read with `cd -- "$(< file)"`. No shell parsing, no escaping, no injection surface. Safe to pass through to alias/hook child processes. - **`WORKTRUNK_DIRECTIVE_EXEC_FILE`** — arbitrary shell (from `--execute`), sourced by the wrapper. Scrubbed from alias/hook child environments so hook bodies cannot inject shell into the parent session. When a nested `wt` inside an alias body tries `--execute` without the EXEC file, the command is dropped with a warning linking to #2101 for user feedback. The old `WORKTRUNK_DIRECTIVE_FILE` is silently honored for one release (users who upgrade wt without restarting their shell). Bash, zsh, fish, and PowerShell self-update on restart; nushell requires `wt config shell install`. Closes #2101 > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-04-12 13:52:04 -07:00
Maximilian Roos	4a7219b230	Sweep stale .git/wt/trash entries on wt remove (#2039) ... ## Summary Background `wt remove` renames worktrees into `.git/wt/trash/<name>-<timestamp>` before firing a detached `rm -rf`. If that cleanup is interrupted (SIGKILL, reboot, disk full), the directory is orphaned and nothing reclaims the space. After `wt remove` prints its primary output, it now sweeps trash entries older than 24 hours via a single detached `rm -rf`. Age is parsed from the filename suffix, so the sweep is deterministic under `WORKTRUNK_TEST_EPOCH`. ## Notes - Runs **after** `handle_remove_output` so it never delays time-to-first-output. `.claude/skills/writing-user-outputs/SKILL.md` documents the general rule for future handlers. - Uses existing `spawn_detached` infrastructure and a new `InternalOp::TrashSweep` log name (`.git/wt/logs/wt-trash-sweep.log`). - Unparseable entries are left alone — the sweep only touches names worktrunk produced. - Integration test seeds stale + fresh entries, runs `wt remove`, and polls for the stale entry to disappear while the fresh one stays. Ref #1974 — related safety improvement (data lingers in trash briefly before deletion), but doesn't resolve the prune confirmation prompt discussion there. > _This was written by Claude Code on behalf of max-sixty_ Co-authored-by: Claude <noreply@anthropic.com>	2026-04-09 13:54:41 -07:00
Maximilian Roos	65e58d3647	fix: JSON output follow-ups from --format=json rollout (#2001) ... Four targeted fixes identified as follow-ups from the `--format=json` rollout (#1969, #1998): 1. **Sort stability in `partition_log_files_json`** — added filename tiebreaker to match `render_log_table`'s sort. Without this, entries with identical timestamps could appear in non-deterministic order in JSON output. 2. **for-each `error` field always present on failure** — previously, the `"error"` field only appeared for `SpawnFailed` errors, not `ExitCode`. Consumers parsing the JSON had to handle two shapes. Now both variants include `"error"`: `"exit code N"` for ExitCode, `"spawn failed: ..."` for SpawnFailed, `"killed by signal"` for signal termination. Error formatting lives in `CommandError::Display` impl with a unit test covering all three variants. 3. **Test: `config show --format=json` outside a git repo** — verifies that project path/config are `null` when not in a git repository. 4. **Test: multi-remove `--format=json` with current worktree** — covers the `plans.current` code path where the current worktree appears in the multi-remove target list (deferred to last in output). > _This was written by Claude Code on behalf of Maximilian Roos_ --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-04-07 21:54:15 -07:00
Maximilian Roos	85c3dadf17	feat: add --format=json to config state subcommands (#1969) ... Add `--format=json` to 10 commands across two commits. ## Config state subcommands (commit 1) - `logs get` — `{command_log: [...], hook_output: [...]}` with file/size/modified_at per entry - `ci-status get` — `{status, source, stale, url}` (richer than the text mode's bare status string) - `marker get` — `{branch, marker, set_at}` or `null` - `vars list` — `{key: value, ...}` object - `hints get` — `["hint-name", ...]` array Also extracts a shared `log_entry_to_json` helper, deduplicating the DirEntry→JSON conversion. ## Top-level commands (commit 2) - `config show` — serialized user/project/system config with paths and existence flags - `step prune --dry-run` — array of candidates with branch, path, kind, reason, target - `remove` — array of removed worktrees with branch, path, deletion status - `merge` — summary with branch, target, committed, squashed, rebased, removed - `step for-each` — per-worktree results with branch, path, exit_code, success For action commands (remove, merge, for-each), progress goes to stderr as usual; JSON summary goes to stdout at the end — same pattern as `wt switch --format=json`. > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-04-07 14:41:21 -07:00
Maximilian Roos	20b8b22f6b	fix: use effective integration target in RemovedWorktree display (#1993) ... The background removal path was showing the local default branch (e.g., `main`) instead of the effective target (e.g., `origin/main`) when the upstream was ahead. This was a pre-existing inconsistency between foreground and background removal — foreground gets the effective target from `delete_branch_if_safe`, but background used `target_branch` directly. Now `prepare_worktree_removal` Phase 5 uses the effective target returned by `compute_integration_reason` (which was added in #1989 but discarded with `_`). One-line fix: `let target_branch = effective_target.or(target_branch);` Added `test_remove_worktree_squash_merged_on_remote` which creates a worktree where `origin/main` is ahead of local `main` after a squash merge, and verifies the background removal output shows `origin/main`. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com>	2026-04-07 14:37:56 -07:00
Maximilian Roos	679fe539fe	Deprecate --no-verify in favor of --no-hooks (#1932) ... `--no-hooks` describes what the flag does — skip hooks. `--no-verify` was inherited from git's naming but doesn't match worktrunk's semantics (there's no "verification" step being skipped). `--no-verify` remains as a hidden alias that emits a deprecation warning, retained for at least one release cycle per the project's deprecation policy. Changes across switch, remove, merge, step commit, and step squash: - `--no-hooks` is the canonical visible flag - `--no-verify` hidden, emits `▲ --no-verify is deprecated; use --no-hooks instead` - Error hints (`↳ To skip pre-merge hooks, re-run with --no-hooks`), info messages, help text, docs, and config examples all updated - `resolve_verify()` helper in main.rs deduplicates the deprecation logic - Backward-compatibility test verifies `--no-verify` still works > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-04-06 15:01:18 -07:00
worktrunk-bot	f6e89009e5	fix: detect squash-merged branches when merge-tree conflicts (#1820) ... ## Problem `wt step prune` (and `wt remove`) fail to detect squash-merged branches when the default branch has since modified the same files that the branch touched. This is because `git merge-tree --write-tree` reports conflicts when both sides changed the same files, and the code conservatively treated conflicts as "not integrated." The same issue affects `wt list` — the `WouldMergeAdd` task calls the same function, so the integration symbol `⊂` is not shown for these branches. ## Solution When `git merge-tree --write-tree` conflicts, fall back to **patch-id matching**: compute the branch's squashed patch-id (`git diff-tree -p merge-base..branch | git patch-id --verbatim`) and check if any commit on the target has a matching patch-id. This detects squash merges because the squash-merge commit on the target has the exact same content changes as the branch. Uses `--verbatim` (not `--stable`) to avoid false positives from whitespace normalization — `--stable` strips whitespace, so tabs-vs-spaces would produce matching patch-ids even though file content differs. The fallback is only triggered when merge-tree conflicts — the happy path (no conflicts) is unchanged. The merge-tree → patch-id sequence is extracted into `Repository::merge_integration_probe()`, a shared method used by both `wt list` (parallel tasks) and `wt remove`/`wt merge` (sequential path). This fixes a drift where the two call sites handled patch-id errors differently. ## Known limitation The patch-id fallback checks if the branch's diff was **ever** applied to the target, not whether the effect **persists**. If a squash merge is later reverted on the target and then the same files are modified (causing merge-tree conflicts), the historical squash-merge commit's patch-id still matches. This is documented in #1818. A follow-up could add a revert-detection check after finding a match. ## Testing - `test_remove_squash_merged_then_same_files_modified` — reproduces the exact scenario from #1818 (branch modifies file, squash-merged, then target modifies same file) - `test_prune_squash_merged_same_files_modified` — verifies `wt step prune --dry-run` detects the branch - All existing squash-merge tests continue to pass (5 remove + 22 prune tests) Closes #1818 > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Co-authored-by: Maximilian Roos <m@maxroos.com>	2026-04-01 19:05:54 -07:00
Maximilian Roos	0d37405b38	fix: only create placeholder directory when removing current worktree (#1868) ... The empty placeholder at the removed worktree path (for Nushell PWD validity) was being created unconditionally for all background removals. It's only needed when removing the *current* worktree — where the shell's PWD points and the wrapper needs to cd away. Threads `changed_directory` through `spawn_background_removal` → `execute_instant_removal_or_fallback` → `build_remove_command_staged`. When `false`, skips `create_dir` and emits just `rm -rf` instead of `sleep 1 && rmdir ...; rm -rf`. Test assertions for non-current worktree removals tightened from `assert_worktree_removed()` (accepts empty dirs) to `!path.exists()`. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com>	2026-04-01 09:13:58 -07:00
Maximilian Roos	bd87f2a146	fix: background removal blocks for 1s due to shell parsing bug (#1858) ... `spawn_detached_unix` constructed the shell command as `sh -c "sleep 1 && rmdir ...; rm -rf ... &"`. In POSIX shell, `;` has lower precedence than `&`, so this parses as two statements: `sleep 1 && rmdir ...` runs **synchronously** (1 second block), then only `rm -rf ... &` is backgrounded. Every `wt remove` paid a 1-second penalty. The fix wraps compound commands in braces — `{ sleep 1 && rmdir ...; rm -rf ...; } &` — so `&` backgrounds the entire group. This affects all callers of `spawn_detached` (remove, prune, merge, hooks). Tests that asserted `!path.exists()` after removal now use `assert_worktree_removed()` which accepts an empty placeholder directory (the placeholder is cleaned up by the now-correctly-backgrounded `sleep 1 && rmdir`). Also fixes a pre-existing race in `test_standalone_hook_post_merge` / `post_create` where background hooks were checked immediately instead of polled. Also adds `benches/remove.rs` for measuring end-to-end remove performance. > _This was written by Claude Code on behalf of maximilian_ --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-03-31 21:05:20 -07:00
Maximilian Roos	68869761da	Fix fork CI queries, branch tracking remote check, and diverged integration target (#1812) ... Three edge-case fixes found via Codex: **1. CI check-runs fallback queries the wrong repo for fork workflows** — `detect_github_commit_checks` was using `github_owner_repo()` (first GitHub remote in the repo) instead of the branch's actual push destination. For fork workflows where branches push to a different repository, this queried the upstream repo's API and found no check runs. Added `github_owner_repo_for_branch()` that resolves through `pushRemote` → `remote.pushDefault` → tracking remote, with fallback to repo-wide remote. **2. `branch_tracks_ref` only checked `.merge`, not `.remote`** — When `wt switch pr:N` found an existing branch tracking `refs/pull/N/head`, it considered it a match regardless of which remote was configured. A branch could track the right ref path but have the wrong remote, so push/pull would be wired to the wrong repository. Now also checks `branch.<name>.remote` when an expected remote is provided. **3. `effective_integration_target` missed remote merges when local main diverged** — When local `main` had extra commits (diverged from `origin/main`), the function returned `main` instead of `origin/main`, causing `wt remove` to miss branches that were squash-merged on the remote. Now prefers upstream in the diverged case, since remote merges should still count as integrated. All three have regression tests. > _This was written by Claude Code on behalf of maximilian_ --------- Co-authored-by: Maximilian Roos <maximilian@Maximilians-MacBook-Pro.local> Co-authored-by: Claude <noreply@anthropic.com>	2026-03-30 10:07:28 -07:00
Maximilian Roos	a2482f1da9	refactor: change test git_command() to return Cmd instead of Command (#1718) ... The test infrastructure's `git_command()` returned `std::process::Command`, so test git commands bypassed `Cmd`'s debug logging (`$ git status [ctx]`) and timing traces (`[wt-trace]`). This changes it to return `worktrunk::shell_exec::Cmd`. ## Approach Added `configure_git_env(Cmd, &Path) -> Cmd` alongside the existing `configure_git_cmd(&mut Command)`. The `Command` version is kept because `configure_wt_cmd` (which configures wt binary commands) still needs it — wt commands go through `Command`, not `Cmd`. Key changes in `tests/common/mod.rs`: - `TestRepoBase::git_command()` and `TestRepo::git_command()` now return `Cmd` - All wrapper methods (`run_git`, `git_output`, `head_sha`, etc.) use `.run()` instead of `.output()` - `BareRepoTest::new()` and `NestedBareRepoTest::new()` use `Cmd::new("git")` for init 247 call sites across 25 files converted: `.output()` → `.run()` and `.status()` → `.run()` on git command chains. Zero `Command::new("git")` remaining in the test directory. Follows #1714 and #1716 which converted raw `Cmd::new("git")` and `Command::new("git")` in test bodies to `repo.run_command()`. > _This was written by Claude Code on behalf of maximilian_ Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-03-25 00:23:23 -07:00