From 1ea1d6dec718c29603e5a6bcd3cb98d6ee38963f Mon Sep 17 00:00:00 2001 From: ellmos-multi-bot <341099510+ellmos-multi-bot@users.noreply.github.com> Date: Wed, 21 Oct 2026 04:01:10 +0200 Subject: [PATCH] fix: confirm API 2026 merges through GraphQL readback Read the merged commit and reviewed head through a read-only GraphQL query when REST omits merge_commit_sha. Preserve a single merge attempt and fail when completion cannot be confirmed. --- src/githubbot_core/typed_actions.py | 28 +++++++++++- tests/test_typed_actions.py | 69 +++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 2 deletions(-) diff --git a/src/githubbot_core/typed_actions.py b/src/githubbot_core/typed_actions.py index 7961e55..92fc00d 100644 --- a/src/githubbot_core/typed_actions.py +++ b/src/githubbot_core/typed_actions.py @@ -124,6 +124,11 @@ def _reviewed_pull(transport, params: dict, *, draft: bool) -> tuple[dict, Optio return pull, None +MERGE_READBACK_QUERY = ( + "query($id: ID!) { node(id: $id) { ... on PullRequest { state headRefOid mergeCommit { oid } } } }" +) + + def exec_merge(transport, params: dict, config: dict): account = str((config.get("multi_bot") or {}).get("merge_account") or "") if account: @@ -143,9 +148,28 @@ def exec_merge(transport, params: dict, config: dict): if error: return None, error after, error = _pull(transport, repo, number) - if error or not after.get("merged") or not after.get("merge_commit_sha"): + if error or after.get("merged") is not True: + return None, "merge not confirmed by readback" + commit_sha = after.get("merge_commit_sha") + if not commit_sha: + # REST 2026-03-10 removed merge_commit_sha from pull request objects. + # Read the actual merged commit; never retry the merge or trust acceptance alone. + if not pull.get("node_id"): + return None, "merge not confirmed by readback" + data, error = transport.request( + "POST", "graphql", {"query": MERGE_READBACK_QUERY, "variables": {"id": pull["node_id"]}} + ) + if error or not isinstance(data, dict) or data.get("errors"): + return None, "merge not confirmed by readback" + payload = data.get("data") + node = payload.get("node") if isinstance(payload, dict) else None + if not isinstance(node, dict) or node.get("state") != "MERGED" or node.get("headRefOid") != params["sha"]: + return None, "merge not confirmed by readback" + commit = node.get("mergeCommit") + commit_sha = commit.get("oid") if isinstance(commit, dict) else None + if not SHA.fullmatch(str(commit_sha or "")): return None, "merge not confirmed by readback" - return {"readback": {"merged": True, "merge_commit_sha": after["merge_commit_sha"]}}, None + return {"readback": {"merged": True, "merge_commit_sha": commit_sha}}, None READY_MUTATION = ( diff --git a/tests/test_typed_actions.py b/tests/test_typed_actions.py index 05c66c3..f6df93d 100644 --- a/tests/test_typed_actions.py +++ b/tests/test_typed_actions.py @@ -5,6 +5,8 @@ import json from types import SimpleNamespace +import pytest + from githubbot_core import actions, cli, typed_actions from githubbot_core.actions import Action from githubbot_core.config import default_config, merge_layers @@ -133,6 +135,73 @@ def test_merge_without_readback_is_failed(): assert receipt["status"] == "failed" and "readback" in receipt["detail"] +class MergeReadbackTransport(Transport): + """The versioned REST readback omits merge_commit_sha; GraphQL still exposes it.""" + + def __init__(self, node=None, *, graphql_error=None, graphql_errors=None, after=None): + gets = base_gets() + gets[f"repos/{REPO}/pulls/7"] = [pull(), pull(merged=True) if after is None else after] + super().__init__(gets) + self.graphql = {"data": {"node": node}, "errors": graphql_errors} + self.graphql_error = graphql_error + + def request(self, method, endpoint, body=None): + if endpoint == "graphql": + self.writes.append((method, endpoint, body)) + return self.graphql, self.graphql_error + return super().request(method, endpoint, body) + + +def merged_node(**overrides): + return {"state": "MERGED", "headRefOid": SHA, "mergeCommit": {"oid": OTHER}, **overrides} + + +def test_merge_api_2026_readback_confirms_the_commit_and_reviewed_head(): + transport = MergeReadbackTransport(merged_node()) + receipt = actions.apply([merge_action()], transport, config())["receipts"][0] + assert receipt["status"] == "applied" + assert receipt["readback"] == {"merged": True, "merge_commit_sha": OTHER} + assert transport.writes[0] == ("PUT", f"repos/{REPO}/pulls/7/merge", {"sha": SHA, "merge_method": "merge"}) + method, endpoint, body = transport.writes[1] + assert (method, endpoint, body["variables"]) == ("POST", "graphql", {"id": "PR_node"}) + assert body["query"].lstrip().startswith("query") + assert len(transport.writes) == 2 # one merge attempt and one read-only query + + +@pytest.mark.parametrize( + "node", + [ + None, + {}, + merged_node(state="OPEN"), + merged_node(state="CLOSED"), + merged_node(headRefOid=OTHER), + merged_node(mergeCommit=None), + merged_node(mergeCommit={}), + merged_node(mergeCommit={"oid": "not-a-commit"}), + ], +) +def test_merge_api_2026_needs_a_confirmed_commit_at_the_reviewed_head(node): + transport = MergeReadbackTransport(node) + receipt = actions.apply([merge_action()], transport, config())["receipts"][0] + assert receipt["status"] == "failed" and "readback" in receipt["detail"] + assert sum(endpoint.endswith("/merge") for _, endpoint, _ in transport.writes) == 1 + + +@pytest.mark.parametrize("kwargs", [{"graphql_error": "Timeout"}, {"graphql_errors": [{"message": "unavailable"}]}]) +def test_merge_api_2026_does_not_accept_a_graphql_error(kwargs): + transport = MergeReadbackTransport(merged_node(), **kwargs) + receipt = actions.apply([merge_action()], transport, config())["receipts"][0] + assert receipt["status"] == "failed" and "readback" in receipt["detail"] + + +def test_merge_api_2026_does_not_query_when_rest_has_not_confirmed_the_merge(): + transport = MergeReadbackTransport(merged_node(), after=pull()) + receipt = actions.apply([merge_action()], transport, config())["receipts"][0] + assert receipt["status"] == "failed" + assert len(transport.writes) == 1 + + def test_ready_uses_graphql_for_the_exact_draft_head(): gets = base_gets() gets[f"repos/{REPO}/pulls/7"] = [pull(draft=True), pull(draft=False)]