fix(hooks): propagate Repository::at errors in pre-remove config resolution#2708
Merged
Conversation
… config Each of the three `pre-remove` config-resolution call sites — the two approval helpers (`main.rs::approve_remove`, `prune::approve_prune_hooks`) and the executor (`handlers::execute_pre_remove_hooks_if_needed`) — used a `match` that incidentally swallowed `Err(_)` from `Repository::at(wt_path)` into the legitimate "no .config/wt.toml" fallback. Rewrite as two-step so `Repository::at` errors propagate; the fallback now fires only when the removed worktree has no project config. Matches the sibling fix in #2701 that replaced `Repository::at(&primary_path) .unwrap_or_else(|_| repo.clone())` with `?` for the same reason. Co-Authored-By: Claude <noreply@anthropic.com>
worktrunk-bot
approved these changes
May 11, 2026
1 task
max-sixty
added a commit
that referenced
this pull request
May 11, 2026
…ve/merge/prune (#2709) ## Summary `wt remove`, `wt merge`, and `wt step prune` all approve `pre-remove` against each removed worktree's own `.config/wt.toml` (with primary-worktree fallback) plus `post-remove`/`post-switch` against the primary's. The pattern was duplicated in three places — subtle enough that #2701 had to fix it separately for prune after #2690 fixed it for remove/merge, and #2708 then had to fix the `Repository::at` `Err`-swallowing in three places independently. This consolidates the three call sites into one shared helper, so the next fix-this-pattern PR only has to touch one place. ## Changes - `src/commands/project_config.rs` — new `collect_remove_hook_commands(primary_repo, paths)`. Returns the `Vec<ApprovableCommand>` for `pre-remove` (per-worktree, with primary fallback) + `post-remove`/`post-switch` (primary), deduped by template. Carries forward #2708's `Repository::at(wt_path)?` propagation. - `src/main.rs::approve_remove` — replaced the per-worktree `approve_or_skip` loop with a single-batch flow. `wt remove` switches from N+1 prompts to one combined batch (strictly fewer prompts in practice; approvals saved by the first prompt would have suppressed later identical ones anyway). - `src/commands/merge.rs::collect_merge_commands` — `pre-remove`/`post-remove`/`post-switch` now come from the helper; merge keeps its inline `pre-commit`/`post-commit`/`pre-merge`/`post-merge` collection and bundles everything into the same `approve_command_batch` call. - `src/commands/step/prune.rs::approve_prune_hooks` — one-line helper call. The "over-approval superset" exception remains in the local docstring. - `src/commands/hooks.rs` — the "Which `.config/wt.toml` a hook reads" spec docstring now points at the shared helper. The executor `output::handlers::execute_pre_remove_hooks_if_needed` still has its own copy of the pattern (an executor-side, not approval-side, concern) — left as-is. ## Test plan - [x] `cargo run -- hook pre-merge --yes` — 3660 tests pass; clippy clean. - Existing `wt remove` / `wt merge` / `wt step prune` approval tests cover both branches of the helper (per-worktree config vs primary fallback). Co-authored-by: Claude <noreply@anthropic.com>
This was referenced May 11, 2026
Merged
max-sixty
added a commit
that referenced
this pull request
May 13, 2026
Release v0.50.0. Highlights: - Experimental Azure DevOps support (#1256, thanks @mikeyroush; fixes #1144 from @dlecan) — `wt switch pr:<N>`, `wt list --full`, and `wt config show --full` recognize Azure DevOps via the `az` CLI. - Experimental Gitea CI-status detection (#2702) on top of the Gitea `pr:` shortcut (#1320, thanks @SjB). - Hooks now resolve `.config/wt.toml` from the worktree they act on — the primary-worktree fallback is gone, and `post-remove` reads the removed worktree's config (snapshotted before removal). Approval prompts collect hook commands from the same worktree. Breaking change for setups that relied on the primary-worktree fallback; the changelog entry has the recovery action. (#2690, #2703, #2714, #2717, #2701, #2708, #2727, #2736, #2748) - The `wt switch` picker's `alt-r` removal no longer runs unapproved project hooks (#2746) — the picker's removal path is now routed through `handle_remove_output` and consults the existing approval state read-only. - `wt config alias show` with no name lists every alias's full definition (#2684, #2691); `wt --help` switches to a compact aliases pointer (#2688). - `wt list --branches` warm-run perf: SHA-keyed cache for `main↕` and `Remote⇅` ahead/behind counts; shared push-remote URL and local-branch scan (#2704, #2718, #2673). - Claude Code plugin ships the `wt-switch-create` skill (#2737, thanks @onetom for #2631). See `CHANGELOG.md` for the full list (8 Improved, 5 Fixed, 5 Internal). semver-checks reports breaking library-API changes (new enum variants without `#[non_exhaustive]`, removed `Branch::github_push_url`, new trait method on `RemoteRefProvider`), which mandates at minimum a minor bump pre-1.0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each of the three
pre-removeconfig-resolution call sites — the two approval helpers (main.rs::approve_remove,prune::approve_prune_hooks) and the executor (handlers::execute_pre_remove_hooks_if_needed) — used amatchthat incidentally swallowedErr(_)fromRepository::at(wt_path)into the legitimate "no.config/wt.toml" fallback. Rewrite as two-step soRepository::aterrors propagate; the fallback now fires only when the removed worktree has no project config.The executor's inline comment notes why we propagate here rather than follow the
UserConfig::load()silent-skip a few lines above: user config can be benignly malformed, but a path git no longer recognizes as a worktree is a different kind of failure.Follows the same shape as the sibling fix in #2701, which replaced
Repository::at(&primary_path).unwrap_or_else(|_| repo.clone())with?for the same reason.merge.rs::collect_merge_commandsalready uses?for itsRepository::at(destination_path)(refactored in #2690), so it's untouched.