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:
@@ -130,6 +130,17 @@ class TestResolveTaskCapability(unittest.TestCase):
|
||||
self.assertIn("author-profile", res["matching_configured_profile"])
|
||||
self.assertIn("reviewer-profile", res["matching_configured_profile"])
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_resolve_mark_issue_author_profile_allowed(self, _auth, _api):
|
||||
# #183: mark_issue must map to gitea.issue.comment (prgs-author allowed op).
|
||||
with patch.dict(os.environ, self._env("author-profile")):
|
||||
res = mcp_server.gitea_resolve_task_capability(task="mark_issue", remote="prgs")
|
||||
self.assertEqual(res["requested_task"], "mark_issue")
|
||||
self.assertEqual(res["required_operation_permission"], "gitea.issue.comment")
|
||||
self.assertTrue(res["allowed_in_current_session"])
|
||||
self.assertIn("gitea.issue.comment", res["active_profile_allowed_operations"])
|
||||
|
||||
def test_resolve_unknown_task_fails_closed(self):
|
||||
with patch.dict(os.environ, self._env("author-profile")):
|
||||
with self.assertRaises(ValueError):
|
||||
|
||||
@@ -21,12 +21,15 @@ import unittest
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
from review_proofs import ( # noqa: E402
|
||||
assess_author_pr_report,
|
||||
assess_capability_evidence,
|
||||
assess_capability_proof,
|
||||
assess_controller_handoff,
|
||||
assess_inventory_completeness,
|
||||
assess_live_state_recheck,
|
||||
assess_review_mutation_final_report,
|
||||
assess_role_boundary,
|
||||
assess_secret_sweep,
|
||||
assess_self_review_contamination,
|
||||
assess_sweep_evidence,
|
||||
assess_validation_report,
|
||||
@@ -190,6 +193,49 @@ def _good_role_boundary_179(**overrides):
|
||||
return assess_role_boundary(**kwargs)
|
||||
|
||||
|
||||
GOOD_HANDOFF = "\n".join([
|
||||
"## Controller Handoff",
|
||||
"",
|
||||
"- Task: review PR #999",
|
||||
"- Repo: Scaled-Tech-Consulting/Gitea-Tools",
|
||||
"- Role: reviewer",
|
||||
"- Identity: sysadmin / prgs-reviewer",
|
||||
"- Issue/PR: #182 / PR #999",
|
||||
"- Branch/SHA: feat/x @ 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||
"- Files changed: review_proofs.py",
|
||||
"- Validation: 700 passed, 6 skipped",
|
||||
"- Mutations: review only",
|
||||
"- Current status: PR open",
|
||||
"- Blockers: none",
|
||||
"- Next: merge if approved",
|
||||
"- Safety: no self-review; no self-merge; no secrets",
|
||||
])
|
||||
|
||||
|
||||
def _good_handoff():
|
||||
return assess_controller_handoff(GOOD_HANDOFF)
|
||||
|
||||
|
||||
def _good_capability_proof():
|
||||
return assess_capability_proof({
|
||||
"comment_issue": {
|
||||
"requested_task": "comment_issue",
|
||||
"required_operation_permission": "gitea.issue.comment",
|
||||
"allowed_in_current_session": True,
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
def _good_secret_sweep(**overrides):
|
||||
sweep = {
|
||||
"method": "git diff prgs/master...HEAD | grep -iE 'token|secret'",
|
||||
"scope": "full PR diff relative to prgs/master",
|
||||
"clean": True,
|
||||
}
|
||||
sweep.update(overrides)
|
||||
return assess_secret_sweep(sweep)
|
||||
|
||||
|
||||
class TestCheckoutProof(unittest.TestCase):
|
||||
"""Required behavior 1 + 2: prove HEAD == pinned PR head or stop."""
|
||||
|
||||
@@ -545,6 +591,9 @@ class TestFinalReport(unittest.TestCase):
|
||||
"live_state": _good_live_state(),
|
||||
"role_boundary": _good_role_boundary(),
|
||||
"review_mutation": _good_review_mutation(),
|
||||
"controller_handoff": _good_handoff(),
|
||||
"capability_proof": _good_capability_proof(),
|
||||
"sweep_proof": _good_secret_sweep(),
|
||||
}
|
||||
kwargs.update(overrides)
|
||||
return build_final_report(**kwargs)
|
||||
@@ -662,6 +711,28 @@ class TestFinalReport(unittest.TestCase):
|
||||
self.assertEqual(report["grade"], "blocked")
|
||||
self.assertFalse(report["merge_allowed"])
|
||||
|
||||
def test_failed_capability_proof_downgrades(self):
|
||||
report = self._report(
|
||||
capability_proof={
|
||||
"proven": False,
|
||||
"reasons": ["task mark_issue not allowed"],
|
||||
}
|
||||
)
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertTrue(
|
||||
any("capability" in r for r in report["downgrade_reasons"])
|
||||
)
|
||||
|
||||
def test_failed_sweep_proof_downgrades(self):
|
||||
report = self._report(
|
||||
sweep_proof={
|
||||
"proven": False,
|
||||
"reasons": ["method missing"],
|
||||
}
|
||||
)
|
||||
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
|
||||
@@ -1231,6 +1302,9 @@ class TestFinalReport179Bar(unittest.TestCase):
|
||||
"live_state": _good_live_state(),
|
||||
"role_boundary": _good_role_boundary(),
|
||||
"review_mutation": _good_review_mutation(),
|
||||
"controller_handoff": _good_handoff(),
|
||||
"capability_proof": _good_capability_proof(),
|
||||
"sweep_proof": _good_secret_sweep(),
|
||||
}
|
||||
kwargs.update(overrides)
|
||||
return build_final_report(**kwargs)
|
||||
@@ -1306,5 +1380,74 @@ class TestFinalReport179Bar(unittest.TestCase):
|
||||
self.assertFalse(report["review_mutation_complete"])
|
||||
|
||||
|
||||
class TestAuthorReporting(unittest.TestCase):
|
||||
"""Harness assertions for author reporting and capability/sweep proofs."""
|
||||
|
||||
def test_good_capability_proof_passes(self):
|
||||
result = _good_capability_proof()
|
||||
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):
|
||||
result = assess_capability_proof({
|
||||
"mark_issue": {
|
||||
"requested_task": "unknown_task",
|
||||
"required_operation_permission": None,
|
||||
"allowed_in_current_session": None,
|
||||
}
|
||||
})
|
||||
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):
|
||||
result = _good_secret_sweep()
|
||||
self.assertTrue(result["proven"])
|
||||
self.assertEqual(result["verdict"], "strong")
|
||||
|
||||
def test_vague_secret_sweep_without_method_is_weak(self):
|
||||
result = _good_secret_sweep(method="")
|
||||
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_weak(self):
|
||||
result = _good_secret_sweep(clean=False)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertEqual(result["verdict"], "weak")
|
||||
|
||||
def test_good_author_pr_report_passes(self):
|
||||
result = assess_author_pr_report({
|
||||
"pr_number": 185,
|
||||
"branch": "feat/issue-184-repo-name-disambiguation",
|
||||
"head_sha": "e2bccbafeeb93124ba068bfb06058d5aa7467cae",
|
||||
})
|
||||
self.assertTrue(result["complete"])
|
||||
|
||||
def test_author_pr_report_without_head_sha_is_incomplete(self):
|
||||
result = assess_author_pr_report({
|
||||
"pr_number": 185,
|
||||
"branch": "feat/issue-184-repo-name-disambiguation",
|
||||
"head_sha": "",
|
||||
})
|
||||
self.assertFalse(result["complete"])
|
||||
self.assertTrue(any("head SHA" in r for r in result["reasons"]))
|
||||
|
||||
def test_author_pr_report_rejects_short_sha(self):
|
||||
result = assess_author_pr_report({
|
||||
"pr_number": 185,
|
||||
"branch": "feat/issue-184-repo-name-disambiguation",
|
||||
"head_sha": "e2bccba",
|
||||
})
|
||||
self.assertFalse(result["complete"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user