Skip to content

feat(switch): run --execute against the picked worktree#3394

Merged
max-sixty merged 3 commits into
mainfrom
feat/issue-3370
Jul 10, 2026
Merged

feat(switch): run --execute against the picked worktree#3394
max-sixty merged 3 commits into
mainfrom
feat/issue-3370

Conversation

@worktrunk-bot

Copy link
Copy Markdown
Collaborator

Closes #3370.

wt switch -x claude (no branch) previously errored with missing <BRANCH> because --execute was declared with requires = "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

  • Relaxed the clap constraint — dropped requires = "branch" from --execute (src/cli/mod.rs). --execute-args still requires = "execute".
  • Threaded the command through the pickerhandle_switch_command now passes execute / execute_args into handle_picker, which forwards them into the shared SwitchPipeline instead of hard-coding None. Because the picker and the argument path were unified onto one SwitchPipeline (refactor(switch): unify the picker and argument-path switch pipelines #2858), hooks, approval, template expansion, and the --execute deprecation warning all come along for free — no drift between the two paths.

Design decision: -x composes with every picker mode

The prior comment on the issue flagged one open question — whether -x should be valid only for the default picker or across --branches / --remotes / --prs too. I went with all modes and removed execute / execute_args from those three conflicts_with_all lists. Every picker mode ends by switching to a worktree, and -x is orthogonal — "run after the switch" — so restricting it to the default picker would be an arbitrary asymmetry. --create / --base / --clobber still conflict with the mode flags (those need an explicit branch name; -x does not). If you'd rather keep it scoped to the default picker, re-adding execute / execute_args to those three lists is a one-line revert.

Testing

Added test_switch_picker_runs_execute_command (shell-integration PTY test): drives wt 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 --help snapshot and the switch.md doc mirrors.

`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 worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 baseafter 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.

@max-sixty

Copy link
Copy Markdown
Owner

@worktrunk-bot

  • respond to the review
  • Why didn't the original agent respond to the review? did an agent successfully spin up? why didn't the notification checks grab it?

@max-sixty

Copy link
Copy Markdown
Owner

@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>
@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

@max-sixty — responded to the review and pushed 73b5971.

Review response

{{ base }} asymmetry (the substantive flag) — fixed rather than documented. The reviewer was right that it's a validation-vs-runtime mismatch: pre-flight validation (SwitchExecute scope) always lists base / base_worktree_path, but the picker set capture_source: false, so an existing-switch {{ base }} stayed unset and wt switch -x 'echo {{ base }}' passed validation then errored on the undefined value after the switch landed. Of the two options offered, I took the "make the paths match" one — it's the option consistent with this PR's whole thesis (no drift between the picker and argument paths). The picker now captures source identity just like the argument path, so {{ base }} resolves to the worktree you came from either way.

Because both entry points now capture, capture_source became an always-true field, so I removed it entirely — source capture is no longer a divergence axis (is_recovered, guarded inside capture_switch_source, remains the sole suppressor). New PTY test test_switch_picker_execute_base_resolves_to_source asserts -x 'echo {{ base }}' through the picker expands to echo main.

Cosmetic doc nit — fixed. Reworded the suggestion_ctx doc to say 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".

Why the original agent didn't respond

Short version: an agent did spin up, and it correctly self-loop-guarded — because this is a bot-reviews-bot situation.

Timeline:

  • tend-review (run 29044522350) posted the review at 19:40:12Z, authored by worktrunk-bot — the same bot that authored this PR.
  • That review's submitted event immediately fired tend-mention (run 29045164521) at 19:40:15Z. Its verify job set should_run=true (PR-author short-circuit), and the handle job ran Claude to completion (success, ~3.5 min). So yes — an agent spun up and read the review.
  • It exited silently. The handle prompt for a non-@-mention review is "If the review requests changes or asks questions, respond appropriately. If the review approves or is between humans, exit silently." Combined with the running-in-ci self-loop guard ("confirm the triggering actor isn't the bot itself"), the agent saw a bot-authored review on the bot's own PR and treated it as the bot talking to itself → silent exit.

Why the notification sweeps didn't catch it either: tend-notifications Layer B marks notifications read for any PR with a recent tend-review/tend-mention run (both ran on #3394), and Layer C drops notifications on bot-authored PRs outright. Even if one had survived, the same self-loop guard applies.

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 COMMENTED review that requests changes / asks a question is not the same as a self-loop), but that lives in the tend-ci-runner plugin, not this repo.

Re your second comment on the 👀-while-working principle: that's directed at @tend-agent and touches the same self-loop machinery (the tend-mention workflow already adds/removes a 👀 reaction for @-mention comments, so it's a matter of generalizing that). I'll leave it for that agent rather than reach into the generated workflow here.

@max-sixty

Copy link
Copy Markdown
Owner

@tend-agent — let's fix that. Did we change this recently? I thought this used to work. We need to trigger in this case

@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

@max-sixty — filed the fix upstream as max-sixty/tend#761. Both surfaces that need changing (the tend-mention prompt branch and the harness self-loop guard) are tend-generated/bundled, so there's no clean local override — the workflow header points changes there too.

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:

  • Before April 1, tend-mention didn't listen to pull_request_review events at all — a review never triggered anything.
  • April 1 (Upgrade tend workflows from 0.0.3 to 0.0.5 #1882, tend 0.0.5) added review support but the workflow-level if filtered bot-authored reviews out (review.user.login != 'worktrunk-bot') — so bot reviews triggered nothing.
  • April 8 (chore: regenerate tend workflows from upstream main #2009) removed that filter (bot reviews now trigger) and added a role-boundary carve-out to the prompt — the stated intent being to let the bot action its own review when it's wearing the reviewer hat. So since April the design has meant to do exactly what you want; the guards just defeat it in practice.

And it's not a trigger gap — the trigger fired. tend-mention run 29045164521 ran on the review's submitted event; verify set should_run=true via the PR-author short-circuit; handle ran Claude to completion (~2m50s, success) and read the review. It then exited silently. So the fix isn't "make it trigger" — it's "stop the self-loop guard from swallowing an actionable self-review."

Why it exits silently — two overlapping guards, no reviewer/author carve-out

The non-mention pull_request_review prompt branch (tend-mention.yaml#L234-L235) says "If the review requests changes or asks questions, respond appropriately. If the review approves or is between humans, exit silently." Combined with the harness self-loop guard ("before responding to a comment or review, confirm the triggering actor isn't the bot itself"), a bot review on the bot's own PR reads as "the bot talking to itself" → silent exit.

The bundled running-in-ci Self-conversation Guard already carves out the right case ("only respond if there is a distinct role boundary — e.g., you are the reviewer on your own PR"), but the blanter harness guard contradicts it and the prompt branch doesn't reinforce it, so the blanket guard wins.

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 tend-review), not a self-loop — action it; only exit silently on a plain approval or no-actionable-content review. And reconcile the harness guard with the same role-boundary exception the bundled skill already documents.

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: tend-mention already adds/removes the 👀 reaction, but only for @-mention comments (the React to mention / Remove eyes reaction steps key on @worktrunk-bot in the body). Generalizing it to any comment the bot picks up for work is the same upstream surface — happy to fold it into tend#761 or file it separately if you'd like it tracked.

@tend-agent

Copy link
Copy Markdown

@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 tend-mention non-mention review branch positively instructs that a review your own review workflow posted on your own PR is your reviewer role speaking — action it, exiting silently only on a plain approval or a review with no actionable content. 256 generator tests green; reaches this repo at the next tend release.

On the 👀-while-working note (that one was aimed at me): the reaction machinery already exists but is scoped to @-mention comments, as worktrunk-bot laid out. Generalizing it to the other triggers is a separate concern from this self-loop fix, so I'll open it as its own PR against max-sixty/tend on your word — reacting on the originating object (tend-triage → the issue, tend-review → the PR), with the caveat that the PR case toggles 👀 on every synchronize push.

@max-sixty max-sixty merged commit bd504f0 into main Jul 10, 2026
36 checks passed
@max-sixty max-sixty deleted the feat/issue-3370 branch July 10, 2026 11:30
@max-sixty max-sixty mentioned this pull request Jul 10, 2026
max-sixty added a commit that referenced this pull request Jul 10, 2026
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_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: Interactive picker should run when --execute is specified without a branch name

3 participants