fix(#28): merge_pr must send MergePullRequestOption.Do + reviewed head_commit_id
The merge request previously posted an empty object, which this Gitea
instance's published MergePullRequestOption schema rejects. Post the
supported merge mode {"Do": "merge"} plus the head_commit_id read from
the PR, so a branch that moved after review cannot be merged
accidentally. Fail closed (GiteaClientError) when the PR record carries
no head commit id. Regression tests assert the exact request shape.
This commit is contained in:
parent
8b17668746
commit
1f2c85b4ff
@ -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"))
|
||||
|
||||
|
||||
@ -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):
|
||||
|
||||
Loading…
Reference in New Issue
Block a user