fix(hooks): structurally close the approval-boundary TOCTOU class

[max-sixty]

Why

Project-defined hook commands (pre-*/post-*) are arbitrary code shipped in a repo the user may have just cloned. They were selected from .config/wt.toml twice: once at the approval gate to build the prompt, and again at execution when register/execute_hook re-read load_project_config(). Between the two reads, the operation itself mutates state — a merge moves the target ref, an auto-rebase rewrites the feature config, a removal scrubs the worktree, git worktree add materializes a --create worktree — so the second read could select a command the user never approved. On a fresh git clone && wt <op> that is remote code execution. On main the post-merge path was entirely unpinned; the others used point-fix config snapshots that the executor could still re-resolve around.

Approach

The gate selects the command set exactly once and freezes it into an immutable, type-state ApprovedHookPlan (new src/commands/hook_plan.rs). Covered executors consume only that value via execute_planned_hook / register_planned and hold no ProjectConfig/Repository for selection, so re-derivation is a compile error, not a review invariant. Rendering stays deferred (post-* hooks legitimately need post-operation context like the merge commit) but consumes the frozen CommandConfig list, never config.

Covered (gate and execution separated by a state mutation): pre-merge, post-merge, pre-remove, post-remove, post-switch, pre-start, post-start. Deliberately not covered — they have no gate→exec mutation window and share the gate's cached Repository: pre-commit, post-commit, pre-switch, wt hook <type>, aliases. This scope boundary is documented in the commands::hooks module spec.

Clean cutover: the point-fix snapshot apparatus is deleted (RemoveResult::removed_project_config, register_with_project_config, collect_remove_hook_commands, collect_merge_commands, removal_hooks_approved, approve_or_skip_with_config) — no parallel path, no compatibility flag.

Reviewer orientation

• src/commands/hook_plan.rs — the whole model: HookPlanBuilder (sole config→commands point), type-state HookPlan → ApprovedHookPlan (constructible only via approve/approve_readonly/empty), lookup/render_planned. Start here.
• merge.rs / main.rs / step/prune.rs / worktree/switch.rs / picker/mod.rs — the five gates that build a plan.
• output/handlers.rs / worktree/finish.rs — the executors that consume it.
• commands::hooks module doc — the canonical "which .config/wt.toml a hook reads" spec, rewritten for the plan model including why the uncovered set is safe (shared never-invalidated config cache).

Behavior parity is preserved: an empty plan (--no-hooks, declined, or no project config) runs no project hooks; the merge approval prompt is unchanged (still lists pre-commit/post-commit); the picker's read-only gate drops only unapproved project pipelines (strictly better than the old all-or-nothing verify boolean). The empty-plan fast path returns before any Approvals load or project-id resolution, so a malformed approvals.toml no longer aborts a command with nothing to authorize, and wt merge --no-hooks no longer parses the destination config. The removal data-safety re-validation, the Ctrl-C signal policy, and source-scoped filtering are untouched.

Testing

cargo run -- hook pre-merge --yes green (3751 tests), clippy + pre-commit clean. New regression tests: test_post_merge_hook_from_merged_feature_config_does_not_run (the TOCTOU itself, causally bounded), test_remove_no_project_hooks_ignores_malformed_approvals, test_merge_no_hooks_ignores_malformed_destination_config, plus hook_plan unit tests (frozen lookup, read-only filter, source-group ordering). Reviewed across eight structurally-distinct passes (adversarial, generalization, evidential, subtraction, metric, classification, holistic) plus a Codex review whose two P2 findings are fixed and locked with the malformed-config tests.

🤖 Generated with Claude Code

[worktrunk-bot]

The structural fix reads well — type-state from HookPlan to ApprovedHookPlan, executors that hold no ProjectConfig/Repository for selection, anchor-keyed exact lookup that returns &[] on mismatch rather than silently running a different worktree's hook. Anchor invariants line up for each covered hook: merge's feature_root and destination_path are computed from the same target_worktree_path/current_wt.root() shape in handle_merge and finish_after_merge; remove threads the same RemoveResult::worktree_path / main_path from gate to executor; switch uses plan.worktree_path() at the gate and the same value via result.path() / worktree_path in execute_switch and spawn_switch_background_hooks; prune resolves through the shared cached list_worktrees() slice. The behaviour-parity claims hold too — wt merge --no-hooks skips both feature_cfg and dest_cfg reads, and the empty-plan fast path in HookPlan::approve returns before Approvals::load / project_id resolution. The test_post_merge_hook_from_merged_feature_config_does_not_run regression is well-framed (a user-source control as the causal bound, with the injected project-source command provably never selected because the destination's gate-time config has nothing in it).

One CI-blocking issue from the Windows 🧪 Pre-merge hooks step: HookPlan::approve_readonly is #[cfg(unix)], but two module-level rustdoc intra-doc links target it. Under RUSTDOCFLAGS='-Dwarnings' cargo doc --no-deps --document-private-items on Windows, the item doesn't exist and rustdoc fails the build:

error: unresolved link to `HookPlan::approve_readonly`
  --> src\commands\hook_plan.rs:40:38
   |
40 | //! interactive / `--yes` gate) or [`HookPlan::approve_readonly`] (the picker,
   |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^ the struct `HookPlan` has no field or associated item named `approve_readonly`

Same error at line 255. Demoting both to plain code spans is the smallest fix; the docstring on approve_readonly itself stays intra-linkable via search since it's still defined on HookPlan for Unix builds, and Unix readers lose nothing because the immediate context already names the method.