feat(author): harden reporting and claim capability per #183
- Add 'mark_issue' to TASK_MAP in resolve_task_capability for exact mutation proof (closes gap on unknown treated as allowed). - Add assess_controller_handoff and integrate into build_final_report (downgrades missing handoff). - Add tests for handoff and update existing. - Update SKILL.md and review-pr.md to mandate exact 'Controller Handoff' section, exact sweep evidence, PR head SHA in reports. - Author profile used throughout; no review/merge. Refs #183
This commit is contained in:
@@ -3337,6 +3337,10 @@ def gitea_resolve_task_capability(
|
||||
"permission": "gitea.issue.write",
|
||||
"role": "author",
|
||||
},
|
||||
"mark_issue": {
|
||||
"permission": "gitea.issue.write",
|
||||
"role": "author",
|
||||
},
|
||||
"create_branch": {
|
||||
"permission": "gitea.branch.create",
|
||||
"role": "author",
|
||||
|
||||
+134
-3
@@ -202,13 +202,15 @@ def assess_validation_report(report):
|
||||
|
||||
*report* keys: ``command``, ``output_read``, ``result`` ('pass'/'fail'),
|
||||
``passed``/``failed``/``skipped`` counts, ``ignored_paths`` (each with a
|
||||
``justification``), ``canonical_command``, ``deviation_justification``.
|
||||
``justification``), ``canonical_command``, ``deviation_justification``,
|
||||
``is_stdout_capture_fix``, ``normal_pytest_summary``.
|
||||
|
||||
Verdicts:
|
||||
- 'invalid' — the result may not be claimed at all (no command stated,
|
||||
or the command output was never read).
|
||||
- 'weak' — claimable but downgraded (missing counts, unjustified
|
||||
ignored paths, unexplained deviation from the canonical command).
|
||||
ignored paths, unexplained deviation from the canonical command, or
|
||||
missing normal summary on stdout capture fix).
|
||||
- 'strong' — full evidence.
|
||||
"""
|
||||
reasons = []
|
||||
@@ -252,6 +254,14 @@ def assess_validation_report(report):
|
||||
"command and the deviation is not justified"
|
||||
)
|
||||
|
||||
# For stdout-capture fixes (e.g. Issue #178)
|
||||
if report.get("is_stdout_capture_fix") is True:
|
||||
if not report.get("normal_pytest_summary"):
|
||||
reasons.append(
|
||||
"stdout-capture fix validation is missing normal pytest summary output "
|
||||
"or still requires junitxml workaround"
|
||||
)
|
||||
|
||||
verdict = "strong" if not reasons else "weak"
|
||||
return {"verdict": verdict, "claimable": True, "reasons": reasons}
|
||||
|
||||
@@ -332,7 +342,8 @@ def assess_self_review_contamination(reviewer_identity, pr_author,
|
||||
|
||||
def build_final_report(checkout_proof, inventory, validation, contamination,
|
||||
identity_eligible, merge_performed,
|
||||
issue_status_verified):
|
||||
issue_status_verified, controller_handoff=None,
|
||||
capability_proof=None, sweep_proof=None):
|
||||
"""Required behavior 6 + acceptance criteria: one report, distinct proofs.
|
||||
|
||||
Combines the individual proof verdicts into the final-report fields the
|
||||
@@ -370,6 +381,19 @@ def build_final_report(checkout_proof, inventory, validation, contamination,
|
||||
if not issue_status_verified:
|
||||
downgrade_reasons.append("linked issue status not verified")
|
||||
|
||||
handoff = assess_controller_handoff(controller_handoff or "")
|
||||
if not handoff.get("present"):
|
||||
downgrade_reasons.append("Controller Handoff section missing or incomplete")
|
||||
downgrade_reasons.extend(handoff.get("reasons", []))
|
||||
|
||||
if capability_proof and not capability_proof.get("proven"):
|
||||
downgrade_reasons.append("capability proof verification failed")
|
||||
downgrade_reasons.extend(capability_proof.get("reasons", []))
|
||||
|
||||
if sweep_proof and not sweep_proof.get("proven"):
|
||||
downgrade_reasons.append("secret/provenance sweep proof verification failed")
|
||||
downgrade_reasons.extend(sweep_proof.get("reasons", []))
|
||||
|
||||
merge_allowed = (
|
||||
identity_eligible
|
||||
and checkout_proven
|
||||
@@ -408,4 +432,111 @@ def build_final_report(checkout_proof, inventory, validation, contamination,
|
||||
"merge_allowed": merge_allowed,
|
||||
"merge_performed": bool(merge_performed),
|
||||
"issue_status_verified": bool(issue_status_verified),
|
||||
"controller_handoff_present": handoff.get("present", False),
|
||||
}
|
||||
|
||||
|
||||
def assess_controller_handoff(report_text: str) -> dict:
|
||||
"""Required for author and reviewer final reports (#183).
|
||||
|
||||
The report must contain an exactly titled 'Controller Handoff' section
|
||||
(compact format preferred). Missing it downgrades the report.
|
||||
"""
|
||||
if not report_text:
|
||||
return {"present": False, "reasons": ["no report text"]}
|
||||
|
||||
text = str(report_text)
|
||||
if "Controller Handoff" not in text:
|
||||
return {
|
||||
"present": False,
|
||||
"reasons": [
|
||||
"final report missing exactly-titled 'Controller Handoff' section"
|
||||
],
|
||||
}
|
||||
return {"present": True, "reasons": []}
|
||||
|
||||
|
||||
def assess_capability_proof(resolved_capabilities: dict) -> dict:
|
||||
"""Required behavior: every mutation must have exact capability proof.
|
||||
|
||||
If a mutation task is unknown, unresolved, or lacks explicit resolver evidence,
|
||||
the workflow must fail closed / be downgraded.
|
||||
"""
|
||||
reasons = []
|
||||
if not resolved_capabilities:
|
||||
return {
|
||||
"proven": False,
|
||||
"reasons": ["no capability proof resolved; fail closed"],
|
||||
}
|
||||
for task, cap in resolved_capabilities.items():
|
||||
allowed = cap.get("allowed_in_current_session")
|
||||
op = cap.get("required_operation_permission")
|
||||
if not allowed:
|
||||
reasons.append(f"task '{task}' requires permission '{op}' which is not allowed in current session")
|
||||
# If the resolved result indicates an unknown task or missing flag
|
||||
if "unknown" in str(cap.get("requested_task", "")).lower() or cap.get("allowed_in_current_session") is None:
|
||||
reasons.append(f"task '{task}' could not be resolved to a known capability")
|
||||
proven = not reasons
|
||||
return {"proven": proven, "reasons": reasons}
|
||||
|
||||
|
||||
def assess_secret_sweep(sweep_report: dict) -> dict:
|
||||
"""Required behavior: secret/provenance sweeps must state exact command/method and scope.
|
||||
|
||||
*sweep_report* keys: ``method``, ``scope``, ``clean`` (bool).
|
||||
Returns dict with ``proven`` (bool), ``verdict`` ('strong', 'weak', 'invalid'), and ``reasons`` list.
|
||||
"""
|
||||
if not sweep_report:
|
||||
return {
|
||||
"proven": False,
|
||||
"verdict": "invalid",
|
||||
"reasons": ["no secret/provenance sweep report provided"],
|
||||
}
|
||||
reasons = []
|
||||
method = (sweep_report.get("method") or "").strip()
|
||||
scope = (sweep_report.get("scope") or "").strip()
|
||||
if not method:
|
||||
reasons.append("secret sweep report missing exact scan command, pattern, or method")
|
||||
if not scope:
|
||||
reasons.append("secret sweep report missing scanned scope")
|
||||
if sweep_report.get("clean") is not True:
|
||||
reasons.append("secret sweep report did not confirm diff is clean")
|
||||
|
||||
verdict = "weak" if reasons else "strong"
|
||||
proven = not reasons and sweep_report.get("clean") is True
|
||||
return {
|
||||
"proven": proven,
|
||||
"verdict": verdict,
|
||||
"reasons": reasons,
|
||||
}
|
||||
|
||||
|
||||
def assess_author_pr_report(pr_report: dict) -> dict:
|
||||
"""Required behavior: author PR creation report must include PR number, branch, and exact head SHA.
|
||||
|
||||
*pr_report* keys: ``pr_number`` (int), ``branch`` (str), ``head_sha`` (str).
|
||||
"""
|
||||
if not pr_report:
|
||||
return {
|
||||
"complete": False,
|
||||
"reasons": ["no PR report provided"],
|
||||
}
|
||||
reasons = []
|
||||
pr_number = pr_report.get("pr_number")
|
||||
branch = (pr_report.get("branch") or "").strip()
|
||||
head_sha = (pr_report.get("head_sha") or "").strip()
|
||||
|
||||
if not isinstance(pr_number, int) or pr_number <= 0:
|
||||
reasons.append("PR number is missing or invalid")
|
||||
if not branch:
|
||||
reasons.append("PR branch name is missing")
|
||||
if not head_sha:
|
||||
reasons.append("PR head SHA is missing")
|
||||
elif not _FULL_SHA.match(head_sha):
|
||||
reasons.append("PR head SHA is not a full 40-hex commit SHA")
|
||||
|
||||
complete = not reasons
|
||||
return {
|
||||
"complete": complete,
|
||||
"reasons": reasons,
|
||||
}
|
||||
|
||||
@@ -287,11 +287,13 @@ Ready-to-copy templates live in [`templates/`](templates/):
|
||||
|
||||
## K. Controller Handoff (required, every task)
|
||||
|
||||
Every LLM task **must end with a `Controller Handoff`** — whether the
|
||||
Every LLM task **must end with a `Controller Handoff`** (exact title) — whether the
|
||||
task was implementation, review, merge, issue triage, documentation,
|
||||
discussion-only, or blocked planning. It lets a controller LLM understand the
|
||||
current state immediately, without rereading the conversation.
|
||||
|
||||
The section title must be exactly "Controller Handoff" (or "Controller Handoff Summary" for long form). Reports without it are downgraded (see review_proofs.assess_controller_handoff).
|
||||
|
||||
**The compact format is the default.** It is written for controller-LLM
|
||||
readability, not as a full human status report. PR bodies still carry the
|
||||
full review detail — the handoff never replaces PR documentation.
|
||||
|
||||
@@ -21,6 +21,7 @@ import unittest
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
from review_proofs import ( # noqa: E402
|
||||
assess_controller_handoff,
|
||||
assess_inventory_completeness,
|
||||
assess_self_review_contamination,
|
||||
assess_validation_report,
|
||||
@@ -314,6 +315,21 @@ class TestValidationReporting(unittest.TestCase):
|
||||
)
|
||||
self.assertEqual(result["verdict"], "strong")
|
||||
|
||||
def test_stdout_capture_fix_without_summary_is_weak(self):
|
||||
result = _good_validation(
|
||||
is_stdout_capture_fix=True,
|
||||
normal_pytest_summary=False
|
||||
)
|
||||
self.assertEqual(result["verdict"], "weak")
|
||||
self.assertTrue(any("stdout-capture" in r for r in result["reasons"]))
|
||||
|
||||
def test_stdout_capture_fix_with_summary_is_strong(self):
|
||||
result = _good_validation(
|
||||
is_stdout_capture_fix=True,
|
||||
normal_pytest_summary=True
|
||||
)
|
||||
self.assertEqual(result["verdict"], "strong")
|
||||
|
||||
|
||||
class TestSelfReviewContamination(unittest.TestCase):
|
||||
"""Required behavior 5: contamination claims need evidence."""
|
||||
@@ -380,6 +396,7 @@ class TestFinalReport(unittest.TestCase):
|
||||
"identity_eligible": True,
|
||||
"merge_performed": False,
|
||||
"issue_status_verified": True,
|
||||
"controller_handoff": "Controller Handoff\n- Task: test",
|
||||
}
|
||||
kwargs.update(overrides)
|
||||
return build_final_report(**kwargs)
|
||||
@@ -464,6 +481,24 @@ class TestFinalReport(unittest.TestCase):
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertFalse(report["merge_allowed"])
|
||||
|
||||
def test_failed_capability_proof_downgrades(self):
|
||||
cap_proof = {
|
||||
"proven": False,
|
||||
"reasons": ["task mark_issue not allowed"]
|
||||
}
|
||||
report = self._report(capability_proof=cap_proof)
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertTrue(any("capability" in r for r in report["downgrade_reasons"]))
|
||||
|
||||
def test_failed_sweep_proof_downgrades(self):
|
||||
sweep_proof = {
|
||||
"proven": False,
|
||||
"reasons": ["method missing"]
|
||||
}
|
||||
report = self._report(sweep_proof=sweep_proof)
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertTrue(any("sweep" in r for r in report["downgrade_reasons"]))
|
||||
|
||||
|
||||
class TestStdoutIsolation(unittest.TestCase):
|
||||
"""Regression test for #178: tests must not close or corrupt stdout/stderr
|
||||
@@ -576,5 +611,105 @@ class TestRepoNameDisambiguation(unittest.TestCase):
|
||||
self.assertTrue(result["can_claim_exhaustive"])
|
||||
|
||||
|
||||
class TestControllerHandoff(unittest.TestCase):
|
||||
"""#183: every final report must contain the Controller Handoff section."""
|
||||
|
||||
def test_report_with_handoff_passes(self):
|
||||
report = "some details\n\nController Handoff\n- Task: foo"
|
||||
result = assess_controller_handoff(report)
|
||||
self.assertTrue(result["present"])
|
||||
|
||||
def test_report_without_handoff_downgrades(self):
|
||||
report = "long details without the section"
|
||||
result = assess_controller_handoff(report)
|
||||
self.assertFalse(result["present"])
|
||||
self.assertTrue(any("Controller Handoff" in r for r in result["reasons"]))
|
||||
|
||||
|
||||
class TestAuthorReporting(unittest.TestCase):
|
||||
"""Harness assertions for author reporting and capability/sweep proofs (#183)."""
|
||||
|
||||
def test_good_capability_proof_passes(self):
|
||||
resolved = {
|
||||
"mark_issue": {
|
||||
"requested_task": "mark_issue",
|
||||
"required_operation_permission": "gitea.issue.write",
|
||||
"allowed_in_current_session": True,
|
||||
}
|
||||
}
|
||||
result = assess_capability_proof(resolved)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_missing_capability_proof_fails_closed(self):
|
||||
result = assess_capability_proof({})
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(any("fail closed" in r for r in result["reasons"]))
|
||||
|
||||
def test_unresolved_capability_proof_fails_closed(self):
|
||||
# unknown task resolving to None/unknown requested task
|
||||
resolved = {
|
||||
"mark_issue": {
|
||||
"requested_task": "unknown_task",
|
||||
"required_operation_permission": None,
|
||||
"allowed_in_current_session": None,
|
||||
}
|
||||
}
|
||||
result = assess_capability_proof(resolved)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(any("could not be resolved" in r for r in result["reasons"]))
|
||||
|
||||
def test_good_secret_sweep_passes(self):
|
||||
sweep = {
|
||||
"method": "git diff | grep -iE 'token|secret'",
|
||||
"scope": "staged diff relative to master",
|
||||
"clean": True,
|
||||
}
|
||||
result = assess_secret_sweep(sweep)
|
||||
self.assertTrue(result["proven"])
|
||||
self.assertEqual(result["verdict"], "strong")
|
||||
|
||||
def test_vague_secret_sweep_without_method_is_weak(self):
|
||||
sweep = {
|
||||
"method": "",
|
||||
"scope": "staged diff",
|
||||
"clean": True,
|
||||
}
|
||||
result = assess_secret_sweep(sweep)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertEqual(result["verdict"], "weak")
|
||||
self.assertTrue(any("method" in r or "scan" in r for r in result["reasons"]))
|
||||
|
||||
def test_unconfirmed_secret_sweep_is_invalid(self):
|
||||
sweep = {
|
||||
"method": "git diff | grep",
|
||||
"scope": "staged diff",
|
||||
"clean": False,
|
||||
}
|
||||
result = assess_secret_sweep(sweep)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertEqual(result["verdict"], "weak")
|
||||
|
||||
def test_good_author_pr_report_passes(self):
|
||||
pr = {
|
||||
"pr_number": 185,
|
||||
"branch": "feat/issue-184-repo-name-disambiguation",
|
||||
"head_sha": "e2bccbafeeb93124ba068bfb06058d5aa7467cae",
|
||||
}
|
||||
result = assess_author_pr_report(pr)
|
||||
self.assertTrue(result["complete"])
|
||||
|
||||
def test_incomplete_author_pr_report_fails(self):
|
||||
pr = {
|
||||
"pr_number": 0,
|
||||
"branch": "",
|
||||
"head_sha": "e2bccba",
|
||||
}
|
||||
result = assess_author_pr_report(pr)
|
||||
self.assertFalse(result["complete"])
|
||||
self.assertTrue(any("number" in r for r in result["reasons"]))
|
||||
self.assertTrue(any("branch" in r for r in result["reasons"]))
|
||||
self.assertTrue(any("SHA" in r for r in result["reasons"]))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user