feat: gate reviewer mutations on canonical workflow load proof (Closes #389)

Add gitea_load_review_workflow and in-process session proof required before
gitea_submit_pr_review, gitea_mark_final_review_decision, and gitea_merge_pr.
Capability and runtime context report workflow-load status; stale or missing
proof fails closed with recovery handoff text that forbids approve/merge replay.
This commit is contained in:
2026-07-07 17:20:10 -04:00
parent cc4741a4ce
commit 8d2608a73e
6 changed files with 445 additions and 6 deletions
+2
View File
@@ -22,6 +22,7 @@ from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import mcp_server # noqa: E402
from mcp_server import ( # noqa: E402
gitea_check_pr_eligibility,
gitea_merge_pr,
@@ -130,6 +131,7 @@ class TestShaCannotBypassSelfReview(unittest.TestCase):
]
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
init_review_decision_lock("prgs", "review_pr")
mcp_server.gitea_load_review_workflow()
gitea_mark_final_review_decision(9, "approve", remote="prgs")
env = self._env(SHA_WOULD_BE_REVIEWER, "reviewer")
with patch.dict(os.environ, env, clear=True):
+14 -5
View File
@@ -55,6 +55,12 @@ _NO_BLOCKER_FEEDBACK = {
}
def _init_reviewer_session(remote="prgs"):
"""Seed review decision lock and required workflow-load proof (#389)."""
init_review_decision_lock(remote, "review_pr")
mcp_server.gitea_load_review_workflow()
def _mark_request_changes_ready(pr_number=8, **kwargs):
"""Mark a request_changes decision ready with the #332 duplicate-
suppression feedback fetch stubbed to 'no existing blocker'."""
@@ -539,6 +545,9 @@ class TestViewPR(unittest.TestCase):
class TestMergePR(unittest.TestCase):
"""Gated merge workflow (#16). gitea_merge_pr is the only merge path."""
def setUp(self):
mcp_server.gitea_load_review_workflow()
def _pr(self, author, state="open", sha="abc123", mergeable=True):
return {
"user": {"login": author},
@@ -1011,8 +1020,7 @@ class TestReviewPR(unittest.TestCase):
{"login": "jcwalker3"}, # /api/v1/user (submit eligibility)
{"user": {"login": "jcwalker3"}, "state": "open", "head": {"sha": "abc1234"}, "mergeable": True}, # /pulls/1
]
from mcp_server import init_review_decision_lock
init_review_decision_lock("prgs", "review_pr")
_init_reviewer_session("prgs")
gitea_mark_final_review_decision(1, "approve", remote="prgs")
result = gitea_review_pr(
pr_number=1,
@@ -1680,7 +1688,7 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
}
def setUp(self):
init_review_decision_lock("prgs", "review_pr")
_init_reviewer_session("prgs")
def _env(self):
return patch.dict(os.environ, {
@@ -1775,7 +1783,7 @@ class TestSubmitPrReview(unittest.TestCase):
"""Gated review-mutation tool (#15)."""
def setUp(self):
init_review_decision_lock("prgs", "review_pr")
_init_reviewer_session("prgs")
gitea_mark_final_review_decision(8, "approve", remote="prgs")
def _pr(self, author, state="open", sha="abc123", mergeable=True):
@@ -2168,7 +2176,7 @@ class TestSubmitPrReview(unittest.TestCase):
os.remove(spoof_path)
def test_mark_final_decision_rejects_remote_mismatch(self):
init_review_decision_lock("prgs", "review_pr")
_init_reviewer_session("prgs")
r = gitea_mark_final_review_decision(8, "approve", remote="dadeschools")
self.assertFalse(r["marked_ready"])
self.assertTrue(any("does not match locked remote" in x for x in r["reasons"]))
@@ -2250,6 +2258,7 @@ if __name__ == "__main__":
class TestTrackerHygieneCleanup(unittest.TestCase):
def setUp(self):
mcp_server.gitea_load_review_workflow()
self.mock_api = patch("mcp_server.api_request").start()
self.mock_auth = patch("mcp_server.get_auth_header", return_value=FAKE_AUTH).start()
patch("gitea_audit.audit_enabled", return_value=True).start()
+2
View File
@@ -230,6 +230,7 @@ class TestEligibilityDenialReport(PermissionReportBase):
return PR_PAYLOAD
mock_api.side_effect = fake_api
mcp_server.init_review_decision_lock("prgs", "review_pr")
mcp_server.gitea_load_review_workflow()
mcp_server.gitea_mark_final_review_decision(42, "approve", remote="prgs")
with patch.dict(os.environ, self._env("author-profile")):
res = mcp_server.gitea_submit_pr_review(
@@ -272,6 +273,7 @@ class TestReviewCommentPathUsesCanonicalOp(PermissionReportBase):
return PR_PAYLOAD
mock_api.side_effect = fake_api
mcp_server.init_review_decision_lock("prgs", "review_pr")
mcp_server.gitea_load_review_workflow()
mcp_server.gitea_mark_final_review_decision(42, "comment", remote="prgs")
with patch.dict(os.environ, self._env("author-profile")):
res = mcp_server.gitea_submit_pr_review(
+151
View File
@@ -0,0 +1,151 @@
"""Tests for canonical review workflow load proof (#389)."""
import os
import sys
import unittest
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import review_workflow_load
import mcp_server
class TestReviewWorkflowLoadModule(unittest.TestCase):
def setUp(self):
review_workflow_load.clear_review_workflow_load()
mcp_server._save_review_decision_lock(None)
def test_load_records_hash_and_schema(self):
root = str(__import__("pathlib").Path(__file__).resolve().parent.parent)
recorded = review_workflow_load.record_review_workflow_load(root)
self.assertEqual(
recorded["workflow_source"],
review_workflow_load.WORKFLOW_REL_PATH,
)
self.assertEqual(recorded["task_mode"], "review-merge-pr")
self.assertRegex(recorded["workflow_hash"], r"^[0-9a-f]{12}$")
self.assertEqual(
recorded["final_report_schema_path"],
review_workflow_load.SCHEMA_REL_PATH,
)
status = review_workflow_load.workflow_load_status(root)
self.assertTrue(status["workflow_load_proof_present"])
self.assertTrue(status["workflow_load_valid"])
def test_stale_session_pid_blocks(self):
root = str(__import__("pathlib").Path(__file__).resolve().parent.parent)
review_workflow_load.record_review_workflow_load(root)
review_workflow_load._REVIEW_WORKFLOW_LOAD["session_pid"] = 0
blockers = review_workflow_load.review_workflow_load_blockers(root)
self.assertTrue(any("different process" in b for b in blockers))
def test_prompt_conflict_detected(self):
conflict, reasons = review_workflow_load.assess_prompt_conflict(
"Run work-issue author implementation only")
self.assertTrue(conflict)
self.assertTrue(reasons)
class TestReviewWorkflowLoadGates(unittest.TestCase):
def setUp(self):
review_workflow_load.clear_review_workflow_load()
mcp_server.init_review_decision_lock("prgs", "review_pr")
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", "reviewer")
def _load_workflow(self):
return mcp_server.gitea_load_review_workflow()
def test_mcp_helper_returns_required_fields(self):
res = self._load_workflow()
self.assertTrue(res["success"])
self.assertTrue(res["loaded"])
self.assertIn("workflow_source", res)
self.assertIn("workflow_hash", res)
self.assertIn("final_report_schema_path", res)
self.assertIn("final_report_schema_hash", res)
def test_mark_final_blocked_without_load(self):
res = mcp_server.gitea_mark_final_review_decision(
42, "approve", remote="prgs")
self.assertFalse(res["marked_ready"])
self.assertTrue(any(
"gitea_load_review_workflow" in r for r in res["reasons"]))
self.assertTrue(any(
"approve/merge replay" in r.lower() or "Do not call" in r
for r in res["reasons"]))
def test_submit_review_blocked_without_load(self):
with patch("mcp_server.gitea_check_pr_eligibility") as elig:
elig.return_value = {
"eligible": True,
"authenticated_user": "rev",
"profile_name": "prgs-reviewer",
"pr_author": "author",
"head_sha": "abc123",
"reasons": [],
}
res = mcp_server.gitea_submit_pr_review(
42,
"approve",
remote="prgs",
final_review_decision_ready=True,
)
self.assertFalse(res["performed"])
self.assertTrue(any(
"gitea_load_review_workflow" in r for r in res["reasons"]))
def test_merge_blocked_without_load(self):
res = mcp_server.gitea_merge_pr(
42,
confirmation="MERGE PR 42",
remote="prgs",
)
self.assertFalse(res["performed"])
self.assertTrue(any(
"gitea_load_review_workflow" in r for r in res["reasons"]))
def test_resolve_capability_reports_missing_load(self):
with patch.object(mcp_server, "_ensure_matching_profile"):
with patch.object(
mcp_server.gitea_config, "is_runtime_switching_enabled",
return_value=False):
with patch.object(
mcp_server, "_authenticated_username",
return_value="rev"):
res = mcp_server.gitea_resolve_task_capability(
"review_pr", remote="prgs")
proof = res.get("workflow_load_proof") or {}
self.assertFalse(proof.get("workflow_load_valid"))
self.assertTrue(any(
"gitea_load_review_workflow" in g
for g in res.get("task_role_guidance") or []))
def test_init_review_lock_clears_prior_load(self):
self._load_workflow()
mcp_server.init_review_decision_lock("prgs", "review_pr")
blockers = review_workflow_load.review_workflow_load_blockers(
str(__import__("pathlib").Path(__file__).resolve().parent.parent))
self.assertTrue(blockers)
def test_dry_run_allowed_without_load(self):
with patch("mcp_server.gitea_check_pr_eligibility") as elig:
elig.return_value = {
"eligible": True,
"authenticated_user": "rev",
"profile_name": "prgs-reviewer",
"pr_author": "author",
"head_sha": "abc123",
"reasons": [],
}
res = mcp_server.gitea_dry_run_pr_review(
42, "approve", remote="prgs")
self.assertNotIn(
"gitea_load_review_workflow",
" ".join(res.get("reasons") or []),
)
if __name__ == "__main__":
unittest.main()