From dc24fe3afec11c556ff25ad50f0a99f8f2942a7a Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 09:31:45 -0400 Subject: [PATCH] feat: enforce canonical reconciliation handoff schema (Closes #307) Reconciliation final reports must emit only the canonical reconciliation handoff schema: - Expand _RECONCILE_REQUIRED_FIELDS from 5 to the full 27-field canonical schema (Task through No review/merge confirmation), so incomplete reconciliation handoffs downgrade with per-field reasons. - Add 'issue lock proof', 'claim/comment status', and 'workspace mutations' to _RECONCILE_STALE_FIELDS so stale author/reviewer fields block. - Make the 'PR number opened' rejection conditional on a new session_pr_opened proof kwarg: the field is allowed only when a PR was actually opened in the session. - Close the git-fetch reporting gap: a fetch observed only in the action log (not report text) now requires a Git ref mutations entry as well. - Update the reconciliation handoff fixture to the canonical schema and add TestCanonicalReconcileSchema covering blocked, successful-close, and comment-only reconciliation plus missing-close-capability, stale-field, and conditional PR-number-opened paths. Co-Authored-By: Claude Opus 4.8 (1M context) --- final_report_validator.py | 46 ++++++- tests/test_final_report_validator.py | 186 ++++++++++++++++++++++++++- 2 files changed, 222 insertions(+), 10 deletions(-) diff --git a/final_report_validator.py b/final_report_validator.py index bff9467..9d3f6d7 100644 --- a/final_report_validator.py +++ b/final_report_validator.py @@ -101,11 +101,37 @@ _RECONCILE_STALE_FIELDS = ( "scratch worktree used", "review decision", "merge result", + "issue lock proof", + "claim/comment status", + "workspace mutations", ) +# Issue #307: reconciliation handoffs must carry the full canonical schema. _RECONCILE_REQUIRED_FIELDS = ( + "Task", + "Repo", + "Role/profile", + "Identity", "Selected PR", - "Eligibility class", + "PR live state", + "Candidate head SHA", + "Target branch", + "Target branch SHA", + "Ancestor proof", + "Linked issue", "Linked issue live status", + "Eligibility class", + "Capabilities proven", + "Missing capabilities", + "PR comments posted", + "Issue comments posted", + "PRs closed", + "Issues closed", + "Git ref mutations", + "Worktree mutations", + "MCP/Gitea mutations", + "External-state mutations", + "Read-only diagnostics", + "Blocker", "Safe next action", "No review/merge confirmation", ) @@ -346,13 +372,15 @@ def _rule_reviewer_git_fetch_readonly( "classify git fetch under Git ref mutations, not read-only diagnostics", ) ] - if not git_ref_reported and _GIT_FETCH_RE.search(text): + if not git_ref_reported: + # Issue #307: an observed git fetch (report text or action log) + # must be listed under Git ref mutations. return [ validator_finding( "reviewer.git_fetch_readonly", "downgrade", "Git ref mutations", - "git fetch mentioned without Git ref mutation classification", + "git fetch occurred without Git ref mutation classification", "record git fetch under Git ref mutations", ) ] @@ -541,10 +569,18 @@ def _rule_reconcile_controller_handoff(report_text: str) -> list[dict[str, str]] return [] -def _rule_reconcile_stale_author_fields(report_text: str) -> list[dict[str, str]]: +def _rule_reconcile_stale_author_fields( + report_text: str, + *, + session_pr_opened: bool = False, +) -> list[dict[str, str]]: text = (report_text or "").lower() findings = [] for field in _RECONCILE_STALE_FIELDS: + if field == "pr number opened" and session_pr_opened: + # Issue #307: allowed only when a PR was actually opened + # in this session. + continue if f"{field}:" in text: findings.append( validator_finding( @@ -803,6 +839,7 @@ def assess_final_report_validator( mutations_observed: bool = False, local_edits: bool = False, issue_filing_lock: dict | None = None, + session_pr_opened: bool = False, ) -> dict[str, Any]: """Validate final-report text against task-specific proof rules (#327). @@ -857,6 +894,7 @@ def assess_final_report_validator( "action_log": action_log, "mutations_observed": mutations_observed, "local_edits": local_edits, + "session_pr_opened": session_pr_opened, } for rule in _RULES_BY_TASK.get(normalized_kind, ()): diff --git a/tests/test_final_report_validator.py b/tests/test_final_report_validator.py index f8e8848..4c1644c 100644 --- a/tests/test_final_report_validator.py +++ b/tests/test_final_report_validator.py @@ -53,26 +53,47 @@ def _review_handoff(**overrides): return text -def _reconcile_handoff(**overrides): +def _reconcile_handoff(drop=(), **overrides): lines = [ "## Controller Handoff", "", "- Task: reconcile already-landed PR #99", "- Repo: Scaled-Tech-Consulting/Gitea-Tools", - "- Role: reconciler", - "- Identity: sysadmin / prgs-author", + "- Role/profile: reconciler / prgs-reconciler", + "- Identity: sysadmin", "- Selected PR: #99", - "- Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED", + "- PR live state: open", + "- Candidate head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", + "- Target branch: master", + "- Target branch SHA: 5e023dc71b0e2b813a0b1eee6b0f42630c47a95c", + "- Ancestor proof: candidate head is ancestor of target branch SHA", "- Linked issue: #98", "- Linked issue live status: not verified in this session", + "- Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED", + "- Capabilities proven: gitea.read, gitea.pr.comment", + "- Missing capabilities: gitea.pr.close", + "- PR comments posted: 1 reconciliation comment on PR #99", + "- Issue comments posted: none", + "- PRs closed: none", + "- Issues closed: none", "- File edits by reconciler: none", "- Git ref mutations: git fetch prgs master", + "- Worktree mutations: none", + "- MCP/Gitea mutations: PR comment posted", + "- External-state mutations: none", + "- Read-only diagnostics: gitea_view_pr, gitea_view_issue", "- Reconciliation mutations: PR comment posted", "- Current status: reconciliation complete", - "- Safe next action: none", + "- Blocker: missing gitea.pr.close capability", + "- Safe next action: hand off to a profile with gitea.pr.close", "- No review/merge confirmation: confirmed", ] - text = "\n".join(lines) + kept = [ + line + for line in lines + if not any(line.startswith(f"- {name}:") for name in drop) + ] + text = "\n".join(kept) for key, value in overrides.items(): if f"- {key}:" in text: text = text.replace(f"- {key}:", f"- {key}: {value}") @@ -251,6 +272,159 @@ class TestReconciliationRules(unittest.TestCase): ) +class TestCanonicalReconcileSchema(unittest.TestCase): + """Issue #307: reconciliation workflows emit only the canonical schema.""" + + def test_comment_only_reconciliation_passes(self): + result = assess_final_report_validator( + _reconcile_handoff(), + "reconcile_already_landed", + ) + self.assertEqual(result["grade"], "A") + self.assertEqual(result["findings"], []) + + def test_successful_close_reconciliation_passes(self): + report = _reconcile_handoff().replace( + "- Capabilities proven: gitea.read, gitea.pr.comment", + "- Capabilities proven: gitea.read, gitea.pr.comment, gitea.pr.close", + ).replace( + "- Missing capabilities: gitea.pr.close", + "- Missing capabilities: none", + ).replace( + "- PRs closed: none", + "- PRs closed: #99", + ).replace( + "- Blocker: missing gitea.pr.close capability", + "- Blocker: none", + ) + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertEqual(result["grade"], "A") + + def test_blocked_reconciliation_passes(self): + report = _reconcile_handoff().replace( + "- Capabilities proven: gitea.read, gitea.pr.comment", + "- Capabilities proven: gitea.read", + ).replace( + "- Missing capabilities: gitea.pr.close", + "- Missing capabilities: gitea.pr.close, gitea.pr.comment", + ).replace( + "- PR comments posted: 1 reconciliation comment on PR #99", + "- PR comments posted: none", + ).replace( + "- MCP/Gitea mutations: PR comment posted", + "- MCP/Gitea mutations: none", + ).replace( + "- Reconciliation mutations: PR comment posted", + "- Reconciliation mutations: none", + ) + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertEqual(result["grade"], "A") + + def test_missing_capabilities_field_required(self): + report = _reconcile_handoff(drop=("Missing capabilities",)) + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertEqual(result["grade"], "downgraded") + self.assertTrue( + any( + f["rule_id"] == "reconcile.controller_handoff" + and "Missing capabilities" in f["reason"] + for f in result["findings"] + ) + ) + + def test_missing_ancestor_proof_and_candidate_sha_downgrade(self): + report = _reconcile_handoff(drop=("Ancestor proof", "Candidate head SHA")) + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertEqual(result["grade"], "downgraded") + reasons = " ".join(f["reason"] for f in result["findings"]) + self.assertIn("Ancestor proof", reasons) + self.assertIn("Candidate head SHA", reasons) + + def test_stale_issue_lock_proof_blocks(self): + report = _reconcile_handoff() + "\n- Issue lock proof: locked before diff" + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertTrue(result["blocked"]) + self.assertTrue( + any( + f["rule_id"] == "reconcile.stale_author_reviewer_fields" + and f["field"] == "issue lock proof" + for f in result["findings"] + ) + ) + + def test_stale_claim_comment_status_blocks(self): + report = _reconcile_handoff() + "\n- Claim/comment status: claimed" + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertTrue(result["blocked"]) + self.assertTrue( + any( + f["rule_id"] == "reconcile.stale_author_reviewer_fields" + and f["field"] == "claim/comment status" + for f in result["findings"] + ) + ) + + def test_stale_workspace_mutations_blocks_without_observed_mutations(self): + report = _reconcile_handoff() + "\n- Workspace mutations: None" + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertTrue(result["blocked"]) + self.assertTrue( + any( + f["rule_id"] == "reconcile.stale_author_reviewer_fields" + and f["field"] == "workspace mutations" + for f in result["findings"] + ) + ) + + def test_pr_number_opened_allowed_when_session_opened_pr(self): + report = _reconcile_handoff() + "\n- PR number opened: #12" + result = assess_final_report_validator( + report, + "reconcile_already_landed", + session_pr_opened=True, + ) + self.assertFalse( + any( + f["rule_id"] == "reconcile.stale_author_reviewer_fields" + and f["field"] == "pr number opened" + for f in result["findings"] + ) + ) + + def test_pr_number_opened_still_blocked_without_session_proof(self): + report = _reconcile_handoff() + "\n- PR number opened: #12" + result = assess_final_report_validator( + report, + "reconcile_already_landed", + session_pr_opened=False, + ) + self.assertTrue(result["blocked"]) + self.assertTrue( + any( + f["rule_id"] == "reconcile.stale_author_reviewer_fields" + and f["field"] == "pr number opened" + for f in result["findings"] + ) + ) + + def test_action_log_fetch_requires_git_ref_mutation_entry(self): + report = _reconcile_handoff().replace( + "- Git ref mutations: git fetch prgs master", + "- Git ref mutations: none", + ) + result = assess_final_report_validator( + report, + "reconcile_already_landed", + action_log=[{"command": "git fetch prgs master", "performed": True}], + ) + self.assertTrue( + any( + f["rule_id"] == "reviewer.git_fetch_readonly" + for f in result["findings"] + ) + ) + + class TestEntryPoint(unittest.TestCase): def test_unknown_task_kind_blocks(self): result = assess_final_report_validator("report", "unknown_mode")