Merge pull request 'fix: remove duplicate verify_preflight_purity in gitea_adopt_merger_pr_lease (closes #548)' (#550) from issue-548-fix-duplicate-preflight into master

This commit was merged in pull request #550.
This commit is contained in:
2026-07-08 20:54:40 -05:00
2 changed files with 64 additions and 2 deletions
-1
View File
@@ -5599,7 +5599,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()
+64 -1
View File
@@ -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": "<!-- mcp-review-lease-adoption:v1 --> 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()