From b1515f70fcde0c88c683cab27be63b05ce3c5aeb Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 04:17:42 -0700 Subject: [PATCH] fix(mcp): pin gittensory_propose_action to the PR's current head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit proposeAction built the staged action's params from only label/reviewBody/ mergeMethod/closeComment — it never fetched or recorded the PR's head SHA. So every MCP-staged action had expectedHeadSha === undefined, and the approval- queue accept path's force-push guard (stagedHead && stagedHead !== pr.headSha) is a silent no-op on a falsy stagedHead: a maintainer/agent could propose a merge, the contributor could force-push unreviewed code, and accept would merge the new commit with no supersede/rejection — exactly the threat that guard exists to catch, just never engaged for this entrypoint. Fetch the PR's current stored head via getPullRequest before building params and pin expectedHeadSha to it, matching the invariant the live webhook/sweep staging path (stageForApproval) already upholds. Advances #1936. Closes #2255. --- src/mcp/server.ts | 7 ++++++ test/unit/mcp-automation-state.test.ts | 32 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 2330a13d15..48c65224b2 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -18,6 +18,7 @@ import { getInstallation, getIssue, getPendingAgentAction, + getPullRequest, getRepository, getRepositorySettings, isGlobalAgentFrozen, @@ -2444,11 +2445,17 @@ export class GittensoryMcp { await this.requireRepoManageAccess(fullName); const repo = await getRepository(this.env, fullName); if (!repo?.installationId) throw new Error("Cannot propose an action: the Gittensory App is not installed on this repository."); + // Pin the staged action to the head the proposer actually saw. Without this, the approval-queue accept + // path's force-push freshness guard (stagedHead && stagedHead !== pr.headSha) is a silent no-op for every + // MCP-staged action, since a falsy stagedHead never triggers it — an unreviewed force-push between + // proposal and accept would then merge/close/approve undetected. (#2255) + const pr = await getPullRequest(this.env, fullName, input.pullNumber); const params = { ...(input.label !== undefined ? { label: input.label } : {}), ...(input.reviewBody !== undefined ? { reviewBody: input.reviewBody } : {}), ...(input.mergeMethod !== undefined ? { mergeMethod: input.mergeMethod } : {}), ...(input.closeComment !== undefined ? { closeComment: input.closeComment } : {}), + ...(pr?.headSha ? { expectedHeadSha: pr.headSha } : {}), }; const { action, created } = await createPendingAgentActionIfAbsent(this.env, { repoFullName: fullName, diff --git a/test/unit/mcp-automation-state.test.ts b/test/unit/mcp-automation-state.test.ts index 9d1fdd6457..b2f76c9c5a 100644 --- a/test/unit/mcp-automation-state.test.ts +++ b/test/unit/mcp-automation-state.test.ts @@ -124,6 +124,38 @@ describe("MCP gittensory_propose_action (#784)", () => { expect(staged?.params).toMatchObject({ label: "gittensory:blocked", reviewBody: "please fix", closeComment: "closing as noise" }); }); + it("pins a proposed action to the PR's current head (expectedHeadSha) so the accept-time force-push guard can fire (#2255)", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h-proposed" }, labels: [], body: "x" }); + const client = await connect(env); + await client.callTool({ name: "gittensory_propose_action", arguments: { owner: "owner", repo: "repo", pullNumber: 7, actionClass: "merge", mergeMethod: "squash" } }); + const [staged] = await listPendingAgentActions(env, { repoFullName: "owner/repo", status: "pending" }); + expect(staged?.params).toMatchObject({ expectedHeadSha: "h-proposed" }); + }); + + it("an MCP-staged merge is superseded on accept if the PR is force-pushed after proposal — the guard now actually fires (#2255)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + await upsertInstallation(env, { + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write" }, events: ["pull_request"] }, + repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], + }); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h-proposed" }, labels: [], body: "x" }); + const client = await connect(env); + const proposed = await client.callTool({ name: "gittensory_propose_action", arguments: { owner: "owner", repo: "repo", pullNumber: 7, actionClass: "merge", mergeMethod: "squash" } }); + const { action } = proposed.structuredContent as { action: { id: string } }; + + // Force-push after staging: the head moves, but nothing re-evaluates the pending row until it's decided. + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h-force-pushed" }, labels: [], body: "x" }); + + const decided = await client.callTool({ name: "gittensory_decide_pending_action", arguments: { owner: "owner", repo: "repo", id: action.id, decision: "accept" } }); + const result = decided.structuredContent as { status: string; executionOutcome?: string }; + expect(result.status).toBe("rejected"); + expect(result.executionOutcome).toBe("head_moved"); + }); + it("allows a session that maintains the repo (owned installation)", async () => { const env = createTestEnv(); await upsertInstallation(env, {