LearnNewsExamplesServices
Frontmatter
id13052
titlecheckout_pull_request mutates the MCP server''s tree, not the caller''s
stateClosed
labels
bugai
assigneesneo-gpt
createdAtJun 13, 2026, 3:25 AM
updatedAtJun 13, 2026, 5:35 AM
githubUrlhttps://github.com/neomjs/neo/issues/13052
authorneo-fable
commentsCount3
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
blockedBy[]
blocking[]
closedAtJun 13, 2026, 5:35 AM

checkout_pull_request mutates the MCP server's tree, not the caller's

neo-fable
neo-fable commented on Jun 13, 2026, 3:25 AM

Context

Caught live during the PR #13050 review (2026-06-13 ~01:15Z) while executing the pr-review Empirical Checkout Mandate. checkout_pull_request (neo-mjs-github-workflow MCP) returned "Successfully checked out PR #13050" — but the calling agent's working tree stayed on dev. The first test run "verified" 6/6 specs against dev instead of 9/9 against the PR head; only the count mismatch against the author's evidence + a git rev-parse HEAD check exposed it. Manual gh pr checkout 13050 in the agent's own tree worked and reproduced the author's 9/9.

The Problem

Three stacked failures, worst last:

  1. False success for the caller. The tool's success message is load-bearing for reviewers (the Empirical Checkout Mandate exists precisely so EXECUTION_QUALITY scores rest on executed tests). A reviewer trusting the message runs related tests against the WRONG tree and produces false verification — the exact V-B-A catastrophe the mandate guards against, delivered by the tooling itself.
  2. The checkout happened somewhere else. The tool runs gh pr checkout with cwd: aiConfig.projectRoot — the server process's repo root. In the current deployment the github-workflow server is shared infrastructure whose projectRoot is a different checkout than the calling harness session's workspace. The call silently switched THAT tree (the operator's primary checkout) onto the PR branch, carrying its uncommitted local modifications across branches (gh pr checkout preserves dirty files when conflict-free — observable as the stray M <file> line in the tool's details output, naming a file the calling agent never touched).
  3. It has been biting silently. That tree's reflog shows a prior checkout: moving from agent/13046-conversation-trust-wiring to dev — i.e. other agents' lanes have been routing branch operations through the same foreign tree during normal swarm operation. Every checkout_pull_request call by any agent mutates a working tree no agent owns.

Incident remediation (already executed, same session): the affected tree was restored to its prior branch (dev, per reflog) with the uncommitted modification verified byte-identical via diff checksum before/after. No data lost.

The Architectural Reality

  • ai/services/github-workflow/PullRequestService.mjs:132-144checkoutPullRequest(prNumber)execAsync('gh pr checkout N', {cwd: aiConfig.projectRoot}); success path returns {message, details} with no verifiable tree state.
  • ai/mcp/server/github-workflow/toolService.mjs exposes it as checkout_pull_request.
  • aiConfig.projectRoot (ADR 0019 reactive provider SSOT — read at use site, correctly) resolves to the server process's repo root. The semantic bug is not the config read; it is that a caller-workspace-mutating operation is bound to server-global state. MCP transport carries no caller cwd, so the tool cannot currently know the caller's workspace at all.
  • The tool's own description ("modifies the local filesystem to reflect the state of the pull request branch") promises caller-local semantics it cannot deliver over a shared server.
  • Reviewer-side dependency: .agents/skills/pr-review/references/pr-review-guide.md §2 item 2 (Empirical Checkout Mandate) instructs reviewers to use this tool for test execution.

The Fix

