sysadmin
3fd02a3c63
Address REQUEST_CHANGES on PR #680: Blocker A — role vs capability agreement - authorization_compatible allows reviewer/merger when they hold the task's required permission (e.g. gitea.issue.comment on comment_issue/mark_issue/ set_issue_labels/lock_issue) without broad role-bypass. - Unauthorized escalation without the permission remains fail closed. - Regression coverage for allowed and denied reviewer/merger cases. Blocker B — MUTATION_TASKS matches runtime wiring - Remove unenforced entries; document dedicated-gate exclusions (mark_final_review_decision, save/resume_review_draft, etc.). - Wire non-closing gitea_edit_pr as edit_pr; acquire lease uses acquire_reviewer_pr_lease task through verify_preflight_purity. - Inventory↔wiring consistency tests replace set-membership-only coverage. Blocker C — entrypoint side-effect ordering - Behavioral tests for merge_pr, submit_pr_review, and comment_issue prove the assessor block aborts before Gitea API mutation and local durable writes. Validation: 43 focused anti-stomp + related suites green; full suite 2639 passed / 6 skipped. Closes #604
1033 lines
40 KiB
Python
1033 lines
40 KiB
Python
"""Regression coverage for the common anti-stomp preflight (#604).
|
|
|
|
Acceptance criteria:
|
|
|
|
1. All mutation tools call the common preflight (wired via
|
|
``verify_preflight_purity`` / ``_run_anti_stomp_preflight``).
|
|
2. Failure returns a typed blocker and exact next action.
|
|
3. Tests cover wrong repo defaulting to Timesheet, stale runtime, wrong
|
|
worktree, foreign lease, terminal lock, and contaminated approval.
|
|
4. No mutation can proceed using stale prompt data if live state disagrees.
|
|
5. Existing successful paths continue to work (happy-path assessment).
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
import unittest
|
|
from unittest import mock
|
|
|
|
import anti_stomp_preflight as asp
|
|
|
|
|
|
LOCAL_GITEA_TOOLS_URL = "https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git"
|
|
STARTUP = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
|
|
ADVANCED = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
|
|
HEAD_A = "1111111111111111111111111111111111111111"
|
|
HEAD_B = "2222222222222222222222222222222222222222"
|
|
|
|
|
|
def _happy_kwargs(**overrides):
|
|
base = dict(
|
|
task="create_issue",
|
|
remote="prgs",
|
|
resolved_org="Scaled-Tech-Consulting",
|
|
resolved_repo="Gitea-Tools",
|
|
local_remote_url=LOCAL_GITEA_TOOLS_URL,
|
|
org_explicit=True,
|
|
repo_explicit=True,
|
|
profile_name="prgs-author",
|
|
profile_role="author",
|
|
required_role="author",
|
|
workspace_path="/repo/branches/issue-604",
|
|
project_root="/repo",
|
|
current_branch="feat/issue-604-anti-stomp-preflight",
|
|
root_head_sha=STARTUP,
|
|
root_porcelain="",
|
|
remote_master_sha=STARTUP,
|
|
startup_head=STARTUP,
|
|
current_code_head=STARTUP,
|
|
source_contaminated=False,
|
|
manual_bypass_attempted=False,
|
|
)
|
|
base.update(overrides)
|
|
return base
|
|
|
|
|
|
class TestIsMutationTask(unittest.TestCase):
|
|
def test_core_mutation_tasks(self):
|
|
for task in (
|
|
"create_issue",
|
|
"comment_issue",
|
|
"set_issue_labels",
|
|
"acquire_reviewer_pr_lease",
|
|
"submit_pr_review",
|
|
"approve_pr",
|
|
"request_changes_pr",
|
|
"merge_pr",
|
|
"cleanup_stale_claims",
|
|
"delete_branch",
|
|
):
|
|
self.assertTrue(asp.is_mutation_task(task), task)
|
|
|
|
def test_gitea_prefix_accepted(self):
|
|
self.assertTrue(asp.is_mutation_task("gitea_create_issue"))
|
|
self.assertTrue(asp.is_mutation_task("gitea_merge_pr"))
|
|
|
|
def test_read_only_not_mutation(self):
|
|
self.assertFalse(asp.is_mutation_task("list_prs"))
|
|
self.assertFalse(asp.is_mutation_task("whoami"))
|
|
self.assertFalse(asp.is_mutation_task(""))
|
|
self.assertFalse(asp.is_mutation_task(None))
|
|
|
|
|
|
class TestHappyPath(unittest.TestCase):
|
|
def test_allowed_when_all_checks_pass(self):
|
|
result = asp.assess_anti_stomp_preflight(**_happy_kwargs())
|
|
self.assertTrue(result["allowed"])
|
|
self.assertFalse(result["block"])
|
|
self.assertEqual(result["blockers"], [])
|
|
self.assertEqual(result["exact_next_action"], "proceed")
|
|
self.assertIsNone(result["blocker_kind"])
|
|
|
|
def test_reviewer_happy_path_skips_author_worktree(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="review_pr",
|
|
profile_name="prgs-reviewer",
|
|
profile_role="reviewer",
|
|
required_role="reviewer",
|
|
# Under branches/ the root guard short-circuits for non-merger.
|
|
workspace_path="/repo/branches/review-pr-42",
|
|
project_root="/repo",
|
|
current_branch="review/pr-42",
|
|
foreign_lease=False,
|
|
terminal_lock_blocks=False,
|
|
expected_head_sha=HEAD_A,
|
|
live_head_sha=HEAD_A,
|
|
workflow_hash_valid=True,
|
|
)
|
|
)
|
|
self.assertTrue(result["allowed"], result.get("reasons"))
|
|
self.assertTrue(result["checks"]["worktree"].get("skipped"))
|
|
|
|
|
|
class TestWrongRepoTimesheet(unittest.TestCase):
|
|
"""AC3: wrong repo defaulting to Timesheet."""
|
|
|
|
def test_prgs_default_timesheet_vs_local_gitea_tools(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
resolved_org="Scaled-Tech-Consulting",
|
|
resolved_repo="Timesheet",
|
|
local_remote_url=LOCAL_GITEA_TOOLS_URL,
|
|
org_explicit=False,
|
|
repo_explicit=False,
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_WRONG_REPO)
|
|
self.assertTrue(result["exact_next_action"])
|
|
self.assertIn("org=", result["exact_next_action"])
|
|
self.assertTrue(
|
|
any("Timesheet" in r or "does not match" in r for r in result["reasons"])
|
|
)
|
|
|
|
def test_explicit_org_repo_skips_mismatch(self):
|
|
# Explicit intent is authoritative (remote_repo_guard contract).
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
resolved_repo="Timesheet",
|
|
org_explicit=True,
|
|
repo_explicit=True,
|
|
)
|
|
)
|
|
self.assertTrue(result["allowed"])
|
|
|
|
|
|
class TestStaleRuntime(unittest.TestCase):
|
|
"""AC3: stale runtime."""
|
|
|
|
def test_stale_runtime_blocks(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
startup_head=STARTUP,
|
|
current_code_head=ADVANCED,
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_STALE_RUNTIME)
|
|
self.assertIn("Restart", result["exact_next_action"])
|
|
self.assertTrue(result["checks"]["stale_runtime"]["stale"])
|
|
|
|
def test_in_parity_allows(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
startup_head=STARTUP,
|
|
current_code_head=STARTUP,
|
|
)
|
|
)
|
|
self.assertTrue(result["allowed"])
|
|
|
|
|
|
class TestWrongWorktree(unittest.TestCase):
|
|
"""AC3: wrong worktree."""
|
|
|
|
def test_author_on_control_checkout_blocked(self):
|
|
# Clean master control checkout: root guard may pass for a clean master
|
|
# HEAD, but the author worktree guard must still refuse mutation from
|
|
# outside branches/.
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
workspace_path="/repo",
|
|
project_root="/repo",
|
|
current_branch="master",
|
|
root_porcelain="",
|
|
root_head_sha=STARTUP,
|
|
remote_master_sha=STARTUP,
|
|
profile_role="author",
|
|
required_role="author",
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_WRONG_WORKTREE)
|
|
self.assertIn("branches/", result["exact_next_action"])
|
|
|
|
def test_author_under_branches_allowed(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
workspace_path="/repo/branches/issue-604",
|
|
project_root="/repo",
|
|
)
|
|
)
|
|
self.assertTrue(result["allowed"])
|
|
|
|
|
|
class TestForeignLease(unittest.TestCase):
|
|
"""AC3: foreign lease."""
|
|
|
|
def test_foreign_lease_blocks(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="merge_pr",
|
|
profile_role="merger",
|
|
required_role="merger",
|
|
# Merger is not auto-exempted under branches/; pass clean master.
|
|
workspace_path="/repo",
|
|
project_root="/repo",
|
|
current_branch="master",
|
|
root_porcelain="",
|
|
foreign_lease=True,
|
|
lease_reasons=["lease owned by other-session"],
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_FOREIGN_LEASE)
|
|
self.assertIn("foreign lease", result["exact_next_action"].lower())
|
|
|
|
def test_lease_required_without_ownership_fails_closed(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="review_pr",
|
|
profile_role="reviewer",
|
|
required_role="reviewer",
|
|
workspace_path="/repo/branches/review-pr-1",
|
|
project_root="/repo",
|
|
lease_required=True,
|
|
lease_owner_session="sess-A",
|
|
active_session_id="sess-B",
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_FOREIGN_LEASE)
|
|
|
|
def test_owned_lease_allows(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="review_pr",
|
|
profile_role="reviewer",
|
|
required_role="reviewer",
|
|
workspace_path="/repo/branches/review-pr-1",
|
|
project_root="/repo",
|
|
foreign_lease=False,
|
|
)
|
|
)
|
|
self.assertTrue(result["allowed"])
|
|
|
|
|
|
class TestTerminalLock(unittest.TestCase):
|
|
"""AC3: terminal lock."""
|
|
|
|
def test_terminal_lock_blocks(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="approve_pr",
|
|
profile_role="reviewer",
|
|
required_role="reviewer",
|
|
workspace_path="/repo/branches/review-pr-9",
|
|
project_root="/repo",
|
|
terminal_lock_blocks=True,
|
|
terminal_lock_reasons=["#332 terminal lock active for this head"],
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_TERMINAL_LOCK)
|
|
self.assertIn("#332", result["exact_next_action"])
|
|
|
|
def test_no_terminal_lock_allows(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="approve_pr",
|
|
profile_role="reviewer",
|
|
required_role="reviewer",
|
|
workspace_path="/repo/branches/review-pr-9",
|
|
project_root="/repo",
|
|
terminal_lock_blocks=False,
|
|
)
|
|
)
|
|
self.assertTrue(result["allowed"])
|
|
|
|
|
|
class TestHeadShaStalePrompt(unittest.TestCase):
|
|
"""AC4: no mutation with stale prompt head SHA."""
|
|
|
|
def _merger_kwargs(self, **overrides):
|
|
base = _happy_kwargs(
|
|
task="merge_pr",
|
|
profile_role="merger",
|
|
required_role="merger",
|
|
workspace_path="/repo",
|
|
project_root="/repo",
|
|
current_branch="master",
|
|
root_porcelain="",
|
|
root_head_sha=STARTUP,
|
|
remote_master_sha=STARTUP,
|
|
)
|
|
base.update(overrides)
|
|
return base
|
|
|
|
def test_head_mismatch_blocks(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**self._merger_kwargs(
|
|
expected_head_sha=HEAD_A,
|
|
live_head_sha=HEAD_B,
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_HEAD_SHA)
|
|
self.assertIn("stale prompt", " ".join(result["reasons"]).lower())
|
|
|
|
def test_require_head_sha_missing_blocks(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**self._merger_kwargs(
|
|
require_head_sha=True,
|
|
expected_head_sha=None,
|
|
live_head_sha=HEAD_A,
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_HEAD_SHA)
|
|
|
|
def test_matching_head_allows(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**self._merger_kwargs(
|
|
expected_head_sha=HEAD_A,
|
|
live_head_sha=HEAD_A,
|
|
)
|
|
)
|
|
self.assertTrue(result["allowed"])
|
|
|
|
|
|
class TestContaminatedApproval(unittest.TestCase):
|
|
"""AC3: contaminated approval / source contamination."""
|
|
|
|
def test_source_contamination_blocks(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="merge_pr",
|
|
profile_role="merger",
|
|
required_role="merger",
|
|
workspace_path="/repo",
|
|
project_root="/repo",
|
|
current_branch="master",
|
|
root_porcelain="",
|
|
source_contaminated=True,
|
|
contamination_reasons=[
|
|
"session contaminated by direct stable-branch push attempt"
|
|
],
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_SOURCE_CONTAMINATION)
|
|
self.assertIn("reconciler", result["exact_next_action"].lower())
|
|
|
|
def test_manual_bypass_blocks(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
manual_bypass_attempted=True,
|
|
manual_bypass_reasons=[
|
|
"attempted manual deletion of session-state lock files"
|
|
],
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_MANUAL_BYPASS)
|
|
|
|
|
|
class TestWrongRole(unittest.TestCase):
|
|
def test_author_cannot_merge(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="merge_pr",
|
|
profile_role="author",
|
|
required_role="merger",
|
|
workspace_path="/repo/branches/issue-604",
|
|
project_root="/repo",
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_WRONG_ROLE)
|
|
|
|
def test_reconciler_may_run_author_class_tasks(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="comment_issue",
|
|
profile_name="prgs-reconciler",
|
|
profile_role="reconciler",
|
|
required_role="author",
|
|
workspace_path="/repo",
|
|
project_root="/repo",
|
|
current_branch="master",
|
|
root_porcelain="",
|
|
)
|
|
)
|
|
self.assertTrue(result["allowed"], result.get("reasons"))
|
|
self.assertTrue(asp.roles_compatible("reconciler", "author"))
|
|
self.assertFalse(asp.roles_compatible("author", "merger"))
|
|
|
|
|
|
class TestCapabilityAuthorizedRoles(unittest.TestCase):
|
|
"""Blocker A: reviewer/merger holding issue-comment capability may mutate.
|
|
|
|
Nominal task role is still ``author`` in task_capability_map, but the
|
|
shared anti-stomp gate must not WRONG_ROLE-block when the active profile
|
|
holds ``gitea.issue.comment``. Unauthorized escalation without the
|
|
permission remains fail closed.
|
|
"""
|
|
|
|
_ISSUE_COMMENT_TASKS = (
|
|
"comment_issue",
|
|
"mark_issue",
|
|
"set_issue_labels",
|
|
"lock_issue",
|
|
)
|
|
_ISSUE_COMMENT_PERM = "gitea.issue.comment"
|
|
|
|
def _role_kwargs(self, task, role, *, allowed_ops, permission=None):
|
|
return _happy_kwargs(
|
|
task=task,
|
|
profile_name=f"prgs-{role}",
|
|
profile_role=role,
|
|
required_role="author",
|
|
required_permission=permission if permission is not None else self._ISSUE_COMMENT_PERM,
|
|
allowed_operations=allowed_ops,
|
|
workspace_path=f"/repo/branches/{role}-ws",
|
|
project_root="/repo",
|
|
current_branch=f"review/{role}-task",
|
|
)
|
|
|
|
def test_reviewer_allowed_with_issue_comment_capability(self):
|
|
ops = ["gitea.read", "gitea.issue.comment", "gitea.pr.review"]
|
|
for task in self._ISSUE_COMMENT_TASKS:
|
|
with self.subTest(task=task):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**self._role_kwargs(task, "reviewer", allowed_ops=ops)
|
|
)
|
|
self.assertTrue(result["allowed"], result.get("reasons"))
|
|
self.assertFalse(result["block"])
|
|
self.assertTrue(
|
|
result["checks"]["role"].get("capability_authorized")
|
|
)
|
|
|
|
def test_merger_allowed_with_issue_comment_capability(self):
|
|
ops = ["gitea.read", "gitea.issue.comment", "gitea.pr.merge"]
|
|
for task in self._ISSUE_COMMENT_TASKS:
|
|
with self.subTest(task=task):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**self._role_kwargs(task, "merger", allowed_ops=ops)
|
|
)
|
|
self.assertTrue(result["allowed"], result.get("reasons"))
|
|
self.assertFalse(result["block"])
|
|
|
|
def test_reviewer_denied_without_issue_comment_capability(self):
|
|
# Holds review permission only — not issue comment.
|
|
ops = ["gitea.read", "gitea.pr.review", "gitea.pr.approve"]
|
|
for task in self._ISSUE_COMMENT_TASKS:
|
|
with self.subTest(task=task):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**self._role_kwargs(task, "reviewer", allowed_ops=ops)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_WRONG_ROLE)
|
|
|
|
def test_merger_denied_without_issue_comment_capability(self):
|
|
ops = ["gitea.read", "gitea.pr.merge"]
|
|
for task in self._ISSUE_COMMENT_TASKS:
|
|
with self.subTest(task=task):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**self._role_kwargs(task, "merger", allowed_ops=ops)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_WRONG_ROLE)
|
|
|
|
def test_not_broad_reviewer_to_author_bypass(self):
|
|
# Reviewer with only issue.comment must not pass create_pr (needs create).
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="create_pr",
|
|
profile_name="prgs-reviewer",
|
|
profile_role="reviewer",
|
|
required_role="author",
|
|
required_permission="gitea.pr.create",
|
|
allowed_operations=["gitea.read", "gitea.issue.comment", "gitea.pr.review"],
|
|
workspace_path="/repo/branches/review-ws",
|
|
project_root="/repo",
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_WRONG_ROLE)
|
|
|
|
def test_not_broad_merger_to_author_bypass(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="create_issue",
|
|
profile_name="prgs-merger",
|
|
profile_role="merger",
|
|
required_role="author",
|
|
required_permission="gitea.issue.create",
|
|
allowed_operations=["gitea.read", "gitea.issue.comment", "gitea.pr.merge"],
|
|
workspace_path="/repo/branches/merge-ws",
|
|
project_root="/repo",
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_WRONG_ROLE)
|
|
|
|
def test_authorization_compatible_helper(self):
|
|
self.assertTrue(
|
|
asp.authorization_compatible(
|
|
"reviewer",
|
|
"author",
|
|
required_permission="gitea.issue.comment",
|
|
allowed_operations=["gitea.issue.comment"],
|
|
)
|
|
)
|
|
self.assertFalse(
|
|
asp.authorization_compatible(
|
|
"reviewer",
|
|
"author",
|
|
required_permission="gitea.issue.comment",
|
|
allowed_operations=["gitea.pr.review"],
|
|
)
|
|
)
|
|
# Missing ops list fails closed when permission is required and roles differ.
|
|
self.assertFalse(
|
|
asp.authorization_compatible(
|
|
"reviewer",
|
|
"author",
|
|
required_permission="gitea.issue.comment",
|
|
allowed_operations=None,
|
|
)
|
|
)
|
|
|
|
|
|
class TestWorkflowHash(unittest.TestCase):
|
|
def test_stale_workflow_hash_blocks(self):
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="review_pr",
|
|
profile_role="reviewer",
|
|
required_role="reviewer",
|
|
workspace_path="/repo/branches/review-pr-3",
|
|
project_root="/repo",
|
|
workflow_hash_valid=False,
|
|
workflow_hash_reasons=["stored workflow hash is stale"],
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
self.assertEqual(result["blocker_kind"], asp.BLOCKER_WORKFLOW_HASH)
|
|
|
|
|
|
class TestTypedBlockerResponse(unittest.TestCase):
|
|
"""AC2: typed blocker + exact next action in response payload."""
|
|
|
|
def test_block_response_shape(self):
|
|
assessment = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
task="review_pr",
|
|
profile_role="reviewer",
|
|
required_role="reviewer",
|
|
workspace_path="/repo/branches/review-pr-7",
|
|
project_root="/repo",
|
|
foreign_lease=True,
|
|
)
|
|
)
|
|
payload = asp.block_response(assessment, pr_number=42)
|
|
self.assertFalse(payload["success"])
|
|
self.assertFalse(payload["performed"])
|
|
self.assertTrue(payload["blocked"])
|
|
self.assertTrue(payload["anti_stomp"])
|
|
self.assertEqual(payload["blocker_kind"], asp.BLOCKER_FOREIGN_LEASE)
|
|
self.assertTrue(payload["exact_next_action"])
|
|
self.assertEqual(payload["pr_number"], 42)
|
|
self.assertTrue(payload["blockers"])
|
|
self.assertEqual(payload["blockers"][0]["kind"], asp.BLOCKER_FOREIGN_LEASE)
|
|
|
|
def test_format_error_includes_kind_and_next_action(self):
|
|
assessment = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
startup_head=STARTUP,
|
|
current_code_head=ADVANCED,
|
|
)
|
|
)
|
|
msg = asp.format_anti_stomp_error(assessment)
|
|
self.assertIn("#604", msg)
|
|
self.assertIn(asp.BLOCKER_STALE_RUNTIME, msg)
|
|
self.assertIn("exact_next_action", msg)
|
|
|
|
|
|
class TestRootCheckoutContamination(unittest.TestCase):
|
|
def test_dirty_control_checkout_blocks_author(self):
|
|
# Workspace is control checkout with dirty porcelain.
|
|
result = asp.assess_anti_stomp_preflight(
|
|
**_happy_kwargs(
|
|
workspace_path="/repo",
|
|
project_root="/repo",
|
|
current_branch="master",
|
|
root_porcelain=" M gitea_mcp_server.py\n",
|
|
root_head_sha=STARTUP,
|
|
remote_master_sha=STARTUP,
|
|
)
|
|
)
|
|
self.assertTrue(result["block"])
|
|
# Author worktree check or root checkout may fire first.
|
|
self.assertIn(
|
|
result["blocker_kind"],
|
|
{asp.BLOCKER_ROOT_CHECKOUT, asp.BLOCKER_WRONG_WORKTREE},
|
|
)
|
|
|
|
|
|
class TestMutationTaskInventory(unittest.TestCase):
|
|
"""AC1 + Blocker B: declared inventory matches runtime wiring."""
|
|
|
|
def test_issue_required_paths_covered(self):
|
|
required = {
|
|
"create_issue",
|
|
"comment_issue",
|
|
"set_issue_labels",
|
|
"acquire_reviewer_pr_lease",
|
|
"submit_pr_review",
|
|
"approve_pr",
|
|
"request_changes_pr",
|
|
"merge_pr",
|
|
"cleanup_merged_pr_branch",
|
|
"cleanup_stale_claims",
|
|
"edit_pr",
|
|
}
|
|
missing = required - asp.MUTATION_TASKS
|
|
self.assertFalse(missing, f"missing mutation tasks: {missing}")
|
|
|
|
def test_disputed_helpers_classified_as_dedicated_or_shared(self):
|
|
"""Blocker B explicit classification for disputed mutation helpers."""
|
|
# Shared preflight inventory (wired).
|
|
self.assertIn("edit_pr", asp.MUTATION_TASKS)
|
|
# Dedicated fail-closed gates — must NOT claim shared preflight.
|
|
for name in (
|
|
"mark_final_review_decision",
|
|
"save_review_draft",
|
|
"resume_review_draft",
|
|
):
|
|
self.assertNotIn(name, asp.MUTATION_TASKS, name)
|
|
self.assertIn(name, asp.DEDICATED_GATE_MUTATIONS, name)
|
|
self.assertTrue(asp.DEDICATED_GATE_MUTATIONS[name].strip())
|
|
|
|
def test_inventory_and_wiring_consistent(self):
|
|
"""Every MUTATION_TASKS member is referenced as a live task kwarg.
|
|
|
|
Accepts:
|
|
* task=\"name\" / task='name' on verify_preflight_purity / _run_anti_stomp
|
|
* review anti_task map values (approve_pr / request_changes_pr / …)
|
|
* gitea_ alias form when the bare name is also declared
|
|
"""
|
|
import pathlib
|
|
import re
|
|
|
|
server_src = pathlib.Path(__file__).resolve().parents[1] / "gitea_mcp_server.py"
|
|
text = server_src.read_text(encoding="utf-8")
|
|
|
|
# Collect string literals used as task= kwargs.
|
|
task_kw = set(re.findall(r'task\s*=\s*["\']([a-z0-9_]+)["\']', text))
|
|
# anti_task map values: "approve": "approve_pr",
|
|
anti_map = set(
|
|
re.findall(
|
|
r'"(?:approve|request_changes|comment)"\s*:\s*"([a-z0-9_]+)"',
|
|
text,
|
|
)
|
|
)
|
|
wired = task_kw | anti_map
|
|
# Also accept f-string / conditional close_pr|edit_pr style already in task_kw.
|
|
|
|
missing = set()
|
|
for task in asp.MUTATION_TASKS:
|
|
bare = task.removeprefix("gitea_")
|
|
if task in wired or bare in wired or f"gitea_{bare}" in wired:
|
|
continue
|
|
# Alias pairs: gitea_commit_files ↔ commit_files
|
|
if task.startswith("gitea_") and bare in asp.MUTATION_TASKS and bare in wired:
|
|
continue
|
|
if f"gitea_{task}" in asp.MUTATION_TASKS and task in wired:
|
|
continue
|
|
missing.add(task)
|
|
self.assertFalse(
|
|
missing,
|
|
f"MUTATION_TASKS declared but not wired in gitea_mcp_server.py: {sorted(missing)}",
|
|
)
|
|
|
|
# Dedicated exclusions must not appear as shared anti-stomp task kwargs
|
|
# for the three disputed local helpers (except resume→submit_pr_review).
|
|
for name in ("mark_final_review_decision", "save_review_draft"):
|
|
# May appear as string metadata but not as task="..." anti-stomp target.
|
|
# Allow mention in comments; assert not as task= kwarg.
|
|
self.assertNotIn(name, task_kw, f"{name} must not be shared-preflight task=")
|
|
|
|
|
|
class TestServerWiring(unittest.TestCase):
|
|
"""Smoke: MCP server imports anti_stomp and exposes the runner."""
|
|
|
|
def test_server_imports_anti_stomp(self):
|
|
import gitea_mcp_server as server
|
|
|
|
self.assertTrue(hasattr(server, "_run_anti_stomp_preflight"))
|
|
self.assertTrue(hasattr(server, "anti_stomp_preflight"))
|
|
self.assertIs(server.anti_stomp_preflight, asp)
|
|
|
|
def test_runner_skipped_under_pytest_by_default(self):
|
|
import gitea_mcp_server as server
|
|
|
|
# Default pytest path must not raise (suite isolation).
|
|
self.assertIsNone(
|
|
server._run_anti_stomp_preflight(
|
|
"create_issue",
|
|
remote="prgs",
|
|
org="Scaled-Tech-Consulting",
|
|
repo="Timesheet",
|
|
)
|
|
)
|
|
|
|
def test_runner_blocks_when_forced(self):
|
|
import gitea_mcp_server as server
|
|
|
|
with mock.patch.dict(
|
|
os.environ,
|
|
{"GITEA_TEST_FORCE_ANTI_STOMP": "1"},
|
|
clear=False,
|
|
):
|
|
# Force assessor to return a block without full git/workspace setup.
|
|
blocked = {
|
|
"allowed": False,
|
|
"block": True,
|
|
"blockers": [{
|
|
"kind": asp.BLOCKER_STALE_RUNTIME,
|
|
"reasons": ["stale"],
|
|
"exact_next_action": "restart",
|
|
"detail": {},
|
|
}],
|
|
"reasons": ["stale"],
|
|
"exact_next_action": "restart",
|
|
"blocker_kind": asp.BLOCKER_STALE_RUNTIME,
|
|
"checks": {},
|
|
}
|
|
with mock.patch.object(
|
|
asp, "assess_anti_stomp_preflight", return_value=blocked
|
|
):
|
|
with self.assertRaises(RuntimeError) as ctx:
|
|
server._run_anti_stomp_preflight(
|
|
"create_issue",
|
|
remote="prgs",
|
|
org="Scaled-Tech-Consulting",
|
|
repo="Gitea-Tools",
|
|
)
|
|
self.assertIn("#604", str(ctx.exception))
|
|
self.assertIn(asp.BLOCKER_STALE_RUNTIME, str(ctx.exception))
|
|
|
|
|
|
class TestEntrypointSideEffectOrdering(unittest.TestCase):
|
|
"""Blocker C / AC4: anti-stomp aborts before Gitea mutation side effects.
|
|
|
|
Invokes real entrypoint functions with external boundaries mocked.
|
|
Forces the assessor to block and proves api_request POST/merge paths
|
|
and durable local writes never run.
|
|
"""
|
|
|
|
def _blocked_assessment(self, kind=asp.BLOCKER_FOREIGN_LEASE, next_action="stop"):
|
|
return {
|
|
"allowed": False,
|
|
"block": True,
|
|
"blockers": [{
|
|
"kind": kind,
|
|
"reasons": ["forced block for ordering test"],
|
|
"exact_next_action": next_action,
|
|
"detail": {},
|
|
}],
|
|
"reasons": ["forced block for ordering test"],
|
|
"exact_next_action": next_action,
|
|
"blocker_kind": kind,
|
|
"checks": {},
|
|
}
|
|
|
|
def test_merge_pr_blocks_before_merge_api(self):
|
|
import gitea_mcp_server as server
|
|
|
|
blocked = self._blocked_assessment(
|
|
kind=asp.BLOCKER_HEAD_SHA,
|
|
next_action="re-pin expected_head_sha and retry",
|
|
)
|
|
api_mock = mock.Mock(side_effect=AssertionError("Gitea API must not be called"))
|
|
save_lock = mock.Mock(side_effect=AssertionError("local lock must not mutate"))
|
|
|
|
with mock.patch.dict(
|
|
os.environ,
|
|
{"GITEA_TEST_FORCE_ANTI_STOMP": "1"},
|
|
clear=False,
|
|
):
|
|
with mock.patch.object(
|
|
server, "_verify_role_mutation_workspace", return_value="/repo/branches/x"
|
|
), mock.patch.object(
|
|
server, "_review_workflow_load_gate_reasons", return_value=[]
|
|
), mock.patch.object(
|
|
server, "_live_namespace_health_gate", return_value=None
|
|
), mock.patch.object(
|
|
server, "terminal_review_hard_stop_reasons", return_value=[]
|
|
), mock.patch.object(
|
|
server,
|
|
"gitea_check_pr_eligibility",
|
|
return_value={
|
|
"eligible": True,
|
|
"authenticated_user": "merger-bot",
|
|
"profile_name": "prgs-merger",
|
|
"pr_author": "other-user",
|
|
"head_sha": HEAD_A,
|
|
"mergeable": True,
|
|
"reasons": [],
|
|
},
|
|
), mock.patch.object(
|
|
server, "_reviewer_pr_lease_gate", return_value=[]
|
|
), mock.patch.object(
|
|
server, "_pr_work_lease_reviewer_block", return_value={"block": False}
|
|
), mock.patch.object(
|
|
server, "get_profile", return_value={
|
|
"profile_name": "prgs-merger",
|
|
"allowed_operations": ["gitea.pr.merge", "gitea.read"],
|
|
"forbidden_operations": [],
|
|
}
|
|
), mock.patch.object(
|
|
server, "_role_kind", return_value="merger"
|
|
), mock.patch.object(
|
|
asp, "assess_anti_stomp_preflight", return_value=blocked
|
|
) as assess_mock, mock.patch.object(
|
|
server, "api_request", api_mock
|
|
), mock.patch.object(
|
|
server, "_save_review_decision_lock", save_lock
|
|
):
|
|
result = server.gitea_merge_pr(
|
|
pr_number=680,
|
|
confirmation="MERGE PR 680",
|
|
expected_head_sha=HEAD_A,
|
|
remote="prgs",
|
|
org="Scaled-Tech-Consulting",
|
|
repo="Gitea-Tools",
|
|
worktree_path="/repo/branches/merge-680",
|
|
)
|
|
|
|
self.assertFalse(result.get("performed"))
|
|
self.assertTrue(any("#604" in r or "anti-stomp" in r.lower() or asp.BLOCKER_HEAD_SHA in r
|
|
for r in (result.get("reasons") or [])),
|
|
result.get("reasons"))
|
|
joined = " ".join(result.get("reasons") or [])
|
|
self.assertIn(asp.BLOCKER_HEAD_SHA, joined)
|
|
self.assertIn("exact_next_action", joined)
|
|
assess_mock.assert_called()
|
|
api_mock.assert_not_called()
|
|
save_lock.assert_not_called()
|
|
|
|
def test_submit_pr_review_blocks_before_review_api(self):
|
|
import gitea_mcp_server as server
|
|
|
|
blocked = self._blocked_assessment(
|
|
kind=asp.BLOCKER_FOREIGN_LEASE,
|
|
next_action="stop; do not stomp foreign lease",
|
|
)
|
|
api_mock = mock.Mock(side_effect=AssertionError("Gitea review API must not be called"))
|
|
save_lock = mock.Mock(side_effect=AssertionError("decision lock must not mutate"))
|
|
|
|
with mock.patch.dict(
|
|
os.environ,
|
|
{"GITEA_TEST_FORCE_ANTI_STOMP": "1"},
|
|
clear=False,
|
|
):
|
|
with mock.patch.object(
|
|
server, "_verify_role_mutation_workspace", return_value="/repo/branches/r"
|
|
), mock.patch.object(
|
|
server, "_review_workflow_load_gate_reasons", return_value=[]
|
|
), mock.patch.object(
|
|
server, "_live_namespace_health_gate", return_value=None
|
|
), mock.patch.object(
|
|
server, "check_review_decision_gate", return_value=[]
|
|
), mock.patch.object(
|
|
server,
|
|
"gitea_check_pr_eligibility",
|
|
return_value={
|
|
"eligible": True,
|
|
"authenticated_user": "reviewer-bot",
|
|
"profile_name": "prgs-reviewer",
|
|
"pr_author": "other-user",
|
|
"head_sha": HEAD_A,
|
|
"reasons": [],
|
|
},
|
|
), mock.patch.object(
|
|
server, "_reviewer_pr_lease_gate", return_value=[]
|
|
), mock.patch.object(
|
|
server, "_pr_work_lease_reviewer_block", return_value={"block": False}
|
|
), mock.patch.object(
|
|
server, "_load_review_decision_lock", return_value={
|
|
"ready_expected_head_sha": HEAD_A,
|
|
"final_review_decision_ready": True,
|
|
"ready_pr_number": 680,
|
|
"ready_action": "request_changes",
|
|
}
|
|
), mock.patch.object(
|
|
server, "get_profile", return_value={
|
|
"profile_name": "prgs-reviewer",
|
|
"allowed_operations": [
|
|
"gitea.pr.review",
|
|
"gitea.pr.request_changes",
|
|
"gitea.read",
|
|
],
|
|
"forbidden_operations": [],
|
|
}
|
|
), mock.patch.object(
|
|
asp, "assess_anti_stomp_preflight", return_value=blocked
|
|
) as assess_mock, mock.patch.object(
|
|
server, "api_request", api_mock
|
|
), mock.patch.object(
|
|
server, "_save_review_decision_lock", save_lock
|
|
), mock.patch.object(
|
|
server, "_submit_pending_pull_review", api_mock
|
|
):
|
|
result = server.gitea_submit_pr_review(
|
|
pr_number=680,
|
|
action="request_changes",
|
|
body="needs work",
|
|
expected_head_sha=HEAD_A,
|
|
remote="prgs",
|
|
org="Scaled-Tech-Consulting",
|
|
repo="Gitea-Tools",
|
|
final_review_decision_ready=True,
|
|
worktree_path="/repo/branches/review-680",
|
|
)
|
|
|
|
self.assertFalse(result.get("performed"))
|
|
joined = " ".join(result.get("reasons") or [])
|
|
self.assertIn(asp.BLOCKER_FOREIGN_LEASE, joined)
|
|
self.assertIn("exact_next_action", joined)
|
|
assess_mock.assert_called()
|
|
# Assessor must run for request_changes_pr (anti_task map).
|
|
call_task = None
|
|
for c in assess_mock.call_args_list:
|
|
kwargs = c.kwargs if c.kwargs else {}
|
|
if "task" in kwargs:
|
|
call_task = kwargs["task"]
|
|
elif c.args:
|
|
call_task = c.args[0] if False else kwargs.get("task")
|
|
# assess_anti_stomp is called with keyword task=
|
|
if c.kwargs.get("task"):
|
|
call_task = c.kwargs["task"]
|
|
# _run_anti_stomp passes task as first positional to assess via keyword.
|
|
self.assertTrue(assess_mock.called)
|
|
api_mock.assert_not_called()
|
|
save_lock.assert_not_called()
|
|
|
|
def test_comment_issue_blocks_before_comment_api(self):
|
|
"""Representative issue-mutation class for AC4 coverage."""
|
|
import gitea_mcp_server as server
|
|
|
|
blocked = self._blocked_assessment(
|
|
kind=asp.BLOCKER_WRONG_ROLE,
|
|
next_action="switch namespace",
|
|
)
|
|
api_mock = mock.Mock(side_effect=AssertionError("comment API must not be called"))
|
|
|
|
with mock.patch.dict(
|
|
os.environ,
|
|
{"GITEA_TEST_FORCE_ANTI_STOMP": "1"},
|
|
clear=False,
|
|
):
|
|
with mock.patch.object(
|
|
server, "verify_preflight_purity", wraps=None
|
|
) as purity, mock.patch.object(
|
|
server, "api_request", api_mock
|
|
):
|
|
# Drive verify_preflight_purity → _run_anti_stomp by calling purity
|
|
# path: patch _run_anti_stomp to raise via assessor.
|
|
def _purity_side_effect(*args, **kwargs):
|
|
# Call real runner under force so assessor block surfaces.
|
|
return server._run_anti_stomp_preflight(
|
|
kwargs.get("task") or (args[2] if len(args) > 2 else None),
|
|
remote=args[0] if args else kwargs.get("remote"),
|
|
worktree_path=kwargs.get("worktree_path"),
|
|
org=kwargs.get("org"),
|
|
repo=kwargs.get("repo"),
|
|
raise_on_block=True,
|
|
)
|
|
|
|
purity.side_effect = None
|
|
with mock.patch.object(
|
|
asp, "assess_anti_stomp_preflight", return_value=blocked
|
|
), mock.patch.object(
|
|
server, "verify_preflight_purity", side_effect=lambda *a, **k: (
|
|
server._run_anti_stomp_preflight(
|
|
k.get("task"),
|
|
remote=a[0] if a else k.get("remote"),
|
|
worktree_path=k.get("worktree_path"),
|
|
org=k.get("org"),
|
|
repo=k.get("repo"),
|
|
)
|
|
)
|
|
), mock.patch.object(
|
|
server, "_profile_operation_gate", return_value=[]
|
|
), mock.patch.object(
|
|
server, "_canonical_comment_gate", return_value={"blocked": False}
|
|
), mock.patch.object(
|
|
server, "get_profile", return_value={
|
|
"profile_name": "prgs-author",
|
|
"allowed_operations": ["gitea.issue.comment", "gitea.read"],
|
|
"forbidden_operations": [],
|
|
}
|
|
):
|
|
with self.assertRaises(RuntimeError) as ctx:
|
|
server.gitea_create_issue_comment(
|
|
issue_number=604,
|
|
body="handoff",
|
|
remote="prgs",
|
|
org="Scaled-Tech-Consulting",
|
|
repo="Gitea-Tools",
|
|
worktree_path="/repo/branches/issue-604",
|
|
)
|
|
self.assertIn(asp.BLOCKER_WRONG_ROLE, str(ctx.exception))
|
|
self.assertIn("exact_next_action", str(ctx.exception))
|
|
api_mock.assert_not_called()
|
|
|
|
|
|
if __name__ == "__main__":
|
|
unittest.main()
|