fix(hooks): resolve project config from the worktree the hook acts on#2690
Conversation
A hook reads `.config/wt.toml` from `ctx.repo`, so whichever worktree that Repository is rooted at decides which file is used. Several call sites rooted it at the wrong worktree: - `pre-remove` resolved config from the primary worktree (`main_path`), not the worktree being removed — so a `pre-remove` added on a feature branch never fired when removing that branch's worktree until the change reached the default branch. It now reads the removed worktree's config (it's still on disk at hook time), falling back to the primary if that worktree has no `.config/wt.toml` (so a project-wide `pre-remove` on the default branch still applies to worktrees branched before it was added). The `wt remove` approval is split to match: `pre-remove` is approved per removed worktree against that worktree's config, `post-remove`/`post-switch` against the primary worktree's — closing the `TODO(pre-remove-context)` in main.rs. - `post-merge` resolved config from the feature worktree, though it runs in the target worktree. It now reads the target worktree's config; `collect_merge_commands` splits accordingly so the approval prompt matches what runs (also fixes a pre-existing `wt merge` approval/execution mismatch for `post-remove`/`post-switch`). - `wt step commit --branch <b>` resolved config from the cwd worktree, not `<b>`'s. `CommandEnv::for_branch` now roots the Repository at the target worktree, so `pre-commit`/`post-commit` for it read `<b>`'s config. This also fixes the diffstat suffix on its "Generating commit message…" line, which was empty because computed from the cwd worktree's (unstaged) index. The rule is documented as a spec in the `commands::hooks` module docstring, with the per-hook mapping; `CommandContext.repo`, `Repository::project_config_path`, and `execute_hook`/`prepare_background_pipelines` point at it. The one exception: `wt switch`'s `pre-start`/`post-start`/`post-switch` still read the invoking worktree's config, since the new worktree doesn't exist when those hooks are approved (and for `wt switch --create` it's a fresh checkout of that worktree's HEAD anyway). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
worktrunk-bot
left a comment
There was a problem hiding this comment.
One observation on the merge-side approval — non-blocking, possibly worth a follow-up. Rest of the change reads cleanly; the spec docstring in commands::hooks is a nice anchor.
…rompt `collect_merge_commands` bucketed `pre-remove` with the feature-worktree hooks unconditionally, so when the feature worktree has no `.config/wt.toml`, the batch loaded nothing for `pre-remove` — yet `execute_pre_remove_hooks_if_needed` falls back to the destination's config in that case (since `main_path` is the destination for `wt merge`). Result: a destination-scoped `pre-remove` would fire under `wt merge --remove` without appearing in the approval prompt. Resolve `pre-remove` in `collect_merge_commands` the same way the executor does: the feature worktree's config if it has one, else the destination's. Update the `commands::hooks` spec row to note the fallback is `RemoveResult.main_path` (primary for `wt remove`, destination for `wt merge`), and extend the merge teardown test to cover the destination-fallback `pre-remove` path alongside `post-merge`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`wt step prune` approved `[pre-remove, post-remove, post-switch]` as one batch against the *cwd worktree's* `.config/wt.toml`. But each pruned worktree's `pre-remove` actually executes against *that worktree's* own config (with a primary-worktree fallback), so a `pre-remove` in a pruned worktree's branch-local config that the cwd config lacks — and that isn't already in `approvals.toml` — ran without ever appearing in the approval prompt. The analogous gap was fixed for `wt remove` and `wt merge` in #2690; this brings `wt step prune` in line. ### Approach A new `approve_prune_hooks` in `src/commands/step/prune.rs` mirrors the executors' config resolution (`output::handlers::execute_pre_remove_hooks_if_needed` and `spawn_hooks_after_remove`): collect `pre-remove` from every `CheckSource::Linked` worktree (each against its own config, falling back to the primary's), plus `post-remove`/`post-switch` once against the primary, then approve the deduped union in a single `approve_command_batch` call — the same shape as `merge::collect_merge_commands`. `gather_check_items` runs before the approval so the candidate set is known. The parallel integration checks haven't run when approval happens, so this approves `pre-remove` for *every* linked worktree prune might prune — a superset of what executes. The dedup-by-template collapses the common case where multiple worktrees fall back to the primary's config. Stale-metadata and orphan-branch removals are correctly excluded: they resolve to `RemoveResult::BranchOnly`, which runs no hooks. The new spec entry in `src/commands/hooks.rs` documents the over-approval. Also dropped the `Repository::at(&primary_path).unwrap_or_else(|_| repo.clone())` defensive fallback in `main.rs`'s `approve_remove` and in the new `approve_prune_hooks`. `home_path()` already propagates; the fallback only added silent substitution of the cwd-rooted `repo` (potentially a worktree being removed) when `Repository::at` somehow fails on a path `home_path()` just produced. Plain `?` is honest, and identical to every code path tests exercise. ### Tests - `test_prune_branch_local_pre_remove_needs_approval` — the regression: a pruned worktree carrying a branch-local `pre-remove` the cwd config lacks aborts prune (needs-approval, non-interactive). Without the fix, the command succeeds and the hook runs. - `test_prune_runs_branch_local_pre_remove_hook` — `--yes` runs the branch-local hook and prunes the worktree. - `test_approval_prompt_prune_decline` (PTY) — declining the prompt prints "Commands declined, continuing removal" and prune continues without the hook. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tree (#2703) PR #2690 made every hook resolve its `.config/wt.toml` from the worktree it acts on, with one documented exception: `wt switch`'s `pre-start` / `post-start` / `post-switch` still read the invoking worktree's config. The carve-out was there because the new worktree doesn't exist yet when those hooks are *approved* — `git worktree add` runs after the approval gate. This PR eliminates that exception by reading the base ref's committed `.config/wt.toml` for the pre-creation approval and template pre-flight, so the prompt always lists the exact commands `execute_switch` and the background spawn will run. ### What changed - **`execute_switch`** and **`spawn_switch_background_hooks`** now root their hook `CommandContext` at the new/destination worktree (via the new `hook_repo_for_worktree`), falling back to the primary worktree's config if the destination has none — same rule as `execute_pre_remove_hooks_if_needed`. - **`approve_switch_hooks`** and **`validate_switch_templates`** take a pre-resolved `Option<&ProjectConfig>` rather than reading `ctx.repo.load_project_config()`. `run_switch` (and the picker) computes it once via the new `switch_hook_project_config`: - `SwitchPlan::Existing` → `Repository::at(dest).load_project_config()`. - `SwitchPlan::Create` / `Regular` → `git show <base-ref>:.config/wt.toml` via the new `base_ref_for_create`, which mirrors how `execute_switch` picks the `git worktree add` argument: explicit `--create` base if one was resolved, else the existing local branch, else its single remote's tracking ref, else (multi-remote) `<checkout.defaultRemote>/<branch>` when that matches one of the remotes — replicating git's DWIM so a `pre-start` on the resolved remote ref can't run unapproved. - `SwitchPlan::Create` / `ForkRef` → `git fetch -- <remote> <ref_path>` then `git show FETCH_HEAD:.config/wt.toml`. `execute_switch` re-fetches at run time (idempotent). `wt switch pr:N` / `mr:N` is explicit network work, so fetching at the approval gate is in keeping; the security note this closes is that for a fork PR the approval prompt now reads the PR's tree, not the local repo's, so what's listed and what runs can't diverge. - Skipped entirely when `--no-hooks` / `--no-verify` is in effect, so the resolution (including any fetch or primary-config parse) doesn't fire when the result is unused. - **`Repository::project_config_at_ref(gitref)`** is the helper for the `git show` path: pipes the content through `worktrunk::config::migrate_content` so deprecated patterns still parse, then `toml::from_str::<ProjectConfig>`. Never writes a `.new` migration file — the content comes from an arbitrary ref, not the user's working copy. `WORKTRUNK_PROJECT_CONFIG_PATH` overrides the path regardless of the ref, the same as `project_config_path`. - **`approve_or_skip_with_config`** in `command_approval.rs` is the switch-side approval entry point. Takes the already-resolved `ProjectConfig` and reuses `collect_commands_for_hooks` / `approve_command_batch`; the approvals namespace (`project_identifier`) still comes from `ctx.repo` since it's repo-wide. - **Spec**: `commands::hooks` module docstring drops the `wt switch` exception row. The new row describes the new/destination-worktree resolution and the `git show` mechanism for `--create`, and the trailing paragraph now states the invariant directly — every hook is approved against the same `.config/wt.toml` it executes against. The `wt step prune` over-approval caveat from #2701 is preserved as the one documented exception. ### Testing Four integration tests in `tests/integration_tests/switch.rs`, each verified to fail before the change: - `test_switch_create_reads_base_branch_config` — `wt switch --create new --base other-base` runs `pre-start` / `post-start` from `other-base`'s committed config; the cwd worktree has none. - `test_switch_existing_reads_destination_worktree_config` — `wt switch dest` runs `post-switch` from `dest`'s `.config/wt.toml`. - `test_switch_pr_reads_pr_ref_config` — `wt switch pr:42` against a PR ref that carries `.config/wt.toml`: non-interactive shows the PR's hook in the approval prompt and bails without running it; with `--yes` the PR's `post-start` runs against the new worktree. - `test_switch_create_honors_project_config_path_override` — `WORKTRUNK_PROJECT_CONFIG_PATH` wins everywhere, including the base-ref preview. Plus two focused unit tests: `base_ref_for_create_picks_the_checkout_ref` (the four arms, including the multi-remote `checkout.defaultRemote` DWIM) and `project_config_at_ref_reads_committed_config` (committed, missing, bad-ref, and malformed-TOML cases). The existing `test_switch_pr_hooks_see_pr_vars` still passes — it exercises the primary-worktree fallback (PR head ref has no `.config/wt.toml`, so the on-disk config in the cwd-which-is-also-primary wins). `cargo run -- hook pre-merge --yes` is green. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lution (#2708) 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. 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_commands` already uses `?` for its `Repository::at(destination_path)` (refactored in #2690), so it's untouched. Co-authored-by: Claude <noreply@anthropic.com>
…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>
… errors (#2714) ## Summary Each worktree's `.config/wt.toml` now stands alone. When the worktree a hook acts on has no `.config/wt.toml`, no project hooks run — the primary worktree's config is no longer consulted as a fallback. A present-but-malformed config aborts the operation with the parse error instead of silently using a different one. ## Why The fallback (#2690, extended to `wt switch` in #2703) was an internal detail meant to keep worktrees branched off *before* a hook was added still firing the hook. But it made which file each hook actually reads ambiguous — Codex flagged this in review of #2703 (P3): a destination worktree with a malformed `.config/wt.toml` silently ran the *primary's* hooks instead of surfacing the parse error. Removing the fallback gives a cleaner model: each worktree owns its config. Users who want a hook to apply project-wide commit it on the default branch — new worktrees branched off that branch inherit it. Worktrees branched off *before* the commit no longer fire the hook silently, which was the leaky part. ## Behavior change A `pre-remove` hook added on `main` no longer fires when removing a feature worktree that predates the commit adding it. To restore that behavior, branch the feature off `main` again (or commit the hook on the feature branch itself). ## What changed - `Repository::project_config_at_ref` now returns `anyhow::Result<Option<ProjectConfig>>` — `Ok(None)` for refs with no `.config/wt.toml`, `Err` for present-but-malformed. - Dropped the fallback from `commands::project_config::collect_remove_hook_commands` (the shared helper introduced in #2709) and from `output::handlers::execute_pre_remove_hooks_if_needed`, `worktree::switch::{switch_hook_project_config, hook_repo_for_worktree, spawn_switch_background_hooks}`, and `worktree::finish::finish_after_merge`. - Updated the `commands::hooks` spec docstring — table rows lose the fallback sub-clauses; trailing paragraph drops the fallback handling. ## Tests - Unit test `project_config_at_ref_reads_committed_config` now asserts `Err` on malformed committed config (was `None`). - Added `test_switch_existing_aborts_on_malformed_destination_config` and `test_remove_aborts_on_malformed_worktree_config` for the end-to-end abort path. - Updated three tests that relied on the fallback to commit configs on the relevant branches instead: `test_switch_pr_hooks_see_pr_vars` (commits on `pr-source`), `test_merge_teardown_hooks_read_destination_worktree_config` → `..._read_acting_worktree_config` (feature carries pre-remove), `test_approval_prompt_remove_decline` (commits before creating the worktree to remove). Snapshot regenerated for the last. Co-authored-by: Claude <noreply@anthropic.com>
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.
|
My Also, the team may not use some tools locally that I do like I created a new issue for this discussion #2818 |
A hook resolves its project config from
ctx.repo.load_project_config(), so whichever worktree thatRepositoryis rooted at decides which.config/wt.tomlit reads. Several call sites rooted it at the wrong worktree — most visiblypre-remove, which read the primary worktree's config rather than the worktree being removed, so apre-removeyou add on a feature branch never fired when removing that branch's worktree until the change landed on the default branch. This makes the rule uniform:ctx.repois rooted at the worktree the hook acts on — and adds a spec docstring for it.What changed
pre-remove(output/handlers.rsexecute_pre_remove_hooks_if_needed) now resolves config from the worktree being removed (it's still on disk at pre-remove time), falling back to the post-removal working directory's config (the primary worktree forwt remove, the destination forwt merge) if that worktree has no.config/wt.toml— so a project-widepre-removestill applies to worktrees branched before it was added. The approval is split to mirror this:main.rsapprovespre-removeper removed worktree against that worktree's config (post-remove/post-switchagainst the primary's), andmerge.rscollect_merge_commandsresolvespre-removethe same way the executor does — which closes theTODO(pre-remove-context)inmain.rs.post-merge(worktree/finish.rs) now resolves config from the target worktree (where it runs), not the feature worktree.collect_merge_commandssplits the approval accordingly (feature config forpre-commit/post-commit/pre-merge; destination config forpost-merge/post-remove/post-switch), which also fixes a pre-existingwt mergeapproval/execution mismatch onpost-remove/post-switch.wt step commit --branch <b>(commands/context.rsCommandEnv::for_branch) now roots theRepositoryat<b>'s worktree rather than the cwd, sopre-commit/post-commitfor it read<b>'s config. As a side effect this fixes the diffstat suffix on that command's "Generating commit message…" line, which was empty because it was computed from the cwd worktree's (unstaged) index — see the updatedstep_commit_branch_flagsnapshot.commands::hooksmodule docstring now has a "Which.config/wt.tomla hook reads" section with the per-hook mapping and the construction site for each;CommandContext.repo,Repository::project_config_path, andexecute_hook/prepare_background_pipelinespoint at it.The one exception, documented in the spec:
wt switch'spre-start/post-start/post-switchstill read the invoking worktree's config — the new worktree doesn't exist yet when those hooks are approved, and forwt switch --createit's a fresh checkout of that worktree's HEAD anyway, so the config matches.Testing
Three integration tests where the only worktree carrying a
.config/wt.tomlis the one the hook acts on, each verified to fail before the change:test_pre_remove_hook_reads_removed_worktree_config(remove.rs),test_merge_teardown_hooks_read_destination_worktree_config(merge.rs — coverspost-mergeand thepre-removedestination fallback), and the diffstat-suffix change instep_commit_branch_flag(merge.rs). Existing hook/merge/remove/step suites pass;wt hook pre-merge --yes(full suite + lints) is green.