diff --git a/mcp_server.py b/mcp_server.py index 26850df..ae06038 100644 --- a/mcp_server.py +++ b/mcp_server.py @@ -1098,16 +1098,30 @@ def gitea_merge_pr( payload["MergeMessageField"] = message api_request("POST", merge_url, auth, payload) # Best-effort read-back of the merge commit SHA (redacted on error). + merged = None try: merged = api_request( "GET", f"{repo_api_url(h, o, r)}/pulls/{pr_number}", auth ) result["merge_commit"] = (merged or {}).get("merged_commit_sha") - - cleanup = cleanup_in_progress_for_pr(merged or {}, remote, host, org, repo) - result["cleanup_status"] = cleanup.get("cleanup_status") except Exception: result["merge_commit"] = None + + # Tracker cleanup (#98): never blocks the merge, and always surfaces an + # explicit cleanup_status once the merge mutation has happened. Cleanup + # needs the PR title/body/branch, which only the read-back payload + # carries here — so a failed read-back is an explicit skip, not a + # silent one. + if merged is None: + result["cleanup_status"] = "skipped (merge read-back failed)" + else: + try: + cleanup = cleanup_in_progress_for_pr(merged, remote, host, org, repo) + result["cleanup_status"] = cleanup.get("cleanup_status") + except Exception as cleanup_exc: # noqa: BLE001 — redact before surfacing + result["cleanup_status"] = ( + f"skipped (cleanup error: {_redact(str(cleanup_exc))})" + ) except Exception as exc: # noqa: BLE001 — redact before surfacing reasons.append(f"merge failed: {_redact(str(exc))}") return result diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 79a42d9..94d4975 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -370,6 +370,55 @@ class TestMergePR(unittest.TestCase): expected_changed_files=["b.py", "a.py"], remote="prgs") self.assertTrue(r["performed"]) + # -- read-back / cleanup surfacing (#98) ----------------------------------- + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_readback_failure_reports_skipped_cleanup(self, _auth, mock_api): + """Merge OK + read-back GET failure => explicit cleanup skip, not silence.""" + mock_api.side_effect = [ + {"login": "merger-bot"}, self._pr("author-bot"), + {}, # merge POST + RuntimeError("HTTP 502: Gitea upstream unavailable"), # read-back fails + ] + 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") + # The merge itself is still reported performed/successful. + self.assertTrue(r["performed"]) + self.assertEqual(r["merge_result"], "PR #8 merged via 'merge'.") + self.assertIsNone(r["merge_commit"]) + # 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) + for c in mock_api.call_args_list: + self.assertNotEqual(c.args[0], "DELETE") + + @patch("mcp_server.cleanup_in_progress_for_pr", + side_effect=RuntimeError("boom token secret-xyz")) + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_cleanup_exception_surfaced_and_redacted(self, _auth, mock_api, _cleanup): + """Unexpected cleanup exception => merge still succeeds; error surfaced redacted.""" + mock_api.side_effect = [ + {"login": "merger-bot"}, self._pr("author-bot"), + {}, # merge POST + {"merged_commit_sha": "c9"}, # read-back OK + ] + 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.assertTrue(r["performed"]) + self.assertEqual(r["merge_commit"], "c9") + self.assertTrue(r["cleanup_status"].startswith("skipped (cleanup error:")) + self.assertNotIn("secret-xyz", r["cleanup_status"]) + # -- confirmation --------------------------------------------------------- @patch("mcp_server.api_request")