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

2026-07-07T04:14:54-05:00

jcwalker3 commented

2026-07-07 04:14:54 -05:00

Summary

Closes #320.

Reviewer controller handoffs no longer use the ambiguous legacy Workspace mutations field. The assess_controller_handoff validator now enforces the precise mutation categories that schemas/review-merge-final-report.md already documents.

Changes

review_proofs.py
- New HANDOFF_REVIEW_MUTATION_FIELDS constant with the nine precise 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. These are now required fields for role="review" handoffs.
- assess_controller_handoff(role="review") drops the Workspace mutations base-field requirement and fails closed (verdict incomplete, downgraded) when any Workspace mutations: line is present — including Workspace mutations: none — with an actionable reason pointing at the precise categories.
- Author, create-issue, issue-filing, inventory, and continuation roles keep their existing contracts unchanged.
tests/test_review_proofs.py
- New TestReviewHandoffPreciseMutationCategories (10 tests): complete handoffs for approval, request-changes, merge, worktree-cleanup, fetch-only, and blocked scenarios; rejection of Workspace mutations: none and of non-none values; missing precise categories listed in missing_fields; Workspace mutations never reported missing for review role; author role still requires the legacy field.
- Updated test_review_role_requires_review_fields to the new contract (legacy line stripped, precise categories included in the complete fixture).

Acceptance criteria mapping (#320)

Reviewer handoffs do not include Workspace mutations — presence is rejected fail-closed.
Final-report validator rejects Workspace mutations: none.
Reports distinguish no file edits (File edits by reviewer: none) from worktree/index mutations (separate required field).
Tests cover request-changes, approval, merge, worktree cleanup, fetch-only, and blocked handoffs.

Validation

python -m pytest tests/ in the issue worktree: 1120 passed, 1 failed, 6 skipped, 20 subtests passed.

The single failure (tests/test_agent_temp_artifacts.py::TestIssueLockArtifactWarning::test_lock_success_includes_artifact_warning) is pre-existing: it fails identically on a clean prgs/master checkout (f92d1a2) with the change stashed, so this PR does not cause it.

Docs were already aligned (workflows/review-merge-pr.md §31 and schemas/review-merge-final-report.md forbid the legacy field); the validator was the missing enforcement.

Note: #319 requests this behavior too, with a subset of these acceptance criteria; this change appears to satisfy it as well.

🤖 Generated with Claude Code

## Summary Closes #320. Reviewer controller handoffs no longer use the ambiguous legacy `Workspace mutations` field. The `assess_controller_handoff` validator now enforces the precise mutation categories that `schemas/review-merge-final-report.md` already documents. ## Changes - `review_proofs.py` - New `HANDOFF_REVIEW_MUTATION_FIELDS` constant with the nine precise 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. These are now required fields for `role="review"` handoffs. - `assess_controller_handoff(role="review")` drops the `Workspace mutations` base-field requirement and fails closed (verdict `incomplete`, downgraded) when any `Workspace mutations:` line is present — including `Workspace mutations: none` — with an actionable reason pointing at the precise categories. - Author, create-issue, issue-filing, inventory, and continuation roles keep their existing contracts unchanged. - `tests/test_review_proofs.py` - New `TestReviewHandoffPreciseMutationCategories` (10 tests): complete handoffs for approval, request-changes, merge, worktree-cleanup, fetch-only, and blocked scenarios; rejection of `Workspace mutations: none` and of non-none values; missing precise categories listed in `missing_fields`; `Workspace mutations` never reported missing for review role; author role still requires the legacy field. - Updated `test_review_role_requires_review_fields` to the new contract (legacy line stripped, precise categories included in the complete fixture). ## Acceptance criteria mapping (#320) - [x] Reviewer handoffs do not include `Workspace mutations` — presence is rejected fail-closed. - [x] Final-report validator rejects `Workspace mutations: none`. - [x] Reports distinguish no file edits (`File edits by reviewer: none`) from worktree/index mutations (separate required field). - [x] Tests cover request-changes, approval, merge, worktree cleanup, fetch-only, and blocked handoffs. ## Validation `python -m pytest tests/` in the issue worktree: **1120 passed, 1 failed, 6 skipped, 20 subtests passed**. The single failure (`tests/test_agent_temp_artifacts.py::TestIssueLockArtifactWarning::test_lock_success_includes_artifact_warning`) is pre-existing: it fails identically on a clean `prgs/master` checkout (f92d1a2) with the change stashed, so this PR does not cause it. Docs were already aligned (`workflows/review-merge-pr.md` §31 and `schemas/review-merge-final-report.md` forbid the legacy field); the validator was the missing enforcement. Note: #319 requests this behavior too, with a subset of these acceptance criteria; this change appears to satisfy it as well. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jcwalker3 added 1 commit 2026-07-07 04:14:54 -05:00

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

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]>

jcwalker3 referenced this pull request

2026-07-07 04:15:08 -05:00

Reviewer controller handoffs must remove legacy Workspace mutations field #320

sysadmin approved these changes 2026-07-07 04:33:20 -05:00

sysadmin left a comment

All tests pass successfully. Reviewed the implementation in review_proofs.py and test coverage in tests/test_review_proofs.py. It correctly rejects the legacy Workspace mutations field in reviewer handoffs, enforcing precise mutation categories instead, matching the acceptance criteria of Issue #320. Tested inside the review worktree, and the single failing test is pre-existing on master.

sysadmin merged commit e1dabd1b0f into master

2026-07-07 04:33:32 -05:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#357