* fix: prevent cross-PR push from subagent-induced branch switch
A workflow_dispatch run for zed-industries/cloud (workflow run 26036155393)
force-pushed the orchestrator's work onto an unrelated engineer's PR branch
(origin/reactivate-pro-plan, PR #2582). The orchestrator's reviewfrog subagent
called checkout_pr({pull_number: 2582}), which (1) moved the shared working
tree to pr-2582 and (2) persisted pushDest pointing at reactivate-pro-plan.
The orchestrator's subsequent commit + push_branch then clobbered the victim
PR. Recovery + disclosure in PR #2584.
Three compounding bugs closed here:
1. checkout_pr dirty-tree guard had a first-call hole: the previous condition
required ctx.toolState.issueNumber to already be set, so on workflow_dispatch
runs the first checkout_pr (commonly from a subagent) bypassed the guard
entirely. Now any PR switch with a dirty tree is refused, including the
first switch of a run. Idempotent same-PR re-checkouts are still absorbed
by alreadyOnBranch inside checkoutPrBranch.
2. push_branch trusted sticky pushDest blindly. Added a backstop: refuse
pushes where the local branch matches /^pr-(\d+)$/ AND pushDest.remoteBranch
differs from it AND the current run is not scoped to PR N (event.is_pr === true
&& event.issue_number === N). This catches subagent-induced silent branch
switches even if a future bug reintroduces a first-call hole in fix#1.
3. Build-mode self-review prompt told the orchestrator to ship "the output of
git diff" to the reviewer. The model in this run synthesized
git diff main...HEAD, which excludes uncommitted work — and Build self-review
runs BEFORE the commit, so the reviewer saw an empty diff and thrashed,
eventually calling checkout_pr on a random PR to find something to look at.
Prompt now specifies git diff origin/<base-branch> (two-dot, no HEAD),
which compares the working tree against the remote base.
Refs:
zed-industries/cloud workflow run 26036155393
zed-industries/cloud#2582 (victim)
zed-industries/cloud#2584 (disclosure)
* review: key dirty-tree guard on current branch + drop 'two-dot' misnomer
Address review feedback on PR #796.
1. checkout_pr dirty-tree guard now keys off the live current branch
(git rev-parse --abbrev-ref HEAD), not ctx.toolState.issueNumber.
issueNumber is ALSO set by get_issue / get_issue_comments /
get_issue_events, so a subagent doing get_issue(N) followed by
checkout_pr(N) on a dirty tree would have bypassed the original guard
(issueNumber === pull_number). The current branch is the actual
primitive for "would this call move HEAD" — querying it directly avoids
correlating on toolState that other tools write to.
2. modes.ts: drop the wrong "two-dot" label on git diff origin/<base>.
That's the single-rev form, not two-dot. Copilot was right that the
label was confusing/contradictory with the actually-shown command.
Colin McDonnell
6c166ac1cc