diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 3aa0229..dd921ca 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -5591,7 +5591,6 @@ def gitea_adopt_merger_pr_lease( _verify_role_mutation_workspace( remote, worktree=worktree, task="adopt_merger_pr_lease" ) - verify_preflight_purity(remote, task="adopt_merger_pr_lease") h, o, r = _resolve(remote, host, org, repo) auth = _auth(h) profile = get_profile() diff --git a/tests/test_merger_lease_adoption.py b/tests/test_merger_lease_adoption.py index 688c14f..8cd681d 100644 --- a/tests/test_merger_lease_adoption.py +++ b/tests/test_merger_lease_adoption.py @@ -258,7 +258,9 @@ class TestMergerHandoffMutationGate(unittest.TestCase): class TestAdoptTool(unittest.TestCase): def setUp(self): leases.clear_session_lease() - patch("mcp_server.verify_preflight_purity", return_value=None).start() + # Patch only the role workspace verifier (which internally does the single + # verify_preflight_purity); the direct duplicate call was removed to prevent + # capability state clearing. See fix for #548. patch("mcp_server._verify_role_mutation_workspace", return_value=None).start() patch("gitea_audit.audit_enabled", return_value=False).start() mcp_server = __import__("mcp_server") @@ -363,6 +365,67 @@ class TestAdoptTool(unittest.TestCase): ) self.assertFalse(gate["block"]) + def test_adopt_preserves_preflight_capability_state_no_duplicate_clear(self): + """Prove fix for #548: adopt calls _verify (single purity) and does not + trigger duplicate verify that would clear capability state, avoiding + need for temp local patches or raw fallbacks. + """ + from unittest.mock import patch, MagicMock + import mcp_server as mcp_server_mod + + # fresh preflight state (as controller/reconciler would have from resolve) + mcp_server_mod.record_preflight_check("whoami") + mcp_server_mod.record_preflight_check( + "capability", "merger", resolved_task="adopt_merger_pr_lease" + ) + initial_cap_called = mcp_server_mod._preflight_capability_called + + verify_patch = patch("mcp_server.verify_preflight_purity", wraps=mcp_server_mod.verify_preflight_purity) + def _role_side(*a, **k): + # simulate the real _verify which calls verify once with path + mcp_server_mod.verify_preflight_purity(k.get("remote") or a[0] if a else None, worktree_path=k.get("worktree") or "branches/work", task=k.get("task")) + return "branches/work" + role_patch = patch("mcp_server._verify_role_mutation_workspace", side_effect=_role_side) + with verify_patch as vmock, role_patch, \ + patch("mcp_server.get_auth_header", return_value="token test"), \ + patch("mcp_server._authenticated_username", return_value="sysadmin"): + # minimal mocks to let adopt reach end without full api + with patch("mcp_server.get_profile") as gp: + gp.return_value = { + "profile_name": "prgs-merger", + "allowed_operations": ["gitea.read", "gitea.pr.comment", "gitea.pr.merge"], + "forbidden_operations": [], + } + with patch("mcp_server.api_request") as api: + def _side(url, *a, **k): + if "/pulls/" in str(url) and "/comments" not in str(url): + return {"state": "open", "head": {"sha": "deadbeef"*5}, "merged": False, "number": 999} + if "/comments" in str(url): + return [] # list of comments for fetch + return {"id": 42} + api.side_effect = _side + with patch("mcp_server.gitea_get_pr_review_feedback", return_value={"approval_at_current_head": True}): + with patch("merger_lease_adoption.assess_adopt_merger_lease", return_value={ + "adopt_allowed": True, + "active_lease": {"session_id": "rev", "profile": "prgs-reviewer", "reviewer_identity": "sysadmin", "issue_number": 548, "target_branch": "master"}, + "adoption_body": " adopted", + }): + res = mcp_server_mod.gitea_adopt_merger_pr_lease( + pr_number=999, + worktree="branches/merge-test", + expected_head_sha="deadbeef"*5, + issue_number=548, + remote="prgs", + ) + self.assertTrue(res.get("success")) + # verify_preflight_purity should have been invoked exactly once (via the _verify_role path) + # not twice (old dup would have cleared state mid-way, requiring temp patches) + self.assertEqual(vmock.call_count, 1) + # capability state management: the single verify consumes (clears) as designed; + # prior state was valid, no unexpected clear between "checks" or hidden reset + # (the removed dup line was the source of the #548 clearing bug) + self.assertTrue(initial_cap_called) + # final may be set by other records in flow, but key is single invocation and success without bypasses if __name__ == "__main__": unittest.main() \ No newline at end of file