mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
codex/remove-codex-cloud-specific-tests
1582 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Worktrunk Bot
|
246c6bd919 |
fix(list): keep [list] columns out of the --format json plan (#3812)
Closes the `[list] columns` half of #3787, per the call in [this
comment](https://github.com/max-sixty/worktrunk/issues/3787#issuecomment-5273942067):
JSON always emits the same shape, and `list.columns` only affects the
actual columns.
Before, `--format json` planned `all_columns` (source `Default`)
*unioned* with the selection's forced-on columns, so the selection
reached JSON in one direction only — it couldn't narrow the emitted
fields, but a listed `ci` did force the forge fetch on without `--full`.
That made a presentation setting decide whether a machine-readable call
talks to GitHub, which is the thing the Neovim plugin in #3787 had to
pin `--config-set 'list.columns=[…]'` against. Now the JSON branch plans
`all_columns` alone; `--full` is the only switch for the gated data, and
it's the one a caller controls.
The table and the `wt switch` picker are untouched — a listed `ci` still
renders the CI column without `--full`, and the picker still unions the
selection in so its table matches `wt list`'s.
Only `ci` and `summary` are affected: every other column is ungated, so
`full_plan()` already covered them, and custom columns require no
background task.
**For the release note — this changes schema 1 too.** A caller with
`[list] columns = […, "ci"]` and no `--full` used to get the `ci` object
in schema-1 JSON and now won't; schema 1 has no `collected` envelope to
say why. The schema-1 `ci` row already documented `` `--full` only ``,
so the docs get *more* accurate, but the observable output changes for
anyone who was relying on the forcing path. Schema 2 reports the same
narrowing through `collected.ci`.
Docs updated in `after_long_help` (the `[list] columns` section plus the
schema-2 `pr`, `summary`, and `checks` rows — `summary` now names
`--full` alongside `[list] summary = true`, and `checks` names the
`--full` gate it shares with `pr`), with the generated mirrors,
`dev/config.example.toml`, and the `--help` snapshots regenerated. The
`CLAUDE.md` network inventory and the `collect` planning comment now
record the exemption too.
<details><summary>Test</summary>
`test_list_json_columns_selection_does_not_force_ci` in
`tests/integration_tests/list_config.rs` asserts schema 2's
`collected.ci` across three configs: unset (false), `columns =
["branch", "ci"]` without `--full` (false — the regression this fixes),
and the same with `--full` (true). `collected` records what the plan
requested rather than what a fetch returned, so the test needs no forge
and no `gh` on PATH. It sits next to
`test_list_json_ignores_columns_selection`, which owns the narrowing
direction, and `test_list_config_listed_column_overrides_full_gate`,
which owns the table's forcing behaviour and still passes unchanged.
Ran locally: full `cargo test --test integration` and `cargo test --lib
--bins`, plus `cargo clippy --all-targets` and `cargo fmt --check`. One
unrelated failure,
`test_copy_ignored_preserves_file_executable_permissions`, is a umask
artifact of this sandbox (expects `0644`, the runner's `umask 002`
produces `0664`); it touches no code in this diff.
The docs-row follow-up in
|
||
|
Maximilian Roos
|
92dfb686bb |
feat(approvals): let wt config approvals add --yes record approvals without a TTY (#3819)
`wt config approvals add` refused every non-interactive run — even with `--yes`, whose hint then suggested the flag already passed — so there was no way to pre-approve a project's commands unattended. An orchestrator (tend's Codex Cloud container was the motivating case) had to hand-write `approvals.toml` from `wt config approvals list --format=json` output, a third-party reimplementation of `add` that breaks whenever the schema changes. The `wt config approvals` docs already promised "`--yes` to bypass prompts in CI" and described `stale` entries as "what `--yes` would silently re-approve"; behavior now matches them. The two `--yes` meanings stay distinct: on a command that runs project commands it grants consent for that run alone and records nothing (unchanged), while on `add` — whose product is the record — it lists what it trusts and writes it. `add` no longer routes through `approve_command_batch` (the execution gate) for this: it prompts or announces, then saves itself, which also makes a failed `approvals.toml` write fail the command instead of warning behind a `✓ saved` line and exit 0 — an orchestrator reading only the exit code would otherwise walk into the prompt it just paid to avoid. The non-interactive hint's pre-approval suggestion now carries `--yes` (`run wt config approvals add --yes`), since a hint reached in CI must name a command that runs there. Per the existing `list --format=json` docs, `add --yes` re-approves templates edited since an earlier approval without comment; the `add` help now says so and points at the `stale` field for reading them first, and the worktrunk skill's escalation rule tells agents not to reach for it on a user's behalf. > _This was written by Claude Code on behalf of max-sixty_ |
||
|
Caleb Cox
|
aa9d8c43df |
feat: add remote_repo variable (#3745)
Add a `remote_repo` variable that returns the repo name from the remote URL. Unlike `repo`, it stays consistent even if the clone was renamed. Feel free to reject, or suggest other names for the variable. But this change would improve my workflow. I hope you don't mind my submitting a PR before opening an issue. Thanks for an amazing developer tool! AI Disclosure 🤖: I used Claude Code to generate the changes, but reviewed every line and made adjustments. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Worktrunk Bot
|
96c6c846f7 |
fix(shell): register completions under the --cmd name, not clap's (#3817)
## Problem
`wt config shell init <shell> --cmd <name>` renames the shell wrapper
and its lazy completion loader, but the registration that loader evals
comes from clap, which derives every identifier in it from its own
compile-time `Command` name (`wt`) — not from `argv[0]` and not from
`--cmd`. The two halves never agreed:
```console
$ wt config shell init zsh --cmd wot | grep _clap
if ! (( $+functions[_clap_dynamic_completer_wot] )); then
_clap_dynamic_completer_wot "$@"
$ COMPLETE=zsh wt | grep -oE '_clap_dynamic_completer_[a-z_]*' | sort -u
_clap_dynamic_completer_wt
```
Nothing completed, and because the guard never became true the
completion script was regenerated and re-evaluated on *every* TAB. Same
shape in bash (`_clap_complete_*`); PowerShell emitted
`Register-ArgumentCompleter -Native -CommandName wt`, so the `--cmd`
name was never registered at all. The documented `--cmd=git-wt` case
(the Windows Terminal conflict) was broken too — including for a binary
genuinely installed under that name, since clap's name comes from the
declaration rather than `argv[0]`.
There is a second, sharper edge: zsh's registration ends with `compdef
<completer> <cmd>`, so the first TAB on `wot` also bound worktrunk's
completer to plain `wt` — handing completions to the *other* `wt` that
`--cmd` exists to step around.
fish and nushell were unaffected. Both register a completer that shells
out to the binary rather than depending on a clap-emitted identifier, so
the reporter's "unverified" row for fish is a pass.
## Solution
The bash, zsh, and PowerShell loaders now pass the name they bind in
`WORKTRUNK_COMPLETE_NAME`, and `registration_name()` in
`src/completion.rs` emits the registration under that name (validated
through the same `validate_shell_command_name` guard `--cmd` uses, since
the value lands verbatim in generated shell code). The fallback is
`binary_name()`, which covers a binary installed as `git-wt` and invoked
directly. The templates apply clap's own `-` → `_` escaping to the
function they call, so `--cmd git-wt` guards on `_clap_complete_git_wt`
rather than the invalid `_clap_complete_git-wt`.
That fixes all four shells and the stray `compdef` in one place, rather
than pinning the templates to clap's internal naming:
```console
$ WORKTRUNK_COMPLETE_NAME=wot COMPLETE=zsh wt | grep -oE '_clap_dynamic_completer_[a-z_]*|compdef .*' | sort -u
_clap_dynamic_completer_wot
compdef _clap_dynamic_completer_wot wot
```
## Testing
Two reproduction tests in `tests/integration_tests/completion.rs`, both
failing before the change:
- `test_init_custom_cmd_defines_clap_completer_in_bash` drives the whole
chain through a real bash — generate the init script with `--cmd`, call
the loader it defines, then assert clap's completer function exists
afterwards. Printed `MISSING` before, `DEFINED` after. Cases for `wot`
and `git-wt`.
- `test_completion_registration_uses_shell_integration_cmd_name` covers
zsh and PowerShell, which CI can't drive: the identifier the init script
references must be the one the registration defines, and the `compdef` /
`-CommandName` target must be the `--cmd` name.
`cargo test --lib --bins` and `cargo test --test integration` are
otherwise green (one unrelated failure locally,
`test_copy_ignored_preserves_file_executable_permissions`, from this
sandbox's `umask 0002`), and `cargo clippy --all-targets --all-features`
/ `cargo fmt --check` are clean.
---
Closes #3816 — automated triage
---------
Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
|
||
|
Worktrunk Bot
|
bdce107d91 |
fix(config): rank env vars and --config-set above project entries (#3790)
Fixes #3788. Layer and specificity were separate steps. `load_with_warnings` flattened system config → user config → `WORKTRUNK_*` env vars → `--config-set` into one document, and the accessors then resolved specificity on that document, so a `[projects."<id>"]` entry answered for the global key of the same name whichever layer set it. `WORKTRUNK_WORKTREE_PATH` could therefore not override a project's `worktree-path`, and a global `--config-set` hit the same wall. Per @max-sixty in the issue thread — "env vars should indeed take precedence over the user project config, we should fix this throughout" — the two invocation layers now cross the axes: they're typed for one run, so they outrank a project entry as well as the global key. Load applies them at both scopes (`apply_invocation_layer_over_projects`, the last step before `finalize`): whatever the layer set is dropped from every project entry, leaving the global key it also set to answer for it. Two kinds of key are held back: - **Keys the layer restates under `projects."<name>"`** — `--config-set 'projects."github.com/owner/repo".worktree-path = …'` is both the highest layer *and* the most specific key, so it still wins over the same layer's global key. - **Composing keys** — hooks, aliases, and `step.copy-ignored.exclude` — whose project-scoped values append to the global ones rather than replacing them. Both already apply, so an env-set hook was never outranked, and dropping the project's copy would silently stop it running. Hook names come from `HooksConfig`'s schema, so a new hook can't be forgotten. Two sections have to go as a unit rather than leaf by leaf. `[commit.generation]`'s mutually exclusive pairs: `template` and `template-file` clear one another in `merge_with` *and* are rejected together by `validate`, so overriding either has to displace both at project scope — otherwise the project's partner would still win the merge. `exclusive_sibling` names those pairs. And `[list.custom-columns]`, which `ListConfig::merge_with` extends per whole column, so a partial removal leaves the project's column replacing the global one anyway — and `ListColumnConfig::template` is required, so it can also strand a column that no longer deserializes. `is_atomic_section` names that table. Both are enumerations, so the pass degrades as a unit behind them: the removals land on a candidate, kept only if it still deserializes and validates. That is the guarantee the env and `--config-set` layers already have, and without it the next required field would answer a stranded leaf with `UserConfig::default()` — costing the user their whole config for that invocation rather than one project entry's precedence. The precedence table now reads: | Source of `worktree-path` | Loses to | |---|---| | `--config-set 'worktree-path = …'` | — | | `WORKTRUNK_WORKTREE_PATH` | `--config-set` | | `[projects."github.com/owner/repo"]` in a config file | either invocation layer | | global `worktree-path` in a config file | all of the above | ## Docs The help text had no precedence section at all — the gap that made this read as a bug — so this adds one under **Environment variables**, plus a pointer from **User project-specific settings**. That supersedes #3789, which documented the old behavior; I'll close it in favour of this. ## Testing Nine unit tests in `src/config/user/tests.rs` cover the table-level rule (both layers, pattern entries, restated project-scoped overrides, untouched sibling keys, composing keys, the exclusive pair, the atomic custom column, a rolled-back layer, and the no-override no-op), and `test_switch_create_invocation_layers_outrank_project_worktree_path` proves it end-to-end — a real process is the only thing that reads `WORKTRUNK_WORKTREE_PATH` off the environment. That test keeps a control showing the project entry still beats the config file's own global key, so it can't pass by project entries having stopped applying. The reproduction from the issue now lands where it says it should: ```console $ WORKTRUNK_CONFIG_PATH="$tmp/wt.toml" WORKTRUNK_WORKTREE_PATH="$tmp/from-environment" \ wt switch --create feature --no-cd --no-hooks --yes --format=json {"action":"created","branch":"feature","path":"/tmp/tmp.jQiTAuNPFd/from-environment",…} ``` <details><summary>Local suite</summary> `cargo test --lib --bins` and `cargo test --test integration` are green apart from `test_copy_ignored_preserves_file_executable_permissions`, which fails in this sandbox because its umask is `0002` (file created `0664`, test expects `0644`) — unrelated to this change and not reproducible on a `0022` runner. `cargo fmt --check` and `cargo clippy --all-targets --all-features` are clean. </details> --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Maximilian Roos <m@maxroos.com> |
||
|
Maximilian Roos
|
7aba380f0c |
fix(remove): gate removal on the registration, not the repository (#3808)
`wt remove --force` deleted a live worktree of this same repository, uncommitted work included, whenever that worktree had been moved onto another worktree's registered path. The guard added in #3785 asks which *repository* the occupant answers to: a linked worktree's git dir sits under `<common>/worktrees/`, the main worktree's *is* the common dir, anything else is someone else's. A sibling worktree moved onto the path satisfies that — its git dir sits under `<common>/worktrees/` like any worktree of this repo — so it passed, and the fast path renamed the directory into trash and handed the `rm -rf` to a detached process. It is not prunable either: its gitdir file points at a location that exists, so the `is_prunable` arm from the same PR doesn't catch it. Git's own validation is one level finer. `validate_worktree` requires the directory to point back at *this registration*, and refuses this removal with `--force`: ```console $ git -C repo worktree remove --force ../repo.feature fatal: validation failed, cannot remove working tree: '.../repo.feature' does not point back to '.git/worktrees/repo.feature' ``` <details> <summary>Reproducer, verified against a build of main</summary> The occupant has to be *moved* onto the path rather than created there — `git worktree add` refuses a registered path, which is what leaves a plain `mv` as the way this state arises. ```console $ git -C repo worktree add ../repo.feature -b feature $ git -C repo worktree add ../repo.bar -b bar $ rm -rf ../repo.feature && mv ../repo.bar ../repo.feature $ echo PRECIOUS > ../repo.feature/precious.txt $ wt remove --force --yes feature ◎ Removing feature worktree (--force) & branch in background (same commit as main, _) $ ls ../repo.feature ls: ../repo.feature: No such file or directory ``` </details> ## The fix The gate is now git's comparison at git's granularity: the directory's `.git` must name *this* registration, and that registration's `gitdir` file must name the directory back. Repository-level ownership stays as the weaker half of the conjunction — it is what rejects a `.git` file pointing at another repository — and the main worktree is the same test where there is no registration to point back at. `ensure_belongs_to_repo` becomes `ensure_holds_this_worktree`, since it no longer merely asks about repository membership. Resolution moves to `Repository::git_dir_at`, the fs-only resolver the `wt list` prewarm already used (`derive_worktree_git_dir`), generalized to answer for a directory rather than for a known worktree of this repo: its main-worktree branch returned `git_common_dir()` on trust, and now canonicalizes the `.git` it actually found. It also never walks up to a parent, which is what git reads too — `git rev-parse --git-dir` in an emptied worktree can resolve the *enclosing* repository. That settles a second thing the old docstring got wrong. It claimed the plan→rename window was "narrower than `ensure_clean`'s"; in fact `ensure_clean` re-runs `git status` while this gate answered from `GIT_DIRS`, memoized process-wide, so the second call was vacuous and the window — which contains the approval prompt and the `pre-remove` hook — was unguarded. `git_dir_at` reads the filesystem on every call, so the check at the rename now re-decides. The refusal was `Directory @ … is not this repository's worktree`, which is false in the sibling case: it *is* one of this repository's worktrees, just not the one registered there. Its hint didn't fit either — "move the directory aside, then run `git worktree prune`" is a repo-wide prune, and with a sibling moved aside *both* registrations are prunable, so following it clears both and leaves a live checkout that has stopped being a worktree: ```console $ mv ../repo.feature ../repo.aside && git worktree prune -v Removing worktrees/repo.feature: gitdir file points to non-existent location Removing worktrees/repo.other: gitdir file points to non-existent location $ git -C ../repo.aside status fatal: not a git repository: (null) ``` So the error carries where the occupant's own registration records it, and each case gets the remedy that fits. Moving it back to that path leaves prune with only the stale entry to clear: ```console $ wt remove --force --yes feature ✗ Directory @ ../repo.feature does not hold the worktree registered there ↳ Removing it could destroy the worktree registered @ ../repo.other; move the directory back there, then run git worktree prune ``` That path is read through `canonicalize_with_parents`, because a relative `gitdir` entry resolves against `<common>/worktrees/<id>` and would otherwise reach the hint with the `..` chain still in it — and plain canonicalization can't normalize a directory that no longer exists, which is the case the arm is reached for. Normalizing there also makes `crate::path::paths_match`, the crate's canonicalizing comparison over that same helper, the right test for the gate, so there is no second comparison beside it. The gate's fail-closed behavior now rests on that helper resolving `..` through the filesystem rather than collapsing it lexically — across a symlink the two readings name different directories — so `src/path.rs` records the constraint where a lexical rewrite would otherwise read as a tidy-up. The FAQ's "What can Worktrunk delete?" paragraph carried the same "a *different* repository" framing and is corrected. ## Scope Pre-existing, and 0.73.0 already narrowed it — 0.72.0 had no ownership check at all and deleted foreign clones too. The guard has two call sites (`prepare_worktree_removal` at planning, `stage_worktree_removal` at the rename), so this reaches `wt merge --remove`, `wt step prune`, and picker removal, not only `wt remove`. One incidental tightening: `wt remove <bare-repo-path>` previously passed the guard (a bare root's git dir *is* the common dir) and was stopped only by the dirty check, which `--force` skips. It now refuses at the guard. <details> <summary>One residual, left alone</summary> `git worktree repair <path>` after the `mv` produces a *double registration*: both `worktrees/repo.bar/gitdir` and `worktrees/repo.feature/gitdir` come to record the same path, and `git worktree list` reports two worktrees there. In that state the new gate accepts the removal — the occupant does point at the `feature` registration, and that registration does point back — while git refuses, because its path→worktree lookup happens to match the `bar` entry first. Closing it means knowing the registration id at the gate, or scanning every `worktrees/*/gitdir` for duplicate claims. Unchanged by this PR, and reachable only via `mv` followed by `repair`. </details> ## Testing Five new tests, each confirmed to fail with the line it covers reverted and to leave the others passing. Three drive the binary: - **the sibling case** — follows #3785's data-safety model: asserts the filesystem afterwards, not just the exit code, since removal stages by rename and deletes in a detached process. Snapshots the refusal, so the hint and the path it names are pinned. Fails with the pointer-back conjunct removed, while the foreign-repo test still passes without it — the two cover different halves. - **the re-check at the rename** — a `pre-remove` hook repoints the worktree's `.git` at a sibling's registration after planning has already cleared it, which is what makes the second gate's freshness observable. Fails when resolution routes back through the `GIT_DIRS`-cached `git_dir()`. - **a relative `gitdir` entry** — removal succeeds, and git reads the rewritten entry back, which is what makes it the form git itself writes. Rewriting the entry rather than setting `worktree.useRelativePaths` keeps the test independent of the git version that introduced the option. Two sit at the gate, where the CLI can't reach: - **both worktree shapes are accepted** — including the main worktree, whose git dir *is* the common dir. `wt remove` rejects the main worktree well upstream of this gate and a bare repository's worktrees are all linked, so nothing through the CLI would notice that arm inverting. - **the refusal names a normalized path** — asserted against `Diagnostic::render`, since the path is in the hint and `Display` carries only the title. Local gate green: 4607 tests, lints, doctests, rustdoc under `-Dwarnings`. > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
667c6efaf0 |
docs(switch): note that Alt-x never forces (#3811)
Requested by @max-sixty in [#3809](https://github.com/max-sixty/worktrunk/issues/3809#issuecomment-5273484502) — a couple of words clarifying that `Alt-x`'s removal is safe-only. The keybinding table read `Remove selected worktree/branch`, which doesn't say the removal never forces; that's what sent the reporter looking for a force-remove that isn't there. The picker hardcodes the safe path — [`prepare_removal`](https://github.com/max-sixty/worktrunk/blob/7a2a3e003e7eed138ff5f2dcd2296f6bbd8e86d4/src/commands/picker/mod.rs#L323-L330) passes `BranchDeletionMode::SafeDelete` and `force_worktree: false`. Deliberately scoped to the table cell, per the "(only)" in the request. The bigger question — whether `Alt-x` should ever pass `-D` — is still open on the issue and isn't touched here. Primary source is `after_long_help` in `src/cli/mod.rs`; the three mirrors and the `--help` snapshot are regenerated. Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Worktrunk Bot
|
1636b78ddf | fix(gitlab): forward glab's verdict when the project lookup fails (#3799) | ||
|
Maximilian Roos
|
cf822f75cf |
fix(worktree): guard a registered path that no longer holds its worktree (#3785)
A registered worktree's path was trusted to still hold that worktree.
Three defects followed, one of them destroying data, and the check that
would have caught them — where it existed at all — was `Path::exists()`.
## `wt remove --force` deleted an unrelated repository
A clone that came to sit at a stale registration's path was removed
whole, including uncommitted work and, for a repo never pushed, the only
copy of its objects. wt's own hint routed the user there: the dirty gate
reads `git status` in that directory, reports the occupant's changes as
this worktree's, and offers `--force` as the cure.
```console
$ wt remove feature
✗ Cannot remove worktree: feature has uncommitted changes
?? precious.txt # ← the other repo's file
↳ ... to lose uncommitted changes, run wt remove --force feature
```
git refuses that same removal, `--force` included (`validation failed …
is not a .git file`). Worktrunk's fast path renames the directory into
trash rather than asking git to, so git's validation never ran.
`ensure_belongs_to_repo` makes it, comparing the directory's git dir
against this repository's: a linked worktree's sits under
`<common>/worktrees/`, the main worktree's *is* the common dir, anything
else answers to someone else. One comparison covers both worktree kinds
and also rejects a `.git` file pointing at another repo, which git's
shape test accepts.
It runs at planning, ahead of the dirty gate, and again at the rename
for callers that stage without planning. Now:
```console
$ wt remove --force feature
✗ Directory @ ../repo.feature is not this repository's worktree
↳ Removing it could destroy unrelated data; move the directory aside, then run git worktree prune
```
## A recreated worktree directory leaked git's exit 128
`wt switch`, `wt merge`, and `wt step push` walked into `git rev-parse
--git-dir failed (exit 128)`. Two of them probed `Path::exists()` first,
which a deleted-and-recreated directory passes; the third asked nothing.
`worktree_is_unusable` is the union of both tests, because neither
implies the other. `exists()` catches the absent directory; git's
`prunable` catches the recreated one. `prunable` alone is *not* the
wider test it looks like — git withholds the attribute from a **locked**
worktree even when its directory is gone, since prunability is its
pruning policy and a lock means "don't prune this":
```console
$ git worktree list --porcelain # wt2 locked, all three directories removed
worktree /tmp/ptest/wt1
prunable gitdir file points to non-existent location
worktree /tmp/ptest/wt2
locked removable media # ← no prunable line
worktree /tmp/ptest/wt3
prunable gitdir file points to non-existent location
```
A locked worktree on an unmounted volume is exactly what
`prepare_worktree_removal`'s lock guard exists for, so a `prunable`-only
test would read it as healthy. All three commands now give the message
the merely-deleted case already gave.
`wt remove` keeps `exists()`, deliberately: there it is the precondition
for the cleanup path rather than a health test, since
`prune_worktree_entry` unregisters via `git worktree remove`, which
skips validation only while the directory is absent. No scoped git
command clears the recreated case, so it reports and names the repo-wide
`git worktree prune` that does.
## `wt switch docs/` missed a branch sitting right there
Git's ref format forbids a trailing `/`, so the branch lookup never had
a candidate — and shell completion produces exactly that spelling
whenever a `docs` directory sits beside the branch. Selectors are
normalized before resolution.
## Resolving selectors through one ladder
The three fixes landed in three of the four places that assemble "expand
shortcuts, try the branch, try the path, classify the failure" by hand.
Each gated its path attempt on "did something rewrite this token?",
answered by comparing an expansion's output against its input:
| where | the comparison |
|---|---|
| `resolve_worktree` | `branch == name` |
| `plan_switch` | `target.branch == branch` |
| `target_worktree_at_path` | `target.filter(\|t\| *t == resolved)` |
| `resolve_base_ref` | `resolved == base` |
That is a fact the rewriting step knows, re-derived downstream from its
output, and it is wrong in both directions. A shortcut can expand to the
token it was given — `-` pointing at the branch you are already on — and
string equality reads that as a literal, turning the path arm back on
for a token nobody typed. Normalization breaks it the other way, which
is why the trailing-separator fix needed threading through three call
sites.
`Selector` carries the fact instead: `expand_shortcut` reports whether
it fired, `wt switch` reports its `pr:`/`mr:` dispatch and remote-prefix
strip, and `names_a_path()` replaces all four comparisons.
`resolve_selector` is the ladder, and `plan_switch` expands into it
rather than re-implementing its phases.
`names_a_path()` gates both path steps together — the worktree-by-path
lookup and the directory verdict — which is what `wt switch --create`
needs: the argument names a branch to create, so `branch_only()` takes
the arm off at the producer rather than each consumer re-testing
`create`.
It also reaches the directory verdict, so `ResolvedWorktree` gains
`NoWorktreeAtPath` and the four sites that called `path_selector_error`
themselves stop re-deriving it. The docstring defending that laziness
didn't survive checking — the function returns on `is_valid_branch_name`
before touching the filesystem, so every ordinary branch name already
short-circuited.
| | before | after |
|---|---|---|
| `normalize_selector` call sites | 3 | 1 |
| `path_selector_*` call sites | 4 | 2 |
| "was it rewritten?" comparisons | 4 | 0 |
## Navigating the diff
- `src/git/repository/mod.rs` — `Selector`, `normalize_selector`, the
new `ResolvedWorktree` variant.
- `src/git/repository/worktrees.rs` — `expand_shortcut`,
`expand_selector`, `resolve_selector`, `usable_worktree_for_branch`.
- `src/git/repository/working_tree.rs` — `ensure_belongs_to_repo`, the
ownership check.
- `src/git/remove.rs`, `src/commands/repository_ext.rs` — where it gates
removal, and why before the dirty gate.
- Call sites: `commands/worktree/switch.rs`,
`commands/worktree/push.rs`, `commands/merge.rs`, `commands/remove.rs`,
`git/repository/config.rs`.
## Size
Comments and docstrings are the largest share: the ownership check and
the four conditions behind the directory verdict all look like things to
simplify away, so the reason each exists is recorded where it's
enforced.
| | + | − |
|---|---:|---:|
| Production code | 277 | 142 |
| Comments & docstrings | 320 | 75 |
| Tests | 325 | 8 |
| Snapshots | 186 | 0 |
| Docs | 6 | 0 |
| **Total** | **1114** | **225** |
## Testing
Seven new tests. The data-safety one drives the real binary and asserts
the filesystem afterwards, not just the exit code — removal stages by
rename and deletes in a detached process, so a passing exit would not
have caught a staged-then-deleted tree. The others cover the recreated
directory (switch and remove), the trailing separator, `--create`
against a worktree registered at that path, and, at the unit boundary,
the four states of `worktree_is_unusable` — healthy, absent,
locked-and-absent, recreated — and the selector's path-ness, including
the degenerate case string equality got wrong. Each new test was
confirmed to fail with its fix reverted.
One more covers an omitted merge target in a repo whose default branch
can't be determined. `^` had a test for that error; the omitted-target
route to the same message had none. The gap predates this branch — the
closure is byte-identical to the one it replaces and codecov records
those lines as missed at the base commit too — but relocating them into
`resolve_target_selector` re-counted them as patch lines, which is what
surfaced it.
Local gate green: 4593 tests, lints, doctests, rustdoc under
`-Dwarnings`.
<details>
<summary>Behavioral matrix, verified against a build</summary>
```console
docs/ (trailing sep) ▲ Worktree for docs @ ../repo.docs
detached by path ▲ Worktree for detached worktree @ ../repo.det
leftover dir ✗ No worktree @ ../repo.leftover
recreated dir ✗ Worktree directory missing for rec
shortcut ^ ▲ Worktree for main @ ../repo
remove leftover ✗ No worktree @ ../repo.leftover
--base docs/ ✓ Created branch nf from docs
foreign-repo remove ✗ Directory @ ../repo.frn is not this repository's worktree
precious.txt survives
```
</details>
<details>
<summary>Also swept, and one thing left alone</summary>
Three more instances of the same shape, fixed here:
- `resolve_base_ref` was the fourth copy of the comparison, so `--base
docs/` now resolves too.
- `hint_for_repo` suggested `wt switch ^` after an existence probe a
recreated directory passes, pointing at a worktree the switch then
refuses.
- The pre-switch hook's `target` var used the bare shortcut expander, so
a hook saw `docs/` where the switch resolved `docs`.
The identical unborn/stale default-branch block in
`require_target_branch` and `require_target_ref` is extracted. The rest
of that pair differs in its existence predicate, extra arms, and final
error; sharing it would cost more in parameters than the duplication
does.
Left alone: `live_sibling_checkout` decides whether another worktree
still holds a branch during removal, and also uses `exists()`. Switching
it to `prunable` would make branch deletion *more* likely in a corner
case where the detached path already answers the other way. That is a
data-safety surface and a separate decision.
</details>
> _This was written by Claude Code on behalf of max-sixty_
|
||
|
Maximilian Roos
|
c37d07d87a |
refactor(output): resolve color in one place via anstream's global ColorChoice (#3777)
`wt` had five independent mechanisms deciding whether escape sequences reach the consumer: the anstream macros, a `println_verbatim!` macro that bypassed them, `ProgressiveTable`'s raw stdout writes, clap's per-command `ColorChoice`, and a raw `anstyle::Reset` in `terminate_output`. This collapses them onto the ecosystem primitive: `anstream::ColorChoice::write_global()`, which `AutoStream::choice` consults before tty detection — the mechanism behind every `--color=always` flag. Color now resolves in exactly one place, and code emits styles freely while the stream decides what survives. For navigating the diff: - **Payload surfaces declare their choice once.** The statusline (`src/commands/statusline.rs`) declares `Always` — a shell prompt or Claude Code captures and re-renders its line, so the escapes are the answer, not presentation; `Always` outranks `NO_COLOR` per the no-color.org convention, preserving shipped behavior byte-for-byte. `--help-page` declares `Always`/`Never` per mode and `--help-md` declares `Never` (`src/help.rs`). `println_verbatim!` is deleted; these surfaces print through the ordinary macros. - **`help.rs` keeps one color mapping.** The five per-command `cmd.color(...)` calls were inert — clap consults `color_when` only in `err.print()`, and every path here uses `err.render()` — so the embedded `--help` reference block always renders `.ansi()` and the stream strips or passes it. All 16 generated doc outputs (`--help-page`, `--plain`, `--help-md` across commands) are byte-identical before/after. - **`ProgressiveTable` consults the same choice rather than writing through a stream** (`src/commands/list/progressive_table.rs`) — anstream's strip adapter would eat its cursor-control CSI, so it reads `AutoStream::choice` once and strips content with `anstream::adapter::strip_str`, the exact transform the buffered path applies. `NO_COLOR` now works on default `wt list`. - **One truncator** (`src/styling/line.rs`): `display::truncate_to_width` — which cut escape-blind — is merged into `truncate_visible`, which now trims trailing whitespace before the `…` and appends a reset only when the kept prefix carries an escape. Styled output is byte-identical (`ansi_cut`'s style closers both block the trim and trigger the reset); one snapshot line changes, a plain branch name losing a needless `[0m`. - **stderr joins the model**: `-v` diagnostics route through `AutoStream::auto(stderr)` (`src/logging.rs`), so `NO_COLOR` and piping reach them, and the lone `ceprintln!` (std stderr, hardcoded escapes) becomes the anstream macro (`src/main.rs`). - **The guard widens** (`tests/integration_tests/output_system_guard.rs`): the stdout scan covers all of `src/` rather than `src/commands/`, and the new `test_color_follows_the_consumer` pins six unforced-color cases — the suite's global `CLICOLOR_FORCE=1` previously made anstream's strip/pass decision invisible to every test. Behavior changes, all in the strip direction: `NO_COLOR` and piped-strip now reach progressive `wt list`, `-v` stderr diagnostics, and the clap error tips; `terminate_output`'s reset is stripped when color is off; truncation no longer leaves a `[0m` on plain text or a space before the ellipsis. The statusline's `Always` semantic is now pinned by test rather than incidental. Testing: the full pre-merge gate is green (4581 tests) plus the feature-gated PTY picker suite; `test_color_follows_the_consumer` covers the unforced matrix, and `test_list_progressive_honors_no_color` pins the progressive path on a real PTY (no SGR under `NO_COLOR`, cursor-control CSI still flowing); docs-sync verifies the generated pages byte-for-byte. The `writing-user-outputs` skill paragraph that described `println_verbatim!` now describes the `write_global()` declaration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
f3ea2fd598 |
fix(errors): report a directory holding no worktree as a directory (#3773)
Passing a path that holds no worktree used to be reported as a missing branch: ``` $ wt remove /repo/.claude/worktrees/ghost ✗ No branch named /repo/.claude/worktrees/ghost ↳ To list branches, run wt list --branches --remotes ``` The argument is plainly a path; the error calls it a branch and sends the user to a listing it could never appear in. Now: ``` $ wt remove /repo/.claude/worktrees/ghost ✗ No worktree @ /repo/.claude/worktrees/ghost ↳ The directory exists but is not a worktree; to list worktrees, run wt list ``` #3767 fixed the hook that produced the leftover directory in #3753; this fixes the message a person gets when they type such a path themselves, which judewang named there as why it was hard to diagnose. Ref #3753. Two neighbouring commands had the same cause and are also fixed. `wt switch <path>` and `wt merge <path>` used to offer `wt switch --create <path>`, which fails with `fatal: … is not a valid branch name`; and `wt config state marker set --branch <path> foo` **succeeded silently**, storing state keyed by a path string. ## How the decision is made `Repository::path_selector_error` reports a directory only when all four conditions hold. Each is here because dropping it made `wt` assert something false, reproduced against a build: | Condition | Without it | |---|---| | Git could never accept the selector as a branch name | `wt step rebase HEAD~9` reports revision syntax as a path | | A directory is there | `wt remove docs` stops meaning the branch when `docs/` sits beside it | | No worktree of this repo is registered at it | a *detached* worktree — reachable by path alone — is reported as absent, contradicting the `wt list` in the hint | | It holds no git data | a sibling repo's live checkout, or a bare repository, is called a leftover only the user can delete | That last one is the data-safety case: the new variant's hint says only the user can delete the directory, so a false positive reads as an invitation to `rm -rf` a checkout with uncommitted work. Absence of git data is *established* rather than assumed — `symlink_metadata` with only `NotFound` counting, so an unreadable clone or a dangling `.git` symlink withholds the claim instead of earning it. The first condition is git's own ref-format rules, implemented in-process and pinned against `git check-ref-format` by a test. ## Navigating the diff - `src/git/repository/worktrees.rs` — `Repository::path_selector_error` and `holds_git_data`, with the rationale for each condition recorded where it's enforced. - `src/git/repository/branch.rs` — `is_valid_branch_name`, the ref-format predicate. - `src/git/error.rs` — the new `GitError::WorktreeNotFoundAtPath` variant. - `src/git/repository/config.rs` — `target_branch_at_path` → `target_worktree_at_path`, returning the worktree whole rather than collapsing "no worktree here" with "a detached worktree here". - Call sites: `commands/remove.rs`, `commands/worktree/switch.rs`, `commands/config/state.rs`. Reporting a detached *target* made `DetachedHead`'s fixed hint wrong: `git switch` acts on the tree it runs in, so `wt merge ../B` was telling the user to switch the tree they were standing in. The variant now carries the detached worktree when that isn't the current one, and the hint becomes `git -C ../B switch <branch>` — the same reason `OperationInProgress` carries `branch`. Set at the three sites where the detached worktree isn't the current one (`require_target_branch`, `require_selected_branch`, and `wt step promote`, which raises this about the main worktree); every other site passes `None` and renders as before. 140 of the 257 added production lines are comments and docstrings — three of the four conditions look like obvious things to simplify away, so the reason each exists is written down next to it. ## Testing Unit tests at the resolver boundary cover each condition and its complement, both `directory_exists` arms, the shortcut-expansion route (`wt merge -`), detached targets, revision syntax, unresolvable `.git` entries, and bare repositories. Two integration tests drive the real binary end-to-end against `../repo.leftover` — `wt remove` and `wt switch` — covering the relative-path spelling and each command's wiring, and the `switch` snapshot pins the thing that would regress silently: that no `--create` hint is offered for a name git would reject. The ref-format predicate is verified beyond its committed table: 2,791 generated names were run against real `git check-ref-format` with zero disagreements, and two review passes did the same over their own inputs. That sweep isn't committed — it needs Python and a live `git` — so the committed table is what guards against drift. Not exercised locally: the Windows-specific behaviours are reasoned, not run — `Path::new("foo.").is_dir()` succeeding against `foo`, and drive-relative `C:foo`. CI exercises the platform but not those inputs. <details> <summary>Known gaps this would ship with</summary> - `wt switch docs/` (trailing slash, branch `docs` has a worktree, `./docs` is a source directory) answers `No worktree @ docs/`. True, unhelpful. The real fix is stripping trailing separators before resolution — a resolution change, not a message change. On Windows it reads `No worktree @ …/docs`, because `path_slash`'s `to_slash_lossy` rebuilds the string from `Path::components()` there and drops the trailing separator — still true of the path, minus the character that explains it. The doc comment on `path_selector_error` now says so rather than promising a spelling guarantee that holds on one platform. - A worktree whose registration was pruned out from under it gets a safe-but-vague message. The useful answer is a distinct `git worktree repair`-or-delete diagnosis; that's new detection, and nobody has reported hitting it. - Under `-C`, a `./`-prefixed selector renders a visible `/./` (`wt -C repo remove ./ghost` → `repo/./ghost`). Cosmetic; tidying it would discard the trailing separator the `docs/` case needs. - `require_target_branch` and `require_target_ref` remain near-copies of one ladder, and this adds a line to each. Pre-existing; extracting it is a separate refactor. - `CommandEnv::require_branch` passes `worktree: None` unconditionally, and `CommandEnv::for_selector` builds an env whose `worktree_path` is the worktree the *argument* named — so a future caller combining the two would get the unqualified hint. Nothing reaches that combination today: `merge` and `squash` are the only `require_branch` callers and both build with `for_action`, and the one `for_selector` caller (`wt step commit --branch`) never calls it. Two pre-existing bugs surfaced while reviewing this, unrelated to the diff and reproducible by branch name: `wt remove` will trash a clone that now sits at a registered worktree's path, and a recreated worktree directory leaks a raw `git rev-parse --git-dir failed (exit 128)`. </details> > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
5576eec1a4 |
fix(output): don't panic when a consumer stops reading stdout (#3766)
`wt list | head -3` exited 101 with `failed printing to stdout: Broken pipe`, and so did `wt list statusline | head -1` — the surface a shell prompt runs on every redraw. std's `print!`/`println!` panic on a `BrokenPipe`; anstream's drop it, which is why the `--format=json` surfaces #3746 converted already exit cleanly. Seven stdout surfaces were still on std's macros. Five of them — the `wt list` table, `--version`, `--help-md`, `--help-description`, and `wt config update --print` — are read by a person, so they go through anstream's printer, the canonical color-aware one. ## `wt list` also stops coloring a pipe `wt --help` documents `NO_COLOR` ("Disable colored output") and `CLICOLOR_FORCE` ("Force colored output even when not a TTY"). That second row only means something if color is off when stdout isn't a terminal — and `src/testing/mod.rs` sets `CLICOLOR_FORCE=1` in `STATIC_TEST_ENV_VARS` precisely so ANSI still appears in snapshots. But `wt list` wrote its escapes through std's macros, so it colored a pipe unconditionally and neither variable ever reached it. It now behaves as documented for the piped case: color on a terminal, plain on a pipe, `CLICOLOR_FORCE=1` to keep it on a pipe. `NO_COLOR` is fixed wherever `print_buffered_table` runs, which is the piped case plus `--no-progressive` on a terminal. It does **not** reach the default terminal rendering: `RenderTarget::detect` hands every tty that didn't pass `--no-progressive` to `ProgressiveTable`, which writes to a raw `std::io::stdout()` with each row's escapes already baked in. Closing that last gap means stripping SGR from the progressive rows while leaving the redraw's cursor control and the URL column's OSC 8 intact (`src/styling/hyperlink.rs` states `NO_COLOR` must not affect hyperlink support), so it's a separate change on the most visible surface in the tool. <details> <summary>PTY measurements</summary> On a terminal, with `CLICOLOR_FORCE` unset: | invocation | escapes | |---|---| | `wt list` (progressive, default) | 4590 | | `NO_COLOR=1 wt list` | 4663 — unchanged, the gap above | | `wt list --no-progressive` | 322 | | `NO_COLOR=1 wt list --no-progressive` | 0 | | `NO_COLOR=1 CLICOLOR_FORCE=1 wt list --no-progressive` | 0 — anstream checks `no_color()` first | Piped: 0 escapes by default, 17 under `CLICOLOR_FORCE=1`. Two earlier drafts of this description got this wrong in both directions, and @worktrunk-bot caught each. </details> The five changed snapshots all belong to `BareRepoTest`, whose `configure_wt_cmd` strips `CLICOLOR_FORCE` to capture "the terminal's plain output" and, until now, didn't get it. The diffs remove ANSI and change nothing else — same content, same column widths. ## Two surfaces keep the escapes For the statusline and `--help-page`, the pipe is a courier rather than the destination. A shell prompt or Claude Code captures the statusline and *renders* it; the docs pipeline converts the help page's escapes into HTML spans. Neither consumer is ever a tty, so anstream would strip them every time in production — and no test would catch it, because the suite forces color. They get `crate::output::println_verbatim!`, a sibling of `print_json` that writes the bytes through unchanged. It drops a `BrokenPipe` and still panics on any other write error, matching anstream, so both printers fail the same way on a full disk. Output is byte-identical: the help snapshots and `test_docs_are_in_sync` pass untouched. <details> <summary>Test, and two cleanups that fall out</summary> `test_stdout_surfaces_survive_a_closed_consumer` drives nine invocations across both printers with the child's stdout pipe closed before it is waited on, so the first write has no reader. That's deterministic rather than racing a consumer's exit, and `--version`'s few dozen bytes trip it as readily as the help page's 23KB — `EPIPE` is about whether a reader is attached, not about filling the buffer. Every case was checked to fail against the unfixed code. `statusline.rs` leaves `STDOUT_ALLOWED_PATHS`, since it no longer writes stdout directly; a stray `println!` there fails the guard again. `print_first_buffered_line` is gone. Its one caller is the `WORKTRUNK_FIRST_OUTPUT` benchmark hook, which wrote the same header line through a third path; a LineWriter flushes on the newline that measurement is timing, so the hook is unaffected. </details> > _This was written by Claude Code on behalf of max-sixty_ |
||
|
Maximilian Roos
|
7f8ac8e5d7 |
feat(list): publish a JSON Schema for the schema-2 envelope (#3747)
`wt list --format=json` schema 2 now has a published, machine-readable contract at [worktrunk.dev/schema/list-v2.json](https://worktrunk.dev/schema/list-v2.json). The schema was already derived — `test_schema_generates` built one, asserted it compiled, and threw it away, with a comment saying "until the schema export ships." This ships it: `wt list --print-schema` prints the document (a developer entry point alongside `--help-page`, intercepted before clap), and a new step in `test_docs_are_in_sync` commits it to `docs/static/schema/list-v2.json`, the same generate-and-commit pattern as `llms.txt`. It shells out rather than calling `schema_for!` because `JsonEnvelope` lives in the bin-only `crate::commands` tree. Two things had to be fixed for the document to be usable. **The contract.** `schema_for!` generates under schemars' *deserialize* contract, which marks a `skip_serializing_if` field required — nothing supplies it on the way in. The first document I generated therefore required `default_branch`, `upstream`, `pr`, `checks`, `summary` and `vars` on every item, all of which the absence rule routinely omits, so it rejected the output it documents. Generating under `for_serialize()` fixes it. **The vocabularies.** Four fields — `checks.status`, `display.state`, `default_branch.integration.reason` and `worktree.operation` — were `&'static str`, so the schema described them as bare strings. They are now `JsonCheckStatus`, `JsonMainState`, `JsonIntegrationReason` and `JsonOperation`, each converted from its domain enum by an exhaustive match, so a new `CiStatus`, `MainState`, `IntegrationReason` or `InProgressOperation` variant is a compile error rather than a value silently missing from the published vocabulary. **The emitted JSON is unchanged**; the existing envelope snapshot passes untouched. <details> <summary>Before and after, for one item</summary> ```json // before — rejects its own output, and loses the vocabulary "required": ["default_branch", "upstream", "pr", "checks", "summary", "vars", "display"], "status": { "type": "string" } // after "required": ["branch", "head", "display"], "status": { "enum": ["passed", "running", "failed"] } ``` </details> ## Testing `test_schema_accepts_envelopes` validates a battery — every `CiStatus` over both sources, every `MainState`, a populated worktree row, an integrated row with an upstream and a dev server, plus the absent and null arms of the absence rule — against the same document `--print-schema` emits. Validating proves nothing about a type the battery never instantiates, so the test also pins every non-`Nullable_` type in the document to a path that must carry a non-null value. A new `Json*` type fails until the battery reaches it, and a row that stops populating one fails too — the check reports the type names rather than leaving the gap to a reader. This needed a `jsonschema` dev-dependency: schemars only generates, and derives the document from the types without ever seeing an envelope, so nothing otherwise tied the two together. The test was confirmed to fail on the bug it exists for. Reverting to `for_deserialize()` makes it report `pr`, `checks`, `summary`, `vars` and `display.columns` as wrongly required. The dependency is dev-only: `reqwest`, `rustls` and `async-trait` stay unselected so no HTTP stack comes along, and `cargo tree --package worktrunk --edges normal -i jsonschema` finds no path to it. One direction it deliberately does not cover: a *loosening*. If a field reverted to `&'static str` the schema would say `type: string` and anything would validate. That direction is held by the compiler instead, via the exhaustive matches. ## Notes for review - `schema_document()` lives in `json_v2.rs` beside the types, not in `help.rs`, so `--print-schema` and the test compile the same document rather than two constructions that could drift on the contract setting. - The lychee exclusion for `worktrunk.dev/schema/` follows the entry directly above it: a generated link that 404s until the site deploys. > _This was written by Claude Code on behalf of max-sixty_ |
||
|
Worktrunk Bot
|
2203c414f5 |
fix(docs): convert a config-example link whose text holds a bracketed span (#3731)
`wt config create --project` writes a comment into the user's `.config/wt.toml` — and `wt config create --help` prints the same text — carrying a raw, unresolvable Zola link: ``` # When many repositories share one self-hosted host, name it once in user config with a [pattern-keyed `[projects]` entry](@/config.md#user-project-specific-settings) instead of repeating this block in each repo. ``` Every other cross-reference in that file is a plain URL (`… see \`wt hook\` (https://worktrunk.dev/hook/) …`), because `transform_config_source_to_toml` converts the `after_long_help` markdown to plain text on the way into `dev/wt.example.toml`. This one link isn't converted: `convert_markdown_links_for_config` matched link text with `[^\]]+`, which stops at the first `]` — here the one closing the nested `` `[projects]` `` span — so the regex failed to match and the markdown survived verbatim. The line arrived with #3701; it's the only link in either generated example file with a bracketed span in its text. ## The fix **One rule for `]` in link text.** `ZOLA_LINK_PATTERN`, earlier in the same file, already solves this problem for the skill mirrors — it alternates a backticked code span with any non-`]`-non-backtick char, which is why `skills/worktrunk/reference/config.md` renders this very sentence with a resolved URL while the TOML example didn't. `convert_markdown_links_for_config` now uses that same class rather than a second, weaker one. Brackets in these link texts always sit inside a code span, so the class fits the shape exactly, and it covers `[[…]]` array-of-tables names as well — these sections already document `[[projects."…".post-start]]` pipelines, so a link naming one is the next form to arrive. Regenerating produces the intended form: ``` # When many repositories share one self-hosted host, name it once in user config with a pattern-keyed `[projects]` entry (https://worktrunk.dev/config/#user-project-specific-settings) instead of repeating this block in each repo. ``` **A shape the regex declines now fails loudly.** Widening the class fixes the shapes we know about; it can't fix the next one. `finalize_skill_content` already handled that risk with a guardrail — after the rewrite it scans for a stray `](@/…md` and panics with the offending line, precisely because "the regex declined on an unexpected character in the link text" is the expected failure mode. `transform_config_source_to_toml` had no equivalent, which is why this one reached `dev/wt.example.toml` and the `--help` output. That check is now extracted into `assert_no_untransformed_zola_links` and called from both surfaces, so the next unsupported shape is a test failure naming the line rather than a raw `@/config.md` target in a user's config file. ## Why nothing caught it `test_project_config_source_generates_example_toml` compares `dev/wt.example.toml` against the output of this same transform, so an unconverted link is "in sync" by construction — the sync test can't see the difference between a link that converted and one the regex declined to match. Two tests close that gap: - `test_config_markdown_links_convert_to_plain_text` asserts the transform's output directly. It fails on `main`'s regex with exactly the reported symptom, and pins the forms already working (Zola page, Zola page + anchor, absolute URL, two links on one line, the `[[…]]` array-of-tables name) plus the case that must *not* convert — a bare `` `[forge]` `` span is not a link and has to survive verbatim. - `test_untransformed_zola_link_fails_the_config_transform` covers the backstop itself: an unbalanced backtick in link text makes the rewrite decline, and the assertion turns that into a panic naming the line. ## Files - `tests/integration_tests/readme_sync.rs` — the shared link-text class, the guardrail extraction and its second call site, and both tests. - `dev/wt.example.toml` — regenerated by the sync test (one line). - `tests/snapshots/…help_config_create.snap` — the same line, as `wt config create --help` renders it. Ran locally on the final state: `readme_sync::` (15), `test_help` (47), `cargo clippy --tests --all-features`, and `cargo fmt --check`. All green; the generated files are byte-identical under the new class, so the sync tests pass without regenerating. The full gate runs in CI. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Worktrunk Bot
|
d4538f2047 |
fix(alias): carry the shell's cwd into alias and hook bodies (#3724)
## Problem Since #939 / #3344, `wt switch` and `wt remove` preserve the user's subdirectory position — from `monorepo.feature/subproject/` you land in `monorepo/subproject/`. #3723 reports that this is lost one layer down: with `[aliases] finish = "wt remove -y"`, `wt finish` drops you at the primary worktree root. The resolution reads the user's position from the wt process's own cwd (`resolve_subdir_in_target`, called with `std::env::current_dir()`). That answers "where is the user standing?" only for a top-level invocation. Alias and hook bodies run with the worktree root as their working directory, so the nested `wt` strips the source root off a cwd that *is* the source root, gets an empty relative path, and falls back to the destination root. ## Solution The CD directive file already travels to exactly the children that are allowed to move the user's shell. The shell's directory now travels with it: `apply_cd_directive_env` sets `WORKTRUNK_SHELL_CWD` wherever the CD file is re-added (`Cmd::stream` and the concurrent runner), and `scrub_directive_env_vars` strips it alongside the other directive vars, so an untrusted child neither keeps nor receives it. `shell_exec::shell_cwd()` reads it back, preferring the inherited value over the process cwd — which is what makes nesting compose, since each layer forwards the shell's directory rather than its own. Three sites ask that question and now go through it: `wt switch`, `wt remove` (`prepare_remove_directory_change`), and `wt step relocate`, whose existing comment already asks to behave identically to the other two. Nothing about the working directory of alias or hook *bodies* changes — `{{ cwd }}` is still the worktree root, per the documented contract. The only change is what a nested `wt` believes about the user's position. ## Testing Two integration tests in `tests/integration_tests/step_alias.rs`, both failing before the change with the exact symptom reported: ``` CD file should preserve the subdirectory (…/repo/apps/gateway), got: "…/repo\n" ``` - `test_alias_wrapping_remove_preserves_subdir` — the reported case (`[aliases] finish = "wt remove -y"` run from `feature/apps/gateway`). - `test_alias_wrapping_switch_preserves_subdir` — the same for `wt switch` inside an alias. The existing subdirectory-preservation tests in `directives.rs` (including the fall-back-to-root cases) still pass, as does the full integration suite — apart from `test_copy_ignored_preserves_file_executable_permissions`, which fails identically on `main` in this sandbox (umask `0002`, expects `0644` gets `0664`) and is unrelated. The open question from the first revision is answered: the new remove test leaves two processes with a cwd inside the worktree being removed (the alias parent in the subdirectory, the nested `wt` at the root) where the existing test has one, and `test (windows)` passes on it. Review follow-ups are in |
||
|
Aral
|
e949ed5fda | fix(shell): use command rm in wrapper cleanup to bypass rm aliases (#3714) | ||
|
Maximilian Roos
|
305f8fcd19 |
fix(gitea): key tea api errors on the HTTP status line, not the response shape (#3713)
`tea api` copies the response body to stdout and exits 0 whatever the
HTTP status, so both Gitea call sites guessed from the body: a `message`
key meant the request had failed, anything else was the resource. `tea
api --include` writes the status line and headers to stderr and leaves
stdout the bare body, so the status is available after all.
## Availability
`--include` shipped with the `api` subcommand itself in **v0.12.0** —
`cmd/api.go` doesn't exist at v0.11.1 — and the implementation is
byte-identical through v0.15.1. A `tea` old enough to lack the flag has
no `api` to send it to, so nothing that can make this call rejects it.
tea's own `api_test.go` passes flags ahead of the endpoint (`{"-d",
"@file", "/test"}`), which settles the argument order.
## What the status buys
Two things the shape check couldn't do:
**A body Gitea didn't write is now an error.** A reverse proxy's HTML
page has no `message` key, so it used to read as the resource — and `wt
list` then logged `Failed to parse tea api pulls JSON`, blaming a Gitea
API change for a gateway that never reached Gitea. Verified by restoring
the old key and re-running: `proxy_page_is_an_error` fails against it
and passes with the status.
**Retriability stops being a text sniff.** `is_retriable_error` was
looking for `429` or `connection` somewhere in Gitea's prose, which
never carries the code. 429 and 5xx are now the server saying "later";
every other 4xx is a token or a missing resource that repeating the call
won't change. `gh` and `glab` still sniff because they bury the status
inside prose.
Visible change: a blanked 500 shows the error indicator where it used to
paint the same blank cell as a healthy branch with no CI.
`gitea_blanked_500` and `gitea_not_found` pin both halves of that
contrast.
## The body's three states
The status decides; the body only supplies text, and its three states
are three different things to say — so `api_error_message` reports all
three rather than flattening them. The switch path prints the code in
its headline:
```
✗ Gitea API error 404 for PR #9999 on owner/test-repo: pull request does not exist [index: 9999]
✗ Gitea API error 500 for PR #101 on owner/test-repo, but the response carried no message — Gitea hides 5xx messages from non-admin tokens
✗ Gitea API error 502 for PR #101 on owner/test-repo, and the response body is not one Gitea sends
```
An exit-0 `tea` that writes no status line is a backstop rather than a
fall-through: `--include` prints the moment the response does, so its
absence means nothing arrived, and reading stdout as the resource there
is the failure the status line exists to prevent.
`switch_pr_gitea_no_status_line` covers it.
With the CI path no longer reading the message, `api_error_message`
drops to module-private.
## Tests
Every mock `tea` response now carries the two streams a real one writes,
via `tea_api_include_stderr`. `setup_mock_tea` takes the status
explicitly, so each test states the HTTP response it stands for instead
of leaving it implied — which also collapses four hand-rolled inline
mocks into it.
## Cost
`--include` writes the whole header block, not just the status line, so
captured stderr now holds whatever the server returns — a `Set-Cookie`
among it, on a Gitea that issues one. That reaches disk only under
`-vv`, which logs every subprocess's output to `subprocess.log`. There's
no narrower flag, and the status isn't available any other way; it's
noted in the module docs.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
> _This was written by Claude Code on behalf of Maximilian_
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
10e8bad2c5 |
fix(gitea): key the tea api error shape on the envelope, not on a non-empty message (#3600)
Gitea blanks a 500's message in production unless the token belongs to
an admin (`services/context/api.go`: `if setting.IsProd && !(ctx.Doer !=
nil && ctx.Doer.IsAdmin) { message = "" }`), so the body arrives as
`{"message":"","url":"…/api/swagger"}`. Both Gitea call sites required a
*non-empty* message before they would call a response an error, so that
body fell through to the data path:
- `wt list` logged `Failed to parse tea api pulls JSON`, once per branch
— blaming an API change for an API error. The CI cell was blank either
way, and the stderr layer is off at the default verbosity, so this
reached whoever was already debugging with `-v` or `RUST_LOG`.
- `wt switch pr:<n>` failed with `Failed to parse Gitea API response for
PR #N. This may indicate a Gitea API change.` — the same misattribution,
in the user's face.
## The key
The discriminator is now the presence of the `message` key, which is the
whole shape. `tea api` never reads the HTTP status — `runApi` copies the
body to stdout and returns nil regardless — so the body is the only
channel, and none of the resources read here carries a `message`:
verified against gitea `main` for `PullRequest`, `CombinedStatus`, and
`CommitStatus`.
`url` is deliberately *not* part of the key. The two mistakes aren't
symmetric: an error body that omits `url` would read as the resource,
which is the bug this key exists to prevent, while requiring it would
only guard against a resource one day growing a `message` field. Gitea
already ships error shapes without one (`APIInvalidTopicsError` is
`message` plus `invalidTopics`), and `CombinedStatus` *does* carry
`url`.
## Shape
One parser — `remote_ref::gitea::api_error_message` — now serves both
sites; the CI-status backend drops its duplicate struct, and the PR path
checks the envelope before the resource parse, so "may indicate a Gitea
API change" is reserved for a body that is neither. A blanked message
reaches the user as an error that says why there's no detail:
```
✗ Gitea API error for PR #101 on owner/test-repo, but the response carried no message — Gitea hides 500 messages from non-admin tokens
```
The CI cell for a blanked 500 stays blank, unchanged:
`is_retriable_error` is the one gate every backend uses to turn a
failure into the error indicator, and an empty message isn't retriable.
Only the misleading log line is gone.
## Tests
Extended rather than duplicated:
`test_tea_api_error_reads_the_response_shape` flips its blank-message
assertion (that assertion *was* the bug), a new
`test_api_error_message_reads_the_response_shape` pins the key against
every shape both sides see, and `switch_pr_gitea_blank_error_message`
snapshots the user-visible path alongside the existing 401/403/404
cases.
`test_list_full_gitea_parse_warning_is_reserved_for_unknown_bodies`
covers the PR-list side end to end. It drives `wt list --full` under
`RUST_LOG=warn` and reads stderr rather than snapshotting, since the
warning is a `tracing` record that the stderr layer suppresses at the
default verbosity (`-v` would turn it on but bury it under a template
expansion per worktree). A second case sends a body that is neither the
resource nor the envelope, where the warning does belong — that pins the
wording to unknown shapes and keeps the first case from passing merely
because nothing logs. Checked against the pre-fix key: the blank case
fails there with the misleading warning, the unknown case passes.
## Noted, not done
tea's `api` command has an `--include` flag that writes the status line
to stderr — a genuine structured channel that would beat shape-sniffing.
It's in tea's `main` source but in no released changelog entry, so
sending it would break every user whose `tea` predates it. It would also
compose with this change rather than replace it, since the body still
supplies the message text.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
> _This was written by Claude Code on behalf of Maximilian_
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
5c42c5b7d5 |
feat: machine-readable approval state and branch-removal outcomes (#3710)
Two of the five machine-readable-output requests NathanaelRea opened (#3696–#3700), reviewed as a set and implemented where the gap was real. ## `wt config approvals list --format=json` (#3698) The command already computed the four distinctions an orchestrator needs — no commands, approved, approval-required, and stale — read-only, without prompting or writing. It had no `--format` flag, so the only way to learn that a non-interactive run would stop for approval was to run the operation and catch `NotInteractive`, or to pass `--yes` and approve whatever was there. ```json { "state": "approval_required", "commands": [ {"phase": "post-start", "name": "dev", "template": "npm run dev", "approved": false}, {"phase": "pre-merge", "template": "cargo test", "approved": true} ], "stale": ["some removed command"] } ``` `state` is what a caller branches on. `stale` stays a separate list rather than a fourth `state`, because it co-occurs with all three — and those are the approvals `--yes` would silently re-approve after their command template changed, which is exactly what an orchestrator preserving the approval model needs to see. A flag on the existing read command rather than a new `status` verb, matching `wt config show`, `wt config state get`, `wt config state logs`, and `wt list`. Closes #3698. ## `branch_outcome` on removal (#3700, partly) `wt remove --format=json` reported the branch as one boolean, collapsing five internal outcomes into two values: | Internal outcome | `branch_deleted` was | |---|---| | `Deleted` | `true` | | `Deferred` — handed to a detached process, result never observed | `true` | | `NotAttempted` — no branch, or `--no-delete-branch` | `false` | | `Retained` — a sibling worktree has it checked out | `false` | | `Retained` — **the CAS refused; the ref moved under us** | `false` | The last row is the exact race #3700 asks for protection against. Worktrunk already deletes with `git update-ref -d <ref> <oid>` and already fails closed when the ref has moved — then reported it as the same `false` that means "you asked me not to". And `Deferred` reported `true` on intent. `branch_outcome` names it instead: `deleted`, `deferred`, `not_attempted`, `retained_unmerged`, `retained_checked_out`, `retained_raced`, `retained_failed`. A caller that sees `retained_raced` knows to re-read the ref and retry, which is what the guard detects it for. **This does not close #3700.** That issue asks for an *input* — a caller-supplied expected OID that makes `wt` fail closed against the orchestrator's own observation. This is an *output*. They land in the same place on the default path, because the integration check already refuses to delete unintegrated content, so the caller was never going to lose commits — they just couldn't classify the refusal. Where the gap is real is `--force-delete` / `-D`, which takes the early return in `delete_branch_if_safe` and runs `git branch -D` with no integration check and no CAS. If an `--expected-oid` flag lands, it has to gate that path. ## Notes - **Output-format break.** `branch_deleted` is replaced, not supplemented, on `wt remove --format=json` and on `wt step prune --format=json`'s live path. Per CLAUDE.md, output formatting is on the flexible side of the interface line; flagging it here so the release changelog picks it up. - **`wt step prune --dry-run` keeps `branch_deleted`.** A dry run predicts; it runs nothing to have an outcome. Different thing, different name, documented as such. - **`retained_raced` and `retained_checked_out` have no deterministic CLI trigger.** Both come from windows between `wt`'s own fresh read and the ref mutation, which no hook can be scheduled inside. They're covered at the unit level (`branch_fate_from_result_mapping`, `branch_fate_json_outcome_is_distinct_per_fate`, and `cas_rejects_delete_when_branch_advances` in `src/git/remove.rs`, which drives the race with a stale snapshot). The integration tests cover the two reachable contrasts: `retained_unmerged` via a `pre-remove` hook that commits, and `not_attempted` via `--no-delete-branch`. - **`print_json` lives under `src/commands/list/`** and now has a third caller from outside that module. Worth a more central home; not moved here. ## The other three Reviewed but not implemented: - **#3696** — already possible. `wt --config-set 'list.json-schema = 2' list --format=json` pins the schema per invocation above every config layer, as does `WORKTRUNK_LIST__JSON_SCHEMA`. Answered on the issue; what's left is a docs gap and making an out-of-range value fail rather than degrade in JSON mode. - **#3697** — the machine-readable error channel. A real gap and the one policy call in the set; not started. - **#3699** — aimed at `wt config state logs --format=json`, which is a directory listing reconstructed from paths, under a model that overwrites. The append-only run record it wants is `commands.jsonl`. ## Testing `cargo run -- hook pre-merge --yes` green: 4533 tests, clippy, fmt, doctests, rustdoc, docs sync. > _This was written by Claude Code on behalf of max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
4a6fd1ec44 |
refactor(merge): leave target-worktree changes in place, drop the autostash (#3703)
`wt merge` / `wt step push` previously moved a dirty target worktree's uncommitted changes aside with an autostash (`git stash push -u`) and restored them after the push. That design entered `refs/stash` — a repo-global namespace any process can mutate, the source of #3683's race class — restored staged changes as unstaged, and existed only because the fast-forward's `receive.denyCurrentBranch=updateInstead` refuses any dirty worktree at all. Both strategies now advance the target through one `advance_target`: - a compare-and-swap `update-ref` (fails cleanly if the target moved since the snapshot; reflog entries are labeled), - `update-index -q --refresh` + `read-tree -m -u <old> <new>` in the target worktree — git's documented lenient `push-to-checkout` policy (githooks(5)), - a CAS rollback when the sync can't apply, so branch and worktree move together or not at all. Uncommitted changes at paths the push doesn't touch never move: unstaged edits stay unstaged, staged entries stay staged, untracked files stay put, and `refs/stash` is never involved. The autostash machinery (`TargetWorktreeStash`, `StashData`, `stash_restore_failed` and its exit-code path from #3693) is deleted — including the staged-restored-as-unstaged flaw, which disappears with the restore itself. Behavior changes: - Receive hooks no longer fire on the fast-forward path — there is no `git push`. A `git merge` run in the target wouldn't run them either, which is the line the module spec draws. - A sync that can't apply fails the whole command with the ref rolled back; `--no-ff` previously warned and left the worktree stale behind its own branch. - An untracked-path collision is refused upfront, naming the file, instead of stashed and later maybe-conflicting. Review highlights (three adversarial passes over the diff): - The upfront conflict check reads `status --porcelain -uall`, so untracked files inside untracked directories get the named upfront refusal rather than a generic sync failure (and one subprocess is dropped). - A commit racing into the CAS→sync window is detected by a post-sync ref re-read — receive-pack used to give the fast-forward path this check structurally — and warned about. - `read-tree` runs under `-c submodule.recurse=false`, keeping #1604 fixed for `submodule.recurse=true` users. - The ignored-file carve-out (an ignored file at a path the push tracks is overwritten, as a `git merge` there would) is pinned by an end-to-end test: two review passes claimed `read-tree` refuses it; experiment on git 2.55 refuted both. > _This was written by Claude Code on behalf of max-sixty_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
02a12c7f59 |
feat(config): match [projects."…"] keys by pattern, and carry forge there (#3701)
## Problem Forge platform is readable only from project config (`[forge].platform`) or a brand substring in the remote hostname. A self-hosted host carrying none of `github`/`gitlab`/`gitea` — a GitLab at `git.company.example`, a company git server — needs the same `[forge]` block in every repository's `.config/wt.toml`. Closes #3678. The user-level `[projects."…"]` table is where per-repository settings already live without touching each repo, but its keys are exact, so covering a host means one entry per repository. ## Solution **Pattern keys.** A `[projects]` key containing `*` matches any run of characters, `/` included, so one entry covers every repository on a host, nested groups and all. `*` is the only metacharacter. ```toml [projects."git.company.example/*"] forge.platform = "gitlab" [projects."git.company.example/platform/*"] worktree-path = ".worktrees/{{ branch | sanitize }}" ``` Every matching entry applies, least- to most-specific, so a narrower key wins where two set the same field and leaves the rest alone. A literal key is the most specific of all; specificity is the count of non-`*` characters. Rules and rationale: the `project_match` module docstring. **`forge` on `[projects]`.** Same shape as the repository's own block, carrying `platform` and `hostname`. Both describe the host rather than the repository — which is why an SSH alias resolved through `~/.ssh/config`, a name local to one machine, belongs in user config rather than a repository's committed one. A repository's own `[forge]` still wins field by field, being the more specific of the two: a repository that sets only `platform` still takes a matching entry's `hostname`. **One resolver.** `wt list`, its statusline, `wt switch pr:`, and CI-platform detection each read project config separately, so a configured platform could resolve in one command and read `unknown` in the next. They now share `Repository::configured_forge_platform` (and `forge_hostname` for the API host). ## Approvals `approved-commands` matches by the same rules, so a pattern entry approves its commands for every repository it covers. That widening is the user's to opt into — only a hand-written key is ever a pattern: - `wt config approvals add` and the interactive prompt record under the exact project identifier, so approving in one repository never reaches another. An identifier that itself contains `*` (a starred remote URL or no-remote path fallback) is refused outright — persisting it verbatim would create an entry reads treat as a pattern; the interactive flow degrades to a warning plus a per-run approval. - `wt config approvals clear` empties only the exact entry, leaving a pattern other repositories share intact — and both its outcomes end with a hint naming any pattern entries still approving commands for the project, so a surviving approval is traceable to the hand-written entry supplying it. - `--stale` judges only the exact entry, so one repository's config can't revoke approvals the others rely on. ## Tests `project_match` unit tests cover `*` spanning `/`, `.` staying literal, specificity ordering, and the lexicographic tie-break. Config tests cover a host-wide entry applying to nested groups, exact-over-pattern precedence, field-by-field layering, hooks appending across both entries, and forge platform/hostname. Forge resolution tests cover the unbranded host, nested groups, a narrower entry winning, project config overriding, falling through to inference, and an invalid value leaving the host unresolved. Approvals tests cover pattern lookup plus the two exactness guarantees above. ## Docs `src/cli/mod.rs` (the primary source) gains "Matching several repositories with one entry" and "Forge platform and hostname" under user project-specific settings, plus a pointer from the project-config forge section. Generated mirrors and `--help` snapshots regenerated. ## Review hardening An adversarial review pass surfaced eight findings, all fixed: - **Approval widening (moderate)**: the starred-identifier refusal above. Previously such an approval persisted verbatim and silently approved its commands for every repository the star matched. - **Literal-key tie (moderate)**: a pattern whose stars all match empty (`github.com/owner/repo*`) ties the exact key on literal count and sorted after it, so its values won the fold. Literal keys now outrank any pattern outright. - **Docs vs behavior (moderate)**: the layering paragraph claimed "most specific wins" for everything; hooks and aliases actually append across matching entries (all run, least-specific first). Docs now say so, and state the forge field-by-field precedence. - **Minor**: `matches()` is a two-pointer byte glob (was a per-call regex compile, ~0.7 ms per pattern key, a few hundred calls per `wt list`), pinned by an exhaustive differential test against a reference matcher; the invalid-platform diagnostics name their two possible config homes; a root `[forge]` in user config now points at `[projects."<id>"].forge`; docs note a host-wide key should end in `/*`; `approve_command` delegates to `approve_commands`, unifying their dedup predicates. ## Relationship to #3681 This is an alternative to #3681, which adds a bespoke `[forge-hosts]` section for the same issue. Both can't land — they'd be two ways to write one sentence. This one puts the setting in the table that already carries per-repository user config, and the pattern keys are reusable for the workspace-scoped ask in #3654 where repositories share a host or namespace. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Maximilian Roos
|
bfbc2e2899 |
fix(picker): abbreviate a PR's commits with git, not a fixed 8 (#3677)
Follow-up to #3676, which made `wt list`'s table render git's `%h` instead of `&head[..8]`. The `--prs` `log` tab was the last user-visible fixed-8 abbreviation. ## The defect `wt switch --prs` renders a row's `log` tab from the local `git log` when the PR head is in the object store, and from `gh pr view <n> --json commits` / `glab api …/commits` when it isn't. The forge path sliced each SHA to 8 characters, so on a repo where git abbreviates to 9 the same commit read `abc1234` at 8 before a fetch and 9 after, and `core.abbrev` never reached the pane at all. ## The fix `Repository::abbrev_len` asks git how wide it abbreviates in this repo and caches the answer on `RepoCache`, so a picker with 30 PR rows probes once — clones share the `Arc<RepoCache>`, and `once_cell::sync::OnceCell` collapses the concurrent background rows onto one fork. The pane wants the width rather than `short_sha` because of the branch it sits on: it runs precisely when the head *isn't* local, and `git rev-parse --short` takes a single revision — there is no batched form for a list of absent commits, and looping it would be one subprocess per commit per row. With no objects to disambiguate against, the width is git's whole answer for these SHAs. Both forge arms now parse the full SHA and share one abbreviation step in `render_commit_lines`. GitLab's ready-made `short_id` goes unused: it's the server's abbreviation, blind to the reader's `core.abbrev`, and taking it printed an MR's commits at a different width from a PR's. ## The audit #8 claimed the `--prs` tab was the last one; that came from a grep. Sweeping `.take(N)`, `[..N]`, `{:.N}`, `truncate(N)`, `get(..N)`, and `--abbrev`/`--short=` across `src/` turned up one more user-visible case: **`wt config state`'s CI cache table**, which sliced `cached.head` to 8 and now truncates to `abbrev_len`. One fixed-width slice over a SHA survives deliberately: `collect/tasks.rs` labels a `tracing::debug!` line with `&sha[..8]` as a fallback when a timed-out task has no branch name. It's a trace field rather than rendered output, and the timeout error path is the wrong place to spawn git. Everything else the sweep found hashes something other than a git object — `config::short_hash`'s base36 branch suffix, `summary.rs`'s content digest — or is a column width rather than a truncation. ## Cost Both call sites are flat in the number of SHAs: one `git rev-parse --short HEAD` per repo per process. The first cut of the CI table called `short_sha` per row instead, to keep git's extension of an ambiguous prefix. That's a fork apiece, and it cost more than the whole command was worth — measured on a repo with a cache entry per branch, `wt config state get` at 50 entries: `wt config state get`, 50 cache entries, release build, hyperfine (warmup 20, 80+ runs, quiet machine): | | mean ± σ | vs. baseline | |---|---:|---:| | before this PR (`.take(8)`) | 22.1 ± 0.8 ms | — | | `abbrev_len` (this PR) | 26.1 ± 1.3 ms | +4.0 ms, **1.18× ± 0.07** | | `short_sha` per row | 266.7 ± 3.9 ms | **12×** | The +18% is one `git rev-parse --short HEAD` against a command whose entire runtime is ~22 ms. For scale, a trace of the same command shows `gh --version` at 15.3 ms and `git config --list -z` running three times for ~12 ms, both of which predate this PR. The probe sits inside the non-empty branch, so a repo with no CI cache pays nothing. No Criterion target covers `wt config state` (`time_to_first_output` covers `remove`/`switch`/`list`; `picker_preview` covers worktree rows, not `--prs`, which needs a forge CLI), hence the standalone measurement. Worth noting against the daily suite's own resolution: run-to-run spread on an unchanged bench (`worktree_scaling/warm/8`, last 10 nightlies) is 18%, so a benchmark would not have caught the +18% either — though it would have caught the 12×. So the form here pays one probe and gives up disambiguation. Absent objects have nothing to collide with, and a live collision at 7–9 characters is rare enough for a display column to wear. On the picker, `abbrev_len` lands on a background pool thread behind a `gh`/`glab` call that costs orders of magnitude more, and never on a first-paint path. #3676 itself cost nothing at runtime, for the record: `%h` was already in the pre-skeleton `git log --no-walk` batch (`%ct` from the same fork drives row order), and the format string is byte-identical across that commit. It moved where the already-fetched string is copied onto the row and swapped a `COMMIT_HASH_WIDTH = 8` constant for an O(N) width pass over the rendered SHAs. No fork added, no phase reordered — the pre-skeleton budget table in `collect/mod.rs` reads 23 commands on both sides. ## Tests - `abbrev_len_follows_core_abbrev_and_covers_absent_objects` — the width matches `%h`, `core.abbrev = 12` reaches it, and it equals what git gives for a SHA with no object behind it. - `render_commits_abbreviates_to_the_given_width` — the pane prints the width it's handed, at 7 and at 12. - `parse_gitlab_commits_keeps_order_and_takes_the_full_id` — `id` over `short_id`. - `test_state_get_ci_head_uses_git_abbreviation` — the CI table's Head cell equals `git rev-parse --short`. It can't be a snapshot: the width scales with the repo's object count and a test repo's SHA isn't fixed. `cargo run -- hook pre-merge --yes`: 4494 passed. > _This was written by Claude Code on behalf of max-sixty_ |
||
|
Maximilian Roos
|
e1745db105 |
feat(list): abbreviate the table's SHA with git, not a fixed slice (#3676)
The Commit cell sliced `&head[..8]` while `--format=json`'s `short_sha` carried git's `%h`, so one commit read `1b9f1d96` in the table and `1b9f1d9` in JSON, and `core.abbrev` reached only the JSON. #3675 gave a detached row's Branch cell the same slice, so the disagreement showed up twice on one row. `ListItem::short_sha` becomes the only abbreviation of `head` anywhere: the Commit cell, a detached row's Branch cell, the statusline, and JSON all render it. `abbreviated_head()` is gone. ## Column widths `COMMIT_HASH_WIDTH = 8` is gone. The Commit column and the Branch column's detached budget both measure the SHAs they will render, so `core.abbrev = 12` no longer truncates mid-hash and the default 7 stops reserving a column nothing fills — the freed character goes to Message. ## Latency `collect()` folds `%h` onto the rows before layout instead of after the skeleton. The batch carrying it already gates the skeleton for `%ct` sort order, so this is a map lookup rather than new I/O, and both cells are identity columns with no placeholder — they still paint in the first frame. Measured on a 40-worktree / 400-branch fixture: git subprocess counts are identical (5 pre-skeleton, 108 for the full run). Pre-skeleton wall time is unchanged; running both binaries in each order, the sign of the difference follows run order rather than the binary (+1.5 ms with this branch second, −0.5 ms with it first), so the residual sits inside drift. ## Behavior change Where the commit-details batch fails, the Commit cell is now empty rather than a slice of a SHA git refused. Age and Message already report that failure the same way, under the same warning, and two snapshots show it. `render_text_cell` also stops styling empty text, so a blank cell no longer emits an escape pair around nothing. ## Reading the diff 160 files, but the hand-written part is +91/−79 in `src/commands/list/` plus a +71 test. The rest is generated. The docs mirrors and help snapshots are symmetric. Of the snapshot lines, content is +799/−799 — every changed line a 1-for-1 hash swap — while +1396 is insta `env:` metadata refreshing on the 128 snapshots this happens to touch. `test_list_abbreviated_sha_follows_git` pins the invariant: the table's hash equals JSON's `short_sha` at git's default and at `core.abbrev = 12`, and a longer prefix is ruled out. It fails against the old fixed slice. > _This was written by Claude Code on behalf of max-sixty_ |
||
|
Maximilian Roos
|
0f2d562541 |
fix(forge): classify a forge by the brand in the hostname, not by DNS label (#3673)
Reverts the branded half of the exact-label classifier and deletes the diagnostic built to explain it. `github-enterprise.acme.com`, `mygithub.com`, `gitlab-internal.company.com`, and the `github-personal` SSH alias resolve to their forge again, so CI status, `wt switch --prs`, and `repo.provider` work with no config. ## Why the label boundary goes It looked like an ownership check and wasn't one. An attacker controls their own DNS, so `github.attacker.example` has the exact label `github` and classified fine; `gitlab.evil.co.uk` likewise. What the rule actually excluded was the self-hoster who put the brand in a hyphenated name. It failed open for the adversary and closed for the customer. The residual case for it doesn't survive either. The hostname comes out of the user's own `.git/config`, and whoever can put a host there can put code there too — the trust decision happens at clone time, and by the time worktrunk reads the remote the user is already building from it. All the classification decides is which forge CLI (`gh`, `glab`, `tea`, `az`) runs against it. So the rule is recall-first: any host carrying `github`, `gitlab`, or `gitea` matches, first match winning. The cost is a host that merely sounds like a forge getting a forge CLI run at it, which surfaces as that CLI's error rather than as silence — the better of the two failures, and `forge.platform` overrides it. ## What stays Azure DevOps keeps suffix matching on its two service domains, and for a reason unrelated to security: those are service domains rather than a brand in the host, so every real hosted instance already matches, and the on-prem edition carries neither string. `dev.azure.com.attacker.example` and `evil-visualstudio.com` are outside the domains and carry no brand to fall back on, so they stay unclassified. Userinfo still resolves to the network host, so `https://github.com@attacker.example/…` is `attacker.example`. ## What goes `LegacyForgeAlias`, `Repository::legacy_forge_alias`, `legacy_forge_alias_diagnostic`, and its three emit sites in `wt list`, `wt switch --prs`, and `wt config show --full`. Every host the diagnostic fired on now classifies, so it could only ever return `None`. A host with no brand at all still reaches the existing generic hint, which is the right message there — there is no platform to infer. The end-to-end warning-dedup test goes with it, since no warning is raised from both the collect and `--prs` threads any more; `stash_warning_preserves_order` keeps the mechanism covered. ## Docs `## Forge platform` in `src/cli/mod.rs` described the override as being for SSH aliases and self-hosted instances, which now detect on their own. It states the rule and scopes the override to hosts carrying no brand — a Forgejo instance at `forge.example.com`. Mirrors, `dev/wt.example.toml`, and the two `config` help snapshots regenerate from it. --- The branch's history has a false start — the first commit widened the diagnostic, the second deletes it in favour of relaxing classification — plus a merge of `main` after v0.71.0 shipped. The net diff is the second approach; it all squashes on merge. Also supersedes [#3672](https://github.com/max-sixty/worktrunk/pull/3672), the triage bot's PR for the same issue — it widens the diagnostic rather than removing the need for one, so it should be closed too. Closes #3671 > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
9eb473e056 |
feat(list): name a detached row by its short hash, not - (#3675)
The Branch cell hardcoded `"-"` for a worktree with no branch. It reads as missing data rather than as a state, and it was the odd one out: the skeleton row, the statusline, and `worktree_display_name` all reach for `branch_name()`'s `"(detached)"`, so the same cell changed label as the row settled. Detached worktrees aren't exotic here any more — Codex creates one per session under `~/.codex/worktrees/`, and they sit in `wt list` alongside everything else. The cell now carries the row's abbreviated HEAD in dim yellow. Yellow keeps it from reading as a branch that happens to be named like a SHA; dim keeps a row that isn't on a branch quieter than one that is. `should_dim`'s removable dim still reaches the row's Path and Message cells, so that signal survives the override. ``` Branch Status Path Commit Branch Status Path Commit @ main ^| . 1243e9c0 @ main ^| . 1243e9c0 + - ! ⚑↓ ../codex/… bdc5c663 → + bdc5c663 ! ⚑↓ ../codex/… bdc5c663 + 1p ⊂ ../wt.1p bdc5c663 + 1p ⊂ ../wt.1p bdc5c663 ``` ### What to look at `display_name()` gains a HEAD-prefix fallback so it answers before the `%h` batch lands post-skeleton — the skeleton and settled rows now print the same text in the same style, with no restyle as the row fills in. Both the Branch cell of a detached row and the Commit cell of every row render the new `abbreviated_head()`, so one commit gets one spelling: sourcing the Branch cell from `short_sha` (`core.abbrev`-aware) instead put `1b9f1d9` beside the Commit column's `1b9f1d96` on the same row. The Branch column budgets `COMMIT_HASH_WIDTH` when any row is detached. Sized off branch names alone it truncated the hash — and it already truncated the skeleton's `(detached)` to `(detac` behind a short branch set, so that was a latent bug rather than a new constraint. The picker's matcher text and the statusline follow the display. A detached row now filters by the hash on screen rather than a `"(detached)"` token that matches nothing visible and collapses every detached row onto one key, and a prompt names the same worktree the same way its `wt list` row does. `⚑` on a detached row stays as it was. The flag's axis is "not at home", and a worktree with no branch has no home path to be at — but the docs described only "branch name doesn't match the worktree path", which doesn't cover the case that has no branch at all. They now name it. ### Testing Covered by the existing detached-head list snapshots (all four now show the hash), a new layout test pinning the column width against a short branch set, a unit test for the `display_name` / `abbreviated_head` pair across the skeleton boundary, and the statusline detached test rewritten to assert the hash. Full suite green locally: 4493 tests, clippy and pre-commit clean. > _This was written by Claude Code on behalf of max-sixty_ |
||
|
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>
|
||
|
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_. |
||
|
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>
|
||
|
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
|
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
|
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> |
||
|
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> |
||
|
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
|
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
|
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
|
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> |
||
|
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> |
||
|
Maximilian Roos
|
32f380a27b |
refactor(switch): forward the forge CLI's own error text (#3605)
Follow-up to #3600, independent of it (no shared files). GitHub and GitLab rewrote forge API failures into messages of our own, discarding what the CLI said. GitLab picked its three by prose — `error_text.starts_with("404")` — the pattern #3595/#3597 removed elsewhere. ## Before / after A GitLab MR that doesn't exist: ``` - ✗ MR !9999 not found + ✗ glab api failed for MR !9999 + ▎ glab: 404 Not found (HTTP 404) ``` A GitHub PR that doesn't exist — the one message kept, now with gh's verdict under it rather than instead of it: ``` ✗ PR #999999 not found on max-sixty/worktrunk (gh default). Check that `gh repo set-default` points to the correct repository. + ▎ gh: Not Found (HTTP 404) ``` ## The rule, now written down `src/git/remote_ref/mod.rs` gets an **Error Messages** section: forward the CLI's own line, and write our own only where the CLI *structurally cannot* report the condition. Two cases qualify — GitHub's 404 (it answers about an owner/repo *we* chose, from `gh repo set-default` or a remote; gh can't know that) and Azure's missing `azure-devops` extension (a precondition `az extension list` answers, which `az` never names). Reading more nicely than the CLI doesn't qualify. `cli_api_error` and `cli_api_error_details` gained docstrings for the mechanism: our context as the headline, the CLI's words in the gutter, so a provider with something to add passes it as `message` rather than bailing. ## Evidence Measured, not assumed — both CLIs render an API failure the same way: | | stdout | stderr | |---|---|---| | `gh api …/pulls/999999` | `{"message":"Not Found",…,"status":"404"}` | `gh: Not Found (HTTP 404)` | | `glab api …/merge_requests/…` | `{"message":"401 Unauthorized"}` | `glab: 401 Unauthorized (HTTP 401)` | `cli_api_error_details` prefers stderr, so the gutter gets the human line, not the JSON. That also surfaced a test bug: **the glab mocks never set stderr**, so they'd have enshrined output real glab doesn't produce. Fixed to match the probe. `gh` puts the status in a structured field, so its 404 arm keys on `status`. `glab` puts it only inside the message text — nothing to key on, which is why prose-matching was the only way to keep those arms. ## What this gives up A bad or expired token no longer gets "run gh auth login" / "run glab auth login"; it gets `gh: Bad credentials (HTTP 401)`. The far more common case — no auth configured at all — is unaffected: gh prints its own "please run: gh auth login" and always did fall through to forwarding. Happy to restore the GitHub 401 arm (it's structurally keyable); GitLab's can't come back without prose-matching. ## Testing `cargo run -- hook pre-merge --yes` → 4572 passed. Six snapshots reviewed individually before accepting (five updated, one new). Both paths also driven through the **real** `gh` binary (a live 404 and a live 401), not only mocks. Clippy clean with `--features shell-integration-tests`. Coverage: the GitHub 404 arm and the forwarding fallthrough each have a test, on both forges. `codecov/patch` also flagged one line — re-indenting the 404 block pulled a pre-existing gap into the patch, and it turned out to be real: the `gh repo set-default` half of the hint, the fork workflow the message exists for, had never been tested. `test_switch_pr_not_found_gh_default` closes it. > _This was written by Claude Code on behalf of Maximilian Roos_ |
||
|
Worktrunk Bot
|
bcd1ffdfde |
feat(worktree): warn when a branch is checked out in multiple worktrees (#3480)
## Problem `git worktree add --force <path> <branch>` bypasses git's "already used by worktree" guard, so the same branch can be live in two worktrees at once — breaking worktrunk's branch ⇔ worktree bijection. worktrunk never creates that state itself, but once it exists [`worktree_for_branch`](https://github.com/max-sixty/worktrunk/blob/cfd7fe469/src/git/repository/worktrees.rs#L64) silently resolved to whichever worktree git listed first (roughly creation order), and that choice flowed into every resolution path — `wt switch`, `wt push`, `wt step diff`, `wt context`, the picker — with no warning, error, or way to name the shadowed worktree. This implements the **detect-and-warn** option from #3392: make the invisible choice visible without changing resolution semantics. ## Solution - `worktree_for_branch` now collects **all** worktrees on the branch via a pure `worktree_paths_for_branch` helper; when more than one exists it warns once (per branch, deduplicated per process) and still resolves to the first — semantics are unchanged. - The warning names every path and points at a concrete fix: ``` ▲ Branch feature is checked out in 2 worktrees; wt uses the first: ┃ ~/repo.feature ┃ ~/repo.feature-dup ↳ To drop a duplicate, run git worktree remove ~/repo.feature-dup ``` Detection is separated from emission so the pure list logic is unit-testable and the ambiguity check stays cheap — `list_worktrees()` already holds the full list. ## Testing - Unit test `test_worktree_paths_for_branch_detects_duplicates` — the pure helper returns both paths in git's listing order for a duplicated branch, one path for a unique branch, and none for an absent one. - Integration test `test_step_diff_duplicate_branch_warns` — creates the duplicate with `git worktree add --force`, runs `wt step diff --branch=feature`, and snapshots the rendered warning. The warning only fires when a duplicate exists (which no prior test sets up), so existing snapshots are unaffected. --- Closes #3392 — automated triage --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Maximilian Roos
|
8865f20ab8 |
fix(git): bound the default-branch remote query, and make the bound bound (#3603)
Follow-up to #3596, which walled the test suite off from the network. That PR left the same unbounded call live for real users: `Repository::default_branch()` is the one detection helper allowed to fall through to `git ls-remote`, and nothing limited how long it could take *not* to answer — an unanswered SYN costs ~127 s per address on Linux (`tcp_syn_retries=6`) and git tries each of a host's addresses in turn, so a remote behind a dropped VPN or a dead host stalled `wt list --full` and `wt switch` for minutes. ## `Cmd::timeout` didn't bound wall-clock Adding a bound alone would have been decorative, which is the substance of this PR. `run_with_timeout_impl` killed only the direct child, and a grandchild inherits the child's stderr pipe — so a surviving one held the write end open and `read_to_end` blocked until it exited. Measured on a 3 s timeout over an `ls-remote` whose upload-pack sleeps 120 s: | | elapsed | |---|---| | before | 120.03 s | | after | 3.20 s | `git-remote-https` sitting in `connect()` is exactly that shape: it doesn't notice git died. So a timed child spawns into its own process group and expiry tears down the group — `killpg` with TERM → KILL escalation on Unix, `taskkill /T /F` on Windows, matching `wt step tether`. Every existing `.timeout()` caller (the fsmonitor stop/lsof probes, `reap.rs`, the shell probe) was latently unbounded the same way and is fixed with it. The isolation costs the kernel's tty broadcast: a Ctrl-C no longer reaches a timed child, so the user waits out the remaining bound. That's seconds, against an orphan holding a pipe for as long as its own operation takes, once per spawn. Recorded at `run_with_timeout_impl` and in CLAUDE.md's Signal Handling section, whose "only the current child does" claim was no longer true. ## The cache is the other half A timed-out query takes the local-inference fallback but is *not* persisted to `worktrunk.default-branch`. That cache is what stops later calls from re-detecting, so a guess made while the network was down would otherwise become the repo's permanent answer. `detect_from_remote` now returns a `RemoteDetection` enum so the cacheability decision is explicit and exhaustive rather than an `Option` that loses the distinction. Only a timeout separates cleanly, via `ErrorKind::TimedOut`. `ls-remote` exits 128 whether the network is down or the remote simply has no HEAD, so telling those apart would mean reading git's error text — and re-querying on every command is the cost the cache exists to avoid. Those stay cached, as before. ## Reviewing - `src/shell_exec.rs` — the process-group teardown; its docstring carries the rationale - `src/git/repository/config.rs` — `RemoteDetection`, the 10 s bound, and the no-persist path - `src/git/repository/mod.rs` — `run_command` now delegates to a `run_command_bounded` that takes the bound Also routes the four remaining hand-rolled git test envs (`src/git/remove.rs`, `src/git/repository/tests.rs`, `tests/integration_tests/bare_repository.rs`) through `configure_git_env`, so they carry #3596's `GIT_ALLOW_PROTOCOL` deny rather than re-deriving a subset of it. All four run local-only git commands today, so this is completeness, not a live hole — and a full suite run under `GIT_TRACE` confirms zero `git-remote-*` transport-helper spawns across 4374 tests. ## Testing Three tests, none of which touch the network: the grandchild case via `sh -c 'sleep 30; :'` (which stops the shell `exec`ing sleep, so there really is a grandchild), the end-to-end no-persist behavior via `remote.origin.uploadpack` pointed at a `sleep`, and the fail-fast unresolvable remote, which had no coverage before. The second waits out the real 10 s bound; both hanging-remote tests are `#[cfg(unix)]` since the vehicle needs a POSIX `sleep`, so `test (windows)` doesn't cover the no-persist path. > _This was written by Claude Code on behalf of max_ |
||
|
Maximilian Roos
|
ae363c2b4c |
test: stop the suite reaching the network (#3596)
`ci_status::test_list_full_with_configured_platform_github` hit nextest's 180s timeout in an advisory CI job while the same 2586-test run passed everything else. Its unloaded runtime is 2.2s, and a `-vv` trace of the same fixture shows exactly one non-local subprocess: `git ls-remote --symref origin HEAD` against `https://bitbucket.org/test-owner/test-repo.git`, the URL the test sets on itself. `Repository::default_branch()` falls through to the wire when neither the worktrunk cache nor `origin/HEAD` resolves, which is sanctioned behavior, but nothing bounds it, so the test blocks for as long as the connect takes. Reproduced by pointing that remote at an unroutable address: 11s on macOS, and on Linux an unanswered SYN costs ~127s per address (`tcp_syn_retries=6`), with `bitbucket.org` publishing six. It isn't one test. Setting `GIT_TRACE` across the harness shows a single full-suite run spawning 180 `git-remote-https` processes: 63 to github.com, 42 to gitlab.com, 27 to dev.azure.com, 3 to bitbucket.org, and the rest to names that don't resolve. Every one is unbounded, on three OSes, on every PR. So the fix goes in the harness rather than in the one test. `GIT_ALLOW_PROTOCOL=file` joins `STATIC_TEST_ENV_VARS`, which covers every `wt` subprocess and every PTY env builder including the hand-rolled ones, and the two git-only builders set it as well for direct `git_command()` spawns that don't consult that table. Local paths and `file://` keep working; anything else fails in milliseconds with `transport 'https' not allowed`, and default-branch detection falls back to local inference, so output is unchanged. `GIT_TERMINAL_PROMPT` was never enough on its own: it only suppresses the credential prompt that the host's 401 triggers, by which point the request has already gone out. The rust-lang/rust benchmark fixture clone is the one git call that *should* reach upstream. It opts back in through `allow_network_transports`, which lives next to the constant so the deny and the opt-out can't drift apart. `switch_pr_self_hosted_tea_authed` recorded the resolver's `Could not resolve host` in its snapshot, which is the same defect class (asserted output depending on DNS). It now records the transport refusal. Also folded in: `build_worktree_config_bare_layout` in `src/git/repository/tests.rs` carried a nine-line hand-rolled copy of `configure_git_env`, so it now calls it. Not addressed here: nothing bounds that `ls-remote` for real users either, so an unreachable remote blocks `wt list --full` and `wt switch` for minutes on the first call per repo. That needs a decision about whether a timed-out detection should write the `worktrunk.default-branch` cache. > _This was written by Claude Code on behalf of max_ |
||
|
Maximilian Roos
|
58444440d7 |
fix(ci-status): classify tea failures by shape; report azure-devops extension (#3597)
Two follow-ups from #3595, both in code that talks to a forge CLI. ## Gitea CI status had the same exit-0 blind spot as the switch path `tea api` never reads the HTTP status — it copies the response body to stdout and exits 0 — so the `!output.status.success()` gate in `src/commands/list/ci_status/gitea.rs` never fired for a 4xx/5xx. #3595 fixed this for `wt switch pr:<n>`; the CI-status backend is the second instance of the same bug. The commit-status path was the quieter one: every field of `GiteaCombinedStatus` carries `#[serde(default)]`, so Gitea's `APIError` body deserialized as `{state: "", total_count: 0}` — an error indistinguishable from "this commit has no statuses". The PR-list path was noisier but less harmful: the error body failed to parse and logged `Failed to parse tea api pulls JSON`, blaming an API change for what was an API error. `tea_api_error` now reads the response shape — the `APIError` body, whose message feeds the existing retriable sniff — and a non-zero exit keeps meaning what it always meant, `tea` itself failing (transport error, no login). A proxy's HTML error page stays unclassified, as before: sniffing arbitrary bodies is the string-heuristic pile this removes. ## `wt config show --full` reports the azure-devops extension The whole `az repos` command group ships in that extension, so without it CI status stays blank however well `az` is authenticated — a persistent, user-fixable condition, which is what this surface is for. `azure_devops_extension_installed` (added in #3595) answers it in JSON: ```console ✓ az installed & authenticated ▲ azure-devops extension not installed; run az extension add --name azure-devops ``` An `az` that can't answer counts as installed, so an unanswered check never becomes an accusation. The `wt switch pr:<n>` bail now uses the same sentence — one wording for one condition, which is the one-line change to `switch_pr_azure_extension_not_installed.snap`. ## Testing The two new Gitea tests were checked against the unfixed source: reverting `gitea.rs` alone fails both, so they pin the fix rather than describe it. The two pre-existing retriable tests kept their exit-1 shape but lost their fabricated payload — `exit 1` alongside `429 Too Many Requests` is the combination `tea` can't produce — and now carry a real transport error, which is the case that does exit non-zero. `config show --full` gets a case per extension state. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
c136cbcd6a |
fix(switch): classify forge-CLI failures by structure, not tool prose (#3595)
Follow-ups from #3589's audit of brittle string matching, plus one live bug the audit turned up. ## Gitea: the error path was unreachable From tea's own `cmd/api.go`: `runApi` never inspects `resp.StatusCode` — it copies the response body to stdout and returns nil. So `tea api` **exits 0 for 404, 401, and 403**, which put the whole 404/401/403 classification behind an `if !output.status.success()` that those responses never reach. A missing PR surfaced as `Failed to parse Gitea API response for PR #N. This may indicate a Gitea API change.` The fabricated `{"message":"403 Forbidden"}` fixtures hid this by mocking an exit code tea does not produce. The response *shape* now discriminates — the PR object, or Gitea's `APIError` body whose message is surfaced verbatim. A non-zero exit means `tea` itself failed (no login configured, transport error) and keeps the existing `cli_api_error` path, which shows tea's own stderr. ## Azure: the install command, from JSON #3589 dropped the appended `az extension add --name azure-devops` because detecting the case needed prose matching. `az extension list --output json` answers it structurally instead, and the whole `az repos` command group ships in that extension — so it's a precondition of the call, not one guess among many. The check runs only after a failure, so the happy path pays nothing, and an `az` that can't answer counts as installed: an unanswered check never becomes an accusation the user would act on. ## Statusline detection `is_statusline_configured` matched the substring `"wt "`, which `newt status` also satisfies. That is not only a wrong diagnostic line — `install-statusline` early-returns on it, so a foreign statusline made install refuse. It now asks for the adjacent token pair `list statusline`, which is the actual question and stays agnostic to `wt` / `git-wt` / an absolute path. ## Worktree removal: comment only The remaining `error.contains("not empty")` stays. There is no structured form to recover: the text is `git worktree remove`'s own stderr, so the failure never passes through a Rust `io::Error`, and `remaining_entries` can't substitute because permission-denied also leaves entries behind. The comment claiming an `io::ErrorKind` was lost upstream is corrected. ## Testing Every changed path is covered by an integration test with realistic mocks: the four Gitea tests now mock exit 0 with `APIError` bodies (`gitea_server_error` became `gitea_request_failed`, since a 5xx no longer exits non-zero), the Azure tests mock `az extension list` on both sides — present, and failing so the check comes back unanswered — and a new `install_statusline_foreign_command` test asserts a foreign statusline gets replaced rather than mistaken for worktrunk's. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
321ffe838c |
fix(step): gate every staging path on an unmerged index (#3588)
`wt step relocate --commit` committed conflict markers, the bug #3579 fixed everywhere else. It stages with `git add -A` and calls `CommitGenerator::commit_staged_changes` directly, so it reached neither of that PR's two gates: ```console $ git merge side # CONFLICT (content): Merge conflict in f $ wt step relocate --commit ◎ Committing changes in feature... ✓ Committed changes @ eb4cf19 $ git show HEAD:f <<<<<<< HEAD feature ======= side >>>>>>> side ``` `MERGE_HEAD` is gone and the working tree is clean afterwards, so the broken merge reads as finished. ## The gate belongs at the staging step, not the commit The obvious funnel is `commit_staged_changes`, the one call all three staging paths share. It cannot work, and the reason rules the shape out rather than merely disfavoring it: every caller stages *before* that call, and `git add -A` collapses an unmerged path's three index stages into one entry. After it, `git diff --diff-filter=U` returns nothing and git itself stops refusing. ```console $ git ls-files -u 100644 df967b9… 1 f 100644 ba2906d… 2 f 100644 2299c37… 3 f $ git add -A $ git diff --name-only --diff-filter=U # empty $ git commit -m x # succeeds ``` A check there passes vacuously in exactly the case it exists for. So the funnel sits one step earlier. `WorkingTree::stage(StageMode, action)` refuses, then stages, with nothing in between. It is now the only path to `git add` against a worktree's real index (every other `git add` writes a throwaway index via `temp_index`), so a fifth entrypoint can neither forget the gate nor order it after the staging that destroys the evidence. It also replaces three hand-rolled copies of the `StageMode` to `git add` match, so the callers come out smaller than they went in. ## The second gate is not belt-and-suspenders The callers keep their early refusals, which guard a different window: those run before hooks, prompts, and the LLM call, so a doomed commit runs no project commands. That placement leaves a hole the early refusal structurally cannot see, because a `pre-commit` hook is arbitrary project code, free to conflict the index after the gate has passed. ```console $ cat .config/wt.toml pre-commit = "git merge side" $ wt step commit --yes ◎ Running pre-commit project hook ✓ Committed changes @ 9e89c76 # markers and all ``` So `wt step commit` carried the same bug, reachable through any project config. That path isn't experimental, which may make it the more consequential half of this PR. `test_step_commit_refuses_hook_created_conflict` pins it. ## relocate skips rather than aborting An unresolved conflict is a per-worktree blocker the user has to fix by hand, like a locked worktree. relocate is a bulk command whose every other blocker (locked, uncommitted, template_error, occupied target) skips one worktree and carries on. Aborting the run would leave the other N-1 unrelocated, and would make passing `--commit` strictly less capable than omitting it. ```console $ wt step relocate --commit ▲ Skipping feature (1 path with unresolved conflicts) conflict.txt ✓ Relocated other: ~/code/wrong-clean → ~/code/myproject.other ``` `--format=json` records it as `reason: "unmerged"`, alongside the existing codes. Relocating a conflicted worktree *without* `--commit` is unchanged. Confirmed by hand that `git worktree move` carries the unmerged index, `MERGE_HEAD`, and the markers across, and that git still refuses to commit at the new path. Nothing stages there, so nothing is lost. ## Detection stays git's Nothing greps file contents for `<<<<<<<`. The whole chain is one structured question, `git diff --name-only --diff-filter=U -z`, which asks git which paths its index records as unmerged: the same question `git commit` asks itself. A content grep would false-positive on this very repo, whose docstrings contain the markers. The only marker-content checks in the diff are two test assertions, that the markers stay on disk for the user and never reach a commit. ## Merged with #3587 #3587 landed while this was in flight (as did #3578, whose `wt step push` refusal test shares the same spot in `merge.rs`) and hoisted `ensure_no_unmerged_paths("merge")` to `wt merge`'s entry, so merge refuses in its own name. The two are complementary: that PR gives each entry point a gate in the name the user typed, and this one guarantees the staging step itself is gated no matter who reaches it. Both sides edited the same docstring and added a test at the same spot; the merge keeps both paragraphs, lists all four entry points, and keeps both tests. The interaction leaves one narrow seam. `wt merge --no-squash`, with a `pre-commit` hook that conflicts the index, now refuses with `Cannot commit` rather than `Cannot merge`, because the entry gate already passed and the funnel names the step it fired in. Fixing the wording would mean threading a caller-supplied noun into `CommitOptions`, which #3587 considered and rejected as the wrong shape, so it stays. The default squash path isn't affected: squash stages before its hooks, so the funnel fires ahead of them. ## Verification Three integration tests, each asserting HEAD is unmoved rather than only matching the message: - `test_relocate_refuses_unmerged_paths`, the reported bug. - `test_relocate_unmerged_skips_only_that_worktree`: one conflicted worktree, one merely dirty. The sibling still relocates, and JSON carries `reason: "unmerged"`. - `test_step_commit_refuses_hook_created_conflict`, driven from a `pre-commit` hook, so it can only pass through the check inside `stage`. Gate green on the merged head (4557/4557, lints, fmt, doc sync), plus `cargo nextest run --features shell-integration-tests` (4546/4546 pre-merge) and `cargo clippy --all-targets --features shell-integration-tests`, which the gate doesn't compile. > _This was written by Claude Code on behalf of Maximilian_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |