Merge pull request 'Require canonical workflow load proof for reviewer mutations' (#493) from feat/issue-389-review-workflow-load-proof into master

tests/test_audit.py

@@ -157,6 +157,7 @@ class _AuditWiringBase(unittest.TestCase):
 
     def tearDown(self):
         mcp_server._IDENTITY_CACHE.clear()
+        mcp_server.review_workflow_load.clear_review_workflow_load()
         self._dir.cleanup()
 
     def _env(self, **extra):
@@ -291,12 +292,11 @@ class TestGatedToolAudit(_AuditWiringBase):
 
     def setUp(self):
         super().setUp()
-        from mcp_server import init_review_decision_lock
-        from tests.test_mcp_server import _install_owned_reviewer_lease
+        from tests.test_mcp_server import _init_reviewer_session, _install_owned_reviewer_lease
         import reviewer_pr_lease
 
-        # init_review_decision_lock clears any prior session lease (#407).
-        init_review_decision_lock("prgs", "review_pr")
+        # Session init clears any prior session lease (#407) and loads workflow (#389).
+        _init_reviewer_session("prgs")
         self._lease_patch = _install_owned_reviewer_lease(8)
         self._lease_patch.start()
         self._auth_identity_patch = patch(
@@ -337,6 +337,7 @@ class TestGatedToolAudit(_AuditWiringBase):
         env = self._env(GITEA_PROFILE_NAME="gitea-merger",
                         GITEA_ALLOWED_OPERATIONS="read,merge")
         with patch.dict(os.environ, env, clear=True):
+            mcp_server.gitea_load_review_workflow()
             r = gitea_merge_pr(pr_number=8, confirmation="MERGE PR 8",
                                expected_head_sha="abc123", remote="prgs")
         self.assertTrue(r["performed"])
@@ -356,6 +357,7 @@ class TestGatedToolAudit(_AuditWiringBase):
         env = self._env(GITEA_PROFILE_NAME="gitea-merger",
                         GITEA_ALLOWED_OPERATIONS="read,merge")
         with patch.dict(os.environ, env, clear=True):
+            mcp_server.gitea_load_review_workflow()
             r = gitea_merge_pr(pr_number=8, confirmation="MERGE PR 8", remote="prgs")
         self.assertFalse(r["performed"])
         recs = self._records()

tests/test_llm_agent_sha.py

@@ -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,

tests/test_mcp_server.py

@@ -58,6 +58,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'."""
@@ -179,6 +185,7 @@ def _seed_ready_review_decision(
         "correction_authorized": False,
         "correction_reason": None,
     })
+    _m.gitea_load_review_workflow()
 
 
 # Issue-write tools are profile-gated (#69).
@@ -671,6 +678,7 @@ class TestMergePR(unittest.TestCase):
     def setUp(self):
         import reviewer_pr_lease
 
+        mcp_server.gitea_load_review_workflow()
         self._lease_patch = _install_owned_reviewer_lease(8)
         self._lease_patch.start()
         self._auth_identity_patch = patch(
@@ -1876,7 +1884,7 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
     def setUp(self):
         import reviewer_pr_lease
 
-        init_review_decision_lock("prgs", "review_pr")
+        _init_reviewer_session("prgs")
         self._lease_patch = _install_owned_reviewer_lease(
             self.PR, head_sha=self.SHA,
         )
@@ -1996,7 +2004,7 @@ class TestSubmitPrReview(unittest.TestCase):
     def setUp(self):
         import reviewer_pr_lease
 
-        init_review_decision_lock("prgs", "review_pr")
+        _init_reviewer_session("prgs")
         self._lease_patch = _install_owned_reviewer_lease(8)
         self._lease_patch.start()
         self._auth_identity_patch = patch(
@@ -2416,7 +2424,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", expected_head_sha="abc123")
         self.assertFalse(r["marked_ready"])
         self.assertTrue(any("does not match locked remote" in x for x in r["reasons"]))
@@ -2500,6 +2508,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()

tests/test_permission_reports.py

@@ -99,6 +99,7 @@ class PermissionReportBase(unittest.TestCase):
     def tearDown(self):
         self._remotes.stop()
         mcp_server._IDENTITY_CACHE.clear()
+        mcp_server.review_workflow_load.clear_review_workflow_load()
         gitea_config._active_profile_override = None
         self._dir.cleanup()
 
@@ -250,6 +251,8 @@ class TestEligibilityDenialReport(PermissionReportBase):
                 return {"login": "author-user"}
             return PR_PAYLOAD
         mock_api.side_effect = fake_api
+        mcp_server.init_review_decision_lock("prgs", "review_pr")
+        mcp_server.gitea_load_review_workflow()
         with patch.dict(os.environ, self._env("author-profile")):
             res = mcp_server.gitea_merge_pr(
                 pr_number=42, confirmation="MERGE PR 42",

tests/test_pr_queue_inventory.py

@@ -11,6 +11,7 @@ from mcp_server import (
     gitea_view_pr,
     gitea_review_pr,
     gitea_check_pr_eligibility,
+    gitea_load_review_workflow,
 )
 import gitea_config
 

tests/test_review_workflow_load.py

@@ -0,0 +1,153 @@
+"""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.get_auth_header", return_value="Basic dGVzdA=="), \
+                patch("mcp_server._list_pr_lease_comments", return_value=[]), \
+                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()

tests/test_terminal_review_hard_stop.py

@@ -37,6 +37,7 @@ def _lock(mutations=None, correction=False):
 
 def _seed(mutations=None, correction=False):
     mcp_server._save_review_decision_lock(_lock(mutations, correction))
+    mcp_server.gitea_load_review_workflow()
 
 
 APPROVED_A = {"pr_number": 5, "action": "approve", "review_id": 1,
@@ -48,6 +49,7 @@ RC_A = {"pr_number": 5, "action": "request_changes", "review_id": 2,
 class TestTerminalHardStopReasons(unittest.TestCase):
     def tearDown(self):
         mcp_server._save_review_decision_lock(None)
+        mcp_server.review_workflow_load.clear_review_workflow_load()
 
     def test_no_lock_no_reasons(self):
         mcp_server._save_review_decision_lock(None)
@@ -93,6 +95,7 @@ class TestTerminalHardStopReasons(unittest.TestCase):
 class TestMergeHardStopWiring(unittest.TestCase):
     def tearDown(self):
         mcp_server._save_review_decision_lock(None)
+        mcp_server.review_workflow_load.clear_review_workflow_load()
 
     def test_merge_blocked_after_request_changes(self):
         _seed([RC_A])
@@ -114,6 +117,7 @@ class TestMergeHardStopWiring(unittest.TestCase):
 class TestMarkFinalHardStopWiring(unittest.TestCase):
     def tearDown(self):
         mcp_server._save_review_decision_lock(None)
+        mcp_server.review_workflow_load.clear_review_workflow_load()
 
     def test_mark_ready_blocked_after_terminal_mutation(self):
         _seed([RC_A])
@@ -146,6 +150,7 @@ def _mark(action, pr_number=6, **kwargs):
 class TestDuplicateRequestChangesSuppression(unittest.TestCase):
     def tearDown(self):
         mcp_server._save_review_decision_lock(None)
+        mcp_server.review_workflow_load.clear_review_workflow_load()
 
     def test_duplicate_request_changes_blocked_at_same_head(self):
         _seed()