From ba7915452e17b5b7d93a2bb8bddefe534cbfb6bb Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Thu, 16 Jul 2026 18:38:17 -0400 Subject: [PATCH] fix(capability-map): restore reviewer role on 10 review tasks (break-glass, incident #722) EXCEPTIONAL OPERATOR-AUTHORIZED BREAK-GLASS RECOVERY - incident #722. Commit 970e68b ("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 blob 02826af (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) Claude-Session: https://claude.ai/code/session_01EnHNSVQvJ8nCk7KZ9kL4Ym --- task_capability_map.py | 20 +- tests/test_task_capability_role_invariants.py | 205 ++++++++++++++++++ 2 files changed, 215 insertions(+), 10 deletions(-) create mode 100644 tests/test_task_capability_role_invariants.py diff --git a/task_capability_map.py b/task_capability_map.py index b3ad7b2..02826af 100644 --- a/task_capability_map.py +++ b/task_capability_map.py @@ -66,7 +66,7 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = { }, "review_pr": { "permission": "gitea.pr.review", - "role": "merger", + "role": "reviewer", }, "merge_pr": { "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). "cleanup_obsolete_reviewer_comment_lease": { "permission": "gitea.pr.comment", - "role": "merger", + "role": "reviewer", }, "gitea_cleanup_obsolete_reviewer_comment_lease": { "permission": "gitea.pr.comment", - "role": "merger", + "role": "reviewer", }, "blind_pr_queue_review": { "permission": "gitea.pr.review", - "role": "merger", + "role": "reviewer", }, "pr_queue_cleanup": { "permission": "gitea.pr.review", - "role": "merger", + "role": "reviewer", }, "pr-queue-cleanup": { "permission": "gitea.pr.review", - "role": "merger", + "role": "reviewer", }, "request_changes_pr": { "permission": "gitea.pr.request_changes", - "role": "merger", + "role": "reviewer", }, "approve_pr": { "permission": "gitea.pr.approve", - "role": "merger", + "role": "reviewer", }, # #594: clear durable #332 decision lock only when last terminal PR is # already merged/closed (moot). Apply path requires reviewer review # permission; assessment itself uses gitea.read inside the tool. "cleanup_stale_review_decision_lock": { "permission": "gitea.pr.review", - "role": "merger", + "role": "reviewer", }, "gitea_cleanup_stale_review_decision_lock": { "permission": "gitea.pr.review", - "role": "merger", + "role": "reviewer", }, # #709: truthful absence-of-proof recovery (server-side auth + record + consume). # Dedicated mutation capability — gitea.read is insufficient (review 434 F1). diff --git a/tests/test_task_capability_role_invariants.py b/tests/test_task_capability_role_invariants.py new file mode 100644 index 0000000..a77f570 --- /dev/null +++ b/tests/test_task_capability_role_invariants.py @@ -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()