From 7f2a5a3d3c567053dade714aeba28c18fa82bb34 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Thu, 9 Jul 2026 10:12:17 -0400 Subject: [PATCH 1/2] fix: resolve reviewer lease identity via host not remote alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _authenticated_username expects a host for auth/API calls. Lease acquire/adopt/gate/release paths passed the remote alias (prgs), so identity resolved empty and same-identity release fail-closed across sessions — blocking author conflict-fix pushes after REQUEST_CHANGES. Pass resolved host h from _resolve into _authenticated_username. --- gitea_mcp_server.py | 14 +++++--- tests/test_mcp_server.py | 78 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 49460df..e41b202 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -6153,7 +6153,9 @@ def _reviewer_pr_lease_gate( """Return block reasons when the session lacks an owned PR reviewer lease.""" session = reviewer_pr_lease.get_session_lease() session_id = (session or {}).get("session_id") - identity = _authenticated_username(remote) or "" + # Resolve host first: _authenticated_username expects a host, not remote name. + h, _, _ = _resolve(remote, host, org, repo) + identity = _authenticated_username(h) or "" try: comments = _fetch_pr_comments( pr_number, remote=remote, host=host, org=org, repo=repo) @@ -6207,7 +6209,8 @@ def gitea_acquire_reviewer_pr_lease( h, o, r = _resolve(remote, host, org, repo) auth = _auth(h) profile = get_profile() - identity = _authenticated_username(remote) or profile.get("username") or "" + # Host (not remote name) — remote aliases like "prgs" do not resolve identity. + identity = _authenticated_username(h) or profile.get("username") or "" sid = (session_id or "").strip() or reviewer_pr_lease.new_session_id() repo_label = f"{o}/{r}" @@ -6350,7 +6353,8 @@ def gitea_adopt_merger_pr_lease( auth = _auth(h) profile = get_profile() profile_name = profile.get("profile_name") or "unknown" - identity = _authenticated_username(remote) or profile.get("username") or "" + # Host (not remote name) — remote aliases like "prgs" do not resolve identity. + identity = _authenticated_username(h) or profile.get("username") or "" sid = (session_id or "").strip() or reviewer_pr_lease.new_session_id() repo_label = f"{o}/{r}" @@ -6738,7 +6742,9 @@ def gitea_release_reviewer_pr_lease( "reasons": ["no active reviewer lease found on PR"], } - identity = _authenticated_username(remote) or "" + # Host (not remote name). Passing "prgs"/"dadeschools" yields empty identity + # and incorrectly blocks same-identity release across sessions. + identity = _authenticated_username(h) or "" session = reviewer_pr_lease.get_session_lease() or {} session_id = session.get("session_id") diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index c3b4643..c054d7b 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -4288,3 +4288,81 @@ class TestIssue546Deadlock(unittest.TestCase): self.assertIsNone(reviewer_pr_lease.get_session_lease()) # verify comment phase is released self.assertIn("phase: released", posted_comments[0]["body"]) + + def test_release_reviewer_pr_lease_same_identity_cross_session(self): + """Same reviewer identity may release a lease claimed by another session. + + Regression: release used to call ``_authenticated_username(remote)`` + with the remote alias (e.g. ``prgs``) instead of the resolved host, + so identity resolved empty and same-identity release fail-closed even + when the authenticated user owned the lease. + """ + import mcp_server + import reviewer_pr_lease + + mcp_server.gitea_resolve_task_capability(task="review_pr", remote="prgs") + mcp_server.gitea_load_review_workflow() + reviewer_pr_lease.clear_session_lease() + + foreign_session = "46820-dd78cdc4aacc" + comments = [ + _reviewer_lease_comment( + 457, + session_id=foreign_session, + head_sha="ef287eaf5841d9e542a4e888ce0ae7d679dbd685", + reviewer="sysadmin", + ) + ] + posted = [] + resolved_hosts = [] + + def mock_api(method, url, auth, data=None): + if "/api/v1/user" in url: + return {"login": "sysadmin"} + if "/comments" in url: + if method == "POST": + new_c = { + "id": 8071, + "body": data["body"], + "user": {"login": "sysadmin"}, + } + comments.append(new_c) + posted.append(new_c) + return {"id": 8071} + return comments + if "/pulls/" in url: + return { + "user": {"login": "jcwalker3"}, + "state": "open", + "head": {"sha": "ef287eaf5841d9e542a4e888ce0ae7d679dbd685"}, + "mergeable": False, + } + return {} + + def tracking_username(host): + resolved_hosts.append(host) + # Empty identity when passed a remote alias (the pre-fix bug path). + if host in {"prgs", "dadeschools"}: + return None + return "sysadmin" + + with patch("mcp_server.api_request", side_effect=mock_api), patch( + "mcp_server._authenticated_username", side_effect=tracking_username + ), patch( + "mcp_server._resolve", + return_value=("gitea.prgs.cc", "Scaled-Tech-Consulting", "Gitea-Tools"), + ), patch("mcp_server._auth", return_value={"Authorization": "token x"}), patch( + "mcp_server._verify_role_mutation_workspace", return_value=None + ): + res = mcp_server.gitea_release_reviewer_pr_lease( + pr_number=457, remote="prgs" + ) + + self.assertTrue(res.get("success"), res) + self.assertTrue(res.get("released"), res) + self.assertEqual(res.get("comment_id"), 8071) + self.assertTrue(posted) + self.assertIn("phase: released", posted[0]["body"]) + # Must resolve identity with host, never the remote alias alone. + self.assertIn("gitea.prgs.cc", resolved_hosts) + self.assertNotIn("prgs", resolved_hosts) -- 2.43.7 From fe9faec741960136b9a49140ed2f1a5486cf4035 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Thu, 9 Jul 2026 10:49:31 -0400 Subject: [PATCH 2/2] test: cover reviewer lease identity host resolution --- tests/test_mcp_server.py | 155 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index c054d7b..d52263b 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -4289,6 +4289,161 @@ class TestIssue546Deadlock(unittest.TestCase): # verify comment phase is released self.assertIn("phase: released", posted_comments[0]["body"]) + def test_reviewer_pr_lease_gate_resolves_identity_with_host(self): + """Lease mutation gate must resolve identity using host, not remote alias.""" + import mcp_server + + resolved_hosts = [] + + def tracking_username(host): + resolved_hosts.append(host) + return None if host in {"prgs", "dadeschools"} else "reviewer-bot" + + with _install_owned_reviewer_lease( + 550, + session_id=_DEFAULT_LEASE_SESSION, + head_sha="abc123", + ), patch( + "mcp_server._resolve", + return_value=("gitea.prgs.cc", "Scaled-Tech-Consulting", "Gitea-Tools"), + ), patch( + "mcp_server._authenticated_username", + side_effect=tracking_username, + ): + reasons = mcp_server._reviewer_pr_lease_gate( + pr_number=550, + remote="prgs", + host=None, + org=None, + repo=None, + mutation="approve", + live_head_sha="abc123", + pinned_head_sha="abc123", + ) + + self.assertEqual(reasons, []) + self.assertIn("gitea.prgs.cc", resolved_hosts) + self.assertNotIn("prgs", resolved_hosts) + + def test_acquire_reviewer_pr_lease_resolves_identity_with_host(self): + """Acquire path must not pass a remote alias into identity lookup.""" + import mcp_server + + resolved_hosts = [] + posted = [] + + def tracking_username(host): + resolved_hosts.append(host) + return None if host in {"prgs", "dadeschools"} else "reviewer-bot" + + def mock_api(method, url, auth, data=None): + if "/pulls/" in url: + return { + "user": {"login": "author-user"}, + "state": "open", + "head": {"sha": "abc123"}, + "mergeable": True, + } + if "/comments" in url: + if method == "POST": + posted.append(data["body"]) + return {"id": 1003} + return [] + return {} + + with patch("mcp_server.api_request", side_effect=mock_api), patch( + "mcp_server._authenticated_username", + side_effect=tracking_username, + ), patch( + "mcp_server._resolve", + return_value=("gitea.prgs.cc", "Scaled-Tech-Consulting", "Gitea-Tools"), + ): + res = mcp_server.gitea_acquire_reviewer_pr_lease( + pr_number=550, + worktree="/workspace", + candidate_head="abc123", + remote="prgs", + ) + + self.assertTrue(res.get("success"), res) + self.assertTrue(res.get("acquired"), res) + self.assertEqual(res.get("comment_id"), 1003) + self.assertTrue(posted) + self.assertIn("gitea.prgs.cc", resolved_hosts) + self.assertNotIn("prgs", resolved_hosts) + + def test_adopt_merger_pr_lease_resolves_identity_with_host(self): + """Adopt path must not pass a remote alias into identity lookup.""" + import mcp_server + import reviewer_pr_lease + + reviewer_pr_lease.clear_session_lease() + head = "a" * 40 + resolved_hosts = [] + posted = [] + reviewer_comment = _reviewer_lease_comment( + 550, + session_id="reviewer-session", + head_sha=head, + reviewer="reviewer-bot", + ) + + def tracking_username(host): + resolved_hosts.append(host) + return None if host in {"prgs", "dadeschools"} else "merger-bot" + + def mock_api(method, url, auth, data=None): + if "/pulls/" in url and "/comments" not in url: + return { + "number": 550, + "user": {"login": "author-user"}, + "state": "open", + "head": {"sha": head}, + "mergeable": True, + "merged": False, + } + if "/comments" in url: + if method == "POST": + posted.append(data["body"]) + return {"id": 1004} + return [reviewer_comment] + return {} + + merger_profile = { + "profile_name": "prgs-merger", + "allowed_operations": ["gitea.read", "gitea.pr.comment", "gitea.pr.merge"], + "forbidden_operations": [], + } + with patch("mcp_server.get_profile", return_value=merger_profile), patch( + "mcp_server.api_request", + side_effect=mock_api, + ), patch( + "mcp_server._authenticated_username", + side_effect=tracking_username, + ), patch( + "mcp_server._resolve", + return_value=("gitea.prgs.cc", "Scaled-Tech-Consulting", "Gitea-Tools"), + ), patch( + "mcp_server.gitea_get_pr_review_feedback", + return_value={ + "approval_at_current_head": True, + "latest_approved_head_sha": head, + }, + ): + res = mcp_server.gitea_adopt_merger_pr_lease( + pr_number=550, + worktree="/workspace", + expected_head_sha=head, + remote="prgs", + ) + + self.assertTrue(res.get("success"), res) + self.assertTrue(res.get("adopted"), res) + self.assertEqual(res.get("adoption_comment_id"), 1004) + self.assertTrue(posted) + self.assertIn("gitea.prgs.cc", resolved_hosts) + self.assertNotIn("prgs", resolved_hosts) + def test_release_reviewer_pr_lease_same_identity_cross_session(self): """Same reviewer identity may release a lease claimed by another session. -- 2.43.7