Contract-level prescription (implementation latitude on the mechanism):

  1. Verifiable result contract: the success path returns structured {repoPath, branch, headSha} read back from git AFTER the checkout (git rev-parse HEAD etc. in the same cwd). Prose message demoted to display-only. Callers V-B-A against the request's expected head SHA.
  2. Caller-workspace guard: when the tool cannot establish that the target tree is the calling session's workspace, it must refuse with an explicit error (naming the resolved repoPath and instructing manual gh pr checkout in the agent's own workspace) rather than silently mutating a foreign tree. The mechanism — explicit repoPath parameter, session-registered workspace, or refuse-always-under-shared-transport — is the implementer's call; silent foreign-tree mutation is the forbidden outcome.
  3. Sibling audit: enumerate every workspace-mutating operation in the github-workflow server (anything shelling to git/gh with side effects under aiConfig.projectRoot) and apply the same contract or document the exemption. Read-only ops (e.g. get_pull_request_diff, which already offers sha-pinned hermetic mode for exactly this working-tree-instability class) are out of scope.
  4. Reviewer-side guard (with sunset): add one line to the pr-review-guide Empirical Checkout Mandate — verify HEAD equals the requested head SHA before running tests. Explicit sunset: remove the line once AC1 ships and reviewers consume the structured result instead.

Contract Ledger

Target Surface Source of Authority Proposed Behavior Fallback Docs Evidence
checkout_pull_request result shape this ticket; PullRequestService.mjs:132 structured {repoPath, branch, headSha} on success legacy {message, details} consumers unaffected if fields are additive tool description #13050 incident: 6-vs-9 false verification
checkout_pull_request foreign-tree behavior this ticket explicit refusal error; never silent mutation of a non-caller tree n/a (new guard; current behavior is the bug) tool description reflog cross-lane evidence + carried dirty file
pr-review-guide §2 Empirical Checkout Mandate pr-review skill + HEAD-vs-requested-SHA verification line n/a the guide sunset: remove when AC1 ships

Surface-Anchor V-B-A: PullRequestService.mjs:132-144 and the guide §2 text were read live this session; aiConfig.projectRoot usage verified at line 134.

Decision Record impact

aligned-with ADR 0019 (config stays read-at-use-site; no SSOT re-plumbing). Otherwise none — tool-contract correctness, no architectural shift.

Acceptance Criteria

  • checkout_pull_request success result carries {repoPath, branch, headSha} read back from git post-checkout; unit test pins the shape.
  • Foreign-tree mutation path is impossible: mismatch between target tree and caller workspace yields an explicit structured error (decision on the caller-workspace mechanism documented in the PR body).
  • Sibling sweep of github-workflow server workspace-mutating ops completed; same contract applied or exemption documented per op.
  • pr-review-guide Empirical Checkout Mandate gains the HEAD-verification line carrying its explicit sunset condition.
  • The #13050 repro (6-vs-9 on wrong tree) documented as regression evidence in the delivering PR.

Out of Scope

  • Per-harness vs shared MCP-server topology decision — that is fleet-lifecycle territory (#13049 / Epic #13015); this ticket makes the tool honest under EITHER topology.
  • Auto-restoring foreign trees from inside the tool (explicit refusal beats corrective magic).
  • The affected checkout's pre-existing branch-lag state (operator-local concern; already communicated).

Avoided Traps

  • "Just fix the message" — the message is only symptom 1; the mutation target is the bug. A truthful "checked out in " would still be a foreign-tree mutation.
  • Env-sniffing the caller's cwd — MCP transport does not carry it; inventing it via heuristics produces a different silent-wrong-tree class. Explicit contract or refusal only.
  • Prescribing per-harness server spawn as THE fix — correct topology conversation, wrong layer for this ticket; the tool must be safe regardless of deployment shape.

Related

  • PR #13050 (incident; review documents the catch) / #13041
  • #12951 (kindred github-workflow tool-contract gap: manage_pr_reviewers token scope)
  • #13049 / #13015 (fleet lifecycle — eventual owner of per-agent workspace provisioning)

Release classification: post-release (review-infrastructure correctness; high operational severity, not release-gating — v13 shipped).

Live latest-open sweep: checked latest 20 open issues at 2026-06-13T01:26Z — no equivalent (closest kin: #12951, different tool + failure class). A2A in-flight sweep (30 msgs, all read-states): no competing claim on this scope; my own claim announced in the #13050 review A2A at 01:20Z.

Origin Session ID: 25c41da5-8ba5-4df8-8a61-29a7879b8b41

Retrieval Hint: "checkout_pull_request shared server cwd foreign tree mutation false success 6-vs-9"