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) <[email protected]>
This commit is contained in:
@@ -101,11 +101,37 @@ _RECONCILE_STALE_FIELDS = (
|
|||||||
"scratch worktree used",
|
"scratch worktree used",
|
||||||
"review decision",
|
"review decision",
|
||||||
"merge result",
|
"merge result",
|
||||||
|
"issue lock proof",
|
||||||
|
"claim/comment status",
|
||||||
|
"workspace mutations",
|
||||||
)
|
)
|
||||||
|
# Issue #307: reconciliation handoffs must carry the full canonical schema.
|
||||||
_RECONCILE_REQUIRED_FIELDS = (
|
_RECONCILE_REQUIRED_FIELDS = (
|
||||||
|
"Task",
|
||||||
|
"Repo",
|
||||||
|
"Role/profile",
|
||||||
|
"Identity",
|
||||||
"Selected PR",
|
"Selected PR",
|
||||||
"Eligibility class",
|
"PR live state",
|
||||||
|
"Candidate head SHA",
|
||||||
|
"Target branch",
|
||||||
|
"Target branch SHA",
|
||||||
|
"Ancestor proof",
|
||||||
|
"Linked issue",
|
||||||
"Linked issue live status",
|
"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",
|
"Safe next action",
|
||||||
"No review/merge confirmation",
|
"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",
|
"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 [
|
return [
|
||||||
validator_finding(
|
validator_finding(
|
||||||
"reviewer.git_fetch_readonly",
|
"reviewer.git_fetch_readonly",
|
||||||
"downgrade",
|
"downgrade",
|
||||||
"Git ref mutations",
|
"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",
|
"record git fetch under Git ref mutations",
|
||||||
)
|
)
|
||||||
]
|
]
|
||||||
@@ -541,10 +569,18 @@ def _rule_reconcile_controller_handoff(report_text: str) -> list[dict[str, str]]
|
|||||||
return []
|
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()
|
text = (report_text or "").lower()
|
||||||
findings = []
|
findings = []
|
||||||
for field in _RECONCILE_STALE_FIELDS:
|
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:
|
if f"{field}:" in text:
|
||||||
findings.append(
|
findings.append(
|
||||||
validator_finding(
|
validator_finding(
|
||||||
@@ -803,6 +839,7 @@ def assess_final_report_validator(
|
|||||||
mutations_observed: bool = False,
|
mutations_observed: bool = False,
|
||||||
local_edits: bool = False,
|
local_edits: bool = False,
|
||||||
issue_filing_lock: dict | None = None,
|
issue_filing_lock: dict | None = None,
|
||||||
|
session_pr_opened: bool = False,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Validate final-report text against task-specific proof rules (#327).
|
"""Validate final-report text against task-specific proof rules (#327).
|
||||||
|
|
||||||
@@ -857,6 +894,7 @@ def assess_final_report_validator(
|
|||||||
"action_log": action_log,
|
"action_log": action_log,
|
||||||
"mutations_observed": mutations_observed,
|
"mutations_observed": mutations_observed,
|
||||||
"local_edits": local_edits,
|
"local_edits": local_edits,
|
||||||
|
"session_pr_opened": session_pr_opened,
|
||||||
}
|
}
|
||||||
|
|
||||||
for rule in _RULES_BY_TASK.get(normalized_kind, ()):
|
for rule in _RULES_BY_TASK.get(normalized_kind, ()):
|
||||||
|
|||||||
@@ -53,26 +53,47 @@ def _review_handoff(**overrides):
|
|||||||
return text
|
return text
|
||||||
|
|
||||||
|
|
||||||
def _reconcile_handoff(**overrides):
|
def _reconcile_handoff(drop=(), **overrides):
|
||||||
lines = [
|
lines = [
|
||||||
"## Controller Handoff",
|
"## Controller Handoff",
|
||||||
"",
|
"",
|
||||||
"- Task: reconcile already-landed PR #99",
|
"- Task: reconcile already-landed PR #99",
|
||||||
"- Repo: Scaled-Tech-Consulting/Gitea-Tools",
|
"- Repo: Scaled-Tech-Consulting/Gitea-Tools",
|
||||||
"- Role: reconciler",
|
"- Role/profile: reconciler / prgs-reconciler",
|
||||||
"- Identity: sysadmin / prgs-author",
|
"- Identity: sysadmin",
|
||||||
"- Selected PR: #99",
|
"- 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: #98",
|
||||||
"- Linked issue live status: not verified in this session",
|
"- 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",
|
"- File edits by reconciler: none",
|
||||||
"- Git ref mutations: git fetch prgs master",
|
"- 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",
|
"- Reconciliation mutations: PR comment posted",
|
||||||
"- Current status: reconciliation complete",
|
"- 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",
|
"- 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():
|
for key, value in overrides.items():
|
||||||
if f"- {key}:" in text:
|
if f"- {key}:" in text:
|
||||||
text = text.replace(f"- {key}:", f"- {key}: {value}")
|
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):
|
class TestEntryPoint(unittest.TestCase):
|
||||||
def test_unknown_task_kind_blocks(self):
|
def test_unknown_task_kind_blocks(self):
|
||||||
result = assess_final_report_validator("report", "unknown_mode")
|
result = assess_final_report_validator("report", "unknown_mode")
|
||||||
|
|||||||
Reference in New Issue
Block a user