diff --git a/mcp/checkout.ts b/mcp/checkout.ts index bf33a8c..ce50dd9 100644 --- a/mcp/checkout.ts +++ b/mcp/checkout.ts @@ -805,16 +805,14 @@ export function CheckoutPrTool(ctx: ToolContext) { return inFlight; } - // refuse to clobber an active checkout if the working tree is dirty — - // forces the agent to commit/push or discard before switching contexts - // instead of silently overwriting uncommitted work. `issueNumber` - // tracks any issue/PR the agent has touched (issues and PRs share - // GitHub's number space); the guard fires only when the agent is - // switching to a *different* number with a dirty tree, which captures - // the legitimate "stop, you have unsaved work" case regardless of - // whether the prior number was an issue or a PR. - const current = ctx.toolState.issueNumber; - if (current !== undefined && current !== pull_number) { + // refuse to clobber an uncommitted tree whenever this call would move + // HEAD away from the target pr-N branch. keyed off the live current + // branch (not toolState.issueNumber, which is also written by + // get_issue / get_issue_comments / get_issue_events and so doesn't + // mean "currently checked out"). catches the subagent-sharing-cwd + // case from zed-industries/cloud (2026-05-18). + const currentBranch = $("git", ["rev-parse", "--abbrev-ref", "HEAD"], { log: false }).trim(); + if (currentBranch !== `pr-${pull_number}`) { const dirty = $("git", ["status", "--porcelain"], { log: false }).trim(); if (dirty) { throw new Error( diff --git a/mcp/git.ts b/mcp/git.ts index e0d33c3..9ea85c1 100644 --- a/mcp/git.ts +++ b/mcp/git.ts @@ -253,6 +253,28 @@ export function PushBranchTool(ctx: ToolContext) { // validate push destination matches expected URL const pushDest = validatePushDestination(ctx, branch); + // backstop against subagent-induced cross-PR clobbers: a subagent + // shares cwd + toolState with the orchestrator, so its `checkout_pr(N)` + // moves HEAD to pr-N and persists pushDest pointing at the foreign + // PR's remote branch. refuse pr-N → origin/ pushes unless this + // run is itself scoped to PR N (zed-industries/cloud, 2026-05-18). + const prBranchMatch = branch.match(/^pr-(\d+)$/); + if (prBranchMatch && pushDest.remoteBranch !== branch) { + const prNumber = Number(prBranchMatch[1]); + const event = ctx.payload.event; + const runScoped = event.is_pr === true && event.issue_number === prNumber; + if (!runScoped) { + throw new Error( + `push blocked: local branch '${branch}' would push to '${pushDest.remoteName}/${pushDest.remoteBranch}', ` + + `but this run is not scoped to PR #${prNumber}. ` + + `the 'pr-${prNumber}' branch was created by a prior checkout_pr call (likely from a subagent — subagents share the working tree and toolState with the orchestrator). ` + + `you have probably landed your commit on the wrong branch. ` + + `switch to your own feature branch first (e.g. 'git checkout ') and then push. ` + + `if the push to PR #${prNumber} is intentional, this run needs to be triggered against that PR.` + ); + } + } + // block pushes to default branch in restricted mode if (pushPermission === "restricted" && pushDest.remoteBranch === defaultBranch) { throw new Error( diff --git a/modes.ts b/modes.ts index 01eaff7..bd99f1e 100644 --- a/modes.ts +++ b/modes.ts @@ -201,7 +201,7 @@ export function computeModes(agentId: AgentId): Mode[] { Otherwise delegate the \`${REVIEWER_AGENT_NAME}\` subagent to review your diff with fresh eyes against YOUR TASK. The subagent's baked-in system prompt enforces a non-mutative + non-recursive contract: read-only file/search/web tools and read-only MCP queries only; no writes, shell side effects, state-changing MCP calls, or nested subagent dispatch. Enforcement is prose-only — restate the constraint in your dispatch instructions and do not relax it. - Provide the subagent with YOUR TASK, the output of \`git diff\`, and a tight summary (not raw output) of any lint/typecheck/test failures you fixed during build — what broke, root cause, the fix — so it can check that fixes addressed root causes rather than suppressed symptoms; say "no build-phase failures" if the build path was clean. Instruct it to flag bugs, logic errors, missing edge cases, gaps between request and diff, and unintended changes. + Provide the subagent with YOUR TASK, the output of \`git diff origin/\` (single-rev form, no \`HEAD\` — this compares the working tree against the remote base and captures committed + staged + unstaged work; \`main...HEAD\` and \`--cached\` both miss the uncommitted edits Build self-review runs on, since self-review happens BEFORE the commit), and a tight summary (not raw output) of any lint/typecheck/test failures you fixed during build — what broke, root cause, the fix — so it can check that fixes addressed root causes rather than suppressed symptoms; say "no build-phase failures" if the build path was clean. Instruct it to flag bugs, logic errors, missing edge cases, gaps between request and diff, and unintended changes. Delegation + research discipline (distilled from \`/anneal\` canonical — these are codified learnings from many review rounds, not theoretical best practices): - Do NOT summarize what you implemented — that biases the subagent toward validating the shape of your solution rather than questioning it.