feat: reject legacy Workspace mutations field in reviewer handoffs (Closes #320)

Reviewer controller handoffs must now report the precise mutation
categories (File edits by reviewer, Worktree/index mutations, Git ref
mutations, MCP/Gitea mutations, Review mutations, Merge mutations,
Cleanup mutations, External-state mutations, Read-only diagnostics)
instead of the ambiguous legacy "Workspace mutations" field.

- assess_controller_handoff(role="review") no longer requires
  "Workspace mutations" and fails closed when any legacy
  "Workspace mutations:" line is present, regardless of value.
- New HANDOFF_REVIEW_MUTATION_FIELDS makes the nine precise categories
  required fields for review-role handoffs, matching the canonical
  schemas/review-merge-final-report.md field set.
- Tests cover approval, request-changes, merge, worktree cleanup,
  fetch-only, and blocked handoffs, plus rejection of
  "Workspace mutations: none" and non-none values.
- Author/create-issue/inventory roles keep their existing contracts.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
2026-07-07 05:11:05 -04:00
co-authored by Claude Opus 4.8
parent f92d1a29c2
commit c2bba27b86
2 changed files with 188 additions and 3 deletions
+34 -1
View File
@@ -1593,6 +1593,20 @@ HANDOFF_BASE_FIELDS = (
("Safety", ("safety",)), ("Safety", ("safety",)),
) )
# Issue #320: reviewer handoffs replace the legacy ambiguous
# "Workspace mutations" field with these precise mutation categories.
HANDOFF_REVIEW_MUTATION_FIELDS = (
("File edits by reviewer", ("file edits by reviewer",)),
("Worktree/index mutations", ("worktree/index mutations",)),
("Git ref mutations", ("git ref mutations",)),
("MCP/Gitea mutations", ("mcp/gitea mutations",)),
("Review mutations", ("review mutations",)),
("Merge mutations", ("merge mutations",)),
("Cleanup mutations", ("cleanup mutations",)),
("External-state mutations", ("external-state mutations",)),
("Read-only diagnostics", ("read-only diagnostics",)),
)
HANDOFF_ROLE_FIELDS = { HANDOFF_ROLE_FIELDS = {
"review": ( "review": (
("Selected PR", ("selected pr",)), ("Selected PR", ("selected pr",)),
@@ -1608,7 +1622,7 @@ HANDOFF_ROLE_FIELDS = {
("Merge result", ("merge result",)), ("Merge result", ("merge result",)),
("Linked issue status", ("linked issue status", "linked issue")), ("Linked issue status", ("linked issue status", "linked issue")),
("Cleanup status", ("cleanup status", "cleanup")), ("Cleanup status", ("cleanup status", "cleanup")),
), ) + HANDOFF_REVIEW_MUTATION_FIELDS,
"author": ( "author": (
("Selected issue", ("selected issue",)), ("Selected issue", ("selected issue",)),
("Issue lock proof", ("issue lock proof", "lock before diff")), ("Issue lock proof", ("issue lock proof", "lock before diff")),
@@ -1759,6 +1773,25 @@ def assess_controller_handoff(report_text, role=None, local_edits=False):
field for field in required field for field in required
if field[0] not in ("Workspace mutations", "Mutations") if field[0] not in ("Workspace mutations", "Mutations")
] ]
if role == "review":
# Issue #320: reviewer handoffs use the precise mutation categories
# in HANDOFF_REVIEW_MUTATION_FIELDS instead of the legacy ambiguous
# "Workspace mutations" field, which is rejected below.
required = [
field for field in required
if field[0] != "Workspace mutations"
]
if any(label.startswith("workspace mutations") for label in labels):
return {
"verdict": "incomplete",
"downgraded": True,
"missing_fields": [],
"reasons": [
"review handoff must not include legacy "
"'Workspace mutations' field; report the precise "
"mutation categories instead (issue #320)"
],
}
required.extend(HANDOFF_ROLE_FIELDS.get(role or "", ())) required.extend(HANDOFF_ROLE_FIELDS.get(role or "", ()))
missing = [] missing = []
+154 -2
View File
@@ -939,13 +939,18 @@ class TestControllerHandoff(unittest.TestCase):
self.assertIn("Safety", result["missing_fields"]) self.assertIn("Safety", result["missing_fields"])
def test_review_role_requires_review_fields(self): 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.assertEqual(result["verdict"], "incomplete")
self.assertIn("Pinned reviewed head", result["missing_fields"]) self.assertIn("Pinned reviewed head", result["missing_fields"])
self.assertIn("Worktree path", result["missing_fields"]) self.assertIn("Worktree path", result["missing_fields"])
self.assertIn("Merge result", 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", "- Selected PR: #999",
"- Reviewer eligibility: passed", "- Reviewer eligibility: passed",
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", "- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
@@ -957,6 +962,15 @@ class TestControllerHandoff(unittest.TestCase):
"- Merge result: merged", "- Merge result: merged",
"- Linked issue status: closed", "- Linked issue status: closed",
"- Cleanup status: branch deleted", "- 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") result = assess_controller_handoff(complete, role="review")
self.assertEqual(result["verdict"], "complete") self.assertEqual(result["verdict"], "complete")
@@ -1079,6 +1093,144 @@ class TestControllerHandoff(unittest.TestCase):
self.assertEqual(res2["verdict"], "complete") 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): class TestReviewMutationFinalReport(unittest.TestCase):
"""Final reports must list exactly one live review mutation.""" """Final reports must list exactly one live review mutation."""