Merge pull request 'feat: reject legacy Workspace mutations field in reviewer handoffs (Closes #320)' (#357) from feat/issue-320-remove-workspace-mutations into master

tests/test_review_proofs.py

@@ -941,13 +941,18 @@ class TestControllerHandoff(unittest.TestCase):
         self.assertIn("Safety", result["missing_fields"])
 
     def test_review_role_requires_review_fields(self):
-        result = assess_controller_handoff(self.BASE_HANDOFF, role="review")
+        # Issue #320: review handoffs must not carry the legacy
+        # 'Workspace mutations' line, so strip it from the shared base.
+        review_base = "\n".join(
+            line for line in self.BASE_HANDOFF.splitlines()
+            if not line.startswith("- Workspace mutations:"))
+        result = assess_controller_handoff(review_base, role="review")
         self.assertEqual(result["verdict"], "incomplete")
         self.assertIn("Pinned reviewed head", result["missing_fields"])
         self.assertIn("Worktree path", result["missing_fields"])
         self.assertIn("Merge result", result["missing_fields"])
 
-        complete = self.BASE_HANDOFF + "\n" + "\n".join([
+        complete = review_base + "\n" + "\n".join([
             "- Selected PR: #999",
             "- Reviewer eligibility: passed",
             "- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
@@ -959,6 +964,15 @@ class TestControllerHandoff(unittest.TestCase):
             "- Merge result: merged",
             "- Linked issue status: closed",
             "- Cleanup status: branch deleted",
+            "- File edits by reviewer: none",
+            "- Worktree/index mutations: created and removed review worktree",
+            "- Git ref mutations: git fetch prgs",
+            "- MCP/Gitea mutations: none",
+            "- Review mutations: one approve on #999",
+            "- Merge mutations: PR #999 merged",
+            "- Cleanup mutations: removed review worktree",
+            "- External-state mutations: none",
+            "- Read-only diagnostics: git log, git diff",
         ])
         result = assess_controller_handoff(complete, role="review")
         self.assertEqual(result["verdict"], "complete")
@@ -1081,6 +1095,144 @@ class TestControllerHandoff(unittest.TestCase):
         self.assertEqual(res2["verdict"], "complete")
 
 
+class TestReviewHandoffPreciseMutationCategories(unittest.TestCase):
+    """Issue #320: reviewer handoffs drop legacy 'Workspace mutations'."""
+
+    REVIEW_BASE = "\n".join([
+        "## Controller Handoff",
+        "",
+        "- Task: review PR #999",
+        "- Repo: Scaled-Tech-Consulting/Gitea-Tools",
+        "- Role: reviewer",
+        "- Identity: jcwalker3 / prgs-reviewer",
+        "- Issue/PR: PR #999",
+        "- Branch/SHA: feat/x @ 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
+        "- Files changed: review_proofs.py",
+        "- Validation: 700 passed, 6 skipped",
+        "- Mutations: one review submitted",
+        "- Current status: review complete",
+        "- Blockers: none",
+        "- Next: controller decides",
+        "- Safety: no self-review; no self-merge; no secrets",
+        "- Selected PR: #999",
+        "- Reviewer eligibility: passed",
+        "- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
+        "- Worktree path: /repo/branches/review-pr-999",
+        "- Worktree dirty: no",
+        "- Scratch worktree used: yes (/repo/branches/review-pr-999)",
+        "- Unrelated local mutations: none",
+        "- Review decision: approve",
+        "- Merge result: none",
+        "- Linked issue status: open",
+        "- Cleanup status: worktree removed",
+    ])
+
+    PRECISE_CATEGORIES = "\n".join([
+        "- File edits by reviewer: none",
+        "- Worktree/index mutations: created and removed review worktree",
+        "- Git ref mutations: git fetch prgs",
+        "- MCP/Gitea mutations: none",
+        "- Review mutations: one approve on #999",
+        "- Merge mutations: none",
+        "- Cleanup mutations: removed review worktree",
+        "- External-state mutations: none",
+        "- Read-only diagnostics: git log, git diff",
+    ])
+
+    def _complete_handoff(self, **overrides):
+        text = self.REVIEW_BASE + "\n" + self.PRECISE_CATEGORIES
+        for old, new in overrides.items():
+            text = text.replace(old, new)
+        return text
+
+    def test_review_handoff_with_precise_categories_is_complete(self):
+        result = assess_controller_handoff(
+            self._complete_handoff(), role="review")
+        self.assertEqual(result["verdict"], "complete")
+        self.assertFalse(result["downgraded"])
+
+    def test_review_handoff_rejects_legacy_workspace_mutations_none(self):
+        text = self._complete_handoff() + "\n- Workspace mutations: none"
+        result = assess_controller_handoff(text, role="review")
+        self.assertEqual(result["verdict"], "incomplete")
+        self.assertTrue(result["downgraded"])
+        self.assertTrue(any(
+            "workspace mutations" in reason.lower()
+            for reason in result["reasons"]
+        ))
+
+    def test_review_handoff_rejects_legacy_workspace_mutations_any_value(self):
+        text = (self._complete_handoff()
+                + "\n- Workspace mutations: edited review_proofs.py")
+        result = assess_controller_handoff(text, role="review")
+        self.assertEqual(result["verdict"], "incomplete")
+        self.assertTrue(any(
+            "workspace mutations" in reason.lower()
+            for reason in result["reasons"]
+        ))
+
+    def test_review_handoff_missing_precise_categories_is_incomplete(self):
+        result = assess_controller_handoff(self.REVIEW_BASE, role="review")
+        self.assertEqual(result["verdict"], "incomplete")
+        self.assertIn("File edits by reviewer", result["missing_fields"])
+        self.assertIn("Worktree/index mutations", result["missing_fields"])
+        self.assertIn("Read-only diagnostics", result["missing_fields"])
+
+    def test_review_handoff_does_not_require_workspace_mutations(self):
+        result = assess_controller_handoff(
+            self._complete_handoff(), role="review")
+        self.assertNotIn("Workspace mutations", result["missing_fields"])
+
+    def test_request_changes_handoff_with_precise_categories_is_complete(self):
+        text = self._complete_handoff(**{
+            "- Review decision: approve": "- Review decision: request-changes",
+            "- Review mutations: one approve on #999":
+                "- Review mutations: one request_changes on #999",
+        })
+        result = assess_controller_handoff(text, role="review")
+        self.assertEqual(result["verdict"], "complete")
+
+    def test_merge_handoff_with_precise_categories_is_complete(self):
+        text = self._complete_handoff(**{
+            "- Merge result: none": "- Merge result: merged",
+            "- Merge mutations: none": "- Merge mutations: PR #999 merged",
+        })
+        result = assess_controller_handoff(text, role="review")
+        self.assertEqual(result["verdict"], "complete")
+
+    def test_fetch_only_handoff_with_precise_categories_is_complete(self):
+        text = self._complete_handoff(**{
+            "- Review decision: approve": "- Review decision: none",
+            "- Review mutations: one approve on #999": "- Review mutations: none",
+            "- Worktree/index mutations: created and removed review worktree":
+                "- Worktree/index mutations: none",
+            "- Cleanup mutations: removed review worktree":
+                "- Cleanup mutations: none",
+            "- Mutations: one review submitted": "- Mutations: fetch only",
+        })
+        result = assess_controller_handoff(text, role="review")
+        self.assertEqual(result["verdict"], "complete")
+
+    def test_blocked_handoff_with_precise_categories_is_complete(self):
+        text = self._complete_handoff(**{
+            "- Blockers: none": "- Blockers: infra_stop (registry unreachable)",
+            "- Review decision: approve": "- Review decision: none",
+            "- Review mutations: one approve on #999": "- Review mutations: none",
+        })
+        result = assess_controller_handoff(text, role="review")
+        self.assertEqual(result["verdict"], "complete")
+
+    def test_author_role_still_requires_workspace_mutations(self):
+        # Non-review roles keep the legacy contract until their own issues
+        # migrate them.
+        author = TestControllerHandoff.BASE_HANDOFF
+        stripped = "\n".join(
+            line for line in author.splitlines()
+            if not line.startswith("- Workspace mutations:"))
+        result = assess_controller_handoff(stripped, role="author")
+        self.assertEqual(result["verdict"], "incomplete")
+        self.assertIn("Workspace mutations", result["missing_fields"])
+
 
 class TestReviewMutationFinalReport(unittest.TestCase):
     """Final reports must list exactly one live review mutation."""