mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
worktree-path-not-found-error
4850 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Worktrunk Bot
|
0c72c60bcd |
docs(list): fix two stale comments in list collection (#3665)
Nightly code-quality sweep: two stale doc comments in the `wt list` collection code, found while surveying `src/commands/list/collect/mod.rs`. - The `integration_targets` comment referenced "task spawning doesn't happen until line 1090+" — a brittle line number that is both stale and directionally impossible (the comment sits at ~L1377, and the actual spawn calls `work_items_for_worktree` / `work_items_for_branch` are at ~L1731/L1744). Replaced with a phase reference that won't rot. - The ahead/behind comment's parenthetical "(Step 5 deletes `cache.ahead_behind` entirely.)" described a refactor that has already landed: there is no longer an `ahead_behind` field on the in-memory `RepoCache` — ahead/behind is served by `RefSnapshot` (`src/git/repository/ref_snapshot.rs`), keyed by SHA. Dropped the reference to the removed field and the completed future-step note; the remaining sentences stand on their own. Comment-only changes — no behavior change, so no regression test is applicable. Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Maximilian Roos
|
7fe28bf574 |
Consolidate benchmark fixtures and harness (#3669)
Benchmark targets had each reimplemented temporary-repository ownership, linked-worktree paths, subprocess isolation, and warm/cold loops. This adds a shared `FixtureRepo` and canonical command/cache helpers while leaving each scenario-specific workload and destructive lifecycle explicit. All 41 existing Criterion IDs, fixture dimensions, command arguments, and cache ordering are preserved. The completion benchmark now also fails when its measured subprocess fails instead of silently accepting the result. Tests: `cargo run -- hook pre-merge --yes`; `cargo test -p wt-perf`; `cargo check --benches -p worktrunk -p wt-perf`; representative Criterion test-mode runs for completion, list, first-output, remove, and prune. > _This was written by Claude Code on behalf of max_. |
||
|
dependabot[bot]
|
62cc0af7f8 |
chore: bump taiki-e/install-action from 2.85.3 to 2.85.4 (#3659)
Bumps [taiki-e/install-action](https://github.com/taiki-e/install-action) from 2.85.3 to 2.85.4. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/taiki-e/install-action/releases">taiki-e/install-action's releases</a>.</em></p> <blockquote> <h2>2.85.4</h2> <ul> <li> <p>Update <code>uv@latest</code> to 0.11.33.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.15.</p> </li> <li> <p>Update <code>biome@latest</code> to 2.5.6.</p> </li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/taiki-e/install-action/blob/main/CHANGELOG.md">taiki-e/install-action's changelog</a>.</em></p> <blockquote> <h2>[2.85.4] - 2026-07-29</h2> <ul> <li> <p>Update <code>uv@latest</code> to 0.11.33.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.15.</p> </li> <li> <p>Update <code>biome@latest</code> to 2.5.6.</p> </li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/taiki-e/install-action/commit/065d6a08a14e61e89fb0a4c10eecdbdef39c7d8e"><code>065d6a0</code></a> Release 2.85.4</li> <li><a href="https://github.com/taiki-e/install-action/commit/971197ad86f8f6f161d7a8d91f1d711c4c21f628"><code>971197a</code></a> Update <code>uv@latest</code> to 0.11.33</li> <li><a href="https://github.com/taiki-e/install-action/commit/3fc72354cdfd0f85327736793faab9b90a6282bb"><code>3fc7235</code></a> Update syft manifest</li> <li><a href="https://github.com/taiki-e/install-action/commit/7e230c19cf947f65a34e4e6f737c5f594c6779ba"><code>7e230c1</code></a> Update sccache manifest</li> <li><a href="https://github.com/taiki-e/install-action/commit/51f77e860854ede202e22ff29a94094601feca62"><code>51f77e8</code></a> Update <code>mise@latest</code> to 2026.7.15</li> <li><a href="https://github.com/taiki-e/install-action/commit/87ddca437422cbc964934ac7b2d8423c1838090a"><code>87ddca4</code></a> Update <code>biome@latest</code> to 2.5.6</li> <li>See full diff in <a href="https://github.com/taiki-e/install-action/compare/v2.85.3...v2.85.4">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
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_. |
||
|
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_. |
||
|
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>
|
||
|
Maximilian Roos
|
cfd6f099bc |
fix(codex): clear activity marker on session end (#3660)
Codex now exposes `SessionEnd`, but Worktrunk’s Codex plugin only returned the activity marker to idle at turn end. This adds a main-session exit hook that clears the marker, using Codex’s three-second maximum hook timeout so cleanup has the best chance to complete without delaying shutdown. The CLI help, plugin-layout guidance, user documentation, generated mirrors, metadata test, and help snapshot now describe and verify the complete Codex lifecycle. Tested with `cargo run -- hook pre-merge --yes` (4,564 tests passed; 1 skipped). > _This was written by Claude Code on behalf of max_. |
||
|
Maximilian Roos
|
66810a3115 |
refactor(remove): move the stale-entry prune from planning to execution (#3658)
`prepare_worktree_removal` pruned stale worktree metadata *while
planning* the missing-directory fallback, so every "validate first" path
quietly mutated: a refused `wt remove` of the default branch still
pruned its entry, `wt remove` pruned before its approval prompt, and the
picker pruned on skim's event loop during row validation. This PR makes
planning a pure read: the `BranchOnly` plan carries the entry as
`prune_entry: Option<PathBuf>`, and `handle_branch_only_output`
unregisters it at execution — before the branch deletion that
`should_keep`/CAS may still decline.
**For the reviewer:**
- `src/commands/repository_ext.rs` — the fallback records the path
instead of calling `prune_worktree_entry`; the planning-is-pure contract
is now on the trait doc.
- `src/commands/worktree/types.rs` — `pruned: bool` → `prune_entry:
Option<PathBuf>`, with the TOCTOU note: `git worktree remove` validates
before touching anything, so a directory that reappears between plan and
execution is handled by git (clean reconnected worktree removed, dirty
or foreign one refuses loudly). JSON output shape unchanged (`"pruned":
prune_entry.is_some()`).
- `src/output/handlers.rs` — execution performs the prune, first and
unconditionally.
- `src/commands/step/prune.rs` (net −48 lines) — with planning pure, the
scan plans every source (it already backs `--dry-run`), which deletes
the downstream prediction machinery: `try_remove`'s re-plan branch,
`removal_needs_write`'s plan-less arm, the `deletes_branch` sibling
heuristic, `Candidate::remove_target`, and `RemovalContext`'s
`worktrees`/`snapshot` fields. The approval gate now reads the plan
instead of the candidate kind, so a stale entry whose directory still
exists gets the same `(approval required)` skip as any other worktree
removal. Detached stale entries keep their direct prune in `try_remove`
— `prepare` structurally can't plan a branchless missing-directory
entry.
**Behavior changes** (all in the safer direction):
1. A refused or declined removal no longer prunes metadata as a side
effect — the accepted snapshot change is this exact case: `wt remove
main` (refused) used to prune the entry, so the follow-up `-D` printed
"No worktree found"; now the `-D` run prunes it itself ("✓ Pruned stale
worktree & removed branch main").
2. A failing metadata prune (`git worktree remove` erroring at
execution) surfaces as a removal failure and aborts prune's queue,
instead of being silently skipped. Plan-time ineligibility (dirty,
default-branch guard) still skips silently, as for every other source.
3. `--dry-run` now plans stale entries through the same gate as
everything else; a dirty prunable-but-present entry is correctly omitted
from the preview.
**Testing:** `test_prune_surfaces_failing_metadata_prune` pins the
flipped failure contract (entry and branch intact after a failed prune,
non-zero exit); `test_prune_dry_run_leaves_stale_entry_registered` pins
planning purity so a mutation creeping back into `prepare` fails the
preview test; the metadata-concurrency barrier test (both `--foreground`
variants) carried over unchanged. The reoccupied-path case where a
*live-looking* entry's directory is replaced by a foreign repo is out of
scope here — it's the rename-first execution path, tracked separately.
> _This was written by Claude Code on behalf of max_
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
dependabot[bot]
|
f1923e9344 |
chore: bump taiki-e/install-action from 2.85.2 to 2.85.3 (#3649)
Bumps [taiki-e/install-action](https://github.com/taiki-e/install-action) from 2.85.2 to 2.85.3. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/taiki-e/install-action/releases">taiki-e/install-action's releases</a>.</em></p> <blockquote> <h2>2.85.3</h2> <ul> <li> <p>Update <code>xh@latest</code> to 0.26.2.</p> </li> <li> <p>Update <code>ubi@latest</code> to 0.10.0.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.14.</p> </li> <li> <p>Update <code>martin@latest</code> to 1.13.0.</p> </li> <li> <p>Update <code>cargo-shear@latest</code> to 1.13.3.</p> </li> <li> <p>Update <code>cargo-binstall@latest</code> to 1.21.1.</p> </li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/taiki-e/install-action/blob/main/CHANGELOG.md">taiki-e/install-action's changelog</a>.</em></p> <blockquote> <h2>[2.85.3] - 2026-07-28</h2> <ul> <li> <p>Update <code>xh@latest</code> to 0.26.2.</p> </li> <li> <p>Update <code>ubi@latest</code> to 0.10.0.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.14.</p> </li> <li> <p>Update <code>martin@latest</code> to 1.13.0.</p> </li> <li> <p>Update <code>cargo-shear@latest</code> to 1.13.3.</p> </li> <li> <p>Update <code>cargo-binstall@latest</code> to 1.21.1.</p> </li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/taiki-e/install-action/commit/18b1216eba7f8039b0f8d131d5473787f0edce68"><code>18b1216</code></a> Release 2.85.3</li> <li><a href="https://github.com/taiki-e/install-action/commit/3a7eb9d7de04335647f871d88d8831485be842be"><code>3a7eb9d</code></a> Update <code>xh@latest</code> to 0.26.2</li> <li><a href="https://github.com/taiki-e/install-action/commit/3d4a0c1c709bc0f907c2a23b1a17cf6296b08298"><code>3d4a0c1</code></a> Update uv manifest</li> <li><a href="https://github.com/taiki-e/install-action/commit/9e4a53cd83bbbd84b17bc14008d4b067b9e99edb"><code>9e4a53c</code></a> Update <code>ubi@latest</code> to 0.10.0</li> <li><a href="https://github.com/taiki-e/install-action/commit/9f2f6a3c937466e1abfae58f471a04b8835bc6de"><code>9f2f6a3</code></a> Update <code>mise@latest</code> to 2026.7.14</li> <li><a href="https://github.com/taiki-e/install-action/commit/e026bd26eeb6a9542c62d43e1f98ced8d56ac346"><code>e026bd2</code></a> Update <code>martin@latest</code> to 1.13.0</li> <li><a href="https://github.com/taiki-e/install-action/commit/f72efa0e99d8d9c15bf7fa6f14f0d4eb369fd084"><code>f72efa0</code></a> Update <code>cargo-shear@latest</code> to 1.13.3</li> <li><a href="https://github.com/taiki-e/install-action/commit/d19664f75e59dd9c49e306b357f78fcb72cd2711"><code>d19664f</code></a> Update <code>cargo-binstall@latest</code> to 1.21.1</li> <li><a href="https://github.com/taiki-e/install-action/commit/4d9bbfb56af57e022493493eecb97d07bf4fddd5"><code>4d9bbfb</code></a> Update biome manifest</li> <li>See full diff in <a href="https://github.com/taiki-e/install-action/compare/v2.85.2...v2.85.3">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Maximilian Roos
|
1b35950a9c |
test: simplify the integration suite (#3657)
This reduces duplicated and false-confidence integration coverage while preserving the suite's semantic and user-facing contracts. ## What changed - Replaces three overlapping list-layout suites with two representative CLI integrations, leaving exhaustive geometry at the direct layout layer. - Groups Git error render variants into labeled family snapshots and removes command-by-shell wrapper cross-products while retaining shell-specific conformance and regression cases. - Updates test-authoring guidance around boundary choice, minimal contrasts, and PTY use, and runs local and CI coverage through Nextest isolation. ## Results - Test catalog: 4,642 to 4,562 - Snapshots: 1,193 to 1,131 - Warm all-feature runtime: 84.70s to 78.17-80.35s - Full coverage: 97.32% of lines ## Testing - `cargo run -- hook pre-merge --yes` - `cargo llvm-cov nextest --features shell-integration-tests --summary-only` > _This was written by Claude Code on behalf of max_. |
||
|
Maximilian Roos
|
5d106aca90 |
perf(prune): fan out the metadata prunes with everything else (#3655)
Follow-up to #3650, which left a `TODO(prune)` behind. `wt step prune` held two candidate shapes on the write side of its scan lock: stale-detached entries, and the plan-less `Prunable` candidates that prune while planning. The stated reason was that they "don't race sibling prunes", which was about the repo-wide `git worktree prune` those paths used to call. #3650 replaced that with `Repository::prune_worktree_entry(path)`, so each call now names its own entry and the exclusion describes nothing. Both shapes join the read-side fan-out. What stays on the write side is the work that runs arbitrary commands or owns the terminal: hook-bearing candidates, `--foreground` ones, and the deferred current worktree. ## The plan-less arm `removal_needs_write` runs before a plan-less candidate has planned, so it can no longer read the answer off the plan. It asks the candidate's own path instead. That matters because git marks an entry prunable off its `gitdir` file, not off the worktree directory: an entry whose directory is still present plans a full worktree removal, hooks and spinner and all, and has to take the write side. The old `None => true` covered that by accident. ## What I verified rather than inherited The lock exists for a Windows `.git/config` race (#2801), and the spec claimed from inode-watching that `git worktree remove` does not touch the file. Re-checked at the write-attempt level, which is stronger: with `.git/config` made immutable, `git branch -D` reports `error: could not write config file .git/config: Operation not permitted` and both `git worktree remove` forms (stale entry and live worktree) succeed silently. The sharper question was `delete_worktrees_dir_if_empty` in git's `builtin/worktree.c`, which rmdir's `.git/worktrees` when the last entry goes. Two concurrent removals can interleave there, but the rmdir only succeeds on an already-empty directory. Every removal enumerates the directory before deleting its own entry, and every per-worktree command in the chain (`git status`, the fsmonitor stop) runs while that worktree is still registered, so an emptied directory has no in-flight reader left to strand. 720 concurrent scoped removals and 60 runs of the real binary over a fixture that empties the directory every time: no failures. One thing that race can still break is a `git worktree list` in another process, which fatals with `Invalid path '.../.git/worktrees'` between its readdir and its realpath (reproduced at roughly 13% of iterations under load). That is git's own race and it holds however wt schedules its removals, so it is recorded in the `prune_worktree_entry` spec rather than treated as a reason to keep the exclusion. ## Test `test_prune_metadata_removals_run_concurrently` follows the existing `test_prune_removals_run_concurrently` pattern: four stale entries (two carrying a branch, two detached) rendezvous inside a `git` shim that holds each `worktree remove` until all four have started. Serialized, each waits out the shim's 15 s timeout and drops a sentinel the test asserts absent. Against the pre-change behavior it fails in 53.7 s with three timeouts; on this branch it passes in 1.1 s. The stale doc comments went with it, including the `TODO(prune)` marker and the claim that `Prunable` candidates plan in `try_remove` because the mutation "must stay serialized" (the real reason is that the same scan backs `--dry-run`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
7d5befa90e |
docs(release): tag the tip of main the drift check just verified (#3653)
Step 12 of the release skill told the runner to expect a single line from `git log <cut-from>..origin/main`, then to tag whatever a bare `gh pr view --json mergeCommit` resolved. Neither claim survives the step's own remedy for drift. **The check can't clear after its own remedy.** When a commit lands during the release PR's CI wait, step 12's fix is a follow-up squash PR adding changelog coverage. That adds both the drifted commit and the follow-up's own squash commit to the cut-from range, so the list is permanently longer than one line and "re-run the check" has no exit condition. In the 0.70.0 release the range held four lines at tag time. **The tag target was underdetermined.** `gh pr view` with no argument resolves a pull request by head branch, and the release worktree's branch outlives the release. Run in that worktree today it selects #3650, an unrelated fix that reused the branch after v0.70.0 shipped. At tag time it would have landed on the follow-up, but only because the follow-up happened to be opened from the same branch and merged last. The check now tests coverage instead of length: clean means the changelog at `origin/main` documents every user-facing commit in the range, however long the list has grown. The tag then targets `origin/main`, the same ref the check just read, so the two cannot name different commits. That is the right commit both when nothing drifted (the release squash commit) and when the changelog had to grow (the follow-up's squash commit), and it keeps the tag reachable from `main`, the guarantee the old parenthetical about the local branch HEAD carried. The target matters beyond bookkeeping because `release.yaml` builds the binaries and the GitHub release notes from the tree at the tag. The v0.70.0 release body carries the entries that exist only in the follow-up commit, since that is what was tagged by hand. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
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>
|
||
|
Maximilian Roos
|
3c887a14ce |
fix(picker): stop underlining links the picker can't carry (#3651)
The picker rendered its rows with hyperlinks enabled and then stripped the OSC 8 escapes afterwards, keeping the underline. A row showed an underlined `#3604` and `:11486` with nothing behind either. This is the follow-up to #3643, which established the "every link is underlined" policy; the picker was the one place the policy was being asserted about text that wasn't a link. The cause was one boolean answering two questions. `supports_hyperlinks(Stream::Stdout)` decided both whether the terminal draws OSC 8 (which is what lets a dev-server URL collapse to `:port`, since the URL rides inside the escape) and whether this render's output delivers the escape intact (which the picker answers differently, because skim mangles it). Each cell called the probe independently, so a row's cells could disagree, and the fix for one cell wouldn't reach the other. `LinkStyle` now answers both once per render: `Linked` (underlined OSC 8 link), `Unlinked` (the same short text, no escape and no underline) and `Expanded` (no OSC 8 in the terminal, so a dev-server URL prints in full and stays copyable). It lives on `LayoutConfig` and rides into `ColumnGrid`, so `--prs` rows rendered off-thread present references exactly as the worktree rows beside them do rather than hardcoding `false`. `Destination` pairs it with the width because one fact decides both: `wt list` writes to the terminal and gets the full width plus intact escapes, while the picker gets a narrower list (skim keeps the rest for the preview pane) and no escapes. The four post-hoc `strip_osc8_hyperlinks` calls in the picker are gone, since nothing emits links there to strip. Two things fell out. `render_cell` was carrying four `LayoutConfig` fields as separate arguments; it now takes the layout, which is also how it got back under clippy's 7-argument cap without a suppression. And the "collapse to `:port`" decision stayed tied to the terminal probe rather than to link emission, so the picker's URL column keeps its narrow form instead of expanding to the full URL. ## Reviewer notes `src/commands/list/layout.rs` holds `LinkStyle` and `Destination` with the rationale; `src/commands/list/collect/mod.rs` is where the picker and `wt list` diverge, keyed on the same `progressive_handler.is_some()` the surrounding picker forks already use. ## Testing `test_link_markup_is_uniform_across_a_row` renders one item under both styles and asserts the CI reference and the dev-server port agree: both underlined links under `Linked`, both plain with no escape and no underline under `Unlinked`. `test_format_cell` additionally pins that `Expanded` and `Unlinked` read the same for a PR reference, which has no URL to fall back on. Beyond the suite, checked against a scratch repo with a URL template: `wt list` emits the dim-plus-underline-plus-link form closing with `[24m` so the dim survives, and the picker's rows render `:12107` with no escape and no underline. The port still collapses in the picker, which was the regression risk worth confirming. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
Worktrunk Bot
|
13bc27e70a |
fix(shell): preview and confirm legacy-file removal on shell install (#3648)
## Problem
`wt config shell install` reclaims its own legacy wrapper paths after
installing to the canonical location — the fish `conf.d/{cmd}.fish`
(#566) and stranded nushell autoload copies (#2878). That reclaim is
deliberate and documented, but the deletion never appeared in anything
the user saw *before* it happened (#3644):
- `--dry-run` returned an empty `legacy_cleanups`, so a preview run
never mentioned the file it would remove.
- The confirmation prompt was passed only the files to *add* — the
legacy removals weren't in the `?` preview.
- When everything was already configured, the cleanup ran and returned
**without prompting at all**, deleting a hand-written `conf.d/wt.fish`
as a silent side effect.
Removals were reported after the fact (`Removed <old> (deprecated; now
using <new>)`), so nothing was silent — but it was after the fact, and
repo policy is that destructive operations get explicit consent.
## Solution
The two cleanup helpers now take a `dry_run` flag: with it set, the same
detection runs but nothing is removed, so the paths a real install
*would* delete can be computed up front. `handle_configure_shell`
collects that list once and threads it into `show_install_preview` and
`prompt_for_install`, so both `--dry-run` and the `?` preview render a
`Will remove <old> (deprecated; now using <new>)` line — the
after-the-fact message in the future tense.
The already-configured branch now prompts (`Remove deprecated shell
integration files?`) when there is anything to remove, and preserves the
file on decline. `--yes` still skips the prompt and removes as before,
so no existing non-interactive flow changes.
## Testing
- New `test_configure_shell_fish_dry_run_previews_legacy_removal` —
`--dry-run` now names the legacy `conf.d` removal on stdout and leaves
the file in place.
- New
`test_configure_shell_fish_legacy_removal_declined_when_already_configured`
— declining the prompt in the already-configured branch exits non-zero
and preserves the file (the core consent gap).
- The existing
`test_configure_shell_fish_dry_run_does_not_delete_legacy` snapshot was
updated to include the new `Will remove …` preview line; its "must NOT
delete" file assertion still holds.
- Verified manually end-to-end (dry-run preview + declined removal both
preserve the file), and ran the full `configure_shell`,
`shell_integration_prompt`, and nushell test suites plus clippy and the
doc-sync test.
---
Closes #3644 — automated triage
---------
Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
|
||
|
dependabot[bot]
|
5e85e8af71 |
chore: bump the patch group with 4 updates (#3625)
Bumps the patch group with 4 updates: [clap](https://github.com/clap-rs/clap), [serde_json](https://github.com/serde-rs/json), [ignore](https://github.com/BurntSushi/ripgrep) and [tokio](https://github.com/tokio-rs/tokio). Updates `clap` from 4.6.2 to 4.6.3 <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/clap-rs/clap/releases">clap's releases</a>.</em></p> <blockquote> <h2>v4.6.3</h2> <h2>[4.6.3] - 2026-07-20</h2> <h3>Fixes</h3> <ul> <li><em>(derive)</em> Allow <code>"literal".function()</code> as attribute values</li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/clap-rs/clap/blob/master/CHANGELOG.md">clap's changelog</a>.</em></p> <blockquote> <h2>[4.6.3] - 2026-07-20</h2> <h3>Fixes</h3> <ul> <li><em>(derive)</em> Allow <code>"literal".function()</code> as attribute values</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/clap-rs/clap/commit/7e0bccab8cf7be047fc84d804d19c7b30715d3fb"><code>7e0bcca</code></a> chore: Release</li> <li><a href="https://github.com/clap-rs/clap/commit/0f09905658535c4b3a92afc037e8970959f55559"><code>0f09905</code></a> docs: Update changelog</li> <li><a href="https://github.com/clap-rs/clap/commit/970695102173e40bddb0c95b0a3f6fb216f2eb19"><code>9706951</code></a> Merge pull request <a href="https://redirect.github.com/clap-rs/clap/issues/6353">#6353</a> from truffle-dev/feat-completer-arg-index</li> <li><a href="https://github.com/clap-rs/clap/commit/ac0d148f7e21068fd1f544230456f30c95311f78"><code>ac0d148</code></a> feat(complete): Index-aware ValueCompleter</li> <li><a href="https://github.com/clap-rs/clap/commit/1565a3cbb411dedc410154fca0de7ec445fcdb08"><code>1565a3c</code></a> test(complete): Cover indexed multi-value case</li> <li><a href="https://github.com/clap-rs/clap/commit/48fc10ae88e4d723fd581745cbbff40eea9ebc88"><code>48fc10a</code></a> Merge pull request <a href="https://redirect.github.com/clap-rs/clap/issues/6350">#6350</a> from stefankreutz/missing_docs</li> <li><a href="https://github.com/clap-rs/clap/commit/7d4c353d5b843ac3b45473172ebdf9c80e89b697"><code>7d4c353</code></a> docs: Hint at clippy's missing docs lint</li> <li><a href="https://github.com/clap-rs/clap/commit/f6a6701e60f372c2104c6aca413c86b996e3e026"><code>f6a6701</code></a> chore(deps): Update Rust Stable to v1.95 (<a href="https://redirect.github.com/clap-rs/clap/issues/6347">#6347</a>)</li> <li><a href="https://github.com/clap-rs/clap/commit/ac5fda6a799e4c640d671edd1111d4a5e723dc1a"><code>ac5fda6</code></a> chore: Release</li> <li><a href="https://github.com/clap-rs/clap/commit/b73c627638fb8d03497079408f5d684f281d132d"><code>b73c627</code></a> docs: Update changelog</li> <li>Additional commits viewable in <a href="https://github.com/clap-rs/clap/compare/clap_complete-v4.6.2...clap_complete-v4.6.3">compare view</a></li> </ul> </details> <br /> Updates `serde_json` from 1.0.150 to 1.0.151 <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/serde-rs/json/releases">serde_json's releases</a>.</em></p> <blockquote> <h2>v1.0.151</h2> <ul> <li>Add RawValue::from_string_unchecked (<a href="https://redirect.github.com/serde-rs/json/issues/1331">#1331</a>, thanks <a href="https://github.com/WonderLawrence"><code>@WonderLawrence</code></a>)</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/serde-rs/json/commit/de8500740cdcabffb9734f503e4889def823cf10"><code>de85007</code></a> Release 1.0.151</li> <li><a href="https://github.com/serde-rs/json/commit/3b2b3c5f28c20ed988bd081a4147c535e7e65c74"><code>3b2b3c5</code></a> Merge pull request <a href="https://redirect.github.com/serde-rs/json/issues/1331">#1331</a> from WonderLawrence/rawvalue-from-string-unchecked</li> <li><a href="https://github.com/serde-rs/json/commit/0406d96860e9d8b9252e2002fa3e626ae48ca1b0"><code>0406d96</code></a> Debug-assert well-formedness and no-whitespace in from_string_unchecked</li> <li><a href="https://github.com/serde-rs/json/commit/cf16f75d81e28c723323bfc60a68fc02d2994fff"><code>cf16f75</code></a> Add RawValue::from_string_unchecked</li> <li><a href="https://github.com/serde-rs/json/commit/827a315bf2198558f0325b07bcc1e2cd973aba2f"><code>827a315</code></a> Update actions/upload-artifact@v6 -> v7</li> <li><a href="https://github.com/serde-rs/json/commit/cea36a5c017ebffdeb95d0cd0f1aad473bfab758"><code>cea36a5</code></a> Update actions/checkout@v6 -> v7</li> <li>See full diff in <a href="https://github.com/serde-rs/json/compare/v1.0.150...v1.0.151">compare view</a></li> </ul> </details> <br /> Updates `ignore` from 0.4.30 to 0.4.31 <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/BurntSushi/ripgrep/commit/59e318f5ace48db54f37bb67c152535bc17fa153"><code>59e318f</code></a> ignore-0.4.31</li> <li><a href="https://github.com/BurntSushi/ripgrep/commit/a9dc2228fbddd1d25146f38d0926830812c97419"><code>a9dc222</code></a> cargo: set <code>rust-version</code> on all crates</li> <li><a href="https://github.com/BurntSushi/ripgrep/commit/be739c7910288905d966f87607b28435d1576295"><code>be739c7</code></a> index: add grep-index crate</li> <li><a href="https://github.com/BurntSushi/ripgrep/commit/d958105d412b6a1ad014cb0e5832a9b029fa4977"><code>d958105</code></a> cargo: add new build-time <code>unstable-index</code> feature</li> <li>See full diff in <a href="https://github.com/BurntSushi/ripgrep/compare/ignore-0.4.30...ignore-0.4.31">compare view</a></li> </ul> </details> <br /> Updates `tokio` from 1.53.0 to 1.53.1 <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/tokio-rs/tokio/releases">tokio's releases</a>.</em></p> <blockquote> <h2>Tokio v1.53.1</h2> <h1>1.53.1 (July 20th, 2026)</h1> <h3>Fixed</h3> <ul> <li>signal: restore MSRV by removing <code>OnceLock::wait</code> from the Windows handler (<a href="https://redirect.github.com/tokio-rs/tokio/issues/8300">#8300</a>)</li> </ul> <h3>Fixed (unstable)</h3> <ul> <li>time: fix alt timer cancellation and insertion race (<a href="https://redirect.github.com/tokio-rs/tokio/issues/8252">#8252</a>)</li> </ul> <h3>Documented</h3> <ul> <li>runtime: remove dead link definition in Runtime::block_on (<a href="https://redirect.github.com/tokio-rs/tokio/issues/8301">#8301</a>)</li> </ul> <p><a href="https://redirect.github.com/tokio-rs/tokio/issues/8252">#8252</a>: <a href="https://redirect.github.com/tokio-rs/tokio/pull/8252">tokio-rs/tokio#8252</a> <a href="https://redirect.github.com/tokio-rs/tokio/issues/8300">#8300</a>: <a href="https://redirect.github.com/tokio-rs/tokio/pull/8300">tokio-rs/tokio#8300</a> <a href="https://redirect.github.com/tokio-rs/tokio/issues/8301">#8301</a>: <a href="https://redirect.github.com/tokio-rs/tokio/pull/8301">tokio-rs/tokio#8301</a></p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/tokio-rs/tokio/commit/75fef53d0a8590c2d1dbb63672aa7b7d1ef51155"><code>75fef53</code></a> chore: prepare Tokio v1.53.1 (<a href="https://redirect.github.com/tokio-rs/tokio/issues/8303">#8303</a>)</li> <li><a href="https://github.com/tokio-rs/tokio/commit/ae9d01121377cdbef32b9d5e8559843cce9f927e"><code>ae9d011</code></a> signal: restore MSRV by removing OnceLock::wait from the Windows handler (<a href="https://redirect.github.com/tokio-rs/tokio/issues/8300">#8300</a>)</li> <li><a href="https://github.com/tokio-rs/tokio/commit/eb4988dc2ecb85d2617971fbbabc84938c141bfd"><code>eb4988d</code></a> time: fix the loom test of the race between cancellation/insertion (<a href="https://redirect.github.com/tokio-rs/tokio/issues/8302">#8302</a>)</li> <li><a href="https://github.com/tokio-rs/tokio/commit/91d3b4c0bccf2234fc3ed19e605e2cd402f19437"><code>91d3b4c</code></a> time: fix alt timer cancellation and insertion race (<a href="https://redirect.github.com/tokio-rs/tokio/issues/8252">#8252</a>)</li> <li><a href="https://github.com/tokio-rs/tokio/commit/a46338401b9e0ffc9bd68c31100ee99cee717481"><code>a463384</code></a> runtime: remove dead link definition in <code>Runtime::block_on</code> (<a href="https://redirect.github.com/tokio-rs/tokio/issues/8301">#8301</a>)</li> <li>See full diff in <a href="https://github.com/tokio-rs/tokio/compare/tokio-1.53.0...tokio-1.53.1">compare view</a></li> </ul> </details> <br /> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore <dependency name> major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore <dependency name> minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore <dependency name>` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore <dependency name>` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore <dependency name> <ignore condition>` will remove the ignore condition of the specified dependency and ignore conditions </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Maximilian Roos
|
79824f7122 |
feat(styling): underline every hyperlink (#3643)
The statusline underlined its PR reference but not the dev-server port, so nothing marked the port as clickable: ``` ~/w/worktrunk.test-suite-cpu ↕|💬 ↑17 ↓11 ^+585 -258 #3604 :11486 Fable 5 🌕 30% ``` Both links now route through a shared `worktrunk::styling::hyperlink(url, text)`, which emits the OSC 8 sequence and the underline together. It closes with `[24m` rather than a full reset, so a wrapping color (the CI verdict) or dim (a port nothing answers on) survives the link. The rule is recorded as policy in the `writing-user-outputs` skill. Link text is sized to fit a column (`#3604`, `:11486`) and reads as ordinary content, and color already carries state, so the underline is the only thing marking text as clickable. Text that is not a link stays plain: on a terminal without OSC 8 support, `wt list` still prints the dev-server URL in full, unadorned. Tests: a unit test pins the helper output, and a statusline test asserts both segments carry a helper-built link. Snapshots updated for the reordered escapes. 🤖 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 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
1faa7b9e8b |
docs(changelog): cover #3639 and #3641 in 0.70.0 (#3642)
Two commits landed on `main` while #3640's CI ran, so they ship in 0.70.0 but weren't in its changelog. The release's drift check caught them before the tag. - **#3641** — a user-facing fix: `wt hook show` printed a section heading and nothing else for a hook type declared with an empty command list. Added as its own Fixed entry. - **#3639** — help-text only, no behavior change, correcting `wt config alias dry-run`'s description of the `vars.*` preview. Folded into the existing hook-previews bullet, since it's the same feature's documentation catching up rather than a separate change. No version change; `v0.70.0` is tagged after this lands so the tag covers both. > _This was written by Claude Code on behalf of Maximilian_v0.70.0 |
||
|
Maximilian Roos
|
946e360447 |
Release v0.70.0 (#3640)
Cuts 0.70.0 — version bump plus the changelog for the 45 commits since
v0.69.2.
**Minor bump, not patch.** `cargo semver-checks` reports 6 breaking
library changes (`set_command_timeout` and `ListConfig::task_timeout_ms`
removed, `WorkingTree::stage` arity changed, three enum variants added
to exhaustive enums). Worktrunk ships breaking library changes freely,
but semver still puts a break at minor while pre-1.0.
## Release validation
- Local gate green on the release commit: 4631 tests, lints, doctests.
- `nightly` dispatched on the cut-from tip (`a27cbd42`) for the full
cross-platform suite — `full-tests` green on linux, macOS, and Windows,
plus minimal-versions, nix-flake, crate-build, and the release targets.
## Data-loss surface review
The cumulative diff was audited against the deletion surface. One
deliberate widening, signed off for this release with follow-ups to
file:
- **#3602** removes the content check from `wt config shell install`'s
legacy cleanup, so `conf.d/{cmd}.fish` and the stranded nushell
`{cmd}.nu` are now deleted by path, unread. Only that exact filename is
touched and each removal is reported, but the deletion is absent from
both `--dry-run` and the confirmation prompt, and the already-configured
path skips the prompt entirely.
Also noted, none blocking:
- The `!path.exists()` check precedes the lock guard on the
`Path`/`Current` removal arm, so a locked worktree whose directory is
absent loses its branch. This already governed the branch-targeted route
in v0.69.2; #3533 unified the other arms onto it. The FAQ's "Neither
`git worktree remove` nor `wt remove` (even with `--force`) will delete
them" is absolute where the behavior isn't.
- On the default background removal path, `ensure_clean` and
`stop_fsmonitor_daemon` swapped order, so the safety gate is now
answered by the live fsmonitor daemon rather than a full re-stat.
Bounded by trash staging with 24-hour retention, and the foreground and
picker paths were already daemon-served.
Net *improvements* to the same surface: shared-branch retention across
remove/prune/merge (#3533), outcome-accurate removal reporting (#3633,
#3637), and the removal of the thread-local command timeout that could
kill in-flight git commands on worker threads (#3615).
## Changelog accuracy
Entries were verified against the actual diffs rather than commit
messages, which corrected several drafts: the prune figures were one PR
stale (~2.9 s → the real ~0.6 s), `wt merge` takes no worktree argument
so it only gained the retention half of #3533, the Azure DevOps report
is behind `--full`, #3608 never touched `nightly.yaml`, and #3601
inverted what the FAQ change actually said. Two omissions were added —
the `install-statusline` foreign-statusline fix (#3595) and the shipped
`/wt-switch-create` skill change (#3636).
> _This was written by Claude Code on behalf of Maximilian_
|
||
|
Maximilian Roos
|
0e7454890e |
fix(hooks): an empty command list no longer leaves a bare heading (#3641)
`wt hook show` decided whether a section had printed anything from
whether the config held an entry for each hook type, not from whether
any command was rendered. A hook type declared with an empty command
list has an entry but no commands, so the section printed its heading
and then stopped:
```console
$ cat .config/wt.toml
post-switch = []
$ wt hook show
PROJECT HOOKS @ /path/.config/wt.toml
```
Both sections carried it, since the loop and the `(none configured)`
fallback were duplicated in `render_user_hooks` and
`render_project_hooks`. `render_hook_commands` now reports whether it
wrote any rows, and the loop plus fallback live once in
`render_hook_section`, which both callers delegate to. The flag means
"something was printed", which is what the `(none configured)` line
claims.
The execution path was already correct: an empty list announces nothing,
and the JSON output omits it. Verified against the built binary across
four cases (project-only empty, user-only empty, an empty list alongside
a real hook, and a filter naming the empty type), and pinned by
`test_hook_show_empty_command_lists`, which covers both the user and
project halves. Reverting the flag to its old meaning fails the test on
both.
Also corrects a docstring in the same file that still described the
preview's pre-#3638 behavior ("shows a `vars.*` template raw"), the same
drift #3639 fixed for `wt config alias dry-run`.
> _This was written by Claude Code on behalf of max_
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
a07fe6d6f2 |
docs(alias): correct the vars.* claim in alias dry-run (#3639)
#3638 changed the preview renderer so a template expands everything around a `{{ vars.<key> }}` reference and leaves only that reference literal. `wt config alias dry-run` shares that renderer, but its `--help` text and the `handle_alias_dry_run` docstring still described the previous all-or-nothing behavior, where a single `vars.` token left the whole template raw. This corrects both. The docstring's mention of a separate syntax-validation step goes with them: expansion reports syntax errors itself, so there is no longer a distinct validation pass to describe. No behavior change. The `dry-run` long help isn't rendered into any generated doc or snapshot, so no mirrors move with it. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
a27cbd42be |
feat(skill): create the worktree by name, fall back to a path (#3636)
Closes #3555. `/wt-switch-create` now creates through `EnterWorktree({name})` for the common invocation, which asks the user to confirm nothing, and falls back to `wt` plus a path entry for the cases that route can't serve. The confirmation Claude Code 2.1.206 added fires on a call that passes `path` and targets a worktree outside `.claude/worktrees/`. `{name}` passes no `path`, so it never fires. With the plugin installed, `{name}` runs worktrunk's own `WorktreeCreate` hook (`wt switch --create`), so the worktree is the one `wt` would have made, and it shows up in `wt list`. ## Routes - **New branch, this repo** → `EnterWorktree({name})`. No confirmation. - **Anything else** → `wt -C <repo> switch --create … --format=json`, then `EnterWorktree({path})`, keeping the outcome handling below. Three things `{name}` cannot do, each announcing its own fallback in the error it returns, so the skill tries the cheap call and reads the result rather than pre-checking: | Case | What `{name}` returns | |---|---| | Repo argument given | no `-C` equivalent; skipped before the call | | Branch already exists | the hook exits nonzero with `✗ Branch <branch> already exists` | | Session already entered a worktree | `Already in a worktree session. Pass `path` to switch into another existing worktree` | ## The tradeoff, taken deliberately A `{name}` worktree the session never touched is removed when the session ends, branch included, through the plugin's `WorktreeRemove` hook (`wt remove`). Verified end to end: create, `/exit`, and neither the worktree nor the branch remains; one untracked file is enough to keep both. That is wanted at this scale, since a research task that wrote nothing leaves nothing to prune, but it does mean "the worktree persists" was no longer true of every route. Cleanup and the rationale now say which worktrees persist; the user docs stay out of the mechanics. ## A bug fixed along the way A user declining the path-entry confirmation returns an ordinary permission denial, which the skill's single rejection branch caught, and that branch's documented recovery is to `cd` into the worktree and work there. So a declined entry became a worktree entered anyway. Three wordings that asked the agent to classify the denial text failed context-blind probes in turn: branches labeled by who refused, "the denial reports a decision", and that plus the verbatim denial string quoted as an example. Entry outcomes now split structurally instead. The tool's own `Cannot enter` errors key the reachability test; a denial of the call stops and asks by default, with one exception: a session with no user to ask (its denial says it couldn't prompt) takes the recovery, since nothing was decided. A recovery that follows a denial invites the model to route any denial into it, so the denial branch leads with stopping. Context-blind agents route both denial directions and the four invocation shapes above through their intended routes. ## Verification All of it re-run live against Claude Code 2.1.220 in scratch repos, since the analysis in #3555 rested on properties pinned to 2.1.173/177 and could not be checked from CI. Two claims died that way: an earlier draft credited the exit removal to the harness when it is worktrunk's own hook, and asserted that a `permissions.deny` rule produces a classifiable denial, when it removes the tool from the session entirely. Also recorded in the rationale: the confirmation offers no always-allow and `permissions.allow` cannot suppress it, it fires in `default`, `acceptEdits`, and `auto` while `bypassPermissions` allows silently, and a session that cannot prompt denies without asking. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Fable 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
9e7ad0144d |
fix(hooks): expand everything but vars.* in a preview (#3638)
Follow-up to #3635, from two reviews that landed after it merged. ## The `vars.*` preview was worse than #3635 claimed `render_template_preview` short-circuits on `template_references_var(template, "vars")`, returning the raw template. So one `vars.` token disabled expansion for the entire command: ``` pre-commit = "deploy --branch={{ branch }} --repo={{ repo }} --env={{ vars.env }}" before: deploy --branch={{ branch }} --repo={{ repo }} --env={{ vars.env }} after: deploy --branch=main --repo=repo --env={{ vars.env }} ``` #3635 described this as "a `vars.*` template renders raw", which is true but understates it: `{{ branch }}` and `{{ repo }}` stopped expanding too, in a command whose whole job is to show the expansion. That short-circuit predates #3635 and has been degrading `wt hook <type> --dry-run` the same way; #3635 only extended it to `wt hook show --expanded`. The fix is at the source rather than at either caller. A preview now injects a stand-in object for `vars` that renders each reference back as itself, nested access included (`{{ vars.config.port }}` round-trips), while every other variable expands normally. `VarsMode::Resolve` keeps execution reading real values from git config; only previews pass `VarsMode::Literal`. A preview also no longer spawns the git read that resolving `vars` required. `vars.*` stays literal on purpose: those values are read when the step runs, after an earlier step in the pipeline may have written them, so a value resolved at preview time can differ from the one the run uses. Nothing covered this, which is why the suite stayed green through the regression. `test_hook_show_expanded_matches_dry_run` now sets a var and asserts the listing and the dry-run both leave it alone while expanding `{{ branch }}` beside it. ## The syntax gate is a type error now #3635 moved the template syntax check out of `prepare_steps` into a free `validate_pipeline_syntax` that both execution funnels had to remember to call. `prepare_steps` now returns a `PreparedPipeline` the caller must resolve: `.validated()` for the paths that run hooks, `.into_unvalidated()` for the listing, which annotates a broken template in place rather than blanking itself. Forgetting is a compile error, the same property `ApprovedHookPlan` gives hook approval. ## Smaller items Four cross-references went stale when the syntax check moved: `PreparedCommand.template`, `validate_template_syntax`, the `switch.rs` skip comment, and `HOOK_INFRASTRUCTURE_VARS` (which still named two deleted functions). The `--expanded` behavior is now documented in the sentence that already owns `{{ vars.<key> }}` semantics, with its three generated mirrors regenerated. `PreparedStep::commands()` replaces two hand-rolled matches in `hooks.rs`. `default_branch` moves inside `build_manual_hook_template_vars` — only the commit-hook arm reads it, and resolving it can cost a `git ls-remote` on a fresh clone, so the other eight hook types no longer pay for it. The listing carries its expansion state in an `Option<String>` instead of re-deriving "was this expanded?" from whether a context exists. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
Maximilian Roos
|
4888d5246f |
refactor(hooks): derive the hook preview from the execution path (#3635)
`wt hook show --expanded` rebuilt the hook pipeline's template context by hand in `expand_command_template`, duplicating what `prepare_steps` already does for what actually runs: the same `build_hook_context` call, the same `hook_type` and `hook_name` inserts, the same `args` default, the same POSIX escape mode. Two copies of one rule, so the preview drifted from the executed command whenever the execution path gained a context key, with nothing to catch it. The listing now prepares its commands through `prepare_steps` itself (`hook_command_rows` in `hook_commands.rs`) and renders them through `render_template_preview`, the renderer `wt hook <type> --dry-run` already used. `prepare_steps` is the sole producer of hook command contexts, so a key added there reaches both with no second edit. ## The `args` divergence The old preview inserted `args = "[]"` unconditionally; `prepare_steps` defaults it only when unset, because manual `wt hook <type>` supplies real args upstream via `extra_vars`. The shared path keeps the conditional default and the listing simply does not supply `args`. The values coincide (a listing has no CLI args to forward, which is exactly what the default encodes), and keeping the conditional form means the rule stays written once, in the place that has a caller who needs the other branch. ## Where the syntax check went `prepare_steps` used to reject an unparsable template, so a pipeline that could not render in full never started. A listing wants the opposite: `wt hook show` is what you run when your hooks are broken, so it annotates the bad template in place and shows the rest. That check is an execution policy rather than part of building a command, so it moved out of `prepare_steps` into `validate_pipeline_syntax`, called by the two funnels every hook-running path goes through: `prepare_and_check` (foreground, background, dry-run, filtered) and `render_planned` (the plan-backed hooks behind `execute_planned_hook` and `register_planned`). Both are mutation-verified: removing either call fails a test. A newtype that made forgetting the gate a compile error would be stronger, but it threads a wrapper through `SourcedStep`, `ForegroundStep`, and the background pipeline spec for a guard whose failure mode is degraded fail-fast rather than incorrectness (a syntax error still surfaces when its step renders). ## User-visible changes Preview expansion errors now name the hook (`Failed to expand project:lint: ...`) instead of the generic `hook preview`. A template referencing `vars.*` renders raw in `--expanded`, matching `--dry-run`, where before it resolved against git config at preview time. Raw is the honest preview: those values resolve when the step runs, and an earlier step in the pipeline may write them. ## Tests `test_hook_show_expanded_matches_dry_run` pins the listing and the dry-run to the same rendering of `hook_type`, `hook_name`, and `args`. `test_foreground_pipeline_syntax_error_aborts_before_first_step` pins the relocated gate; it runs through `wt merge`'s pre-commit hooks because `wt hook <type>` cannot reach it (the CLI pre-parses every template for shorthand-argument routing and errors first). Also folded in: adding `source: HookSource` to the listing renderer left `approval_context: Option<(&Approvals, Option<&str>)>` encoding the same user-vs-project discriminator, so both sites now call one `needs_approval`. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
062cf57d8e |
ci(affected): pin cargo-affected to the published 0.3.2 (#3634)
Both `Install cargo-affected` steps in `affected.yaml` installed from git with no rev, so the tool version on any run was whatever had last merged to cargo-affected's default branch. That cut both ways today: an upstream regression made `cargo affected run` abort with `git diff stdout was not valid UTF-8: invalid utf-8 sequence of 1 bytes from index 136455` on all three OSes, turning the advisory legs red repo-wide, and the fix (cargo-affected #69) then arrived the same way. Neither change is visible in this repo's history. cargo-affected publishes to crates.io as of 0.3.1, so it can carry a `version: "=X.Y.Z"` pin like every other `baptiste0928/cargo-install` block here. `=0.3.2` is what the unpinned installs already resolve to, so nothing changes behaviorally today. Two things it buys: - The crate joins the weekly tend pin sweep, which reads `version:` strings and now covers `affected.yaml`. The `cargo-install` action's own drift annotation starts firing for it too. - The existing "bump the `db-v{N}` cache marker if cargo-affected ships an on-disk schema change" rule becomes enforceable. It was unfollowable against a floating version, since the schema could change with no diff to notice. The pin appears twice, once per job, which is a new way to be wrong. A drift between the two hits `migrate_legacy_tables`, which drops and rebuilds the coverage tables rather than erroring, so selection silently degrades while the advisory job stays green. That is recorded in the workflow's cache-contract comment and in the tend skill's bump list. `cargo-affected`'s `rust-version` is 1.94, against this repo's pinned 1.96.0 toolchain. ## Testing The `affected tests (…, advisory)` legs on this PR exercise the change directly: they resolve `=0.3.2` from crates.io rather than from git. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
67945d722a |
fix(hooks): make the warm build and the gate agree on their flags (#3627)
The post-merge warm build and the pre-merge insta gate now agree on RUSTFLAGS and the feature set, so the gate reuses the warm build's artifacts instead of rebuilding from scratch. Both use --all-targets --all-features. |
||
|
Maximilian Roos
|
e91b5fa885 |
perf(prune): parallelize removals on the scan lock's read side (#3631)
`wt step prune` executed removals one at a time under the write side of `check_lock` while the candidate scan fanned out on the read side. On the rust-scale fixture that serialized ~2.6 s of a ~2.9 s live prune. Removals now run concurrently: the main thread streams scan results into a FIFO queue of per-candidate jobs, and a worker pool sized like the rayon pool executes them under the read side of the lock. **Why the read side is safe.** The lock exists for the Windows `.git/config` race (#2801): `git branch -D` rewrites config via lockfile plus atomic rename, and a concurrent reader fails on the rename. The removal chain has since moved to the CAS `git update-ref -d`, and (verified empirically, by inode-watching `.git/config` across each command) neither it nor `git worktree remove` / `git worktree prune` touches config; `branch -D` remains reachable only through arms prune never takes. Concurrent `git worktree prune` runs are idempotent and exit 0, and `update-ref -d` does not consult worktree checkout state, so nothing in the hook-free chain needs exclusivity. The write side remains for the removals that do: hook-bearing candidates (a foreground `pre-remove` body is an arbitrary command with streamed output), `--foreground` candidates (TTY spinner), the fail-hard metadata-pruning candidates (`StaleDetached`, plan-less `Prunable`), and the deferred current worktree. `removal_needs_write` in `src/commands/step/prune.rs` is the decision table; the `check_lock` field doc carries the full analysis. **Output and determinism.** Skip lines ride the same FIFO queue as removals, so `RAYON_NUM_THREADS=1` (one worker) reproduces the exact serial output order the snapshot tests pin; with more workers, whole lines interleave (one `anstream` `eprintln!` writes a line under a single stderr lock). Live `--format=json` output is now sorted by scan index, matching dry-run, with the current worktree still last. The first failing removal flips an abort flag that drains the rest of the queue unexecuted, matching the serial loop's abort-on-first-error. **Measured.** `prune_e2e/live` criterion: ~615 ms to ~398 ms. Rust-scale live one-shot (36 worktrees, 24 candidates, warm): 2.9 s to 0.56 s wall, all 24 removals overlapping in the trace (6.5 s of subprocess time). **Abort semantics.** The first failing removal flips an abort flag: in-flight removals complete (their per-candidate lines already printed), queued ones drain unexecuted, no summary prints, and the first error propagates — the same abort-on-first-error the serial loop had, where an error mid-loop likewise left earlier removals done and later candidates untouched. A mid-scan check error takes the same abort path. **Testing.** New `test_prune_removals_run_concurrently` proves the overlap causally: two orphan-branch removals barrier on each other inside a git shim, and serialized removals trip the shim's timeout sentinel (verified: the test fails under forced `RAYON_NUM_THREADS=1`). `test_prune_removal_failure_aborts_remaining_queue` and `test_prune_concurrent_removal_failures_report_first` pin the abort semantics deterministically (single-worker FIFO, and a barrier forcing two concurrent failures). The config-race canary keeps its role with its premise rewritten: a regression to a config-rewriting deletion is what it would now catch. The deterministic-output snapshot tests pass unchanged. The one patch line no test reaches deterministically is the worker's early `return` when the result receiver is gone (a scan error racing an in-flight removal). Accepted residuals, documented in code: a `packed-refs.lock` contended past git's ~1 s retry budget reads as `RetainedRaced` (fail-closed; unobserved at 24-way concurrency on local disks), and concurrent fail-soft `git worktree prune` races leave at worst stale metadata for the next prune to collect. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
4c845597b2 |
fix(prune, remove, ci): count what a removal took, not what it selected (#3633)
Three follow-ups from the review of #3533, which landed shared-branch retention across the removal surface. Each is small and independent; they share a branch because they came out of the same pass. ## `wt step prune` counted kinds, not outcomes A candidate's kind is what the scan selected, not what the removal took. A branch a sibling worktree still has checked out is retained, so a worktree candidate can take the worktree and leave the branch standing. The summary counted the kind anyway: ```console ○ Worktree directory missing for feature; pruned ↳ branch checked out at ~/code/repo.feature ✓ Pruned 1 branch ← the branch is right there ``` `prune_summary` now counts the planned outcome, so that reads `✓ Pruned 1 worktree` — the stale entry, which is all that went. Both `--format=json` payloads gained `branch_deleted` for the same reason: a consumer reading `{"branch": "feature", "kind": "branch_only"}` would reasonably conclude the branch is gone. The predicate has one home now. `RemoveResult::to_json` already computed `branch_deleted` inline; it moved to `RemoveResult::deletes_branch()` and both callers share it. That incidentally fixes a detached worktree reporting `"branch_deleted": true` beside a null branch — it has no branch to delete. `wt step prune --dry-run` had the same defect and no per-item line to contradict it, so it now predicts retention too. A `Prunable` item has no plan until `try_remove` prunes its stale entry, so the dry run asks `live_sibling_checkout` — the same predicate the plan would. ## `RemoveTarget::Branch` now means "a branch with no worktree" All three callers resolve first and pass `Path` for anything with a worktree, so the arm handling "the branch turned out to have a worktree" was unreachable at selection time — codecov confirmed it never executed. Deleting it would have been enough, but the arm was a live hazard rather than dead weight: the picker builds a fresh `Repository` and re-lists worktrees between selecting a row and removing it, so a `wt switch` in another terminal can give a branch-only row a worktree mid-flight. The old code would then have removed that worktree, from a row that said "delete this branch" — the same defect class #3533 fixed. ```console ✗ Branch feature gained a worktree @ ~/code/repo.feature since it was selected; to remove that worktree, run wt remove ~/code/repo.feature ``` `test_prepare_removal_refuses_branch_that_gained_a_worktree` covers the picker path. I checked it fails when the guard is neutralized rather than passing trivially. ## `affected tests (windows, advisory)` was permanently red git calls a file binary only when it finds a NUL byte in the first 8000, and two of the committed loose objects under `tests/fixtures/*/_git/` are zlib streams small enough to have none. Those diff as text, so `git diff` writes raw deflate into its output, and `cargo affected` aborts decoding it as a string before running a test. Reproduced with `git show <commit> -- <object>` (invalid UTF-8, byte `0x95` at position 5403) and confirmed fixed — the same command now reports `Binary files … differ`. The rest of `_git/` stays text worth reading. ## Testing `prune_summary_counts_a_retained_branch_as_worktree_only` covers the counting; `test_prune_retains_branch_checked_out_in_another_worktree` gained an assertion on the summary line, and I verified it fails when the guard is removed. The six `.snap` changes are all the same added `branch_deleted` key. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
dependabot[bot]
|
21ea6d6f1a | chore: bump skim from 5.1.0 to 5.5.0 (#3626) | ||
|
Maximilian Roos
|
835d5f0cc1 |
perf(switch): compute only the template vars --execute names, and give the context a type (#3628)
`wt switch --execute` built its template context with `referenced:
None`, so every invocation resolved every standard variable whether or
not the command named one. That context feeds `expand_template` and
nothing else: the payload reaches the child as a shell string through
the EXEC directive file (or `sh -c`), never as JSON on stdin, and
`--execute` renders no `-v` variables table. Every unnamed variable was
a git subprocess whose result was discarded.
`var_default_branch` is the one with teeth. On a clone with no
`refs/remotes/origin/HEAD` and no cached `worktrunk.default-branch`, it
falls through to `git ls-remote`, so `wt switch feature -x 'echo hi'`
reached the network for a variable the command never mentions. Traced on
a scratch repo in that state:
| `--execute` | before | after |
| --- | --- | --- |
| `echo hi` | 13 subprocesses, 1 `ls-remote` | 8, none |
| `echo {{ commit }}` | 13, 1 | 9, none |
| `echo {{ default_branch }}` | 13, 1 | 11, 1 (correctly still fetches)
|
`referenced_vars_for_templates` unions minijinja's
`undeclared_variables` across the command and its trailing args, which
are two expansion positions over one context map. It does not error on
an unparsable template: `validate_switch_templates` has already parsed
both positions against `ValidationScope::SwitchExecute` before the
switch runs, and reports a syntax error against the position that
carried it.
## The second commit: why `None` was reachable at all
`build_hook_context` returned a bare `HashMap<String, String>`, so each
of its seven callers had to know four separate things about the result:
how to reborrow it as `HashMap<&str, &str>` for `expand_template` (five
copies), how to serialize it for a child's stdin (four copies, two
carrying the same `expect` string), what to insert into it afterwards,
and whether it had been filtered. The doc comment on that last point
said "hooks pass `None`", which described the callers rather than the
rule, so `--execute` inherited `None` by sitting next to code that
needed it.
`TemplateContext` (`src/config/expansion.rs`) owns all of it, with
`expand` / `to_json` / `insert` / `get` / `remove` as the interface and
the map private. It serializes transparently, so the JSON a hook child
reads on stdin and the `PipelineSpec` the background runner deserializes
are the same flat object as before.
The filter argument becomes `VarScope` rather than
`Option<&BTreeSet<String>>`. `VarScope::All` has to be typed out, and
its doc states the discriminator: `Referenced` when only the templates
read the context, `All` when a child reads the JSON or the command's
output is the variable listing. The same enum replaces the second
`Option<&BTreeSet<String>>` on `format_alias_variables`, which meant the
same thing (which vars render as dim `(unused)`).
Two changes fell out of that rather than being planned. `src/llm.rs` had
its own private `TemplateContext` for LLM prompt data, which would have
made two types of that name in one crate; it is now `PromptContext`. And
`expand_shell_template` shrank to a one-line wrapper but stays, because
it still fixes POSIX for its four callers and converts the error type.
Not included: `render_hook_commands`'s preview path reimplements what
`prepare_steps` does (build context, insert `hook_type`, insert
`hook_name`, default `args`, expand POSIX), so the preview drifts from
what actually runs whenever the execution path gains a context key. That
touches `wt hook` output and wants its own change.
## Testing
`test_switch_execute_computes_only_referenced_vars` reads `var_*` spans
out of the `-vv` `trace.jsonl` and pairs the variable-free case with a
`{{ commit }}` control, so an empty span set cannot pass by the trace
simply missing them. Reverted to `None` it fails with `got
["var_commit", "var_default_branch", "var_primary_worktree",
"var_remote"]`. Two unit tests cover `referenced_vars_for_templates`
directly (union across positions, unparsable template contributes
nothing).
The refactor is covered by the existing suite rather than new tests,
which is the point: 4606 tests pass with no snapshot movement, so no
user-visible output changed.
> _This was written by Claude Code on behalf of max_
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Worktrunk Bot
|
648f1a866e | docs(test): drop stale line-number reference in porcelain-parse test (#3630) | ||
|
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> |
||
|
dependabot[bot]
|
e6ca358846 |
chore: bump clechasseur/rs-cargo from 5.0.6 to 5.0.7 (#3624)
Bumps [clechasseur/rs-cargo](https://github.com/clechasseur/rs-cargo) from 5.0.6 to 5.0.7. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/clechasseur/rs-cargo/releases">clechasseur/rs-cargo's releases</a>.</em></p> <blockquote> <h2>v5.0.7</h2> <p>Patch release with updates to vulnerable dependencies.</p> <h2>What's Changed</h2> <ul> <li>chore(deps): bump fast-xml-parser from 5.9.3 to 5.10.1 in the npm_and_yarn group across 1 directory by <a href="https://github.com/dependabot"><code>@dependabot</code></a>[bot] in <a href="https://redirect.github.com/clechasseur/rs-cargo/pull/427">clechasseur/rs-cargo#427</a></li> <li>fix: update <code>@clechasseur/rs-actions-core</code> to 8.0.3, run <code>npm update</code> to get vulnerability fixes by <a href="https://github.com/clechasseur"><code>@clechasseur</code></a> in <a href="https://redirect.github.com/clechasseur/rs-cargo/pull/429">clechasseur/rs-cargo#429</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/clechasseur/rs-cargo/compare/v5.0.6...v5.0.7">https://github.com/clechasseur/rs-cargo/compare/v5.0.6...v5.0.7</a></p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/clechasseur/rs-cargo/commit/8ccb6817fc46c9af3b058a5a7b31932edb82cf13"><code>8ccb681</code></a> Merge pull request <a href="https://redirect.github.com/clechasseur/rs-cargo/issues/429">#429</a> from clechasseur/fix/update-deps</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/a11eddeaaecf8997cf0bcc3c24becc1f09624173"><code>a11edde</code></a> fix: bump version to 5.0.7</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/76685bb9f52e212a4c00f3ddc44842dc5281ca3d"><code>76685bb</code></a> fix: <code>npm update</code></li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/48ea33b367c4c1b8b6fe7fefc203bce3d4e7158a"><code>48ea33b</code></a> fix: update <code>@clechasseur/rs-actions-core</code> to 8.0.3</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/b5a8184fad32ef9f11f0e29418d46300fa1a1986"><code>b5a8184</code></a> chore(deps): bump fast-xml-parser (<a href="https://redirect.github.com/clechasseur/rs-cargo/issues/427">#427</a>)</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/5b3bb18517d1d95a02ac4aa47ff2a192c63da94a"><code>5b3bb18</code></a> chore(deps): update dependency prettier to ^3.9.6 (<a href="https://redirect.github.com/clechasseur/rs-cargo/issues/422">#422</a>)</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/95bd32da1610f3c33ea573b0248b59d5416eefee"><code>95bd32d</code></a> chore(deps): update dependency oxlint to ^1.74.0 (<a href="https://redirect.github.com/clechasseur/rs-cargo/issues/420">#420</a>)</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/de86f7e954c523dbb840a840196e75d2b3f5a56c"><code>de86f7e</code></a> chore(deps): update actions-rust-lang/setup-rust-toolchain action to v1.17.0 ...</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/24b0f4bdb30da53e2b0bd096f5936f516c683c38"><code>24b0f4b</code></a> chore(deps): update dependency <code>@types/node</code> to ^24.13.3 (<a href="https://redirect.github.com/clechasseur/rs-cargo/issues/423">#423</a>)</li> <li><a href="https://github.com/clechasseur/rs-cargo/commit/3ad8301e71b20505890be273a34f0840a9f7701b"><code>3ad8301</code></a> chore(deps): update actions/setup-node action to v7 (<a href="https://redirect.github.com/clechasseur/rs-cargo/issues/426">#426</a>)</li> <li>See full diff in <a href="https://github.com/clechasseur/rs-cargo/compare/v5.0.6...v5.0.7">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
8ef63150b6 |
chore: bump taiki-e/install-action from 2.85.0 to 2.85.2 (#3623)
Bumps [taiki-e/install-action](https://github.com/taiki-e/install-action) from 2.85.0 to 2.85.2. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/taiki-e/install-action/releases">taiki-e/install-action's releases</a>.</em></p> <blockquote> <h2>2.85.2</h2> <ul> <li> <p>Update <code>prek@latest</code> to 0.4.11.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.13.</p> </li> <li> <p>Update <code>kingfisher@latest</code> to 1.109.0.</p> </li> </ul> <h2>2.85.1</h2> <ul> <li> <p>Update <code>vacuum@latest</code> to 0.30.0.</p> </li> <li> <p>Update <code>uv@latest</code> to 0.11.32.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.12.</p> </li> <li> <p>Update <code>cyclonedx@latest</code> to 0.33.1.</p> </li> <li> <p>Update <code>cargo-neat@latest</code> to 0.5.2.</p> </li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/taiki-e/install-action/blob/main/CHANGELOG.md">taiki-e/install-action's changelog</a>.</em></p> <blockquote> <h2>[2.85.2] - 2026-07-26</h2> <ul> <li> <p>Update <code>prek@latest</code> to 0.4.11.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.13.</p> </li> <li> <p>Update <code>kingfisher@latest</code> to 1.109.0.</p> </li> </ul> <h2>[2.85.1] - 2026-07-25</h2> <ul> <li> <p>Update <code>vacuum@latest</code> to 0.30.0.</p> </li> <li> <p>Update <code>uv@latest</code> to 0.11.32.</p> </li> <li> <p>Update <code>mise@latest</code> to 2026.7.12.</p> </li> <li> <p>Update <code>cyclonedx@latest</code> to 0.33.1.</p> </li> <li> <p>Update <code>cargo-neat@latest</code> to 0.5.2.</p> </li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/taiki-e/install-action/commit/41049aa56687c35e0afa74eed4f09cec4f9afabf"><code>41049aa</code></a> Release 2.85.2</li> <li><a href="https://github.com/taiki-e/install-action/commit/dfcf36552b1b743e910b9b9b6b114a08e56a1e5e"><code>dfcf365</code></a> Update <code>prek@latest</code> to 0.4.11</li> <li><a href="https://github.com/taiki-e/install-action/commit/eea03ccfa855b965a301aa52424915fddc32f855"><code>eea03cc</code></a> Update <code>mise@latest</code> to 2026.7.13</li> <li><a href="https://github.com/taiki-e/install-action/commit/81ca2feb84748ed3fa514707ee1004f4f3ad715a"><code>81ca2fe</code></a> Update martin manifest</li> <li><a href="https://github.com/taiki-e/install-action/commit/cc90ed04bc1a258ea90a83789f24b759a77c9671"><code>cc90ed0</code></a> Update <code>kingfisher@latest</code> to 1.109.0</li> <li><a href="https://github.com/taiki-e/install-action/commit/55639a3362f508fda5154d3451072205f5c32ca6"><code>55639a3</code></a> Update cargo-shear manifest</li> <li><a href="https://github.com/taiki-e/install-action/commit/3d7d7cd5ac7f994c1892ae0c06165095b9139094"><code>3d7d7cd</code></a> Release 2.85.1</li> <li><a href="https://github.com/taiki-e/install-action/commit/d09ccb4fe2105ac9ead6376f7a423ff88defeb57"><code>d09ccb4</code></a> Update <code>vacuum@latest</code> to 0.30.0</li> <li><a href="https://github.com/taiki-e/install-action/commit/ac43dee1a92e482c0e244bdb0bb126908159e245"><code>ac43dee</code></a> Update <code>uv@latest</code> to 0.11.32</li> <li><a href="https://github.com/taiki-e/install-action/commit/49b16979f38f30e44b1f68af9115be9a6ffc5215"><code>49b1697</code></a> Update prek manifest</li> <li>Additional commits viewable in <a href="https://github.com/taiki-e/install-action/compare/v2.85.0...v2.85.2">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Maximilian Roos
|
4202cffe61 |
fix(ci): split ci by cadence so coverage uploads on every main commit (#3608)
## What prompted this Getting #3605 green ran into codecov reporting a `base_commit` three commits older than the real merge-base. This audits whether our config causes that. ## The cause Codecov picks a PR's base by walking back to the newest ancestor that has a coverage report. It used the real merge-base for PRs #3480, #3532 and #3602, and a stale one for #3603 and #3605. The difference is whether the merge-base uploaded a report. **29 of the last 40 main commits did not.** `ci` had one concurrency group for main pushes, and GitHub cancels the *pending* run in a group whenever a newer one joins, even with `cancel-in-progress: false`. So the question is how long a run holds the group, and a run isn't done until its slowest job is: | job | duration on main | |-----|------------------| | `fast-checks` | 2 min | | `code-coverage` | 3-4 min | | `test (windows)` | 11 min | | `collect affected coverage (windows)` | 110-129 min | Each main run held the group for ~2 hours, so nearly every subsequent main push was cancelled while queued, taking the 4-minute coverage job with it. Every cancelled main run's `updated_at` lands within a second of the next push's `created_at`. The 2 hours is real work, not queue: 2-5s from `created_at` to `started_at`, then 108 minutes inside `cargo affected collect` — 4181 tests under `-C instrument-coverage` with a per-test LLVM profile, ~5 GB of profraw. ## The fix: one workflow per cadence The three groups of jobs have incompatible needs, and one group was serving all of them. | workflow | cadence on main | why | |----------|-----------------|-----| | `ci` | every commit, ~11 min | required gate + fast checks | | `coverage` | every commit, keyed per-sha | a skipped upload leaves later PRs on a stale base | | `affected` | sampled, ~2 h | a DB a few commits old still anchors a correct superset | `affected` keeps exactly the grouping it has today, so its sampling is unchanged and deliberate. It just no longer drags the other two along. ### Scope of the impact The posted `codecov/patch` check scopes to the PR's own GitHub diff, so a stale base did **not** score PRs against other people's lines. On #3605 the posted 91.66% is exactly `github.rs`'s 11/12, while the stale-base compare object reported 64/65 across 13 files. What a stale base costs: - `codecov/project` reports "compared to \<stale sha\>" - the patch `auto` target is the stale base's project coverage (0.02pp here) - the compare API object widens to `base..head`, which is what made the investigation look like silence Separately, `test`/`lint`/`fast-checks` also stopped completing on main. Nothing load-bearing rode on that (they already ran on the PR), but it left `tend-ci-fix` with nothing to watch, since it doesn't fire on cancelled runs. ## Two smaller fixes - `ignore: "**/tests/**"` compiles to `.*/tests/.*` (confirmed against codecov's validator), which needs a leading directory and so never matched `tests/` itself. Inert today since `cargo llvm-cov` reports only `src/` (verified against a downloaded `cobertura.xml`), but now correct if that changes. Now `tests/**`. - `fail_ci_if_error` gated on `github.repository_owner`, which is the *base* repo's owner on a fork PR too, so the soft-fail its comment describes never applied. It keys off the head repo now. ## Docs The API behaviour was ours to misuse, not codecov's to explain. Three traps, all confirmed against the live API: - `file_report/<path>/` 404s with `coverage info not found` because the route swallows the trailing slash into the path. Without it the endpoint returns `line_coverage`. - `?pullid=N` always compares the PR's **current** head. `?base=&head=` asks about an earlier commit. - the compare response has no `patch_totals` key, and `.name` is `{base, head}` rather than a string, so a filename lookup silently matches nothing. A working recipe already existed in `running-tend`, but that skill is scoped to CI. `tests/CLAUDE.md` owns coverage investigation, so the queries go there and `running-tend` points at them instead of keeping a second copy. Re-running the corrected query against #3605's failing commit reproduces the miss exactly: `src/git/remote_ref/github.rs:164`, the `gh repo set-default` hint, matching what the session eventually found by hand. ## This PR demonstrates it It changes no Rust at all, only YAML and markdown. Codecov still reported a **10-file, 111-line patch** on its first commit, because it based the comparison on `203603909` rather than the real merge-base `32f380a27`. Every main commit in between has no report: | commit | ci run | report | |--------|--------|--------| | `32f380a27` | queued | no | | `9645e3e13` | cancelled | no | | `bcd1ffdfd` | cancelled | no | | `8865f20ab` | cancelled | no | Every one of those 111 patch lines belongs to somebody else's merged commit. It passed at 100% only because those commits are well covered. > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Maximilian Roos
|
9cb128328f |
refactor(tests): deny network transports for in-process git, consolidate fixtures (#3620)
An in-process `Repository::run_command()` spawns git with the test process's own environment, so none of the harness env isolation reaches it — no `GIT_CONFIG_GLOBAL`, and crucially no `GIT_ALLOW_PROTOCOL`, meaning a unit test driving the library directly could make a real network connect (reproduced: `ls-remote` against a fixture-style `https://` URL attempted the wire). The repo's own config is the one layer such a spawn still reads, so every fixture repo now carries `LOCAL_TEST_CONFIG` (identity, `commit.gpgsign`, `protocol.allow = never` with a `file` exception), written by a single `TestRepo::assemble` that every constructor routes through. The consolidation on top of that fix: - **`git_test_env()`** is the one home for the nine git-isolation settings; `configure_git_cmd` (Command), `configure_git_env` (`Cmd`), and `pty_env_vars` (PTY) all consume it. Composes with #3618's PTY layering — the env doc in `tests/CLAUDE.md` now reads four layers. - **The test gitconfig is one process-static file** (`test_gitconfig_path()`); the per-fixture copies (including two hand-rolled, drifted ones in `bare_repository.rs`) are gone. - **`BareRepoTest` and `NestedBareRepoTest` compose over `TestRepo`** (new `TestRepo::bare_at()`), so they inherit `assemble`'s guarantees structurally. Their public APIs are unchanged, deliberately — ~220 call-site references and the bare-repo snapshots are written against them. - **The committed `tests/fixtures/standard/` is deleted** (83 files), along with its Taskfile generator and the `_git` rename/sed dance. `build_standard_fixture` constructs the template once per `target/<profile>` (private-dir build + atomic rename, so parallel nextest processes race benignly) and `copy_standard_fixture` keeps only copy + per-copy gitdir rewrite. Two review-worthy subtleties in the fixture rebuild: - The old fixture's commits were stamped `-08:00` (the generator machine's timezone) and **~170 snapshots embed the resulting commit SHAs**, so the builder pins that timestamp as an explicit constant and `test_standard_fixture_template_reproduces_pinned_shas` asserts the four exact SHAs — the template reproduces the committed fixture bit-for-bit. - The old fixture's index files carried an untracked-cache extension from the generator machine's config, which made `git status` print `warning: untracked cache is disabled…` into exactly two `for_each` snapshots. The clean-room template drops it; those two snapshots are regenerated here (that's the whole content diff, plus documented-cosmetic env-block lines). Tested by the full pre-merge gate including `shell-integration-tests` (4583 tests) plus a post-merge-with-main revalidation. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
e548196f89 |
docs(bench): refresh rust-scale prune one-shots after the removal-chain trim (#3622)
Re-measurement on the rebuilt `rust-prune-12-24` fixture after #3617 trimmed the removal chain: live one-shot ~12 s to ~2.9 s (two runs, 2.91 s / 2.89 s), worktree candidates ~155-210 ms each now that the status is fsmonitor-served, branch-only ~40-100 ms. Full-cold dry-run unchanged in shape. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
5ecdf2bb78 |
perf(remove, prune): one check per guarantee in the removal chain (#3617)
`wt step prune` scans in parallel, but each removal ran a serial chain of ~17 git subprocesses under the scan write lock: a re-run of `prepare_worktree_removal` (the scan had already run it and kept only `is_ok()`), a post-hook "safety refresh" (status + fresh snapshot + integration re-compute) that ran even when no pre-remove hooks executed, and a final pre-rename clean check that ran right after the fsmonitor daemon stop, paying a full un-fsmonitored re-stat. At rust-lang/rust scale that post-stop status is the 0.5-1.7 s per-removal driver. This trims the chain to one mechanism per guarantee, with no behavioral additions: - **Prune passes the scan-time plan through** (`CheckOutcome.plan`) instead of re-preparing under the write lock. `Prunable` candidates still prepare in `try_remove` because preparing them prunes stale metadata, which must stay serialized. - **`refresh_removal_safety_after_pre_remove` is deleted.** `prepare_worktree_removal` now computes the integration verdict and effective target for display (carried on `RemoveResult::RemovedWorktree`), the pre-rename clean check catches hook-dirtied worktrees, and `delete_branch_if_safe`'s CAS re-decides the branch deletion against fresh refs. The clean check also moves into `remove_worktree_with_cleanup`, so the foreground and picker paths get their final gate adjacent to the rename rather than relying on the deleted refresh. - **The clean check runs before the fsmonitor daemon stop.** The stop must stay before the rename: on Windows the daemon holds a handle on the worktree that would fail the rename, and git's graceful stop resolves the daemon by worktree path, so it cannot reach it after the move (verified: a healthy daemon self-exits and unlinks its socket within ~500 ms of the rename; the client from the renamed path reports "not running"). - **`prune-gather` stops shelling out**: the main worktree comes from `git worktree list` ordering (documented: main lists first; `list_worktrees()` filters bare entries) instead of a `git rev-parse --git-dir` per worktree. Criterion on the `prune-4-8` fixture: `dry_run_probe_cold` 197 ms to 147 ms (-25%), `dry_run_warm` 110 ms to 62 ms (-43%), `live` 783 ms to 615 ms (-21%). A `prune-12-8` one-shot timeline (24 candidates, live): wall 3.14 s to 2.09 s; per-worktree removals 170-210 ms to 112-134 ms, now dominated by the ~60 ms daemon stop. The rust-scale numbers in `benches/CLAUDE.md` are marked for re-measurement on the next fixture rebuild. **Consistency corner, unchanged in kind but wider in trigger:** a worktree that becomes dirty after `handle_removed_worktree_output` starts (in practice: a pre-remove hook dirtying it) now fails at the pre-rename check, which is after the cd directive is written (and, in background mode, after the "Removing … in background" message and retention hint print), and the shell wrapper applies the cd file regardless of exit code. So in that corner the shell moves to the primary worktree while the dirty worktree is preserved with an error. The same outcome already existed for a file appearing between the refresh and the final check; nothing is removed in either case. Testing: full pre-merge gate green (4,578 tests, no snapshot changes), plus a new shim-based test covering the skip path when a stale (`Prunable`) candidate's preparation fails at removal time. The hook-dirtied corner has no dedicated test; existing dirty-worktree removal tests cover the command-entry gate. The one remaining uncovered added line is the `?` propagation closing the live scan closure (`prune.rs:1023`), which has no deterministic trigger. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
951dac773f |
fix(tests): give the picker's directive files a temp directory of their own, and make a PTY failure explain itself (#3619)
Chasing the Windows-only failure of `test_switch_picker_runs_execute_command` that @max-sixty flagged after #3607 merged: it failed the advisory `affected tests (windows)` leg while the required `test (windows)` leg **passed the same test on the same SHA** in the same run. ## The failure message was the whole problem ``` assertion `left == right` failed: Expected exit code 0 for picker switch with --execute left: 1 right: 0 ``` That is everything CI had. wt prints an error before exiting non-zero, so there was a message — it just never reached the captured frame, because of two harness gaps: - **The post-exit drain was a single non-blocking sweep**, taken the instant `try_wait` reported the child reaped. The child's last writes can still be in the PTY at that moment, so the tail — wt's `✗` line — was dropped. This is also why #3427 concluded a picker-create "self-exited 1 after fully-correct work with no error on screen": the error was there, just not drained. - **`child.kill()` ran unconditionally after the exit poll.** On Windows that is `TerminateProcess(proc, 1)`, so a hung picker and a picker that failed on its own both report exit 1 — the ambiguity that made #3427 reason from wall-clock time to rule out a kill. Both are fixed here: drain until the output goes quiet, and panic on a hang rather than killing silently, so every exit code a caller asserts on is one the child chose. The four directive tests now also dump the screen (and the EXEC file) in their exit-code assertions. ## What reproducing it turned up Loop-based reproduction of the reported symptom is hopeless by the numbers: across the last 38 PR runs, exactly one advisory job failed — this one — i.e. ~1 failure per 160,000 test executions. What *did* reproduce was a different Windows failure in the same five tests, at a rate 1000× higher, once the loop ran with realistic neighbours (the full suite running alongside, as the failing leg had at test 3111/4204): ``` thread '…switch_picker::test_switch_picker_emits_cd_directive_by_default' panicked at tests\integration_tests\switch_picker.rs:2531:45: failed to create cd temp file: … PathError { path: "D:\\tmp\\.tmp3XI9wb", err: Os { code: 5, kind: PermissionDenied, message: "Access is denied." } } ``` **4 of 80 runs of the five directive tests.** `directive_files()` called `NamedTempFile::new()` in the shared temp directory; `tempfile` retries a name collision only when it surfaces as `AlreadyExists`, and on Windows `create_new` against a name held by a *directory* — or by a file in delete-pending state — returns `PermissionDenied`, which it hands back to the caller. A full suite run fills that directory with `.tmpXXXXXX` entries (every `TestRepo` makes one), so the collision is routine rather than astronomical. The helpers now take a `TempDir` and put fixed-named files inside it. `TempDir::new` isn't exposed to the same problem — a directory collision surfaces as `AlreadyExists`, which tempfile retries — and fixed names inside a private directory can't collide with anything. That covers all 75 call sites of `directive_files()` / `legacy_directive_file()`, not just the picker's five, and the switch_picker copy of the helper is gone. ## The reported exit-1 is not proven fixed Stated plainly: the panic above is a *different symptom* from the reported exit 1, so this PR does not prove that failure is gone. What the evidence does say, and what a reader should check first if it recurs: - The failing run took 3.34s where the same test passing on the required leg took 3.11s and the same switch *without* `--execute` takes 2.42s. The `--execute` tail — `build_hook_context`'s git lookups, ~0.7s of the difference — had therefore run, so the failure was at the end of the pipeline, not in the picker or the switch. - After that tail, the only fallible step left is the EXEC directive append. Every other step is infallible or `.ok()`-swallowed. - That write's error was context-free, which is why nobody could place it. `wt` rendered `✗ No such file or directory (os error 2)` — no file, no operation, and positioned right under `◎ Executing (--execute):`, where it reads as if the *command* were missing. It now reads: ``` ✗ Failed to write the command to the directive file /tmp/tmp.aBc No such file or directory (os error 2) ``` The message lives in `append_line` / `write_cd_path` rather than at their four call sites, so every directive write says which file and what didn't get written — including the legacy-protocol arms, which no test can drive to failure. So the next occurrence prints its own diagnosis, in one look, instead of `left: 1, right: 0`. ## Also here **The four directive tests select their row by cursor, not by a typed query.** They typed `target` and waited for `target-branch`'s preview pane before Enter. That gate can only assert what skim *painted*: the filtered item list is swapped into `ItemList` during a render, while `Accept` reads `item_list.selected()` — the cursor's slot in whatever list is current at that moment. The list-pane `>` pointer comes from the same render state the accept reads, which is why this file's own `wait_for_cursor_on_row` calls it the race-free signal. One Down reaches `target-branch` (the picker sorts the current worktree first) and the arrow is re-issued until the pointer settles, so no query — and no matcher — is in the accept path at all. It also drops the wait on the preview pane rendering. ## Testing - Reproducer and confirmation both on Windows CI, via a temporary workflow (deleted in the last commit): 80 runs of the five directive tests with the full suite running alongside. **4 failures before the fix, 0 after.** The concurrent full suite passed 4208/4208 in both. - Local: `switch_picker` (68), plus every suite that calls these helpers — `directives`, `switch`, `remove`, `security`, `bare_repository`, `step_alias`, `user_hooks`, `step_relocate` (861 tests) — green, plus clippy `-Dwarnings` on all targets with `--features shell-integration-tests`. - The two directive-write messages are pinned by tests of their own (`test_exec_directive_write_failure_names_the_file`, `test_cd_directive_write_failure_names_the_file`): an unwritable directive path fails the same way on every platform, so the paths that produce these messages are covered rather than checked by hand. ## Follow-ups, not in here - `build_hook_context` is called with `referenced: None` on the `--execute` path, so a `-x 'echo hi'` with no template variables still runs the `default_branch`, `primary_worktree`, `commit`, `short_commit`, `remote`, and `upstream` lookups — roughly doubling the command's subprocess count (30ms → 60ms locally, 2.42s → 3.11s in the Windows test timings). The alias path already passes its referenced set; a `--execute` command receives no JSON context, so the reason hooks pass `None` doesn't apply to it. - `cancel_background_commands()` is a no-op on Windows (`signal_background_pid`), so the picker's "the picker's exit cancels the unfinished request" promise isn't kept there: preview git children keep reading `.git/config` while the accept path writes `worktrunk.history` and `worktrunk.default-branch` to it. Issue #2564 is the same shape of concurrent-config-access failure on Windows. - `assert_valid_create_exit_code`'s Windows tolerance (#3427) and `assert_valid_abort_exit_code`'s acceptance of `1` both rest on the kill-vs-self-exit ambiguity this PR removes; they can be tightened once a Windows run confirms nothing else produces a stray 1. > _This was written by Claude Code on behalf of Maximilian Roos_ |
||
|
Maximilian Roos
|
9032308400 |
refactor(tests): give the PTY test environment one home (#3618)
Follow-up to #3616, which fixed a PTY snapshot flake by adding one env knob — and to add it I had to touch three separate env builders, none of which knew about the others. This consolidates that surface. ## What was there The environment a test subprocess runs in was assembled at nine sites: five copies of the PTY prologue (`env_clear`, HOME, PATH, the Windows block, coverage passthrough) and four partial restatements of the determinism knobs. There was no rule for which belonged where, so adding a knob meant finding every copy, and a missed copy surfaced later as a flake somewhere unrelated. ## What's there now Three named layers, each with one home in `src/testing/mod.rs`: | Layer | Home | Contents | |---|---|---| | Baseline | `STATIC_TEST_ENV_VARS` | knobs every child needs, whatever it's attached to | | Terminal | `PTY_TEST_ENV_VARS` (new) | knobs only a TTY triggers — `WORKTRUNK_TEST_SPINNERS=0` | | Fixture | `pty_env_vars(TestEnvPaths { … })` (new) | the paths that vary per fixture | `configure_cli_command` and `configure_pty_command` apply them by transport. That turns the standing `// NOTE: TERM is intentionally NOT in STATIC_TEST_ENV_VARS` comment into a consequence rather than an exception: `TERM` is transport-level, so it can't sit in a baseline both transports share. `WORKTRUNK_TEST_SPINNERS` stays out of the shared baseline deliberately. It's inert on a pipe, and insta-cmd records the whole environment into every snapshot it writes (`Info::from_std_command` builds it unconditionally from `cmd.get_envs()` — there's no hook to suppress it), so putting it there would add a no-op line to 1043 snapshot files. `configure_pty_command` is now the only place a PTY child's isolation is set up. `shell_command`, `execute_shell_script`, `configure_pty_environment`, `exec_in_pty_shell`, `exec_bash_truly_interactive` and two `wt switch` spawns all delegate to it. `shell_wrapper`'s `STANDARD_TEST_ENV` and `bare_repository`'s hand-rolled `test_env_vars` are gone, as are `configure_shell`'s hand-copied knobs and four redundant `CLICOLOR_FORCE` lines in `switch_picker`. Net −149 lines. One spawn stays outside: the Windows ConPTY smoke test, which runs PowerShell against a deliberately bare environment and isn't a wt child at all. ## Reviewing Start at `src/testing/mod.rs` — the three layers and their doc comments are the whole design. Everything under `tests/` is deletion plus a delegation call. Two snapshots change: `install_preview_with_gutter` and `install_preview_declined` now carry ANSI, because those two tests previously ran without `CLICOLOR_FORCE`. Text is identical. Arguably a fix — the test named "with_gutter" couldn't see the gutter (a background-color block), while its own prompt line was already colored, so the file was internally inconsistent. ## Testing Full `wt hook pre-merge --yes` green: 4601 tests, `--features shell-integration-tests`, `RUSTFLAGS='-D warnings'`, `insta --check`. The knob's delivery path was verified by probe rather than by inspection. With `sleep 6` in the mock `llm`, `test_readme_example_hooks_pre_merge` passes; flipping `PTY_TEST_ENV_VARS` to `"1"` reproduces the original failure byte for byte: ``` +␛[1G␛[J␛[2m↳␛[22m ␛[2mWaiting for the commit generation command (4s)␛[22m␛[1G␛[J␛[2m↳␛[22m ␛[2mWaiting for the commit generation command (5s)␛[22m ``` So the knob reaches the shell-wrapper PTY child through the shared setup, not through a surviving copy. Both probe edits are reverted. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
d6ef23d573 |
test(pty): suppress the TTY spinners in PTY captures (#3616)
`test_readme_example_hooks_pre_merge` flakes when the whole suite runs together. The mock `commit.generation` command gets starved past the watchdog's 4s threshold, and the PTY capture records the redraw frames a terminal would have erased: ``` ↳ Waiting for the commit generation command (4s) ``` The elapsed seconds vary run to run, so accepting the snapshot just moves the flake. ## The fix Both spinners in `src/progress.rs` now check `WORKTRUNK_TEST_SPINNERS` alongside the existing TTY (and, for `Watchdog`, verbosity) gate, and the PTY env builders set it to `0`. This is the shape the repo already uses for `WORKTRUNK_TEST_DELAYED_STREAM_MS=-1`, whose comment names the same failure mode: "slow CI triggers progress messages that don't appear on faster systems". Only the render is gated, so the counters behind the summary lines the snapshots do assert are untouched. Gating both spinner types rather than the one call site closes the latent exposure elsewhere: `Progress` renders after 300ms in any PTY test that copies or removes files, and `Watchdog` also wraps `wt switch` and the version check. ## Why not filter the frames out of the capture A line-scoped filter in `add_pty_filters` would be a one-liner, but it only covers the single-row block. Past 10s the watchdog escalates to two rows and redraws with a cursor-up, which spans a newline the filter cannot follow, and the revealed command line carries a tempdir path. Closing that would mean modeling cursor arithmetic in a regex that silently deletes bytes from captured output. ## Evidence Reproduced first, by making the mock command slow directly rather than waiting for ambient CPU starvation to do it. The resulting diff is byte-identical to the reported flake, including the `(4s)` / `(5s)` counters: - `sleep 6` in the mock, before the fix: FAIL, same diff as reported - `sleep 6`, after the fix: pass - `sleep 12` (past the 10s escalation tier, where a filter would not have helped): pass - negative control, `sleep 6` with only `WORKTRUNK_TEST_SPINNERS` flipped from `0` to `1`: FAIL again, so the knob is what engages Full gate green: 4601 tests, no pending snapshots. > _This was written by Claude Code on behalf of Maximilian_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
63071709af |
feat(config): deprecate list.task-timeout-ms (#3615)
Removes the `[list] task-timeout-ms` per-task command timeout and the thread-local machinery behind it. The key stops having any effect immediately; a config that still carries it loads, warns, and is stripped by `wt config update`. The timeout killed any git command that outlived its budget, on every collect worker, through a thread-local that `Cmd::run` consulted on each invocation and clamped an explicit `.timeout()` against. Progressive rendering removed the reason for it: `wt list` and the picker paint from local data and stream results in behind the frame, so no single git command can hold up the first paint. What it bounded instead was completion, which `[list] timeout-ms` already bounds directly, and the drain falls back to a hardcoded 120s `DRAIN_TIMEOUT` whenever that is unset (`collect/mod.rs` `drain_deadline`), so removing this cannot introduce an unbounded wait. Both keys default to unset, so the default path never had a per-task timeout at all. `[list] timeout-ms`, the wall-clock budget for the whole collect phase, stays. It is the surviving knob and the more direct expression of the same goal. With the thread-local gone, the two-source `min()` in `Cmd::run` collapses to the command's own `self.timeout`, so every explicit `.timeout()` caller keeps its bound unclamped: `PROBE_TIMEOUT` in `git/reap.rs`, the fsmonitor stop/lsof bounds in `git/remove.rs`, the version check in `config/show.rs`, and `REMOTE_DETECTION_TIMEOUT`. ## Deprecation A `Structural` row in `DEPRECATION_RULES` strips the key from `[list]` in both the section and inline forms, top-level and per-project, following the `[switch.picker] timeout-ms` precedent (also a strip with no equivalent key to migrate into): ``` ▲ User config: list.task-timeout-ms is no longer used — list.timeout-ms bounds the collect phase ``` The env overlay (`WORKTRUNK__LIST__TASK_TIMEOUT_MS`) and `--config-set` route through the same rule and migrate silently, since neither layer has a file for `wt config update` to materialize. Neither errors. ## Testing New unit tests cover detection and migration for the section, inline, and per-project forms plus the warning text, and two cases join the `test_warning_fires_iff_update_changes` battery that pins the warn-iff-update-changes invariant. The three integration tests that exercised the feature are gone; `Cmd::timeout` keeps its own coverage in `shell_exec.rs`, so dropping the four thread-local unit tests loses nothing for the surviving path. Verified end to end that a config carrying the key loads, warns, and has it stripped by `wt config update` with sibling keys intact. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
Maximilian Roos
|
b8299aca34 |
docs(push): record that destination-worktree safety matches git (#3614)
Comment-only. It records a decision rather than changing one: `wt merge`
and `wt step push` overwrite an ignored file in the destination worktree
whose path the incoming commits track, and that is intended.
`prepare_target_worktree` works from `git status --porcelain`, which
omits ignored files. So the existing protection covers modified-tracked
and untracked collisions (refused as `ConflictingChanges`) and
autostashes the rest of a dirty tree, while an ignored file at a
colliding path is neither refused nor stashed.
git draws the line in the same place. A plain `git merge` run in that
worktree, against a scratch repo:
```
ignored locally → Fast-forward, db.sqlite | 1 + (content: TRACKED-FROM-BRANCH)
untracked locally → error: The following untracked working tree files
would be overwritten by merge (content: REAL-LOCAL-DATABASE)
```
So a probe that refused here would make `wt` stricter than the tool it
wraps, over data git treats as expendable. Three places say so now, each
where someone would otherwise reach the opposite conclusion:
- **`src/commands/worktree/push.rs`** — the module spec, at the point
where such a probe would be added. It names the mechanisms rather than
arguing by analogy: the fast-forward hands the checkout to
`receive.denyCurrentBranch=updateInstead`, `--no-ff` syncs with
`read-tree -m -u`, and both run git's unpack-trees checks. Confirmed
against a scratch repo — `read-tree -m -u` silently overwrites an
ignored file at a colliding path and refuses an untracked one (`exit
128`, file intact), the same split `git merge` produces.
- **`src/commands/repository_ext.rs`** — `prepare_target_worktree`,
which owns the status read, pointing at that spec from the
implementation site. (A `[`crate::…`]` intra-doc link was the first
attempt; `push` is a private module, so `RUSTDOCFLAGS=-D warnings`
rejected it. Plain path instead.)
- **`CLAUDE.md`** — Data Safety, bounding "prefer failure over silent
loss" at git's edge. Read unqualified, that rule is what invites the
stricter check.
Follows #3601, which fixed the user-facing half: the FAQ had recommended
`git worktree lock` for "precious ignored data", which the lock does not
protect — it blocks removal only.
## Verification
`RUSTDOCFLAGS="-D warnings" cargo doc`, `cargo clippy --all-targets --
-D warnings`, and pre-commit all clean.
The local gate flagged `test_readme_example_hooks_pre_merge`, which is
the known load flake — an extra `Waiting for the commit generation
command (4s)` spinner line under full-suite CPU contention. It passes in
isolation with no retries. The diff contains no executable lines at all
(every changed line is `///`, `//!`, or markdown), so it cannot be
causal.
> _This was written by Claude Code on behalf of Maximilian Roos_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
||
|
Maximilian Roos
|
04d8d587f2 |
feat(list): flag a branch checked out in more than one worktree (#3606)
## Problem Follow-up to #3480. That PR made a duplicated branch checkout (`git worktree add --force <path> <branch>`) visible at *resolution* time: `worktree_for_branch` warns once per branch, then resolves to whichever worktree git lists first. `wt list` said nothing about it. Two rows named `feature`, and no column that explains why. The nearest thing to a signal was accidental. A force-added duplicate usually lands off-template, since the original holds the template path, so it picks up `⚑` for the location mismatch — while the worktree *at* the template path, the one `wt` actually resolves to, carried no flag at all. Exactly backwards from what's useful. The framing from the request: a worktree in the wrong location gets a status flag; a worktree sharing its branch should get one too. ## Solution `⚑` now covers both, on every worktree of the duplicated branch, resolved one included. Which worktree `wt` picks is git's listing order, so singling out the shadowed rows would imply a legitimacy the ordering doesn't carry. ``` @ main ^| | . 05a4a45d 16h Initial commit + feature ⚑_ ../repo.feature 05a4a45d 16h Initial commit + feature ⚑_ ../repo.feature-dup 05a4a45d 16h Initial commit ``` This started as a seventh glyph (`⧉`) and collapsed onto `⚑` in the second commit. The Status column is a dense alphabet the reader has to learn, and the two states say one thing: this worktree's place in the branch ⇔ worktree map is irregular. Off-template path and branch-claimed-twice are both instances. The table already distinguishes them without a glyph — a repeated Branch cell is the duplicate, an odd Path cell the mismatch — so the flag only has to say "not a rendering glitch, look at the Path column". Sharing the glyph means sharing its dim-yellow styling, since the codebase treats a symbol's color as part of its identity; #3480's warning remains the loud channel, firing the moment any command resolves the branch. **The Path column comes along.** It previously appeared only for a location mismatch, on the reasoning that the path is otherwise redundant with the branch. A duplicate inverts that: the branch name no longer identifies the row, and the path is the only thing telling the two apart. The layout flag is renamed from `has_branch_worktree_mismatch` to `path_is_informative` to say what it now means. **The data model keeps the distinction.** JSON has no cardinality budget, and reporting a duplicate that sits at the template path as `branch_worktree_mismatch` would be false — its path does match. Schema 1's `worktree.state` names the cause (`"duplicate_branch"`), schema 2 gets its own `worktree.duplicate_branch` bool beside `branch_mismatch`, matching that schema's one-fact-per-field shape. The priority between the two `⚑` states now decides only which cause JSON reports. Detection is one pass over the worktree list (`duplicated_branches`, next to #3480's `worktree_paths_for_branch`), in memory, pre-skeleton, no git calls. ## Testing - `test_worktree_paths_for_branch_detects_duplicates` gains the set form and a detached-HEAD worktree, which has no branch to duplicate. - `test_metadata_worktree_state_priority` covers the two `⚑` states' ordering and both yielding to `⊟`/`⊞`. - `test_list_duplicate_branch` snapshots the table above, showing both flagged rows and the Path column earning its place. - `test_list_duplicate_branch_json` asserts both schemas flag exactly the two duplicated rows. The flag only fires on a state no prior test sets up, so the only snapshot churn is the help pages and the schema-2 envelope's new field. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
4d1a669b6f |
fix(list): honor task-timeout-ms for a command with its own bound (#3613)
Two follow-ups from #3603. ## `[list] task-timeout-ms` didn't bound a command that set its own timeout `Cmd::run` resolved its two timeout sources as a precedence chain — `self.timeout.or_else(thread_local)` — so a command carrying an explicit `.timeout()` escaped a shorter budget its caller had set. `wt list` sets exactly such a budget on every collect worker from `[list] task-timeout-ms`, and the 10s bound #3603 put on remote default-branch detection sits inside one. With `task-timeout-ms = 50`, that command ran for the full 10s. Both values are ceilings — one bounds a command, the other bounds every command on the thread — so the tighter of the two now wins. The key's own documentation already promises that it "kills individual git commands that exceed this duration", so this makes the documented behavior true rather than changing it; no doc update goes with it. I confirmed the defect rather than inferring it: with the old line restored, the new test's command runs the full 10.01s against a 50ms budget, 200× over. The setting is unset by default, so nothing changes for a default config. The one nearby thing this could have clamped is the picker's 2s pager bound, and it can't — `pager.rs` calls `child.wait_timeout` on a raw `Child`, not through `Cmd`. `COMMAND_TIMEOUT`'s doc named the `wt switch` picker as its driver and omitted the config key that actually feeds it, which reads as though the thread-local has no production callers at all. It now names the key and says it's unset by default. ## A Windows CI flake in `test_cmd_run_file_current_dir_is_errored` The test created a `NamedTempFile` purely to get a path that isn't a directory, and on Windows the creation itself failed: ``` PathError { path: "D:\\tmp\\.tmpWKLe5F", err: Os { code: 5, kind: PermissionDenied, message: "Access is denied." } } ``` tempfile retries only a *reported* collision (`AlreadyExists` / `AddrInUse`), so a denial propagates. The error the test actually exercises comes from `check_spawn_preconditions` — `metadata` plus `is_dir`, before anything spawns — so the test needs nothing more than an existing non-directory path. The test binary is one, and using it means the test creates nothing under the shared temp root. ### What I did not do The obvious next step is a prefix-separation sweep: `TempDir` and `NamedTempFile` draw names from one namespace (a `.tmp` prefix plus six random characters), and this suite feeds it ~260 default-prefix `tempdir()` sites plus a temp directory per test repo, so a temp *file* name can land on a temp *directory* name. That would explain a denial rather than a collision report — but only if Windows returns `ACCESS_DENIED` where a file creation hits an existing directory name, instead of `FILE_EXISTS`. I can't test that from macOS, and it's the load-bearing link, so I left the other eleven `NamedTempFile::new()` sites alone rather than sweep production (`src/commands/step/commit.rs`, `src/git/repository/working_tree.rs`) on an unverified premise. Worth doing if someone can confirm the semantic. ## Testing The timeout fix carries a regression test that fails without it (`Ok` after 10s) and passes with it. The Windows fix is verified only by the local suite — the flake it removes is rare and Windows-only, so CI going green here is consistent with it but doesn't demonstrate it. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
cad89eaebb |
fix(relocate): skip worktree blocked by an immovable occupant (#3530)
## Problem `wt step relocate` could strand a worktree in the internal staging directory and then error out. When worktree **A**'s target is occupied by worktree **B**, A depends on B vacating. But if B is itself *blocked* — its own target is a non-worktree path and `--clobber` wasn't passed — B never moves. The dependency-resolution loop's no-progress branch assumes "no progress ⟹ a cycle to break," so it temp-moved A into `.git/wt/staging/relocate/`. `finalize_temp_relocations` then ran `git worktree move <temp> <A's target>` into the still-occupied path, which fails and propagates via `?` — leaving A relocated into the staging dir rather than at either its original or expected path. ### Reproduction - A at `/w/a` wants `/w/b`; B at `/w/b` wants `/w/c`; `/w/c` is a plain non-worktree file, no `--clobber`. - B is classified `target_blocked` and never moves. - A is temp-moved by `break_cycle`, then `finalize` fails with "Failed to move worktree from temp to final location", stranding A in staging. ## Fix `is_target_empty` only distinguished "occupant moved" from "occupant present"; it couldn't tell a still-pending occupant from a permanently-blocked one. A new `blocked_occupant` helper detects when a dependent's target is held by a worktree already in the `blocked` set, and the loop propagates the block to the dependent (skipping it cleanly) instead of handing it to `break_cycle`. Setting `made_progress` re-drives the loop so a chain of dependents (A → B → C-blocked) is blocked transitively. After the fix both worktrees are skipped and the command succeeds: ```console ▲ Skipping beta (target blocked: repo.beta) ↳ To backup blocking paths, use --clobber ▲ Skipping alpha (blocked by beta, which can't be relocated) ○ Relocated 0 worktrees, skipped 2 worktrees ``` ## Test `test_relocate_blocked_occupant_skips_dependent` sets up exactly this shape and asserts the command succeeds, both worktrees stay put, and nothing is stranded in `.git/wt/staging/relocate/`. Before the fix the command exited non-zero and `alpha` was left in the staging dir, so both the snapshot (`success: true`) and the location assertions would have failed. Found during the nightly code-quality survey of `src/commands/relocate.rs`. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |