From f1449ff5c8107789c7f2614ffe9875ef4eeaa28b Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Wed, 8 Jul 2026 22:07:59 -0400 Subject: [PATCH] fix: isolate #519 PR lookup from shared lease comment listing Keep _fetch_pr_lease_comments_safe for gitea_assess_conflict_fix_push only. Restore _list_pr_lease_comments to a single comments GET so merge approval feedback and #485 non-list handling keep stable API call order. Closes #519 --- gitea_mcp_server.py | 29 ++++++++++++------- .../test_pr_lease_comments_non_list_guard.py | 23 ++++++--------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 0609f46..af54037 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -2940,16 +2940,25 @@ def _list_pr_lease_comments( repo: str | None, limit: int = 100, ) -> list[dict]: - """Fetch PR/issue thread comments used for reviewer/conflict-fix leases.""" - fetched = _fetch_pr_lease_comments_safe( - pr_number, - remote=remote, - host=host, - org=org, - repo=repo, - limit=limit, - ) - return fetched["comments"] + """Fetch PR/issue thread comments used for reviewer/conflict-fix leases. + + Intentionally does **not** pre-fetch the PR via GET /pulls/{n}. Shared + callers (merge approval feedback, reviewer lease gates) depend on a single + comments GET so mock sequences and fail-open #485 non-list handling stay + stable. Structured PR-lookup failures belong only to + :func:`_fetch_pr_lease_comments_safe` used by conflict-fix push assessment + (#519). + """ + h, o, r = _resolve(remote, host, org, repo) + auth = _auth(h) + api = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments" + comments = api_request("GET", api, auth) + # Fail safe to no lease comments when the API returns a non-list payload + # (e.g. an error object such as an HTTP 401 body): lease state can only be + # proven from real comment entries, never inferred from an error shape (#485). + if not isinstance(comments, list): + return [] + return list(comments[:limit]) def _pr_work_lease_reviewer_block( diff --git a/tests/test_pr_lease_comments_non_list_guard.py b/tests/test_pr_lease_comments_non_list_guard.py index 526c366..9b21a24 100644 --- a/tests/test_pr_lease_comments_non_list_guard.py +++ b/tests/test_pr_lease_comments_non_list_guard.py @@ -16,31 +16,26 @@ AUTHOR_ENV = { "GITEA_PROFILE_NAME": "gitea-author", "GITEA_ALLOWED_OPERATIONS": "gitea.read,gitea.issue.comment", } -OPEN_PR = {"number": 12, "state": "open", "head": {"sha": "abc123"}} - - -def _pr_then_comments(mock_api, comments_payload): - def _api(method, url, auth, *args, **kwargs): - if "/pulls/" in url: - return OPEN_PR - return comments_payload - - mock_api.side_effect = _api class TestPrLeaseCommentsNonListGuard(unittest.TestCase): @patch("mcp_server.api_request") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_list_pr_lease_comments_non_list_payload_returns_empty(self, _auth, mock_api): - _pr_then_comments(mock_api, {"message": "Unauthorized"}) + mock_api.return_value = {"message": "Unauthorized"} result = _list_pr_lease_comments( 12, remote="prgs", host=None, org=None, repo=None) self.assertEqual(result, []) + # Single comments GET only — no pre-flight PR lookup (#519 isolation). + mock_api.assert_called_once() + _method, url, _auth_arg = mock_api.call_args[0][:3] + self.assertIn("/issues/12/comments", url) + self.assertNotIn("/pulls/", url) @patch("mcp_server.api_request") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_list_pr_lease_comments_none_returns_empty(self, _auth, mock_api): - _pr_then_comments(mock_api, None) + mock_api.return_value = None result = _list_pr_lease_comments( 12, remote="prgs", host=None, org=None, repo=None) self.assertEqual(result, []) @@ -49,7 +44,7 @@ class TestPrLeaseCommentsNonListGuard(unittest.TestCase): @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_list_pr_lease_comments_list_payload_unchanged(self, _auth, mock_api): comment = {"id": 7, "body": ""} - _pr_then_comments(mock_api, [comment]) + mock_api.return_value = [comment] result = _list_pr_lease_comments( 12, remote="prgs", host=None, org=None, repo=None, limit=5) self.assertEqual(result, [comment]) @@ -83,4 +78,4 @@ class TestPrLeaseCommentsNonListGuard(unittest.TestCase): if __name__ == "__main__": - unittest.main() \ No newline at end of file + unittest.main()