fix(capability-map): restore reviewer role on 10 review tasks (break-glass, incident #722)
EXCEPTIONAL OPERATOR-AUTHORIZED BREAK-GLASS RECOVERY - incident #722. Commit970e68b("fix: adopt_merger_pr_lease requires merger role") was an unreviewed direct push that flipped 11 task_capability_map.py entries to role="merger" instead of the intended 1. Merger profiles forbid the review permissions, so no configured profile could resolve review_pr / approve_pr / request_changes_pr (matching_configured_profile was empty repository-wide) and no fix PR could be formally reviewed. The operator resolved the catch-22 with a one-time break-glass authorization recorded on issue #722 (comment 11918); this commit is the authorized minimum and nothing more. Changes: - task_capability_map.py restored to blob02826af(the preserved intended correction, local commit c071f8a1): the 10 regressive entries return to role="reviewer"; adopt_merger_pr_lease keeps role="merger"; merge_pr is untouched. - tests/test_task_capability_role_invariants.py added (#723 AC1/AC2): profile-coverage invariant for formal review tasks, map/router agreement, merger-only adopt_merger_pr_lease and merge_pr, merger profiles cannot hold review permissions. Validation: focused role-mapping tests 72 passed (+22 subtests); full suite 2900 passed, 6 skipped, 1 known warning, 195 subtests. Recovery step 1 for incident #722; code-defect follow-ups tracked in #723. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EnHNSVQvJ8nCk7KZ9kL4Ym
This commit is contained in:
+10
-10
@@ -66,7 +66,7 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
|
|||||||
},
|
},
|
||||||
"review_pr": {
|
"review_pr": {
|
||||||
"permission": "gitea.pr.review",
|
"permission": "gitea.pr.review",
|
||||||
"role": "merger",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
"merge_pr": {
|
"merge_pr": {
|
||||||
"permission": "gitea.pr.merge",
|
"permission": "gitea.pr.merge",
|
||||||
@@ -90,42 +90,42 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
|
|||||||
# Apply path posts lease release + audit comments (gitea.pr.comment).
|
# Apply path posts lease release + audit comments (gitea.pr.comment).
|
||||||
"cleanup_obsolete_reviewer_comment_lease": {
|
"cleanup_obsolete_reviewer_comment_lease": {
|
||||||
"permission": "gitea.pr.comment",
|
"permission": "gitea.pr.comment",
|
||||||
"role": "merger",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
"gitea_cleanup_obsolete_reviewer_comment_lease": {
|
"gitea_cleanup_obsolete_reviewer_comment_lease": {
|
||||||
"permission": "gitea.pr.comment",
|
"permission": "gitea.pr.comment",
|
||||||
"role": "merger",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
"blind_pr_queue_review": {
|
"blind_pr_queue_review": {
|
||||||
"permission": "gitea.pr.review",
|
"permission": "gitea.pr.review",
|
||||||
"role": "merger",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
"pr_queue_cleanup": {
|
"pr_queue_cleanup": {
|
||||||
"permission": "gitea.pr.review",
|
"permission": "gitea.pr.review",
|
||||||
"role": "merger",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
"pr-queue-cleanup": {
|
"pr-queue-cleanup": {
|
||||||
"permission": "gitea.pr.review",
|
"permission": "gitea.pr.review",
|
||||||
"role": "merger",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
"request_changes_pr": {
|
"request_changes_pr": {
|
||||||
"permission": "gitea.pr.request_changes",
|
"permission": "gitea.pr.request_changes",
|
||||||
"role": "merger",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
"approve_pr": {
|
"approve_pr": {
|
||||||
"permission": "gitea.pr.approve",
|
"permission": "gitea.pr.approve",
|
||||||
"role": "merger",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
# #594: clear durable #332 decision lock only when last terminal PR is
|
# #594: clear durable #332 decision lock only when last terminal PR is
|
||||||
# already merged/closed (moot). Apply path requires reviewer review
|
# already merged/closed (moot). Apply path requires reviewer review
|
||||||
# permission; assessment itself uses gitea.read inside the tool.
|
# permission; assessment itself uses gitea.read inside the tool.
|
||||||
"cleanup_stale_review_decision_lock": {
|
"cleanup_stale_review_decision_lock": {
|
||||||
"permission": "gitea.pr.review",
|
"permission": "gitea.pr.review",
|
||||||
"role": "merger",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
"gitea_cleanup_stale_review_decision_lock": {
|
"gitea_cleanup_stale_review_decision_lock": {
|
||||||
"permission": "gitea.pr.review",
|
"permission": "gitea.pr.review",
|
||||||
"role": "merger",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
# #709: truthful absence-of-proof recovery (server-side auth + record + consume).
|
# #709: truthful absence-of-proof recovery (server-side auth + record + consume).
|
||||||
# Dedicated mutation capability — gitea.read is insufficient (review 434 F1).
|
# Dedicated mutation capability — gitea.read is insufficient (review 434 F1).
|
||||||
|
|||||||
@@ -0,0 +1,205 @@
|
|||||||
|
"""Invariant tests pinning task_capability_map role assignments (#722/#723).
|
||||||
|
|
||||||
|
Added with the operator-authorized break-glass repair for incident #722:
|
||||||
|
commit 970e68b remapped ten reviewer tasks to ``role="merger"`` while every
|
||||||
|
configured merger profile forbids the review permissions, so no configured
|
||||||
|
profile could resolve any formal review task (``matching_configured_profile``
|
||||||
|
was empty repository-wide). These tests fail loudly if that class of
|
||||||
|
regression recurs:
|
||||||
|
|
||||||
|
- every role-exclusive formal-review task must be satisfiable by at least one
|
||||||
|
canonical role profile (permission AND role together);
|
||||||
|
- the capability map must agree with ``role_session_router`` task sets;
|
||||||
|
- ``adopt_merger_pr_lease`` stays merger-only (the legitimate hunk of
|
||||||
|
970e68b, preserved by the repair);
|
||||||
|
- merger profiles must not be able to resolve review_pr/approve_pr.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import unittest
|
||||||
|
|
||||||
|
import gitea_config
|
||||||
|
from role_session_router import MERGER_TASKS, REVIEWER_TASKS
|
||||||
|
from task_capability_map import required_permission, required_role
|
||||||
|
|
||||||
|
# Canonical role-profile permission shape. Mirrors the configured
|
||||||
|
# author/reviewer/merger/reconciler profiles (profiles.json v2 role split):
|
||||||
|
# reviewers review/approve/request changes but never merge; mergers merge but
|
||||||
|
# never review/approve/request changes.
|
||||||
|
CANONICAL_ROLE_PROFILES = {
|
||||||
|
"author": {
|
||||||
|
"allowed": [
|
||||||
|
"gitea.read",
|
||||||
|
"gitea.branch.create",
|
||||||
|
"gitea.branch.push",
|
||||||
|
"gitea.repo.commit",
|
||||||
|
"gitea.pr.create",
|
||||||
|
"gitea.pr.comment",
|
||||||
|
"gitea.issue.create",
|
||||||
|
"gitea.issue.comment",
|
||||||
|
"gitea.issue.close",
|
||||||
|
],
|
||||||
|
"forbidden": [
|
||||||
|
"gitea.pr.approve",
|
||||||
|
"gitea.pr.request_changes",
|
||||||
|
"gitea.pr.merge",
|
||||||
|
],
|
||||||
|
},
|
||||||
|
"reviewer": {
|
||||||
|
"allowed": [
|
||||||
|
"gitea.read",
|
||||||
|
"gitea.pr.review",
|
||||||
|
"gitea.pr.approve",
|
||||||
|
"gitea.pr.request_changes",
|
||||||
|
"gitea.pr.comment",
|
||||||
|
"gitea.issue.comment",
|
||||||
|
],
|
||||||
|
"forbidden": [
|
||||||
|
"gitea.branch.create",
|
||||||
|
"gitea.branch.push",
|
||||||
|
"gitea.repo.commit",
|
||||||
|
"gitea.pr.create",
|
||||||
|
"gitea.pr.merge",
|
||||||
|
],
|
||||||
|
},
|
||||||
|
"merger": {
|
||||||
|
"allowed": [
|
||||||
|
"gitea.read",
|
||||||
|
"gitea.pr.merge",
|
||||||
|
"gitea.pr.comment",
|
||||||
|
"gitea.issue.comment",
|
||||||
|
],
|
||||||
|
"forbidden": [
|
||||||
|
"gitea.branch.create",
|
||||||
|
"gitea.branch.push",
|
||||||
|
"gitea.repo.commit",
|
||||||
|
"gitea.pr.create",
|
||||||
|
"gitea.pr.approve",
|
||||||
|
"gitea.pr.review",
|
||||||
|
"gitea.pr.request_changes",
|
||||||
|
],
|
||||||
|
},
|
||||||
|
"reconciler": {
|
||||||
|
"allowed": [
|
||||||
|
"gitea.read",
|
||||||
|
"gitea.pr.close",
|
||||||
|
"gitea.pr.comment",
|
||||||
|
"gitea.issue.comment",
|
||||||
|
"gitea.branch.delete",
|
||||||
|
"gitea.decision_lock.irrecoverable_recovery",
|
||||||
|
],
|
||||||
|
"forbidden": [
|
||||||
|
"gitea.pr.approve",
|
||||||
|
"gitea.pr.merge",
|
||||||
|
"gitea.pr.review",
|
||||||
|
"gitea.pr.request_changes",
|
||||||
|
"gitea.pr.create",
|
||||||
|
"gitea.branch.create",
|
||||||
|
"gitea.branch.push",
|
||||||
|
"gitea.repo.commit",
|
||||||
|
],
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
# Role-exclusive formal-review tasks (mirrors the resolver's role-exclusive
|
||||||
|
# handling for review work): permission alone is not enough — the profile's
|
||||||
|
# role kind must also match, so both dimensions are pinned here.
|
||||||
|
FORMAL_REVIEW_TASKS = (
|
||||||
|
"review_pr",
|
||||||
|
"approve_pr",
|
||||||
|
"request_changes_pr",
|
||||||
|
"blind_pr_queue_review",
|
||||||
|
"pr_queue_cleanup",
|
||||||
|
"pr-queue-cleanup",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _profile_satisfies(role_name, task):
|
||||||
|
"""True when the canonical *role_name* profile can perform *task*."""
|
||||||
|
profile = CANONICAL_ROLE_PROFILES[role_name]
|
||||||
|
ok, _reason = gitea_config.check_operation(
|
||||||
|
required_permission(task), profile["allowed"], profile["forbidden"]
|
||||||
|
)
|
||||||
|
return ok and role_name == required_role(task)
|
||||||
|
|
||||||
|
|
||||||
|
class TestFormalReviewProfileCoverage(unittest.TestCase):
|
||||||
|
"""#722: some configured profile must be able to formally review."""
|
||||||
|
|
||||||
|
def test_every_formal_review_task_has_a_satisfying_role_profile(self):
|
||||||
|
for task in FORMAL_REVIEW_TASKS:
|
||||||
|
with self.subTest(task=task):
|
||||||
|
satisfying = [
|
||||||
|
role
|
||||||
|
for role in CANONICAL_ROLE_PROFILES
|
||||||
|
if _profile_satisfies(role, task)
|
||||||
|
]
|
||||||
|
self.assertTrue(
|
||||||
|
satisfying,
|
||||||
|
f"no canonical role profile satisfies both permission "
|
||||||
|
f"{required_permission(task)!r} and role "
|
||||||
|
f"{required_role(task)!r} for task {task!r} — formal "
|
||||||
|
f"review would be impossible for every configured "
|
||||||
|
f"profile (incident #722)",
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_formal_review_tasks_are_reviewer_role(self):
|
||||||
|
for task in FORMAL_REVIEW_TASKS:
|
||||||
|
with self.subTest(task=task):
|
||||||
|
self.assertEqual(required_role(task), "reviewer")
|
||||||
|
|
||||||
|
|
||||||
|
class TestMapRouterAgreement(unittest.TestCase):
|
||||||
|
"""#723 AC2: the map and the role session router must not drift."""
|
||||||
|
|
||||||
|
def test_reviewer_tasks_map_to_reviewer_role(self):
|
||||||
|
for task in sorted(REVIEWER_TASKS):
|
||||||
|
with self.subTest(task=task):
|
||||||
|
self.assertEqual(
|
||||||
|
required_role(task),
|
||||||
|
"reviewer",
|
||||||
|
f"router classifies {task!r} as a reviewer task but the "
|
||||||
|
f"capability map assigns role {required_role(task)!r}",
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_merger_tasks_map_to_merger_role(self):
|
||||||
|
for task in sorted(MERGER_TASKS):
|
||||||
|
with self.subTest(task=task):
|
||||||
|
self.assertEqual(
|
||||||
|
required_role(task),
|
||||||
|
"merger",
|
||||||
|
f"router classifies {task!r} as a merger task but the "
|
||||||
|
f"capability map assigns role {required_role(task)!r}",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestMergerBoundary(unittest.TestCase):
|
||||||
|
"""Preserve the legitimate hunk of 970e68b and the merger fence."""
|
||||||
|
|
||||||
|
def test_adopt_merger_pr_lease_requires_merger_role(self):
|
||||||
|
self.assertEqual(required_role("adopt_merger_pr_lease"), "merger")
|
||||||
|
self.assertEqual(
|
||||||
|
required_permission("adopt_merger_pr_lease"), "gitea.pr.comment"
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_merge_pr_requires_merger_role(self):
|
||||||
|
self.assertEqual(required_role("merge_pr"), "merger")
|
||||||
|
self.assertEqual(required_permission("merge_pr"), "gitea.pr.merge")
|
||||||
|
|
||||||
|
def test_merger_profile_cannot_resolve_formal_review_tasks(self):
|
||||||
|
merger = CANONICAL_ROLE_PROFILES["merger"]
|
||||||
|
for task in ("review_pr", "approve_pr", "request_changes_pr"):
|
||||||
|
with self.subTest(task=task):
|
||||||
|
ok, reason = gitea_config.check_operation(
|
||||||
|
required_permission(task),
|
||||||
|
merger["allowed"],
|
||||||
|
merger["forbidden"],
|
||||||
|
)
|
||||||
|
self.assertFalse(
|
||||||
|
ok,
|
||||||
|
f"merger profile must not hold {task!r} permission "
|
||||||
|
f"(got reason {reason!r})",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
Reference in New Issue
Block a user