fix(mcp): require visible APPROVED reviews before merge (closes #244) #251
+102
-2
@@ -1180,7 +1180,8 @@ _REVIEW_ACTIONS = {
|
||||
# 'comment' posts review findings without an approval/rejection state.
|
||||
# #14 names this eligibility category 'review'.
|
||||
"comment": ("review", "COMMENT"),
|
||||
"approve": ("approve", "APPROVE"),
|
||||
# Gitea ReviewStateType uses APPROVED, not APPROVE — wrong event leaves PENDING (#244).
|
||||
"approve": ("approve", "APPROVED"),
|
||||
"request_changes": ("request_changes", "REQUEST_CHANGES"),
|
||||
}
|
||||
|
||||
@@ -1390,6 +1391,42 @@ def _redact(text: str) -> str:
|
||||
# neither may drive the blocking/approval summaries.
|
||||
_VERDICT_STATES = ("APPROVED", "REQUEST_CHANGES", "COMMENT")
|
||||
|
||||
# Terminal review events that must be visible after live submission (#244).
|
||||
_SUBMIT_VISIBLE_EVENTS = frozenset({"APPROVED", "REQUEST_CHANGES"})
|
||||
|
||||
|
||||
def _latest_review_state_for_reviewer(raw_reviews: list, reviewer: str) -> str | None:
|
||||
"""Return the latest non-COMMENT verdict for *reviewer*, or None."""
|
||||
ordered = sorted(
|
||||
raw_reviews or [],
|
||||
key=lambda rv: ((rv.get("submitted_at") or ""), rv.get("id") or 0),
|
||||
)
|
||||
latest = None
|
||||
for rv in ordered:
|
||||
state = (rv.get("state") or "").upper()
|
||||
login = (rv.get("user") or {}).get("login", "")
|
||||
if login != reviewer or state not in _VERDICT_STATES or state == "COMMENT":
|
||||
continue
|
||||
latest = state
|
||||
return latest
|
||||
|
||||
|
||||
def _submit_pending_pull_review(
|
||||
h: str,
|
||||
o: str,
|
||||
r: str,
|
||||
pr_number: int,
|
||||
review_id: int,
|
||||
event: str,
|
||||
body: str,
|
||||
auth,
|
||||
) -> dict | None:
|
||||
"""Submit a PENDING draft review via Gitea's pending-review endpoint."""
|
||||
submit_url = (
|
||||
f"{repo_api_url(h, o, r)}/pulls/{pr_number}/reviews/{review_id}"
|
||||
)
|
||||
return api_request("POST", submit_url, auth, {"body": body, "event": event})
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_get_pr_review_feedback(
|
||||
@@ -1630,6 +1667,7 @@ def _evaluate_pr_review_submission(
|
||||
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
review_id = None
|
||||
submitted_state = None
|
||||
try:
|
||||
auth = _auth(h)
|
||||
review_url = f"{repo_api_url(h, o, r)}/pulls/{pr_number}/reviews"
|
||||
@@ -1637,10 +1675,43 @@ def _evaluate_pr_review_submission(
|
||||
resp = api_request("POST", review_url, auth, payload)
|
||||
if isinstance(resp, dict):
|
||||
review_id = resp.get("id")
|
||||
submitted_state = (resp.get("state") or "").upper() or None
|
||||
if (
|
||||
submitted_state == "PENDING"
|
||||
and review_id
|
||||
and event in _SUBMIT_VISIBLE_EVENTS
|
||||
):
|
||||
resp = _submit_pending_pull_review(
|
||||
h, o, r, pr_number, review_id, event, body, auth,
|
||||
)
|
||||
if isinstance(resp, dict):
|
||||
submitted_state = (resp.get("state") or "").upper() or None
|
||||
except Exception as exc: # noqa: BLE001 — redact before surfacing
|
||||
reasons.append(f"review submission failed: {_redact(str(exc))}")
|
||||
return result
|
||||
|
||||
if action in _TERMINAL_REVIEW_ACTIONS:
|
||||
try:
|
||||
auth = _auth(h)
|
||||
raw_reviews = (
|
||||
api_request("GET", f"{review_url}", auth) or []
|
||||
)
|
||||
visible = _latest_review_state_for_reviewer(raw_reviews, auth_user)
|
||||
except Exception as exc: # noqa: BLE001 — redact before surfacing
|
||||
reasons.append(
|
||||
f"could not verify submitted review verdict (fail closed): "
|
||||
f"{_redact(str(exc))}"
|
||||
)
|
||||
return result
|
||||
result["submitted_verdict"] = visible
|
||||
result["review_verdict_visible"] = visible == event
|
||||
if visible != event:
|
||||
reasons.append(
|
||||
f"review submission left verdict '{submitted_state or visible or 'PENDING'}'; "
|
||||
f"expected visible '{event}' for reviewer '{auth_user}' (fail closed)"
|
||||
)
|
||||
return result
|
||||
|
||||
record_live_review_mutation(pr_number, action, review_id)
|
||||
result["performed"] = True
|
||||
reasons.append(f"all gates passed; submitted '{event}' review on PR #{pr_number}")
|
||||
@@ -2095,6 +2166,9 @@ def gitea_merge_pr(
|
||||
5. If ``expected_changed_files`` is given and the PR's changed file set
|
||||
differs → refuse.
|
||||
6. Redundant self-merge block (authenticated user == PR author).
|
||||
7. Re-read formal review feedback (#167): refuse when
|
||||
``approval_visible`` is false or undismissed REQUEST_CHANGES block
|
||||
merge — PENDING draft approvals do not count (#244).
|
||||
|
||||
No force / ignore-checks option is exposed. Gitea's own ``mergeable`` signal
|
||||
(which reflects branch-protection required reviews and status checks) must
|
||||
@@ -2219,7 +2293,33 @@ def gitea_merge_pr(
|
||||
reasons.append("self-merge blocked (authenticated user is PR author)")
|
||||
return result
|
||||
|
||||
# Gate 7 — in-process mutation authority (#199): the last check before
|
||||
# Gate 7 — visible formal approval required (#244). PENDING drafts and
|
||||
# absent verdicts do not satisfy this gate even when Gitea mergeable is true.
|
||||
feedback = gitea_get_pr_review_feedback(
|
||||
pr_number=pr_number, remote=remote, host=host, org=org, repo=repo,
|
||||
)
|
||||
if not feedback.get("success"):
|
||||
reasons.append("PR review feedback unavailable before merge (fail closed)")
|
||||
reasons.extend(feedback.get("reasons", []))
|
||||
if feedback.get("permission_report"):
|
||||
result["permission_report"] = feedback["permission_report"]
|
||||
return result
|
||||
result["approval_visible"] = feedback.get("approval_visible")
|
||||
result["has_blocking_change_requests"] = feedback.get(
|
||||
"has_blocking_change_requests")
|
||||
if feedback.get("has_blocking_change_requests"):
|
||||
reasons.append(
|
||||
"undismissed REQUEST_CHANGES review blocks merge (fail closed)"
|
||||
)
|
||||
return result
|
||||
if not feedback.get("approval_visible"):
|
||||
reasons.append(
|
||||
"no visible APPROVED review on PR; verify review submission "
|
||||
"completed before merge (fail closed)"
|
||||
)
|
||||
return result
|
||||
|
||||
# Gate 8 — in-process mutation authority (#199): the last check before
|
||||
# the merge mutation, using the identity the eligibility gate proved.
|
||||
# A profile/identity flip or side-channel override between preflight
|
||||
# and merge fails closed here.
|
||||
|
||||
@@ -237,6 +237,9 @@ Worktree folder = branch with `/` replaced by `-`
|
||||
validation completes, call `gitea_mark_final_review_decision`, then submit
|
||||
exactly one live review via
|
||||
`gitea_submit_pr_review(..., final_review_decision_ready=True)`.
|
||||
After submitting, re-read `gitea_get_pr_review_feedback` and confirm the
|
||||
verdict is visible (`approval_visible` true for APPROVE; PENDING drafts do
|
||||
not count — #244). Do not merge until a visible APPROVED review exists.
|
||||
Final reports must list exactly one review mutation
|
||||
(`review_proofs.assess_review_mutation_final_report`) unless an
|
||||
operator-approved correction flow was invoked and explained.
|
||||
|
||||
+8
-2
@@ -293,9 +293,12 @@ class TestGatedToolAudit(_AuditWiringBase):
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_merge_success_audited(self, _auth, mock_api):
|
||||
# user, pr, merge POST, readback — no extra identity call (uses result).
|
||||
# user, pr, feedback pr+reviews, merge POST, readback.
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
self._pr("author-bot"),
|
||||
[{"id": 1, "user": {"login": "reviewer-bot"}, "state": "APPROVED",
|
||||
"submitted_at": "2026-07-06T10:00:00Z", "dismissed": False}],
|
||||
{}, {"merged_commit_sha": "c1"},
|
||||
]
|
||||
env = self._env(GITEA_PROFILE_NAME="gitea-merger",
|
||||
@@ -332,7 +335,10 @@ class TestGatedToolAudit(_AuditWiringBase):
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_submit_review_success_audited(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 7},
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 7, "state": "APPROVED"},
|
||||
[{"id": 7, "user": {"login": "reviewer-bot"}, "state": "APPROVED",
|
||||
"submitted_at": "2026-07-06T10:00:00Z", "dismissed": False}],
|
||||
]
|
||||
env = self._env(GITEA_PROFILE_NAME="gitea-reviewer",
|
||||
GITEA_ALLOWED_OPERATIONS="read,review,approve")
|
||||
|
||||
+149
-17
@@ -47,6 +47,22 @@ import mcp_server
|
||||
|
||||
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
|
||||
|
||||
|
||||
def _formal_review(reviewer, verdict, sha="abc123", review_id=1):
|
||||
return {
|
||||
"id": review_id,
|
||||
"user": {"login": reviewer},
|
||||
"state": verdict,
|
||||
"commit_id": sha,
|
||||
"submitted_at": "2026-07-06T10:00:00Z",
|
||||
"dismissed": False,
|
||||
}
|
||||
|
||||
|
||||
def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"):
|
||||
return [_formal_review(reviewer, "APPROVED", sha=sha)]
|
||||
|
||||
|
||||
# Issue-write tools are profile-gated (#69).
|
||||
ISSUE_WRITE_ENV = {
|
||||
"GITEA_ALLOWED_OPERATIONS": (
|
||||
@@ -488,6 +504,10 @@ class TestMergePR(unittest.TestCase):
|
||||
f"unexpected merge mutation: {method} {url}",
|
||||
)
|
||||
|
||||
def _feedback_reads(self, author="author-bot", sha="abc123"):
|
||||
"""PR + reviews GETs for gitea_get_pr_review_feedback during merge."""
|
||||
return [self._pr(author, sha=sha), _visible_approval_reviews(sha=sha)]
|
||||
|
||||
# -- success --------------------------------------------------------------
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@@ -495,6 +515,7 @@ class TestMergePR(unittest.TestCase):
|
||||
def test_merge_succeeds_when_all_gates_pass(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
*self._feedback_reads(),
|
||||
{}, # merge POST
|
||||
{"merged_commit_sha": "mergecommit99"}, # read-back
|
||||
]
|
||||
@@ -511,8 +532,8 @@ class TestMergePR(unittest.TestCase):
|
||||
self.assertEqual(r["head_sha"], "abc123")
|
||||
self.assertEqual(r["merge_method"], "squash")
|
||||
self.assertEqual(r["merge_commit"], "mergecommit99")
|
||||
# 3rd call is the merge POST with the requested method/title/message.
|
||||
merge_call = mock_api.call_args_list[2]
|
||||
# 5th call is the merge POST with the requested method/title/message.
|
||||
merge_call = mock_api.call_args_list[4]
|
||||
self.assertEqual(merge_call.args[0], "POST")
|
||||
self.assertTrue(merge_call.args[1].endswith("/pulls/8/merge"))
|
||||
payload = merge_call.args[3]
|
||||
@@ -527,6 +548,7 @@ class TestMergePR(unittest.TestCase):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
[{"filename": "a.py"}, {"filename": "b.py"}], # files
|
||||
*self._feedback_reads(),
|
||||
{}, # merge POST
|
||||
{"merged_commit_sha": "c1"}, # read-back
|
||||
]
|
||||
@@ -546,6 +568,7 @@ class TestMergePR(unittest.TestCase):
|
||||
"""Merge OK + read-back GET failure => explicit cleanup skip, not silence."""
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
*self._feedback_reads(),
|
||||
{}, # merge POST
|
||||
RuntimeError("HTTP 502: Gitea upstream unavailable"), # read-back fails
|
||||
]
|
||||
@@ -561,8 +584,8 @@ class TestMergePR(unittest.TestCase):
|
||||
# The skip is explicit, not silent.
|
||||
self.assertEqual(r["cleanup_status"], "skipped (merge read-back failed)")
|
||||
# No tracker-cleanup API traffic after the failed read-back:
|
||||
# user, PR (eligibility), merge POST, read-back — and nothing more.
|
||||
self.assertEqual(mock_api.call_count, 4)
|
||||
# user, PR (eligibility), feedback PR+reviews, merge POST, read-back.
|
||||
self.assertEqual(mock_api.call_count, 6)
|
||||
for c in mock_api.call_args_list:
|
||||
self.assertNotEqual(c.args[0], "DELETE")
|
||||
|
||||
@@ -574,6 +597,7 @@ class TestMergePR(unittest.TestCase):
|
||||
"""Unexpected cleanup exception => merge still succeeds; error surfaced redacted."""
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
*self._feedback_reads(),
|
||||
{}, # merge POST
|
||||
{"merged_commit_sha": "c9"}, # read-back OK
|
||||
]
|
||||
@@ -761,6 +785,7 @@ class TestMergePR(unittest.TestCase):
|
||||
def test_output_redacts_secrets(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
*self._feedback_reads(),
|
||||
{}, {"merged_commit_sha": "c1"},
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
@@ -778,6 +803,7 @@ class TestMergePR(unittest.TestCase):
|
||||
def test_merge_error_message_redacts_credential(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
*self._feedback_reads(),
|
||||
RuntimeError("HTTP 500: token abc-secret-xyz rejected"),
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
@@ -790,6 +816,45 @@ class TestMergePR(unittest.TestCase):
|
||||
self.assertIn("[REDACTED]", blob)
|
||||
self.assertNotIn("abc-secret-xyz", blob)
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_merge_blocked_without_visible_approval(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
self._pr("author-bot"),
|
||||
[_formal_review("sysadmin", "PENDING")],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8), remote="prgs")
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertFalse(r.get("approval_visible"))
|
||||
self.assertTrue(any("no visible APPROVED review" in x for x in r["reasons"]))
|
||||
self._assert_no_merge_call(mock_api)
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_merge_blocked_on_request_changes(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
self._pr("author-bot"),
|
||||
[
|
||||
_formal_review("reviewer-bot", "APPROVED"),
|
||||
_formal_review("reviewer-bot", "REQUEST_CHANGES", review_id=2),
|
||||
],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8), remote="prgs")
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertTrue(r.get("has_blocking_change_requests"))
|
||||
self.assertTrue(any("REQUEST_CHANGES" in x for x in r["reasons"]))
|
||||
self._assert_no_merge_call(mock_api)
|
||||
|
||||
|
||||
class TestNoUngatedMergePath(unittest.TestCase):
|
||||
"""Prove no other exposed tool can merge (#16 surface audit)."""
|
||||
@@ -1545,7 +1610,9 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_duplicate_terminal_decision_blocked(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 1},
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 1, "state": "APPROVED"},
|
||||
[_formal_review("reviewer-bot", "APPROVED", review_id=1)],
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
]
|
||||
gitea_mark_final_review_decision(
|
||||
@@ -1628,7 +1695,9 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_approve_succeeds_when_eligible(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 7},
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 7, "state": "APPROVED"},
|
||||
[_formal_review("reviewer-bot", "APPROVED", review_id=7)],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||
@@ -1638,16 +1707,63 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertTrue(r["performed"])
|
||||
self.assertTrue(r.get("review_verdict_visible"))
|
||||
self.assertEqual(r["authenticated_user"], "reviewer-bot")
|
||||
self.assertEqual(r["pr_author"], "author-bot")
|
||||
self.assertEqual(r["head_sha"], "abc123")
|
||||
method, url = mock_api.call_args.args[0], mock_api.call_args.args[1]
|
||||
self.assertEqual(method, "POST")
|
||||
self.assertTrue(url.endswith("/pulls/8/reviews"))
|
||||
payload = mock_api.call_args.args[3]
|
||||
self.assertEqual(payload["event"], "APPROVE")
|
||||
post_calls = [
|
||||
c for c in mock_api.call_args_list
|
||||
if c.args[0] == "POST" and c.args[1].endswith("/pulls/8/reviews")
|
||||
]
|
||||
self.assertEqual(len(post_calls), 1)
|
||||
payload = post_calls[0].args[3]
|
||||
self.assertEqual(payload["event"], "APPROVED")
|
||||
self.assertEqual(payload["commit_id"], "abc123")
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_approve_fails_when_verdict_stays_pending(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 7, "state": "PENDING"},
|
||||
{"id": 7, "state": "PENDING"},
|
||||
[_formal_review("reviewer-bot", "PENDING", review_id=7)],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_submit_pr_review(
|
||||
pr_number=8, action="approve", body="LGTM", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertFalse(r.get("review_verdict_visible"))
|
||||
self.assertTrue(any("expected visible 'APPROVED'" in x for x in r["reasons"]))
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_approve_submits_pending_draft_when_api_returns_pending(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 7, "state": "PENDING"},
|
||||
{"id": 7, "state": "APPROVED"},
|
||||
[_formal_review("reviewer-bot", "APPROVED", review_id=7)],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_submit_pr_review(
|
||||
pr_number=8, action="approve", body="LGTM", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertTrue(r["performed"])
|
||||
submit_calls = [
|
||||
c for c in mock_api.call_args_list
|
||||
if c.args[0] == "POST" and "/reviews/7" in c.args[1]
|
||||
]
|
||||
self.assertEqual(len(submit_calls), 1)
|
||||
self.assertEqual(submit_calls[0].args[3]["event"], "APPROVED")
|
||||
|
||||
# -- request_changes ------------------------------------------------------
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@@ -1655,7 +1771,9 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
def test_request_changes_succeeds_when_eligible(self, _auth, mock_api):
|
||||
gitea_mark_final_review_decision(8, "request_changes", remote="prgs")
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 9},
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 9, "state": "REQUEST_CHANGES"},
|
||||
[_formal_review("reviewer-bot", "REQUEST_CHANGES", review_id=9)],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,request_changes"}
|
||||
@@ -1666,7 +1784,12 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertTrue(r["performed"])
|
||||
self.assertEqual(mock_api.call_args.args[3]["event"], "REQUEST_CHANGES")
|
||||
post_calls = [
|
||||
c for c in mock_api.call_args_list
|
||||
if c.args[0] == "POST" and c.args[1].endswith("/pulls/8/reviews")
|
||||
]
|
||||
self.assertEqual(len(post_calls), 1)
|
||||
self.assertEqual(post_calls[0].args[3]["event"], "REQUEST_CHANGES")
|
||||
|
||||
def test_request_changes_blocked_without_eligibility(self):
|
||||
gitea_mark_final_review_decision(8, "request_changes", remote="prgs")
|
||||
@@ -1777,7 +1900,8 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
patch("mcp_server.api_request") as mock_api:
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot", sha="abc123"),
|
||||
{"id": 5},
|
||||
{"id": 5, "state": "APPROVED"},
|
||||
[_formal_review("reviewer-bot", "APPROVED", review_id=5)],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||
@@ -1929,8 +2053,12 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_correction_flow_allows_second_terminal_review(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 42},
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 43},
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 42, "state": "APPROVED"},
|
||||
[_formal_review("reviewer-bot", "APPROVED", review_id=42)],
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 43, "state": "REQUEST_CHANGES"},
|
||||
[_formal_review("reviewer-bot", "REQUEST_CHANGES", review_id=43)],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve,request_changes"}
|
||||
@@ -2003,7 +2131,7 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
patch("gitea_audit.audit_enabled", return_value=True).start()
|
||||
self.mock_audit = patch("gitea_audit.write_event").start()
|
||||
# gitea.pr.close: closing a PR via gitea_edit_pr is capability-gated (#216).
|
||||
patch("mcp_server.get_profile", return_value={"profile_name": "test", "allowed_operations": ["merge", "edit", "close", "gitea.pr.close", "gitea.issue.close"], "audit_label": "test", "forbidden_operations": []}).start()
|
||||
patch("mcp_server.get_profile", return_value={"profile_name": "test", "allowed_operations": ["read", "merge", "edit", "close", "gitea.pr.close", "gitea.issue.close"], "audit_label": "test", "forbidden_operations": []}).start()
|
||||
|
||||
def tearDown(self):
|
||||
patch.stopall()
|
||||
@@ -2051,6 +2179,8 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "GET" and "/user" in url:
|
||||
return {"login": "merger"}
|
||||
if method == "GET" and url.endswith("/reviews"):
|
||||
return [_formal_review("reviewer", "APPROVED", sha="sha123")]
|
||||
if method == "GET" and "pulls/1" in url and "/files" not in url:
|
||||
return {
|
||||
"user": {"login": "author"},
|
||||
@@ -2083,6 +2213,8 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "GET" and "/user" in url:
|
||||
return {"login": "merger"}
|
||||
if method == "GET" and url.endswith("/reviews"):
|
||||
return [_formal_review("reviewer", "APPROVED", sha="sha123")]
|
||||
if method == "GET" and "pulls/1" in url and "/files" not in url:
|
||||
return {
|
||||
"user": {"login": "author"},
|
||||
|
||||
@@ -119,12 +119,28 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
mock_get_all.return_value = [
|
||||
{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": "abc1"}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "other_user"}}
|
||||
]
|
||||
# mock_api: 1) /user (inventory), 2) /user (eligibility), 3) /pulls/1 (eligibility), 4) /pulls/1/reviews (POST review)
|
||||
# mock_api: inventory whoami, eligibility whoami, eligibility PR,
|
||||
# POST review (#244: state + visible-verdict GET reviews).
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer1"}, # inventory whoami
|
||||
{"login": "reviewer1"}, # submit eligibility whoami
|
||||
{"user": {"login": "other_user"}, "state": "open", "head": {"sha": "abc1"}, "mergeable": True}, # submit eligibility PR
|
||||
{"id": 100}, # POST review
|
||||
{"login": "reviewer1"},
|
||||
{"login": "reviewer1"},
|
||||
{
|
||||
"user": {"login": "other_user"},
|
||||
"state": "open",
|
||||
"head": {"sha": "abc1"},
|
||||
"mergeable": True,
|
||||
},
|
||||
{"id": 100, "state": "APPROVED"},
|
||||
[
|
||||
{
|
||||
"id": 100,
|
||||
"user": {"login": "reviewer1"},
|
||||
"state": "APPROVED",
|
||||
"commit_id": "abc1",
|
||||
"submitted_at": "2026-07-06T10:00:00Z",
|
||||
"dismissed": False,
|
||||
}
|
||||
],
|
||||
]
|
||||
|
||||
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
|
||||
|
||||
Reference in New Issue
Block a user