diff --git a/gitea_saas/gitea_client.py b/gitea_saas/gitea_client.py index 0545a3d..6e0a0d8 100644 --- a/gitea_saas/gitea_client.py +++ b/gitea_saas/gitea_client.py @@ -345,14 +345,26 @@ class GiteaClient: """Merge ``owner/repo`` PR ``pr_number``; True when it is merged. Uses the Gitea merge endpoint and confirms the PR is merged after - the request. An already-merged PR short-circuits to True (no - duplicate merge request). Non-2xx responses (e.g. 409 when the - head is not mergeable) raise :class:`GiteaApiError`. + the request. The merge request carries the Gitea schema's + mandatory ``Do`` mode and the reviewed ``head_commit_id`` read from + the PR, so a branch that moved after review cannot be merged + accidentally (the server rejects a stale head commit id). An + already-merged PR short-circuits to True (no duplicate merge + request). Non-2xx responses (e.g. 409 when the head is not + mergeable) raise :class:`GiteaApiError`; a PR record without a + head commit id raises :class:`GiteaClientError` rather than + merging unpinned. """ current = self._request("GET", f"{repo}/pulls/{pr_number}") or {} if current.get("merged"): return True - self._request("POST", f"{repo}/pulls/{pr_number}/merge", {}) + head_commit_id = (current.get("head") or {}).get("sha") + if not head_commit_id: + raise GiteaClientError( + f"merge_pr: PR {pr_number} record has no head commit id; " + "refusing to merge without the reviewed head") + self._request("POST", f"{repo}/pulls/{pr_number}/merge", + {"Do": "merge", "head_commit_id": head_commit_id}) merged = self._request("GET", f"{repo}/pulls/{pr_number}") or {} return bool(merged.get("merged")) diff --git a/tests/test_gitea_client.py b/tests/test_gitea_client.py index 788d33f..3059cf8 100644 --- a/tests/test_gitea_client.py +++ b/tests/test_gitea_client.py @@ -370,7 +370,7 @@ def test_merge_pr_posts_merge_and_confirms(make_client): if method == "GET" and path.endswith("/pulls/7"): state["gets"] += 1 merged = state["gets"] >= 2 # first GET = pre-check, later = confirm - return 200, {**gitea_pr(7), "merged": merged} + return 200, {**gitea_pr(7, head_sha="sha-head"), "merged": merged} if method == "POST" and path.endswith("/pulls/7/merge"): return 200, {"merged": True, "message": "Pull Request was successfully merged"} return 404, {"error": "unexpected route"} @@ -380,7 +380,52 @@ def test_merge_pr_posts_merge_and_confirms(make_client): posts = requests_for(stub, method="POST", path_suffix="/pulls/7/merge") assert len(posts) == 1 - assert json.loads(posts[0]["body"]) == {} # default merge style + # The real request must carry a supported merge mode plus the reviewed + # head commit id — the Gitea schema rejects `{}` and an unpinned merge. + assert json.loads(posts[0]["body"]) == {"Do": "merge", "head_commit_id": "sha-head"} + + +def test_merge_pr_sends_reviewed_head_commit_id_regression(make_client): + # Regression (review bounce): the merge payload must pin the exact head + # commit id read from the PR, so a branch that moved after review cannot + # be merged accidentally; `Do` must be a schema-supported merge mode. + reviewed_sha = "abcd-1234-reviewed-head" + state = {"gets": 0} + + def dispatcher(method, path, query, body): + if method == "GET" and path.endswith("/pulls/7"): + state["gets"] += 1 + merged = state["gets"] >= 2 # first GET = pre-check, later = confirm + return 200, {**gitea_pr(7, head_sha=reviewed_sha), "merged": merged} + if method == "POST" and path.endswith("/pulls/7/merge"): + return 200, {"merged": True, "message": "merged"} + return 404, {"error": "unexpected route"} + + client, stub = make_client(dispatcher) + assert client.merge_pr("acme", 7) is True + + posts = requests_for(stub, method="POST", path_suffix="/pulls/7/merge") + assert len(posts) == 1 + payload = json.loads(posts[0]["body"]) + assert payload == {"Do": "merge", "head_commit_id": reviewed_sha} + assert payload["Do"] in {"merge", "rebase", "rebase-merge", "squash", + "fast-forward-only"} + + +def test_merge_pr_raises_when_pr_has_no_head_commit_id(make_client): + # Without a head commit id the merge cannot be pinned to the reviewed + # head: fail closed instead of sending an unpinned (or empty) request. + def dispatcher(method, path, query, body): + if method == "GET" and path.endswith("/pulls/7"): + pr = gitea_pr(7) + pr["head"] = {"ref": "issue-7"} # sha missing + return 200, pr + return 404, {"error": "unexpected route"} + + client, stub = make_client(dispatcher) + with pytest.raises(GiteaClientError, match="no head commit id"): + client.merge_pr("acme", 7) + assert not requests_for(stub, method="POST", path_suffix="/merge") def test_merge_pr_already_merged_short_circuits(make_client):