Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/commands/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
//! |---|---|---|
//! | `pre-remove`, `post-remove` | the worktree being removed (`pre-remove` reads it on disk; `post-remove` reads a snapshot taken before removal, stashed on [`super::worktree::RemoveResult::RemovedWorktree::removed_project_config`]) | `output::handlers::execute_pre_remove_hooks_if_needed` and `output::handlers::spawn_hooks_after_remove`; the three approval call sites (`main.rs`'s `approve_remove` for `wt remove`, `merge::collect_merge_commands` for `wt merge`, `step::prune::approve_prune_hooks` for `wt step prune`) all delegate to [`super::project_config::collect_remove_hook_commands`] |
//! | `post-merge` | the merge destination (target branch's worktree if it has one, otherwise the primary) | `worktree::finish::finish_after_merge`; approval in `merge::collect_merge_commands` |
//! | `post-switch` after a removal | the post-removal working directory (where the user landed) — for `wt remove` / `wt step prune` the primary worktree, for `wt merge` the merge destination | `output::handlers::spawn_hooks_after_remove`; approval through `collect_remove_hook_commands` |
//! | `post-switch` after a removal | the post-removal working directory the user lands in ([`super::worktree::RemoveResult::destination_path`]) — usually the primary worktree, cwd when the primary worktree is itself removed (bare repo), the merge destination for `wt merge` | `output::handlers::spawn_hooks_after_remove`; approval through `collect_remove_hook_commands` |
//! | `pre-commit` / `post-commit` | the worktree being committed — the cwd worktree, or `<b>`'s worktree for `wt step commit --branch <b>` | `commands::commit`, `step::commit` (via `CommandEnv::for_branch`) |
//! | `wt switch`'s `pre-start` / `post-start` / `post-switch` | the new (`--create`) or destination (`wt switch <existing>`) worktree | `worktree::switch` — `hook_repo_for_worktree` (execution); `switch_hook_project_config` (approval / pre-flight, see Snapshot paths below) |
//! | `pre-switch`, `pre-merge`, `wt hook <type>`, aliases | the worktree `wt` was invoked in | the respective command entry points |
Expand Down
4 changes: 3 additions & 1 deletion src/commands/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,11 @@ fn collect_merge_commands(
if verify && will_remove {
let current_wt = repo.current_worktree();
let feature_path = current_wt.path();
// The feature worktree is removed; the user lands in the merge
// destination, so `post-switch` reads `destination_path`'s config.
all_commands.extend(collect_remove_hook_commands(
&destination_repo,
&[feature_path],
&[destination_path],
)?);
}
}
Expand Down
21 changes: 13 additions & 8 deletions src/commands/picker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ impl PickerCollector {
..
} => {
let main_repo = Repository::at(main_path)?;
let verify = removal_hooks_approved(&main_repo, worktree_path, approvals)?;
let verify =
removal_hooks_approved(&main_repo, main_path, worktree_path, approvals)?;
let mut announcer = HookAnnouncer::new(&main_repo, main_repo.user_config(), false);
handle_remove_output(
result,
Expand Down Expand Up @@ -353,17 +354,21 @@ impl CommandCollector for PickerCollector {
/// Whether every `pre-remove` / `post-remove` / `post-switch` command this
/// removal would run is already approved — a read-only check, no prompt.
///
/// `wt remove` / `wt merge` collect the same command set and prompt for it at
/// the gate. The picker can't prompt mid-render, so it runs the removal's hooks
/// only when they're already approved (e.g. from a prior `wt remove` /
/// `wt merge`) and skips them otherwise — unapproved project commands must
/// never run. See CLAUDE.md → "Project Commands Run Only After Approval".
/// `main_path` is the post-removal destination (where `post-switch` reads its
/// config, same as the executor); `worktree_path` is the worktree being
/// removed (where `pre-remove` / `post-remove` read theirs). `wt remove` /
/// `wt merge` collect the same command set and prompt for it at the gate; the
/// picker can't prompt mid-render, so it runs the removal's hooks only when
/// they're already approved (e.g. from a prior `wt remove` / `wt merge`) and
/// skips them otherwise — unapproved project commands must never run. See
/// CLAUDE.md → "Project Commands Run Only After Approval".
fn removal_hooks_approved(
main_repo: &Repository,
main_path: &Path,
worktree_path: &Path,
approvals: &Approvals,
) -> anyhow::Result<bool> {
let commands = collect_remove_hook_commands(main_repo, &[worktree_path])?;
let commands = collect_remove_hook_commands(&[worktree_path], &[main_path])?;
if commands.is_empty() {
return Ok(true); // nothing to run — `verify` is moot
}
Expand Down Expand Up @@ -1176,7 +1181,7 @@ pub mod tests {
// Empty approvals → the hook is unapproved.
let approvals = Approvals::default();
assert!(
!removal_hooks_approved(&repo, &wt_path, &approvals).unwrap(),
!removal_hooks_approved(&repo, test.path(), &wt_path, &approvals).unwrap(),
"an unapproved pre-remove hook is not approved"
);

Expand Down
31 changes: 24 additions & 7 deletions src/commands/project_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,25 +49,31 @@ pub fn collect_commands_for_hooks(
///
/// Each `pre-remove` and `post-remove` reads the removed worktree's own
/// `.config/wt.toml` — both hooks are *about* that worktree, and at approval
/// time it's still on disk. `post-switch` reads `primary_repo`'s config (the
/// post-removal working directory, where the user lands). For `wt merge`,
/// `primary_repo` is the merge destination — same rule, different "primary".
/// time it's still on disk. `post-switch` reads the destination worktree's
/// config — the post-removal working directory the user lands in, which
/// `prepare_worktree_removal` records as [`super::worktree::RemoveResult`]'s
/// `main_path` (the primary worktree, except cwd when the primary worktree is
/// itself being removed; the merge destination for `wt merge`). Pass those
/// `destination_path()`s in — the gate must read the same config the executor
/// will (`output::handlers::spawn_hooks_after_remove`).
///
/// No fallback to another worktree's config, mirroring the executors
/// (`output::handlers::execute_pre_remove_hooks_if_needed` and the
/// `post-remove` snapshot path in `spawn_hooks_after_remove`). A
/// present-but-malformed worktree config surfaces as an error so the user
/// fixes it rather than silently running a different one.
///
/// Templates are deduped so the approval prompt shows each command once.
/// Templates are deduped so the approval prompt shows each command once — so
/// `destination_paths` may contain duplicates (the common case: every removal
/// lands in the same primary worktree).
///
/// Callers feed the result into [`super::command_approval::approve_command_batch`].
/// `wt merge` prepends its own `pre-commit` / `post-commit` / `pre-merge` /
/// `post-merge` commands to the same batch; `wt remove` and `wt step prune`
/// approve the helper's output on its own.
pub fn collect_remove_hook_commands(
primary_repo: &Repository,
removed_worktree_paths: &[&Path],
destination_paths: &[&Path],
) -> anyhow::Result<Vec<ApprovableCommand>> {
let mut commands: Vec<ApprovableCommand> = Vec::new();

Expand All @@ -83,8 +89,19 @@ pub fn collect_remove_hook_commands(
}
}

if let Some(cfg) = primary_repo.load_project_config()? {
commands.extend(collect_commands_for_hooks(&cfg, &[HookType::PostSwitch]));
// `destination_paths` is usually one worktree repeated (every removal lands
// in the same primary) — read each one's config at most once.
let mut seen_dests: HashSet<&Path> = HashSet::new();
for &dest_path in destination_paths {
if !seen_dests.insert(dest_path) {
continue;
}
// Propagate a `Repository::at` failure rather than silently skipping
// — same as the removed-worktree loop above. See #2708.
let dest_repo = Repository::at(dest_path)?;
if let Some(cfg) = dest_repo.load_project_config()? {
commands.extend(collect_commands_for_hooks(&cfg, &[HookType::PostSwitch]));
}
}

let mut seen = HashSet::new();
Expand Down
9 changes: 5 additions & 4 deletions src/commands/step/prune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,9 @@ fn render_dry_run(
/// metadata and orphan-branch removals delete just the branch — no
/// `pre-remove`/`post-remove`/`post-switch`). The integration checks haven't
/// run yet, so every linked worktree is fed to the helper — its `pre-remove`
/// approval is a superset of what executes. `post-remove` / `post-switch`
/// resolve from the primary worktree's config (the same helper input). No
/// approval is a superset of what executes. `post-switch` resolves from the
/// primary worktree's config: a prune candidate is never the primary worktree,
/// so each removal's `RemoveResult::destination_path()` is `home_path()`. No
/// fallback between worktrees — each `.config/wt.toml` stands alone.
///
/// Returns `true` when hooks should run, `false` when the user declined.
Expand All @@ -401,7 +402,6 @@ fn approve_prune_hooks(
yes: bool,
) -> anyhow::Result<bool> {
let primary_path = repo.home_path()?;
let primary_repo = Repository::at(&primary_path)?;

let removed_worktree_paths: Vec<&Path> = check_items
.iter()
Expand All @@ -410,7 +410,8 @@ fn approve_prune_hooks(
_ => None,
})
.collect();
let all_commands = collect_remove_hook_commands(&primary_repo, &removed_worktree_paths)?;
let all_commands =
collect_remove_hook_commands(&removed_worktree_paths, &[primary_path.as_path()])?;

if all_commands.is_empty() {
return Ok(true);
Expand Down
11 changes: 11 additions & 0 deletions src/commands/worktree/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,17 @@ impl RemoveResult {
}
}

/// Post-removal working directory — where the user lands, and the worktree
/// whose `.config/wt.toml` `post-switch` reads. `None` for branch-only
/// deletions (no worktree was removed, so nothing was switched away from).
/// See the `main_path` field docs on [`RemoveResult::RemovedWorktree`].
pub fn destination_path(&self) -> Option<&Path> {
match self {
RemoveResult::RemovedWorktree { main_path, .. } => Some(main_path),
RemoveResult::BranchOnly { .. } => None,
}
}

/// Convert to a JSON value for structured output.
pub fn to_json(&self) -> serde_json::Value {
match self {
Expand Down
87 changes: 51 additions & 36 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -894,30 +894,33 @@ fn handle_remove_command(args: RemoveArgs, yes: bool) -> anyhow::Result<()> {
}

// Helper: approve every command the removal will run, in one batch.
// `pre-remove` runs in — and resolves its `.config/wt.toml` from —
// each worktree being removed (no fallback to the primary worktree's
// config, same rule as `execute_pre_remove_hooks_if_needed`);
// `post-remove` and `post-switch` run in the primary worktree
// afterwards and resolve their config from there. The shared helper
// assembles both, dedup'd by template. Returns `true` when the prompt
// was accepted or there was nothing to approve.
let approve_remove = |removed_worktree_paths: &[&Path], yes: bool| -> anyhow::Result<bool> {
let primary_path = repo.home_path()?;
let primary_repo = Repository::at(&primary_path)?;
let commands =
collect_remove_hook_commands(&primary_repo, removed_worktree_paths)?;
if commands.is_empty() {
return Ok(true);
}
let project_id = repo.project_identifier()?;
let approvals = Approvals::load().context("Failed to load approvals")?;
let approved =
approve_command_batch(&commands, &project_id, &approvals, yes, false)?;
if !approved {
eprintln!("{}", info_message("Commands declined, continuing removal"));
}
Ok(approved)
};
// `pre-remove` / `post-remove` resolve their `.config/wt.toml` from
// each worktree being removed (`removed_worktree_paths`); `post-switch`
// resolves from each removal's post-removal destination
// (`destination_paths` — `RemoveResult::destination_path()`, normally
// the primary worktree, cwd when the primary worktree is itself the
// removal target). No fallback between worktrees, same rule as the
// executors. The shared helper assembles them all, dedup'd by template.
// Returns `true` when the prompt was accepted or there was nothing to
// approve.
let approve_remove =
|removed_worktree_paths: &[&Path], destination_paths: &[&Path], yes: bool| -> anyhow::Result<bool> {
let commands = collect_remove_hook_commands(
removed_worktree_paths,
destination_paths,
)?;
if commands.is_empty() {
return Ok(true);
}
let project_id = repo.project_identifier()?;
let approvals = Approvals::load().context("Failed to load approvals")?;
let approved =
approve_command_batch(&commands, &project_id, &approvals, yes, false)?;
if !approved {
eprintln!("{}", info_message("Commands declined, continuing removal"));
}
Ok(approved)
};

let branches = args.branches;

Expand All @@ -941,8 +944,12 @@ fn handle_remove_command(args: RemoveArgs, yes: bool) -> anyhow::Result<()> {
}

// "Approve at the Gate": approval happens AFTER validation passes
let removed_path = result.removed_worktree_path();
let run_hooks = verify && approve_remove(removed_path.as_slice(), yes)?;
let run_hooks = verify
&& approve_remove(
result.removed_worktree_path().as_slice(),
result.destination_path().as_slice(),
yes,
)?;

let mut announcer = HookAnnouncer::new(&repo, &config, false);
handle_remove_output(
Expand Down Expand Up @@ -984,16 +991,24 @@ fn handle_remove_command(args: RemoveArgs, yes: bool) -> anyhow::Result<()> {
}

// Approve hooks (only if we have valid plans). Each removed
// worktree's `pre-remove` is approved against that worktree's
// own config — see `approve_remove` above.
let pre_remove_targets: Vec<&Path> = plans
.others
.iter()
.chain(&plans.branch_only)
.chain(plans.current.iter())
.filter_map(|r| r.removed_worktree_path())
.collect();
let run_hooks = verify && approve_remove(&pre_remove_targets, yes)?;
// worktree's `pre-remove` / `post-remove` is approved against
// that worktree's config, and its `post-switch` against the
// worktree the user lands in — see `approve_remove` above.
// (`destination_targets` is mostly the primary worktree
// repeated; the helper dedups by template.)
let all_plans = || {
plans
.others
.iter()
.chain(&plans.branch_only)
.chain(plans.current.iter())
};
let removed_targets: Vec<&Path> =
all_plans().filter_map(|r| r.removed_worktree_path()).collect();
let destination_targets: Vec<&Path> =
all_plans().filter_map(|r| r.destination_path()).collect();
let run_hooks =
verify && approve_remove(&removed_targets, &destination_targets, yes)?;

// Execute all validated plans: others first, branch-only next, current last
let show_branch =
Expand Down
Loading