feat: add PR-only queue cleanup mode with per-PR dispatch gates (Closes #390)
Adds canonical pr-queue-cleanup workflow, report verifier, reviewer-only task routing, and runbook guidance so operators can drain open PRs one canonical review at a time with fail-closed boundaries on batching and unauthorized merges. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
Claude Opus 4.8
@@ -16,6 +16,7 @@ def test_all_workflow_files_exist():
|
||||
"reconcile-landed-pr.md",
|
||||
"create-issue.md",
|
||||
"work-issue.md",
|
||||
"pr-queue-cleanup.md",
|
||||
):
|
||||
assert (SKILL_DIR / "workflows" / name).is_file(), name
|
||||
|
||||
@@ -26,6 +27,7 @@ def test_skill_references_all_workflow_files():
|
||||
"workflows/reconcile-landed-pr.md",
|
||||
"workflows/create-issue.md",
|
||||
"workflows/work-issue.md",
|
||||
"workflows/pr-queue-cleanup.md",
|
||||
):
|
||||
assert name in SKILL
|
||||
|
||||
@@ -36,6 +38,7 @@ def test_skill_contains_mode_isolation_language():
|
||||
assert "reconcile-landed-pr" in SKILL
|
||||
assert "create-issue" in SKILL
|
||||
assert "work-issue" in SKILL
|
||||
assert "pr-queue-cleanup" in SKILL
|
||||
assert "Do not mix modes" in SKILL or "do not mix modes" in SKILL.lower()
|
||||
|
||||
|
||||
@@ -99,12 +102,24 @@ def test_work_issue_workflow_contract():
|
||||
assert "## 21. PR creation rules" in text
|
||||
|
||||
|
||||
def test_pr_queue_cleanup_workflow_contract():
|
||||
text = (SKILL_DIR / "workflows" / "pr-queue-cleanup.md").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
assert "canonical: true" in text
|
||||
assert "exactly one" in text
|
||||
assert "check_cleanup_task_allowed" in text
|
||||
assert "resolve_cleanup_run_state" in text
|
||||
assert "Next suggested PR" in text
|
||||
|
||||
|
||||
def test_final_report_schemas_exist():
|
||||
for name in (
|
||||
"review-merge-final-report.md",
|
||||
"reconcile-landed-final-report.md",
|
||||
"create-issue-final-report.md",
|
||||
"work-issue-final-report.md",
|
||||
"pr-queue-cleanup-final-report.md",
|
||||
):
|
||||
assert (SKILL_DIR / "schemas" / name).is_file(), name
|
||||
|
||||
@@ -124,6 +139,20 @@ def test_start_issue_template_references_work_issue_workflow():
|
||||
assert "workflows/work-issue.md" in text
|
||||
|
||||
|
||||
def test_pr_queue_cleanup_template_references_workflow():
|
||||
text = (SKILL_DIR / "templates" / "pr-queue-cleanup.md").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
assert "workflows/pr-queue-cleanup.md" in text
|
||||
assert "pr_queue_cleanup" in text
|
||||
|
||||
|
||||
def test_pr_queue_cleanup_verifier_exported():
|
||||
from review_proofs import assess_pr_queue_cleanup_report
|
||||
|
||||
assert callable(assess_pr_queue_cleanup_report)
|
||||
|
||||
|
||||
def test_reviewer_fallback_verifier_exported():
|
||||
from review_proofs import assess_reviewer_fallback_report
|
||||
|
||||
@@ -199,4 +228,23 @@ def test_mutation_categories_verifier_exported():
|
||||
def test_worktree_ownership_verifier_exported():
|
||||
from review_proofs import assess_worktree_ownership_report
|
||||
|
||||
assert callable(assess_worktree_ownership_report)
|
||||
assert callable(assess_worktree_ownership_report)
|
||||
|
||||
|
||||
def test_pr_queue_cleanup_verifier_exported():
|
||||
from review_proofs import assess_pr_queue_cleanup_report
|
||||
|
||||
assert callable(assess_pr_queue_cleanup_report)
|
||||
|
||||
|
||||
def test_pr_queue_cleanup_workflow_contract():
|
||||
text = (SKILL_DIR / "workflows" / "pr-queue-cleanup.md").read_text(encoding="utf-8")
|
||||
assert "canonical: true" in text
|
||||
assert "task_mode: pr-queue-cleanup" in text
|
||||
assert "## 1. Mode isolation" in text
|
||||
assert "## 4. Terminal mutation chain" in text
|
||||
assert "## 5. Fresh run per PR" in text
|
||||
assert "exactly one" in text.lower()
|
||||
assert "review-merge-pr.md" in text
|
||||
assert "Next suggested PR" in text
|
||||
assert "schemas/pr-queue-cleanup-final-report.md" in text
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
"""Tests for PR-only queue cleanup mode (#390)."""
|
||||
import sys
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from pr_queue_cleanup import (
|
||||
CLEANUP_WORKFLOW_PATH,
|
||||
STOP_AFTER_REQUEST_CHANGES,
|
||||
STOP_APPROVED_NO_MERGE_AUTH,
|
||||
CONTINUE_TO_SAME_PR_MERGE,
|
||||
assess_pr_queue_cleanup_report,
|
||||
check_cleanup_task_allowed,
|
||||
resolve_cleanup_run_state,
|
||||
)
|
||||
from review_proofs import assess_pr_queue_cleanup_report as proofs_assess
|
||||
|
||||
|
||||
GOOD_REPORT = f"""
|
||||
Task: pr-queue-cleanup
|
||||
Workflow source: {CLEANUP_WORKFLOW_PATH}
|
||||
PR inventory pagination proof: inventory_complete=true, total_count=12
|
||||
Selected PR: #401
|
||||
Review decision: approved
|
||||
Merge authorized for PR: true
|
||||
Merge result: not attempted
|
||||
Next suggested PR: #402
|
||||
Issue mutations: none
|
||||
Branch mutations: none
|
||||
"""
|
||||
|
||||
|
||||
class TestCleanupRunState(unittest.TestCase):
|
||||
def test_request_changes_stops_run(self):
|
||||
result = resolve_cleanup_run_state("request_changes")
|
||||
self.assertEqual(result["outcome"], STOP_AFTER_REQUEST_CHANGES)
|
||||
self.assertFalse(result["further_mutation_allowed"])
|
||||
|
||||
def test_approved_without_merge_auth_stops(self):
|
||||
result = resolve_cleanup_run_state("approved")
|
||||
self.assertEqual(result["outcome"], STOP_APPROVED_NO_MERGE_AUTH)
|
||||
self.assertFalse(result["further_mutation_allowed"])
|
||||
|
||||
def test_approved_with_merge_auth_continues_to_merge(self):
|
||||
result = resolve_cleanup_run_state(
|
||||
"approved",
|
||||
merge_authorized_for_pr=True,
|
||||
merge_gates_passed=True,
|
||||
)
|
||||
self.assertEqual(result["outcome"], CONTINUE_TO_SAME_PR_MERGE)
|
||||
self.assertTrue(result["further_mutation_allowed"])
|
||||
|
||||
|
||||
class TestCleanupTaskGate(unittest.TestCase):
|
||||
def test_author_tasks_blocked(self):
|
||||
for task in (
|
||||
"lock_issue",
|
||||
"create_issue",
|
||||
"create_branch",
|
||||
"create_pr",
|
||||
"commit_files",
|
||||
):
|
||||
allowed, reasons = check_cleanup_task_allowed(task)
|
||||
self.assertFalse(allowed, task)
|
||||
self.assertTrue(reasons)
|
||||
|
||||
|
||||
class TestCleanupReportVerifier(unittest.TestCase):
|
||||
def test_good_report_passes(self):
|
||||
result = assess_pr_queue_cleanup_report(GOOD_REPORT)
|
||||
self.assertTrue(result["proven"])
|
||||
self.assertFalse(result["block"])
|
||||
|
||||
def test_multi_pr_selection_blocked(self):
|
||||
report = GOOD_REPORT + "\nSelected PR: #403\n"
|
||||
result = assess_pr_queue_cleanup_report(report)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertIn("multiple prs", result["reasons"][0].lower())
|
||||
|
||||
def test_missing_pagination_proof_blocked(self):
|
||||
report = GOOD_REPORT.replace(
|
||||
"PR inventory pagination proof: inventory_complete=true, total_count=12",
|
||||
"PR inventory pagination proof: inventory listed",
|
||||
)
|
||||
result = assess_pr_queue_cleanup_report(report)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(
|
||||
any("pagination" in r.lower() for r in result["reasons"])
|
||||
)
|
||||
|
||||
def test_merge_without_authorization_blocked(self):
|
||||
report = GOOD_REPORT.replace(
|
||||
"Merge authorized for PR: true",
|
||||
"Merge authorized for PR: false",
|
||||
).replace("Merge result: not attempted", "Merge result: merged abc123")
|
||||
result = assess_pr_queue_cleanup_report(report)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(
|
||||
any("merge" in r.lower() for r in result["reasons"])
|
||||
)
|
||||
|
||||
def test_issue_mutations_forbidden(self):
|
||||
report = GOOD_REPORT.replace(
|
||||
"Issue mutations: none",
|
||||
"Issue mutations: gitea_mark_issue",
|
||||
)
|
||||
result = assess_pr_queue_cleanup_report(report)
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_review_proofs_wrapper_matches_module(self):
|
||||
direct = assess_pr_queue_cleanup_report(GOOD_REPORT)
|
||||
wrapped = proofs_assess(GOOD_REPORT)
|
||||
self.assertEqual(direct["proven"], wrapped["proven"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -275,6 +275,28 @@ class TestResolveTaskCapability(unittest.TestCase):
|
||||
self.assertEqual(res["required_role_kind"], "author")
|
||||
self.assertIn("author", res["exact_safe_next_action"].lower())
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_resolve_pr_queue_cleanup_author_profile_blocked(self, _auth, _api):
|
||||
with patch.dict(os.environ, self._env("author-profile")):
|
||||
res = mcp_server.gitea_resolve_task_capability(
|
||||
task="pr_queue_cleanup", remote="prgs"
|
||||
)
|
||||
self.assertEqual(res["required_role_kind"], "reviewer")
|
||||
self.assertFalse(res["allowed_in_current_session"])
|
||||
self.assertTrue(res["stop_required"])
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "reviewer-user"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
|
||||
def test_resolve_pr_queue_cleanup_reviewer_profile_allowed(self, _auth, _api):
|
||||
with patch.dict(os.environ, self._env("reviewer-profile")):
|
||||
res = mcp_server.gitea_resolve_task_capability(
|
||||
task="pr_queue_cleanup", remote="prgs"
|
||||
)
|
||||
self.assertEqual(res["required_operation_permission"], "gitea.pr.review")
|
||||
self.assertTrue(res["allowed_in_current_session"])
|
||||
self.assertFalse(res["stop_required"])
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_close_pr_known_and_lookalike_tasks_still_fail_closed(self, _auth, _api):
|
||||
|
||||
@@ -99,6 +99,24 @@ class TestRoleSessionRouter(unittest.TestCase):
|
||||
self.assertFalse(route["downstream_allowed"])
|
||||
self.assertIn("Wrong role/session for reviewer task", route["message"])
|
||||
|
||||
def test_pr_queue_cleanup_under_author_profile_wrong_role_stop(self):
|
||||
with patch.dict(os.environ, self._env("prgs-author")):
|
||||
route = mcp_server.gitea_route_task_session(
|
||||
task_type="pr_queue_cleanup", remote="prgs"
|
||||
)
|
||||
self.assertEqual(route["route_result"], role_session_router.ROUTE_WRONG_ROLE)
|
||||
self.assertFalse(route["downstream_allowed"])
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "sysadmin"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
|
||||
def test_pr_queue_cleanup_under_reviewer_profile_allowed(self, _auth, _api):
|
||||
with patch.dict(os.environ, self._env("prgs-reviewer")):
|
||||
route = mcp_server.gitea_route_task_session(
|
||||
task_type="pr_queue_cleanup", remote="prgs"
|
||||
)
|
||||
self.assertEqual(route["route_result"], role_session_router.ROUTE_ALLOWED)
|
||||
self.assertTrue(route["downstream_allowed"])
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_issue_creation_blocked_after_reviewer_wrong_role_stop(
|
||||
|
||||
Reference in New Issue
Block a user