feat(switch): run --execute against the picked worktree#3394
Conversation
`wt switch -x <cmd>` with no branch argument was rejected at parse time because `--execute` declared `requires = "branch"`. The picker path and the argument path already share a single `SwitchPipeline` (#2858), which carries the `execute` fields — the picker just hard-coded `execute: None`. Relax the clap constraint and thread `execute` / `execute_args` from `handle_switch_command` through `handle_picker` into the pipeline, so a picked worktree runs the command exactly as the argument path does. This also drops `execute` / `execute_args` from the `--branches` / `--remotes` / `--prs` conflict lists: every picker mode ends by switching to a worktree, so `-x` (run after switch) composes with all of them rather than only the default picker. Closes #3370 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
worktrunk-bot
left a comment
There was a problem hiding this comment.
The thread-through is clean and the design note on composing -x with every picker mode is convincing — every mode ends in a switch, so --execute is orthogonal. One newly-introduced edge worth a decision, plus a minor comment nit.
{{ base }} in a picker --execute errors on an existing switch. The picker constructs SwitchPipeline with capture_source: false, so source_branch/source_path are empty. For an Existing / AlreadyAt result, TemplateVars::for_post_switch (src/commands/template_vars.rs) only sets base when source_branch is non-empty, so the var is absent. But pre-flight validate_switch_templates runs under ValidationScope::SwitchExecute, which always adds base / base_worktree_path to the placeholder set (vars_available_in in src/config/expansion.rs), so validation passes. The foreground expansion then runs under UndefinedBehavior::SemiStrict and errors on the undefined base — after the switch has already completed.
So wt switch -x 'echo {{ base }}' → pick an existing worktree → Enter lands you in the worktree, then fails with Failed to expand --execute command: undefined value. The same template on the argument path (wt switch <existing> -x 'echo {{ base }}') succeeds because capture_source: true populates base with the source worktree.
This is niche and non-destructive (the switch itself succeeds; {{ base }} is documented as requiring --create, and the picker create path populates it correctly). But the argument-vs-picker asymmetry for an accepted-by-validation template is a small surprise. Options: leave it (the picker's empty base is arguably closer to the documented "requires --create" contract) and note the asymmetry, or have the picker capture_source: true so the two paths match. Flagging for a call rather than blocking.
Secondary, cosmetic: the reworded suggestion_ctx doc — "the picker, whose selection can't fail plan_switch that way" — isn't quite right; a picker create (alt-c, or a --remotes/--prs selection) does reach plan_switch with create: true and can fail there. suggestion_ctx: None is still correct, just for a different reason — the picker has no branch argument to embed in the enriched wt switch <branch> --execute=… hint. Wording like "the picker has no branch argument to suggest" would age better.
|
|
@tend-agent consider an principle where when something is being worked on, the originating comment gets the 👀 emoji And then when it's responded, the emoji is removed, so we don't have a littering of emojis everywhere, but humans have some indication that it's being worked on |
…ktree
Review flagged a validation-vs-runtime asymmetry: pre-flight
`validate_switch_templates` runs under `SwitchExecute`, which always lists
`base` / `base_worktree_path` as available, but the picker set
`capture_source: false`, so an existing-switch `{{ base }}` stayed unset and
`wt switch -x 'echo {{ base }}'` (no branch) errored on the undefined value
*after* the switch had already landed. The argument path never hit this
because it captures the source worktree.
Capture source identity on the picker path too, matching the argument path.
`{{ base }}` in a picker `-x` now resolves to the worktree the user came from,
exactly as `wt switch <branch> -x …` produces. Source capture is no longer a
divergence axis, so drop the now-always-true `capture_source` field entirely
(`is_recovered` remains the sole suppressor, guarded inside
`capture_switch_source`).
Also correct the `suggestion_ctx` doc: `None` for the picker because it has no
branch argument to embed in the suggested `wt switch <branch> --execute=…`
hint — not because its selection "can't fail plan_switch".
Adds `test_switch_picker_execute_base_resolves_to_source` (PTY): drives
`wt switch -x 'echo {{ base }}'` through the picker from the `main` worktree and
asserts the EXEC directive contains the expanded `echo main`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@max-sixty — responded to the review and pushed 73b5971. Review response
Because both entry points now capture, Cosmetic doc nit — fixed. Reworded the Why the original agent didn't respondShort version: an agent did spin up, and it correctly self-loop-guarded — because this is a bot-reviews-bot situation. Timeline:
Why the notification sweeps didn't catch it either: So nothing failed — the guard did exactly what it's designed to do. The gap is that it can't distinguish "bot chatting with itself" from "the bot's review workflow surfaced a real, actionable finding on the bot's own PR." When the reviewer and the author are both the bot, an actionable self-review gets swallowed, and it only got actioned because you asked by hand. That's a real edge worth a guard refinement upstream (a bot-authored Re your second comment on the 👀-while-working principle: that's directed at |
|
@tend-agent — let's fix that. Did we change this recently? I thought this used to work. We need to trigger in this case |
|
@max-sixty — filed the fix upstream as max-sixty/tend#761. Both surfaces that need changing (the Did we change this recently? Not in the way it feels. The honest answer is this flow has never reliably worked — but the intent flipped toward "should work" in April, which is probably what you're remembering:
And it's not a trigger gap — the trigger fired. Why it exits silently — two overlapping guards, no reviewer/author carve-outThe non-mention The bundled The proposed fix (in tend#761): make the non-mention review branch positively instruct that a bot-authored review requesting changes / asking questions is the reviewer role speaking (from Distinct from tend#607, which argues bot self-reviews are pure no-op waste — the opposite premise from this case, where the self-review found a real bug you wanted fixed. On your 👀-while-working note: |
|
@max-sixty — shipped the fix as max-sixty/tend#762, closing the issue worktrunk-bot filed (tend#761). It covers both surfaces #761 names: the harness self-loop guard now carves out the reviewer/author role boundary, and the On the 👀-while-working note (that one was aimed at me): the reaction machinery already exists but is scoped to |
Release v0.67.0: version bump and changelog. Highlights since v0.66.0: experimental `wt remove --reap` (#3396), `wt switch -x` opening the picker (#3394), the termimad panic containment in the picker's PR-comments preview (#3408), the `wt config show` SIGTTOU fix (#3327), the `GIT_*` discovery-var scrub for `wt step for-each` and the `--execute` fallback (#3400), and the `wt list` prompt-reserve fix (#3409). Seven commits landed on `main` after the initial cut; `origin/main` is merged back in and the two user-facing ones (#3394, #3411) plus the BY CONTEXT profile table (#3403) are folded into the changelog. Pre-release validation: local pre-merge gate green on the merged tree (4389 tests); nightly cross-platform suite green on the initial cut ([run 29086956095](https://github.com/max-sixty/worktrunk/actions/runs/29086956095)), with the seven post-cut commits each validated by their own PR CI; `cargo semver-checks` reports no breaking changes; data-loss surface review of the cumulative diff (including the drift commits) found one new destructive capability (`--reap`, explicit opt-in, adjudicated acceptable). > _This was written by Claude Code on behalf of max_
Closes #3370.
wt switch -x claude(no branch) previously errored withmissing <BRANCH>because--executewas declared withrequires = "branch", so clap rejected it before any picker code ran. Now it opens the interactive picker and runs the command against the selected worktree.What changed
requires = "branch"from--execute(src/cli/mod.rs).--execute-argsstillrequires = "execute".handle_switch_commandnow passesexecute/execute_argsintohandle_picker, which forwards them into the sharedSwitchPipelineinstead of hard-codingNone. Because the picker and the argument path were unified onto oneSwitchPipeline(refactor(switch): unify the picker and argument-path switch pipelines #2858), hooks, approval, template expansion, and the--executedeprecation warning all come along for free — no drift between the two paths.Design decision:
-xcomposes with every picker modeThe prior comment on the issue flagged one open question — whether
-xshould be valid only for the default picker or across--branches/--remotes/--prstoo. I went with all modes and removedexecute/execute_argsfrom those threeconflicts_with_alllists. Every picker mode ends by switching to a worktree, and-xis orthogonal — "run after the switch" — so restricting it to the default picker would be an arbitrary asymmetry.--create/--base/--clobberstill conflict with the mode flags (those need an explicit branch name;-xdoes not). If you'd rather keep it scoped to the default picker, re-addingexecute/execute_argsto those three lists is a one-line revert.Testing
Added
test_switch_picker_runs_execute_command(shell-integration PTY test): driveswt switch -x 'echo picker-exec-ran'through the picker, selects a worktree, and asserts the command lands in the EXEC directive file — the observable proof the pipeline ran--execute. Regenerated the--helpsnapshot and theswitch.mddoc mirrors.