Scaled-Tech-Consulting/Gitea-Tools
2
0
Fork 0
Code Issues 78 Pull Requests 5 Actions Packages Projects Releases Wiki Activity

Merge branch 'master' into fix/issue-702-stale-binding-lease-recovery

Browse Source
This commit is contained in:
jcwalker3
2026-07-17 10:33:46 -05:00
parent e33d8874a5 a19af4de22
commit 07083eae09
10 changed files with 3511 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
anti_stomp_preflight.py
+807
View File
@@ -0,0 +1,807 @@
"""Common anti-stomp preflight for every MCP mutation tool (#604).
Even with leases, mutation tools need a **shared** preflight that prevents
stale sessions, wrong worktrees, wrong repos, old prompts, terminal locks,
foreign leases, contaminated approvals, and root-checkout mutations from
slipping through.
This module is the pure assessment core. Callers (MCP mutation entrypoints)
gather live facts and pass them in — nothing here performs git, network, or
durable-state I/O. Existing happy-path guards remain authoritative; this
module composes them into one typed, fail-closed result.
Required checks (issue #604):
* repo/org verification
* profile/role verification
* root checkout clean and not used for mutation (when role requires branches/)
* worktree under branches when required
* active lease ownership (when lease is required for the mutation)
* terminal lock status (when applicable)
* expected head SHA (when pinned)
* stale runtime status
* workflow hash (when a workflow load is required)
* source contamination status
* no manual state/mtime/source-file bypass
Failure returns a typed blocker and an exact next action. There is **no**
agent-facing bypass flag.
"""
from __future__ import annotations
from typing import Any
import author_mutation_worktree
import master_parity_gate
import remote_repo_guard
import root_checkout_guard
import stable_branch_push_guard
# ── public constants ──────────────────────────────────────────────────────────
# Typed blocker kinds returned in the structured result.
BLOCKER_WRONG_REPO = "wrong_repo"
BLOCKER_WRONG_ROLE = "wrong_role"
BLOCKER_ROOT_CHECKOUT = "root_checkout_mutation"
BLOCKER_WRONG_WORKTREE = "wrong_worktree"
BLOCKER_FOREIGN_LEASE = "foreign_lease"
BLOCKER_TERMINAL_LOCK = "terminal_lock"
BLOCKER_HEAD_SHA = "head_sha_mismatch"
BLOCKER_STALE_RUNTIME = "stale_runtime"
BLOCKER_WORKFLOW_HASH = "workflow_hash"
BLOCKER_SOURCE_CONTAMINATION = "source_contamination"
BLOCKER_MANUAL_BYPASS = "manual_bypass"
BLOCKER_KINDS = frozenset({
BLOCKER_WRONG_REPO,
BLOCKER_WRONG_ROLE,
BLOCKER_ROOT_CHECKOUT,
BLOCKER_WRONG_WORKTREE,
BLOCKER_FOREIGN_LEASE,
BLOCKER_TERMINAL_LOCK,
BLOCKER_HEAD_SHA,
BLOCKER_STALE_RUNTIME,
BLOCKER_WORKFLOW_HASH,
BLOCKER_SOURCE_CONTAMINATION,
BLOCKER_MANUAL_BYPASS,
})
# Mutation tasks that must invoke the shared anti-stomp preflight before
# acting (issue #604 AC1). Every member must appear as a live task= kwarg
# to verify_preflight_purity / _run_anti_stomp_preflight (or the review
# anti_task map) in gitea_mcp_server.py. Inventory↔wiring tests fail if
# a declared task is not wired.
MUTATION_TASKS = frozenset({
"create_issue",
"comment_issue",
"close_issue",
"mark_issue",
"lock_issue",
"set_issue_labels",
"create_label",
"create_pr",
"close_pr",
"edit_pr",
"commit_files",
"gitea_commit_files",
"delete_branch",
"cleanup_merged_pr_branch",
"cleanup_stale_claims",
"reconcile_merged_cleanups",
"reconcile_already_landed_pr",
"reconcile_close_superseded_pr",
"post_heartbeat",
"acquire_reviewer_pr_lease",
"gitea_acquire_reviewer_pr_lease",
"adopt_merger_pr_lease",
"review_pr",
"submit_pr_review",
"approve_pr",
"request_changes_pr",
"merge_pr",
})
# Intentionally excluded from MUTATION_TASKS. Each has a dedicated
# fail-closed gate that preserves decision-lock ownership, workflow-hash,
# head-SHA, and repository consistency. Do not re-add without either
# wiring shared preflight or updating this rationale (#604 Blocker B).
DEDICATED_GATE_MUTATIONS: dict[str, str] = {
"mark_final_review_decision": (
"Local review-decision lock only. Enforced by session lock ownership, "
"workflow-hash, expected head SHA, PR work lease, eligibility, and "
"terminal-lock gates. No Gitea review POST; shared anti-stomp would "
"duplicate without side-effect-ordering value."
),
"save_review_draft": (
"Local session-state draft save with live PR head/base consistency "
"checks. Not a Gitea review/merge mutation; dedicated head-SHA and "
"worktree resolution gates apply."
),
"resume_review_draft": (
"Live-state resume with terminal lock, lease ownership, head/base SHA, "
"and parity checks. When submit=True, delegates to gitea_submit_pr_review "
"which runs shared anti-stomp before the Gitea review POST."
),
"cleanup_stale_review_decision_lock": (
"Moot-lock cleanup with identity match, live PR merged/closed proof, "
"and reviewer capability gate (#594). Distinct from shared anti-stomp."
),
"gitea_cleanup_stale_review_decision_lock": (
"Alias of cleanup_stale_review_decision_lock; dedicated #594 path."
),
"heartbeat_reviewer_pr_lease": (
"Lease heartbeat posts via verify_preflight_purity(task='review_pr') "
"plus in-session lease ownership checks; not a separate mutation class."
),
"release_reviewer_pr_lease": (
"Lease release uses workspace binding + ownership checks; posts only "
"when the session owns the active lease."
),
}
# Roles that must mutate from a branches/ worktree (not the control checkout).
_BRANCHES_REQUIRED_ROLES = frozenset({"author"})
# Reviewer/merger mutations that require an owned lease + head pinning when
# the caller supplies those facts.
_LEASE_AWARE_TASKS = frozenset({
"review_pr",
"submit_pr_review",
"approve_pr",
"request_changes_pr",
"merge_pr",
"acquire_reviewer_pr_lease",
"gitea_acquire_reviewer_pr_lease",
"adopt_merger_pr_lease",
})
_NEXT_ACTIONS: dict[str, str] = {
BLOCKER_WRONG_REPO: (
"Pass explicit org= and repo= matching the local git remote "
"(e.g. org=Scaled-Tech-Consulting repo=Gitea-Tools) and retry."
),
BLOCKER_WRONG_ROLE: (
"Call gitea_resolve_task_capability, switch to the required "
"namespace/profile, re-verify with gitea_whoami, then retry."
),
BLOCKER_ROOT_CHECKOUT: (
"Leave the root control checkout on clean master; create or switch "
"to a session-owned worktree under branches/ and retry the mutation "
"with worktree_path set."
),
BLOCKER_WRONG_WORKTREE: (
"Create or bind a session-owned worktree under branches/, set "
"worktree_path (or GITEA_ACTIVE_WORKTREE), and retry."
),
BLOCKER_FOREIGN_LEASE: (
"Stop. Do not stomp a foreign lease. Wait for expiry, request "
"takeover through the sanctioned path, or hand off to the owner."
),
BLOCKER_TERMINAL_LOCK: (
"Stop. A terminal review-decision lock is active for this head. "
"Do not re-approve/merge; follow the #332/#620 recovery path."
),
BLOCKER_HEAD_SHA: (
"Re-fetch the live PR head with gitea_view_pr, re-pin "
"expected_head_sha to the current head, re-validate, then retry."
),
BLOCKER_STALE_RUNTIME: (
"Restart the Gitea MCP server so it reloads master's capability "
"gates, re-run gitea_whoami + gitea_resolve_task_capability, then retry."
),
BLOCKER_WORKFLOW_HASH: (
"Reload the canonical workflow via gitea_load_review_workflow, then "
"rerun the full review-merge workflow from inventory (no approve/merge replay)."
),
BLOCKER_SOURCE_CONTAMINATION: (
"Stop. Session is source-contaminated. Hand off to a reconciler for "
"audit/clear; do not clear markers by deleting session-state files."
),
BLOCKER_MANUAL_BYPASS: (
"Stop. Manual state/mtime/source-file bypass is forbidden. Use "
"sanctioned MCP tools only; never delete or rewrite session-state "
"or lock files by hand."
),
}
def is_mutation_task(task: str | None) -> bool:
"""True when *task* is in the #604 mutation set (normalized)."""
name = (task or "").strip().lower().removeprefix("gitea_")
if not name:
return False
if name in MUTATION_TASKS:
return True
# Accept gitea_ prefixed membership for aliases already in the set.
return f"gitea_{name}" in MUTATION_TASKS
def roles_compatible(active_role: str | None, required_role: str | None) -> bool:
"""Whether *active_role* may perform a task that maps to *required_role*.
Exact match always passes. Reconcilers may run author-class mutations
(comment/close/cleanup) that the capability map stamps as ``author`` —
matching the long-standing reconciler exemption for control-checkout
work. No other cross-role substitutions are allowed.
Capability-authorized multi-role tasks (e.g. reviewer holding
``gitea.issue.comment`` for ``comment_issue``) are handled by
:func:`authorization_compatible`, not by expanding this role matrix.
"""
active = (active_role or "").strip().lower()
required = (required_role or "").strip().lower()
if not active or not required:
return True
if active == required:
return True
if active == "reconciler" and required in {"author", "reconciler"}:
return True
return False
def authorization_compatible(
active_role: str | None,
required_role: str | None,
*,
required_permission: str | None = None,
allowed_operations: Any = None,
) -> bool:
"""Whether the active session may run a task given role *and* capability.
Order:
1. :func:`roles_compatible` (exact role or narrow reconciler→author).
2. Capability possession: when *required_permission* is non-empty and
present in *allowed_operations*, allow even if the nominal task role
differs (e.g. reviewer/merger with ``gitea.issue.comment`` on
``comment_issue`` / ``mark_issue`` / ``set_issue_labels`` /
``lock_issue``).
Fail closed otherwise. This is **not** a broad reviewer→author or
merger→author role rewrite: without the specific permission the check
still denies. Missing *allowed_operations* when a permission is required
also fails closed (callers must supply the live profile op list).
"""
if roles_compatible(active_role, required_role):
return True
perm = (required_permission or "").strip()
if not perm:
return False
if allowed_operations is None:
return False
try:
ops = {str(o).strip() for o in allowed_operations if o is not None}
except TypeError:
return False
return perm in ops
def _blocker(
kind: str,
reasons: list[str],
*,
exact_next_action: str | None = None,
detail: dict | None = None,
) -> dict[str, Any]:
if kind not in BLOCKER_KINDS:
raise ValueError(f"unknown anti-stomp blocker kind: {kind!r}")
return {
"kind": kind,
"reasons": list(reasons),
"exact_next_action": exact_next_action or _NEXT_ACTIONS[kind],
"detail": dict(detail or {}),
}
def _allowed_result(checks: dict[str, Any]) -> dict[str, Any]:
return {
"allowed": True,
"block": False,
"blockers": [],
"reasons": [],
"exact_next_action": "proceed",
"blocker_kind": None,
"checks": checks,
}
def _blocked_result(
blockers: list[dict[str, Any]],
checks: dict[str, Any],
) -> dict[str, Any]:
primary = blockers[0]
all_reasons: list[str] = []
for b in blockers:
for r in b.get("reasons") or []:
if r not in all_reasons:
all_reasons.append(r)
return {
"allowed": False,
"block": True,
"blockers": blockers,
"reasons": all_reasons,
"exact_next_action": primary["exact_next_action"],
"blocker_kind": primary["kind"],
"checks": checks,
}
def assess_anti_stomp_preflight(
*,
task: str | None = None,
# repo/org
remote: str | None = None,
resolved_org: str | None = None,
resolved_repo: str | None = None,
local_remote_url: str | None = None,
org_explicit: bool = False,
repo_explicit: bool = False,
check_repo: bool = True,
# profile/role + capability
profile_name: str | None = None,
profile_role: str | None = None,
required_role: str | None = None,
required_permission: str | None = None,
allowed_operations: Any = None,
check_role: bool = True,
# root checkout + worktree
workspace_path: str | None = None,
project_root: str | None = None,
current_branch: str | None = None,
root_head_sha: str | None = None,
root_porcelain: str | None = None,
remote_master_sha: str | None = None,
check_root_checkout: bool = True,
check_worktree: bool = True,
# stale runtime (master parity)
startup_head: str | None = None,
current_code_head: str | None = None,
check_stale_runtime: bool = True,
# lease ownership
lease_required: bool = False,
foreign_lease: bool | None = None,
lease_owner_session: str | None = None,
active_session_id: str | None = None,
lease_owner_identity: str | None = None,
active_identity: str | None = None,
lease_reasons: list[str] | None = None,
# terminal lock
terminal_lock_blocks: bool | None = None,
terminal_lock_reasons: list[str] | None = None,
# expected head SHA
require_head_sha: bool = False,
expected_head_sha: str | None = None,
live_head_sha: str | None = None,
# workflow hash
workflow_hash_valid: bool | None = None,
workflow_hash_reasons: list[str] | None = None,
# source contamination (stable-branch push / approval contamination)
source_contaminated: bool | None = None,
contamination_reasons: list[str] | None = None,
# manual bypass
manual_bypass_attempted: bool = False,
manual_bypass_reasons: list[str] | None = None,
) -> dict[str, Any]:
"""Fail-closed common anti-stomp assessment (pure).
Only checks for which facts are supplied (or which are required by the
mutation category) are evaluated. Unsupplied optional facts are recorded
as ``skipped`` so callers can see coverage without inventing defaults.
Returns a structured dict with ``allowed``/``block``, typed ``blockers``,
aggregated ``reasons``, ``exact_next_action``, and per-check ``checks``.
"""
task_name = (task or "").strip()
role = (profile_role or "").strip().lower() or None
req_role = (required_role or "").strip().lower() or None
req_perm = (required_permission or "").strip() or None
checks: dict[str, Any] = {
"task": task_name or None,
"profile_name": profile_name,
"profile_role": role,
"required_role": req_role,
"required_permission": req_perm,
}
blockers: list[dict[str, Any]] = []
# ── manual bypass (always first; never skippable when attempted) ─────────
if manual_bypass_attempted:
reasons = list(manual_bypass_reasons or [
"manual state/mtime/source-file bypass attempt detected (fail closed)"
])
blockers.append(_blocker(BLOCKER_MANUAL_BYPASS, reasons))
checks["manual_bypass"] = {"block": True, "reasons": reasons}
else:
checks["manual_bypass"] = {"block": False, "skipped": False}
# ── repo/org ─────────────────────────────────────────────────────────────
if check_repo and resolved_org is not None and resolved_repo is not None:
repo_assessment = remote_repo_guard.assess_remote_repo_match(
remote=remote or "",
resolved_org=resolved_org,
resolved_repo=resolved_repo,
local_remote_url=local_remote_url,
org_explicit=org_explicit,
repo_explicit=repo_explicit,
)
checks["repo"] = {
"block": bool(repo_assessment.get("block")),
"reasons": list(repo_assessment.get("reasons") or []),
"resolved_org": resolved_org,
"resolved_repo": resolved_repo,
}
if repo_assessment.get("block"):
blockers.append(
_blocker(
BLOCKER_WRONG_REPO,
list(repo_assessment.get("reasons") or ["repo/org mismatch"]),
detail={
"resolved_org": resolved_org,
"resolved_repo": resolved_repo,
"local_remote_url": local_remote_url,
},
)
)
else:
checks["repo"] = {"block": False, "skipped": True}
# ── profile/role + capability ────────────────────────────────────────────
# Role match OR possession of the task's required permission (Blocker A).
# Reviewer/merger may run issue-comment-class tasks when they hold
# gitea.issue.comment; unauthorized escalation without the permission
# still fails closed.
if check_role and req_role and role:
authorized = authorization_compatible(
role,
req_role,
required_permission=req_perm,
allowed_operations=allowed_operations,
)
if not authorized:
perm_clause = (
f", required_permission='{req_perm}'" if req_perm else ""
)
reasons = [
f"active profile role '{role}' is not authorized for task "
f"'{task_name or '(unknown)'}' (required_role='{req_role}'"
f"{perm_clause}; profile={profile_name or '(unknown)'})"
]
checks["role"] = {
"block": True,
"reasons": reasons,
"required_permission": req_perm,
"capability_authorized": False,
}
blockers.append(
_blocker(
BLOCKER_WRONG_ROLE,
reasons,
detail={
"profile_name": profile_name,
"profile_role": role,
"required_role": req_role,
"required_permission": req_perm,
},
)
)
else:
checks["role"] = {
"block": False,
"reasons": [],
"required_permission": req_perm,
"capability_authorized": bool(
req_perm
and allowed_operations is not None
and req_perm in {
str(o).strip() for o in (allowed_operations or [])
if o is not None
}
and not roles_compatible(role, req_role)
),
}
else:
checks["role"] = {"block": False, "skipped": True}
# ── stale runtime (master parity) ────────────────────────────────────────
if check_stale_runtime and (
startup_head is not None or current_code_head is not None
):
parity = master_parity_gate.assess_master_parity(
{"startup_head": startup_head},
current_code_head,
)
stale_reasons = master_parity_gate.parity_block_reasons(parity)
checks["stale_runtime"] = {
"block": bool(stale_reasons),
"reasons": list(stale_reasons),
"startup_head": startup_head,
"current_code_head": current_code_head,
"stale": bool(parity.get("stale")),
}
if stale_reasons:
blockers.append(
_blocker(
BLOCKER_STALE_RUNTIME,
list(stale_reasons),
detail={
"startup_head": startup_head,
"current_code_head": current_code_head,
},
)
)
else:
checks["stale_runtime"] = {"block": False, "skipped": True}
# ── root checkout ────────────────────────────────────────────────────────
if (
check_root_checkout
and workspace_path is not None
and project_root is not None
and root_porcelain is not None
):
root_assessment = root_checkout_guard.assess_root_checkout_guard(
workspace_path=workspace_path,
canonical_repo_root=project_root,
current_branch=current_branch,
head_sha=root_head_sha,
porcelain_status=root_porcelain,
remote_master_sha=remote_master_sha,
resolved_role=req_role or role,
actual_role=role,
)
checks["root_checkout"] = {
"block": bool(root_assessment.get("block")),
"reasons": list(root_assessment.get("reasons") or []),
}
if root_assessment.get("block"):
blockers.append(
_blocker(
BLOCKER_ROOT_CHECKOUT,
list(root_assessment.get("reasons") or [
"control checkout is not clean master"
]),
detail={
"workspace_path": workspace_path,
"project_root": project_root,
"current_branch": current_branch,
},
)
)
else:
checks["root_checkout"] = {"block": False, "skipped": True}
# ── worktree under branches (author) ─────────────────────────────────────
worktree_role = role or req_role
if (
check_worktree
and workspace_path is not None
and project_root is not None
and worktree_role in _BRANCHES_REQUIRED_ROLES
):
wt = author_mutation_worktree.assess_author_mutation_worktree(
workspace_path=workspace_path,
project_root=project_root,
current_branch=current_branch,
)
checks["worktree"] = {
"block": bool(wt.get("block")),
"reasons": list(wt.get("reasons") or []),
"under_branches": wt.get("under_branches"),
}
if wt.get("block"):
blockers.append(
_blocker(
BLOCKER_WRONG_WORKTREE,
list(wt.get("reasons") or [
"mutation worktree is not under branches/"
]),
detail={
"workspace_path": workspace_path,
"project_root": project_root,
},
)
)
else:
checks["worktree"] = {
"block": False,
"skipped": worktree_role not in _BRANCHES_REQUIRED_ROLES
or workspace_path is None,
}
# ── foreign lease ────────────────────────────────────────────────────────
lease_needed = lease_required or (
task_name.removeprefix("gitea_") in {
t.removeprefix("gitea_") for t in _LEASE_AWARE_TASKS
}
and foreign_lease is not None
)
if lease_needed or foreign_lease is True:
is_foreign = bool(foreign_lease)
if foreign_lease is None and lease_required:
# Ownership must be proven when lease_required; missing ownership
# facts fail closed.
owner_ok = (
(not lease_owner_session and not active_session_id)
or (
lease_owner_session
and active_session_id
and lease_owner_session == active_session_id
)
)
identity_ok = (
(not lease_owner_identity and not active_identity)
or (
lease_owner_identity
and active_identity
and lease_owner_identity == active_identity
)
)
if not owner_ok or not identity_ok:
is_foreign = True
reasons = list(lease_reasons or [])
if is_foreign and not reasons:
reasons = [
"active lease is owned by a foreign session or identity "
"(anti-stomp fail closed)"
]
checks["lease"] = {
"block": is_foreign,
"reasons": reasons if is_foreign else [],
"lease_owner_session": lease_owner_session,
"active_session_id": active_session_id,
}
if is_foreign:
blockers.append(
_blocker(BLOCKER_FOREIGN_LEASE, reasons)
)
else:
checks["lease"] = {"block": False, "skipped": True}
# ── terminal lock ────────────────────────────────────────────────────────
if terminal_lock_blocks is not None:
reasons = list(terminal_lock_reasons or [])
if terminal_lock_blocks and not reasons:
reasons = [
"terminal review-decision lock is active for this head "
"(anti-stomp fail closed)"
]
checks["terminal_lock"] = {
"block": bool(terminal_lock_blocks),
"reasons": reasons if terminal_lock_blocks else [],
}
if terminal_lock_blocks:
blockers.append(_blocker(BLOCKER_TERMINAL_LOCK, reasons))
else:
checks["terminal_lock"] = {"block": False, "skipped": True}
# ── expected head SHA ────────────────────────────────────────────────────
if require_head_sha or (
expected_head_sha is not None and live_head_sha is not None
):
exp = (expected_head_sha or "").strip().lower()
live = (live_head_sha or "").strip().lower()
mismatch = False
reasons: list[str] = []
if require_head_sha and not exp:
mismatch = True
reasons.append(
"expected_head_sha is required before this mutation "
"(anti-stomp fail closed)"
)
elif exp and live and exp != live:
mismatch = True
reasons.append(
f"expected_head_sha '{exp}' does not match live PR head "
f"'{live}' (anti-stomp fail closed; stale prompt data)"
)
elif require_head_sha and exp and not live:
mismatch = True
reasons.append(
"live PR head SHA could not be determined to corroborate "
"expected_head_sha (anti-stomp fail closed)"
)
checks["head_sha"] = {
"block": mismatch,
"reasons": reasons,
"expected_head_sha": expected_head_sha,
"live_head_sha": live_head_sha,
}
if mismatch:
blockers.append(_blocker(BLOCKER_HEAD_SHA, reasons))
else:
checks["head_sha"] = {"block": False, "skipped": True}
# ── workflow hash ────────────────────────────────────────────────────────
if workflow_hash_valid is not None:
reasons = list(workflow_hash_reasons or [])
if not workflow_hash_valid and not reasons:
reasons = [
"workflow load proof missing or hash stale "
"(anti-stomp fail closed)"
]
checks["workflow_hash"] = {
"block": not bool(workflow_hash_valid),
"reasons": reasons if not workflow_hash_valid else [],
}
if not workflow_hash_valid:
blockers.append(_blocker(BLOCKER_WORKFLOW_HASH, reasons))
else:
checks["workflow_hash"] = {"block": False, "skipped": True}
# ── source contamination ─────────────────────────────────────────────────
if source_contaminated is not None:
reasons = list(contamination_reasons or [])
if source_contaminated and not reasons:
reasons = [
"session is source-contaminated (stable-branch push or "
"contaminated approval path); anti-stomp fail closed"
]
checks["source_contamination"] = {
"block": bool(source_contaminated),
"reasons": reasons if source_contaminated else [],
}
if source_contaminated:
blockers.append(
_blocker(BLOCKER_SOURCE_CONTAMINATION, reasons)
)
else:
checks["source_contamination"] = {"block": False, "skipped": True}
if blockers:
return _blocked_result(blockers, checks)
return _allowed_result(checks)
def format_anti_stomp_error(assessment: dict[str, Any]) -> str:
"""Single RuntimeError message for MCP mutation gates."""
kind = assessment.get("blocker_kind") or "unknown"
reasons = "; ".join(
assessment.get("reasons")
or ["anti-stomp preflight blocked the mutation"]
)
next_action = assessment.get("exact_next_action") or "stop and diagnose"
return (
f"Anti-stomp preflight (#604) blocked mutation "
f"[{kind}]: {reasons}. "
f"exact_next_action: {next_action}"
)
def block_response(
assessment: dict[str, Any],
**extra_fields: Any,
) -> dict[str, Any]:
"""Structured tool-return payload for a blocked mutation."""
payload = {
"success": False,
"performed": False,
"blocked": True,
"anti_stomp": True,
"blocker_kind": assessment.get("blocker_kind"),
"blockers": list(assessment.get("blockers") or []),
"reasons": list(assessment.get("reasons") or []),
"exact_next_action": assessment.get("exact_next_action"),
"checks": assessment.get("checks") or {},
}
payload.update(extra_fields)
return payload
def contamination_from_stable_marker(
marker: dict | None,
*,
task: str | None,
actual_role: str | None,
) -> tuple[bool, list[str]]:
"""Derive source-contamination facts from a #671 durable marker."""
if not marker:
return False, []
gate = stable_branch_push_guard.assess_contamination_gate(
marker,
task=task,
actual_role=actual_role,
)
if gate.get("block"):
return True, list(gate.get("reasons") or [])
return False, []
gitea_mcp_server.py
+640 -25
View File
@@ -755,6 +755,188 @@ def _enforce_branches_only_author_mutation(worktree_path: str | None = None) ->
)
def _anti_stomp_in_test_mode() -> bool:
"""Whether the #604 anti-stomp gate is skipped under pytest.
Mirrors remote-repo / stable-contamination test bypasses so the existing
suite (bare remotes, mocked APIs) stays green. Set
``GITEA_TEST_FORCE_ANTI_STOMP=1`` to exercise the live gate under tests.
"""
if not _preflight_in_test_mode():
return False
return not bool(os.environ.get("GITEA_TEST_FORCE_ANTI_STOMP"))
def _run_anti_stomp_preflight(
task: str | None,
*,
remote: str | None = None,
worktree_path: str | None = None,
host: str | None = None,
org: str | None = None,
repo: str | None = None,
expected_head_sha: str | None = None,
live_head_sha: str | None = None,
require_head_sha: bool = False,
lease_required: bool = False,
foreign_lease: bool | None = None,
lease_reasons: list[str] | None = None,
terminal_lock_blocks: bool | None = None,
terminal_lock_reasons: list[str] | None = None,
workflow_hash_valid: bool | None = None,
workflow_hash_reasons: list[str] | None = None,
raise_on_block: bool = True,
) -> dict | None:
"""Shared #604 anti-stomp preflight for MCP mutation entrypoints.
Gathers live workspace/parity/role/repo facts and invokes the pure
:func:`anti_stomp_preflight.assess_anti_stomp_preflight` assessor. On
block, raises ``RuntimeError`` (default) or returns a structured
:func:`anti_stomp_preflight.block_response` when *raise_on_block* is
False. Returns ``None`` when the mutation may proceed.
"""
if _anti_stomp_in_test_mode():
return None
# Only enforce when the caller names a known mutation task. Read-only
# paths and legacy verify_preflight_purity calls without a task stay
# on the pre-#604 gate chain (whoami/capability/root/worktree).
if not task or not anti_stomp_preflight.is_mutation_task(task):
return None
profile = get_profile()
profile_name = profile.get("profile_name")
profile_role = _actual_profile_role()
allowed_operations = list(profile.get("allowed_operations") or [])
req_role = None
req_perm = None
if task:
try:
req_role = task_capability_map.required_role(task)
except KeyError:
req_role = _preflight_resolved_role
try:
req_perm = task_capability_map.required_permission(task)
except KeyError:
req_perm = None
ctx = _resolve_namespace_mutation_context(worktree_path)
workspace = ctx["workspace_path"]
canonical_root = ctx["canonical_repo_root"]
git_state = issue_lock_worktree.read_worktree_git_state(canonical_root)
remote_master_sha = root_checkout_guard.resolve_remote_master_sha(canonical_root)
# Repo/org facts (best-effort; explicit org/repo when provided).
resolved_org = org
resolved_repo = repo
local_remote_url = None
org_explicit = org is not None
repo_explicit = repo is not None
if remote:
try:
local_remote_url = _local_git_remote_url(remote)
except Exception:
local_remote_url = None
if resolved_org is None or resolved_repo is None:
try:
rem = _effective_remote(remote)
if rem in REMOTES:
if resolved_org is None:
resolved_org = REMOTES[rem]["org"]
if resolved_repo is None:
resolved_repo = REMOTES[rem]["repo"]
except Exception:
pass
parity = _current_master_parity()
startup_head = parity.get("startup_head")
current_code_head = parity.get("current_head")
# Source contamination from durable #671 marker (role-aware).
source_contaminated = None
contamination_reasons = None
try:
marker = _load_stable_contamination_marker(remote)
contaminated, cont_reasons = anti_stomp_preflight.contamination_from_stable_marker(
marker,
task=task,
actual_role=profile_role,
)
if marker is not None:
source_contaminated = contaminated
contamination_reasons = cont_reasons
except Exception:
# Fail soft on marker I/O: existing #671 enforcer still runs separately.
source_contaminated = None
# Workflow-hash facts when the task is review/merge oriented.
if workflow_hash_valid is None and task in {
"review_pr",
"submit_pr_review",
"approve_pr",
"request_changes_pr",
"merge_pr",
}:
try:
wf_reasons = _review_workflow_load_gate_reasons()
workflow_hash_valid = not bool(wf_reasons)
workflow_hash_reasons = list(wf_reasons) if wf_reasons else None
except Exception:
pass
# Mirror remote_repo_guard's pytest bypass: under the unit suite bare
# remote=prgs still defaults to Timesheet, and re-checking here would
# break happy-path reconciler/author tests that already rely on the
# #530 gate being opt-in via GITEA_FORCE_REMOTE_REPO_CHECK.
check_repo = True
if "pytest" in sys.modules and not os.environ.get(
"GITEA_FORCE_REMOTE_REPO_CHECK"
):
check_repo = False
assessment = anti_stomp_preflight.assess_anti_stomp_preflight(
task=task,
remote=remote,
resolved_org=resolved_org,
resolved_repo=resolved_repo,
local_remote_url=local_remote_url,
org_explicit=org_explicit,
repo_explicit=repo_explicit,
check_repo=check_repo,
profile_name=profile_name,
profile_role=profile_role,
required_role=req_role or _preflight_resolved_role,
required_permission=req_perm,
allowed_operations=allowed_operations,
workspace_path=workspace,
project_root=canonical_root,
current_branch=git_state.get("current_branch"),
root_head_sha=git_state.get("head_sha"),
root_porcelain=git_state.get("porcelain_status") or "",
remote_master_sha=remote_master_sha,
startup_head=startup_head,
current_code_head=current_code_head,
lease_required=lease_required,
foreign_lease=foreign_lease,
lease_reasons=lease_reasons,
terminal_lock_blocks=terminal_lock_blocks,
terminal_lock_reasons=terminal_lock_reasons,
require_head_sha=require_head_sha,
expected_head_sha=expected_head_sha,
live_head_sha=live_head_sha,
workflow_hash_valid=workflow_hash_valid,
workflow_hash_reasons=workflow_hash_reasons,
source_contaminated=source_contaminated,
contamination_reasons=contamination_reasons,
manual_bypass_attempted=False,
)
if not assessment.get("block"):
return None
if raise_on_block:
raise RuntimeError(anti_stomp_preflight.format_anti_stomp_error(assessment))
return anti_stomp_preflight.block_response(assessment)
def verify_preflight_purity(
remote: str | None = None,
worktree_path: str | None = None,
@@ -762,13 +944,29 @@ def verify_preflight_purity(
*,
target_issue_number: int | None = None,
require_author_lock: bool = False,
expected_head_sha: str | None = None,
live_head_sha: str | None = None,
require_head_sha: bool = False,
lease_required: bool = False,
foreign_lease: bool | None = None,
lease_reasons: list[str] | None = None,
terminal_lock_blocks: bool | None = None,
terminal_lock_reasons: list[str] | None = None,
workflow_hash_valid: bool | None = None,
workflow_hash_reasons: list[str] | None = None,
org: str | None = None,
repo: str | None = None,
):
"""Verify identity/capability order, then production workspace guards.
"""Verify identity/capability order, production workspace guards, then anti-stomp.
#683: pytest/unittest must not skip production root/branches/scope
enforcement when force-on signals request production behavior. The
early return below only skips *preflight-order* purity checks under
pure unit-test isolation — never when production guards are active.
#604: also runs the shared anti-stomp preflight for mutation tasks (typed
blocker + exact next action). Extended lease/head/terminal facts may be
supplied by review/merge callers.
"""
global _preflight_reviewer_violation_files
@@ -779,7 +977,12 @@ def verify_preflight_purity(
# Pure unit-test isolation: skip purity-order unless legacy dirty/porcelain
# force flags request the dirtiness path. #683 FORCE_PRODUCTION_GUARDS alone
# runs production root/branches/scope without requiring whoami/capability.
skip_purity_order = in_test and not workflow_scope_guard.purity_order_forced()
force_anti_stomp = bool(os.environ.get("GITEA_TEST_FORCE_ANTI_STOMP"))
skip_purity_order = (
in_test
and not workflow_scope_guard.purity_order_forced()
and not force_anti_stomp
)
if not skip_purity_order:
if not _preflight_whoami_called:
@@ -874,6 +1077,25 @@ def verify_preflight_purity(
target_issue_number=target_issue_number,
require_author_lock=require_author_lock,
)
# #604: common anti-stomp preflight after legacy + #683 enforcers.
_run_anti_stomp_preflight(
task,
remote=remote,
worktree_path=worktree_path,
org=org,
repo=repo,
expected_head_sha=expected_head_sha,
live_head_sha=live_head_sha,
require_head_sha=require_head_sha,
lease_required=lease_required,
foreign_lease=foreign_lease,
lease_reasons=lease_reasons,
terminal_lock_blocks=terminal_lock_blocks,
terminal_lock_reasons=terminal_lock_reasons,
workflow_hash_valid=workflow_hash_valid,
workflow_hash_reasons=workflow_hash_reasons,
raise_on_block=True,
)
_clear_preflight_capability_state()
return
@@ -888,6 +1110,25 @@ def verify_preflight_purity(
target_issue_number=target_issue_number,
require_author_lock=require_author_lock,
)
if force_anti_stomp:
_run_anti_stomp_preflight(
task,
remote=remote,
worktree_path=worktree_path,
org=org,
repo=repo,
expected_head_sha=expected_head_sha,
live_head_sha=live_head_sha,
require_head_sha=require_head_sha,
lease_required=lease_required,
foreign_lease=foreign_lease,
lease_reasons=lease_reasons,
terminal_lock_blocks=terminal_lock_blocks,
terminal_lock_reasons=terminal_lock_reasons,
workflow_hash_valid=workflow_hash_valid,
workflow_hash_reasons=workflow_hash_reasons,
raise_on_block=True,
)
def _session_issue_lock_snapshot(
@@ -1256,6 +1497,7 @@ import root_checkout_guard # noqa: E402
import workflow_scope_guard # noqa: E402 # #683 production scope / force-on guards
import stable_branch_push_guard # noqa: E402
import remote_repo_guard # noqa: E402
import anti_stomp_preflight # noqa: E402
import issue_claim_heartbeat # noqa: E402
import issue_work_duplicate_gate # noqa: E402
import issue_workflow_labels # noqa: E402
@@ -3617,6 +3859,8 @@ def init_review_decision_lock(remote: str | None, task: str | None, force: bool
"live_mutations": [],
"correction_authorized": False,
"correction_reason": None,
"correction_pr_number": None,
"correction_head_sha": None,
})
@@ -3712,21 +3956,30 @@ def check_review_decision_gate(
if stale_review_decision_lock.prior_live_mutations_block_boundary(
lock, pr_number=pr_number, expected_head_sha=ready_head
):
if prior and not lock.get("correction_authorized"):
scoped = stale_review_decision_lock.correction_applies(
lock, pr_number=pr_number, expected_head_sha=ready_head
)
if prior and not scoped:
reasons.append(
"live review mutation already recorded for this PR head in this "
"run; only one live review mutation is allowed per head unless "
"gitea_authorize_review_correction was invoked (fail closed, #620)"
"gitea_authorize_review_correction was invoked for this same "
"PR/head (fail closed, #620/#693)"
)
elif (
action in _TERMINAL_REVIEW_ACTIONS
and any(m.get("action") in _TERMINAL_REVIEW_ACTIONS for m in prior)
and not lock.get("correction_authorized")
and any(
isinstance(m, dict)
and m.get("action") in _TERMINAL_REVIEW_ACTIONS
and m.get("pr_number") == pr_number
for m in prior
)
and not scoped
):
reasons.append(
"terminal review decision already submitted for this PR head in "
"this run; blocked unless an operator-approved correction was "
"authorized (fail closed, #620)"
"authorized for this same PR/head (fail closed, #620/#693)"
)
return reasons
@@ -3755,6 +4008,8 @@ def record_live_review_mutation(pr_number: int, action: str, review_id: int | No
if lock.get("correction_authorized"):
lock["correction_authorized"] = False
lock["correction_reason"] = None
lock["correction_pr_number"] = None
lock["correction_head_sha"] = None
_save_review_decision_lock(lock)
@@ -3798,11 +4053,13 @@ def terminal_review_hard_stop_reasons(
and last.get("pr_number") == pr_number
):
return []
if operation in ("mark_ready", "review", "resume") and lock.get(
"correction_authorized"
if operation in ("mark_ready", "review", "resume") and (
stale_review_decision_lock.correction_applies(
lock, pr_number=pr_number, expected_head_sha=expected_head_sha
)
):
return []
# #620: same PR, different reviewed head → fresh decision boundary.
# #620 same-PR new head + #693 cross-PR isolation.
if stale_review_decision_lock.terminal_boundary_allows_fresh_decision(
lock,
pr_number=pr_number,
@@ -3824,16 +4081,20 @@ def terminal_review_hard_stop_reasons(
else ""
)
recovery = (
"if that PR is already merged/closed, use "
"gitea_cleanup_stale_review_decision_lock (apply=true) after live-state "
"proof (#594); if the PR is still open but the head moved, mark/submit "
"with the new expected_head_sha (#620); never delete session-state "
"files by hand"
"exact_next_action: if that PR is already merged/closed, use "
"gitea_cleanup_stale_review_decision_lock(apply=true) after live-state "
"proof (#594); if the same PR head moved, mark/submit with the new "
"expected_head_sha (#620); if the target is a different PR, "
"gitea_diagnose_review_decision_lock then mark_final on the target "
"(cross-PR isolation, #693); if same-head verdict was mistaken, "
"gitea_authorize_review_correction with matching target_pr_number "
"(not a generic unlock); never delete session-state files by hand; "
"if still blocked create/update a durable Gitea issue and stop"
)
return [
"terminal review mutation already consumed in this run "
f"({last.get('action')} on PR #{last.get('pr_number')}{head_note}); "
f"{guidance} (fail closed, #332); {recovery}"
f"{guidance} (fail closed, #332/#693); {recovery}"
]
@@ -4393,6 +4654,30 @@ def _evaluate_pr_review_submission(
result["pr_work_lease"] = lease_block
return result
if live:
# #604: common anti-stomp with live head + lease proof (typed blocker).
anti_task = {
"approve": "approve_pr",
"request_changes": "request_changes_pr",
"comment": "submit_pr_review",
}.get(action, "submit_pr_review")
try:
_run_anti_stomp_preflight(
anti_task,
remote=remote,
worktree_path=worktree_path,
org=org,
repo=repo,
expected_head_sha=pinned_sha,
live_head_sha=actual_sha,
require_head_sha=True,
foreign_lease=False,
terminal_lock_blocks=False,
)
except RuntimeError as exc:
reasons.append(str(exc))
return result
if (body or "").strip():
gate = _canonical_comment_gate(body, context="pr_review")
if gate["blocked"]:
@@ -4498,10 +4783,33 @@ def gitea_mark_final_review_decision(
"reasons": [
"review decision lock missing; resolve review_pr capability first"
],
"exact_next_action": (
"gitea_resolve_task_capability(task='review_pr') then retry "
"mark_final"
),
"durable_issue_handoff": {
"required": True,
"instruction": (
"If the lock cannot be seeded, create/update a durable "
"Gitea issue and stop (#693 AC8)"
),
},
}
session_reasons = _review_decision_session_reasons(lock)
if session_reasons:
return {"marked_ready": False, "reasons": session_reasons}
classification = stale_review_decision_lock.classify_review_decision_lock(
lock,
target_pr_number=pr_number,
target_head_sha=expected_head_sha,
session_blockers=session_reasons,
)
fail = stale_review_decision_lock.build_precise_mark_final_failure(
classification
)
return {
"marked_ready": False,
**fail,
}
if remote != lock.get("remote"):
return {
"marked_ready": False,
@@ -4509,6 +4817,7 @@ def gitea_mark_final_review_decision(
f"requested remote '{remote}' does not match locked remote "
f"'{lock.get('remote')}' (fail closed)"
],
"exact_next_action": "use the remote bound on the decision lock",
}
try:
_, resolved_org, resolved_repo = _resolve(remote, None, org, repo)
@@ -4543,7 +4852,26 @@ def gitea_mark_final_review_decision(
pr_number, "mark_ready", expected_head_sha=expected_head_sha
)
if hard_stop:
return {"marked_ready": False, "reasons": hard_stop}
classification = stale_review_decision_lock.classify_review_decision_lock(
lock,
target_pr_number=pr_number,
target_head_sha=expected_head_sha,
active_profile_identity=_decision_lock_binding().get(
"profile_identity"
),
)
fail = stale_review_decision_lock.build_precise_mark_final_failure(
classification
)
# Prefer classification next_action when more precise; keep hard_stop
# text as primary reasons for operators.
return {
"marked_ready": False,
"reasons": hard_stop + list(fail.get("reasons") or []),
"classification": fail.get("classification"),
"exact_next_action": fail.get("exact_next_action"),
"durable_issue_handoff": fail.get("durable_issue_handoff"),
}
if action not in _REVIEW_ACTIONS:
return {
"marked_ready": False,
@@ -4559,17 +4887,63 @@ def gitea_mark_final_review_decision(
"expected_head_sha required before marking final review "
"decision (fail closed, #399)"
],
"exact_next_action": "retry mark_final with expected_head_sha pin",
}
# Safe idempotency: already ready for same PR/action/head (#693 AC4).
ready_head = stale_review_decision_lock.normalize_head_sha(
lock.get("ready_expected_head_sha")
)
target_head = stale_review_decision_lock.normalize_head_sha(expected_head_sha)
if (
lock.get("final_review_decision_ready")
and lock.get("ready_pr_number") == pr_number
and lock.get("ready_action") == action
and ready_head
and target_head
and stale_review_decision_lock.heads_equal(ready_head, target_head)
and lock.get("ready_remote") == remote
and lock.get("ready_org") == org
and lock.get("ready_repo") == repo
):
return {
"marked_ready": True,
"idempotent": True,
"pr_number": pr_number,
"action": action,
"expected_head_sha": expected_head_sha,
"remote": remote,
"org": org,
"repo": repo,
"final_review_decision_ready": True,
"head_scoped": True,
"reasons": [
"final review decision already marked ready for this PR/action/"
"head (idempotent, #693)"
],
}
if stale_review_decision_lock.prior_live_mutations_block_boundary(
lock, pr_number=pr_number, expected_head_sha=expected_head_sha
):
classification = stale_review_decision_lock.classify_review_decision_lock(
lock,
target_pr_number=pr_number,
target_head_sha=expected_head_sha,
)
fail = stale_review_decision_lock.build_precise_mark_final_failure(
classification
)
return {
"marked_ready": False,
"reasons": [
"cannot mark final decision after a live review mutation was "
"already recorded for this PR head unless an operator-approved "
"correction was authorized (fail closed, #620)"
],
"correction was authorized for this same PR/head "
"(fail closed, #620/#693)"
]
+ list(fail.get("reasons") or []),
"classification": fail.get("classification"),
"exact_next_action": fail.get("exact_next_action"),
"durable_issue_handoff": fail.get("durable_issue_handoff"),
}
elig = gitea_check_pr_eligibility(
pr_number=pr_number,
@@ -4653,8 +5027,19 @@ def gitea_authorize_review_correction(
prior_review_state: str,
reason: str,
operator_authorized: bool = False,
target_pr_number: int | None = None,
expected_head_sha: str | None = None,
remote: str = "dadeschools",
org: str | None = None,
repo: str | None = None,
post_audit_comment: bool = True,
) -> dict:
"""Authorize one operator-approved correction after a mistaken live review."""
"""Authorize one operator-approved correction after a mistaken live review.
#693: correction is scoped to the named prior review's **PR and head**.
It cannot unlock a different PR (no generic decision-lock bypass).
Successful authorization posts a thread-visible audit on the target PR.
"""
reason = (reason or "").strip()
prior_review_state = (prior_review_state or "").strip().lower()
lock = _load_review_decision_lock()
@@ -4680,7 +5065,20 @@ def gitea_authorize_review_correction(
last_mutation = prior[-1]
last_review_id = last_mutation.get("review_id")
last_review_state = last_mutation.get("action")
last_pr = last_mutation.get("pr_number")
last_head = stale_review_decision_lock.mutation_head_sha(last_mutation, lock)
reasons = []
if target_pr_number is None:
reasons.append(
"target_pr_number is required so correction cannot become a "
"generic unlock (fail closed, #693)"
)
elif last_pr is not None and int(target_pr_number) != int(last_pr):
reasons.append(
f"target_pr_number #{target_pr_number} does not match last "
f"recorded mutation PR #{last_pr}; correction cannot unlock a "
f"different PR (fail closed, #693)"
)
if last_review_id is not None and last_review_id != prior_review_id:
reasons.append(
f"prior review ID '{prior_review_id}' does not match last "
@@ -4691,14 +5089,202 @@ def gitea_authorize_review_correction(
f"prior review state '{prior_review_state}' does not match last "
f"recorded review state '{last_review_state}' (fail closed)"
)
want_head = stale_review_decision_lock.normalize_head_sha(expected_head_sha)
if want_head and last_head and not stale_review_decision_lock.heads_equal(
want_head, last_head
):
reasons.append(
f"expected_head_sha does not match last mutation head "
f"{last_head[:12]}… (fail closed, #693)"
)
if not reason:
reasons.append("correction reason is required")
try:
actor = None
h, o, r = _resolve(remote, None, org, repo)
try:
actor = _authenticated_username(h)
except Exception:
actor = None
except Exception as exc: # noqa: BLE001
return {
"authorized": False,
"reasons": [f"could not resolve remote for audit: {_redact(str(exc))}"],
}
profile = get_profile()
profile_name = (profile.get("profile_name") or "").strip() or None
audit = stale_review_decision_lock.build_correction_audit_record(
prior_review_id=prior_review_id,
prior_review_state=prior_review_state,
target_pr_number=target_pr_number if target_pr_number is not None else last_pr,
target_head_sha=want_head or last_head,
reason=reason,
actor_username=actor,
profile_name=profile_name,
authorized=False,
)
if reasons:
return {"authorized": False, "reasons": reasons}
return {
"authorized": False,
"reasons": reasons,
"audit": audit,
"exact_next_action": (
"use same-PR correction only, or for a different PR use "
"gitea_diagnose_review_decision_lock / cross-PR isolation "
"(#693) or moot cleanup (#594)"
),
}
scope_pr = int(target_pr_number) if target_pr_number is not None else int(last_pr)
scope_head = want_head or last_head
lock["correction_authorized"] = True
lock["correction_reason"] = reason
lock["correction_pr_number"] = scope_pr
lock["correction_head_sha"] = scope_head
_save_review_decision_lock(lock)
return {"authorized": True, "correction_reason": reason, "reasons": []}
audit = stale_review_decision_lock.build_correction_audit_record(
prior_review_id=prior_review_id,
prior_review_state=prior_review_state,
target_pr_number=scope_pr,
target_head_sha=scope_head,
reason=reason,
actor_username=actor,
profile_name=profile_name,
authorized=True,
)
report = {
"authorized": True,
"correction_reason": reason,
"correction_pr_number": scope_pr,
"correction_head_sha": scope_head,
"audit": audit,
"audit_comment_id": None,
"reasons": [],
"scope": "same_pr_head_only",
}
if post_audit_comment and scope_pr is not None:
comment_block = _profile_operation_gate("gitea.pr.comment")
if comment_block:
report["audit_comment_skipped"] = comment_block
else:
try:
auth = _auth(h)
body = stale_review_decision_lock.format_correction_audit_comment(
audit
)
with _audited(
"comment_pr",
host=h,
remote=remote,
org=o,
repo=r,
pr_number=scope_pr,
request_metadata={"source": "authorize_review_correction"},
):
posted = api_request(
"POST",
f"{repo_api_url(h, o, r)}/issues/{scope_pr}/comments",
auth,
{"body": body},
)
report["audit_comment_id"] = (posted or {}).get("id")
except Exception as exc: # noqa: BLE001
report["audit_comment_error"] = _redact(str(exc))
return report
@mcp.tool()
def gitea_diagnose_review_decision_lock(
target_pr_number: int,
expected_head_sha: str | None = None,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Read-only: classify durable review-decision lock state for a target PR (#693).
Returns a deterministic classification, owner/evidence, exact next_action,
and whether mark_final / cleanup / scoped correction are appropriate.
Never mutates the lock and never authorizes correction.
"""
read_block = _profile_operation_gate("gitea.read")
if read_block:
return {
"success": False,
"reasons": read_block,
"permission_report": _permission_block_report("gitea.read"),
}
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
binding = _decision_lock_binding()
lock = _load_review_decision_lock()
session_blockers = _review_decision_session_reasons(lock) if lock else []
last = stale_review_decision_lock.last_terminal_mutation(lock)
pr_live = None
pr_lookup_error = None
last_pr = last.get("pr_number") if last else None
if last_pr is not None:
try:
pr_live = api_request(
"GET", f"{repo_api_url(h, o, r)}/pulls/{last_pr}", auth
) or {}
if not pr_live:
pr_lookup_error = "empty PR payload"
except Exception as exc: # noqa: BLE001
pr_lookup_error = _redact(str(exc))
classification = stale_review_decision_lock.classify_review_decision_lock(
lock,
target_pr_number=target_pr_number,
target_head_sha=expected_head_sha,
pr_live=pr_live,
pr_lookup_error=pr_lookup_error,
active_profile_identity=binding.get("profile_identity"),
session_blockers=session_blockers,
)
recovery = stale_review_decision_lock.assess_open_pr_decision_lock_recovery(
lock,
target_pr_number=target_pr_number,
target_head_sha=expected_head_sha,
last_terminal_pr_live=pr_live,
pr_lookup_error=pr_lookup_error,
active_profile_identity=binding.get("profile_identity"),
session_blockers=session_blockers,
)
try:
actor = _authenticated_username(h)
except Exception:
actor = None
return {
"success": True,
"remote": remote,
"org": o,
"repo": r,
"authenticated_username": actor,
"active_profile_identity": binding.get("profile_identity"),
"classification": classification.get("classification"),
"mark_final_allowed": classification.get("mark_final_allowed"),
"cleanup_allowed": classification.get("cleanup_allowed"),
"correction_eligible": classification.get("correction_eligible"),
"recovery_allowed": recovery.get("recovery_allowed"),
"recovery_mode": recovery.get("recovery_mode"),
"exact_next_action": classification.get("next_action"),
"owner_profile_identity": classification.get("owner_profile_identity"),
"last_terminal_pr": classification.get("last_terminal_pr"),
"last_terminal_action": classification.get("last_terminal_action"),
"locked_head_sha": classification.get("locked_head_sha"),
"target_pr_number": target_pr_number,
"target_head_sha": stale_review_decision_lock.normalize_head_sha(
expected_head_sha
),
"lock_summary": classification.get("lock_summary"),
"evidence": classification.get("evidence"),
"reasons": list(classification.get("reasons") or []),
"manual_file_delete_forbidden": True,
"correction_as_generic_unlock_forbidden": True,
"durable_issue_handoff": stale_review_decision_lock.build_precise_mark_final_failure(
classification
).get("durable_issue_handoff"),
}
@@ -6672,7 +7258,12 @@ def gitea_edit_pr(
raise ValueError("At least one field to edit (title, body, state, base) must be provided.")
closing = payload.get("state") == "closed"
verify_preflight_purity(remote, task="close_pr" if closing else None)
# Closing uses close_pr; non-closing title/body/base edits use edit_pr so
# the shared #604 anti-stomp inventory stays consistent with wiring.
if closing:
verify_preflight_purity(remote, task="close_pr")
else:
verify_preflight_purity(remote, task="edit_pr")
# PR closure is a first-class capability, distinct from retitling or
# rebasing edits (#216). Gate BEFORE auth/API setup so a blocked close
@@ -7164,6 +7755,25 @@ def gitea_merge_pr(
reasons.extend(lease_block.get("reasons") or [])
result["pr_work_lease"] = lease_block
return result
# #604: common anti-stomp with live head + lease proof (typed blocker).
try:
_run_anti_stomp_preflight(
"merge_pr",
remote=remote,
worktree_path=worktree_path,
org=org,
repo=repo,
expected_head_sha=expected_head_sha,
live_head_sha=actual_sha,
require_head_sha=True,
foreign_lease=False,
terminal_lock_blocks=False,
)
except RuntimeError as exc:
reasons.append(str(exc))
return result
if expected_head_sha and actual_sha and expected_head_sha != actual_sha:
reasons.append(
"expected head SHA does not match current PR head (fail closed)"
@@ -10040,7 +10650,11 @@ def gitea_acquire_reviewer_pr_lease(
"permission_report": _permission_block_report("gitea.pr.comment"),
}
_verify_role_mutation_workspace(remote, worktree=worktree, task="review_pr")
# task=acquire_reviewer_pr_lease so verify_preflight_purity runs shared #604
# anti-stomp for the declared lease-acquire mutation inventory entry.
_verify_role_mutation_workspace(
remote, worktree=worktree, task="acquire_reviewer_pr_lease"
)
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
profile = get_profile()
@@ -10181,6 +10795,7 @@ def gitea_adopt_merger_pr_lease(
"permission_report": _permission_block_report("gitea.pr.merge"),
}
# task=adopt_merger_pr_lease so verify_preflight_purity runs shared #604 anti-stomp.
_verify_role_mutation_workspace(
remote, worktree=worktree, task="adopt_merger_pr_lease"
)
stale_review_decision_lock.py
+457 -13
View File
@@ -1,4 +1,4 @@
"""Stale / moot #332 review-decision lock detection and cleanup policy (#594/#620).
"""Stale / moot #332 review-decision lock detection and cleanup policy (#594/#620/#693).
#332 correctly hard-stops a reviewer session after a terminal live review
mutation. After #559 those locks are durable on disk, so they can outlive the
@@ -12,6 +12,11 @@ on head B of the **same open PR**. Same-head duplicates remain fail-closed.
Open-PR lock *cleanup* remains forbidden unless the PR is truly merged/closed
(#594); new-head re-review is allowed without deleting the durable lock.
#693 scopes the terminal boundary by **PR number**: a terminal on open PR A
must not block a fresh formal decision on open PR B on the same durable
profile lock (cross-PR isolation). Correction authorization is scoped to the
named prior review's PR/head and cannot unlock a different PR.
This module is pure policy (no Gitea I/O). The MCP tool
``gitea_cleanup_stale_review_decision_lock`` fetches live PR state, calls these
helpers, and only then clears durable state when cleanup is allowed.
@@ -80,17 +85,45 @@ def last_terminal_mutation(lock: dict | None) -> dict | None:
return terminals[-1] if terminals else None
def correction_applies(
lock: dict | None,
*,
pr_number: int | None = None,
expected_head_sha: str | None = None,
) -> bool:
"""True when operator correction is active and scoped to the target (#693).
Global ``correction_authorized`` alone is not enough: correction must name
the same PR (and head when recorded) as the decision being re-opened.
Missing scope fields on legacy locks fail closed (do not apply).
"""
if not lock or not lock.get("correction_authorized"):
return False
corr_pr = lock.get("correction_pr_number")
if corr_pr is None:
# Legacy unscoped correction — refuse as generic unlock (#693).
return False
if pr_number is not None and int(corr_pr) != int(pr_number):
return False
corr_head = normalize_head_sha(lock.get("correction_head_sha"))
target = normalize_head_sha(expected_head_sha)
if corr_head and target and not heads_equal(corr_head, target):
return False
return True
def prior_live_mutations_block_boundary(
lock: dict | None,
*,
pr_number: int,
expected_head_sha: str | None,
) -> bool:
"""True when prior live mutations block a new decision for PR+head (#620).
"""True when prior live mutations block a new decision for PR+head (#620/#693).
Historical terminals on a **different head of the same PR** do not block.
Any prior mutation on another PR, the same head, or without a comparable
head SHA fails closed (blocks).
Terminals on a **different PR** do not block (#693 cross-PR isolation).
Same PR + same head (or same PR without a comparable head SHA) blocks
unless a scoped correction applies.
Head resolution uses ``mutation_head_sha(m, lock)`` so pre-#620 ledgers
that only stored ``ready_expected_head_sha`` still compare correctly
@@ -98,7 +131,9 @@ def prior_live_mutations_block_boundary(
"""
if not lock:
return False
if lock.get("correction_authorized"):
if correction_applies(
lock, pr_number=pr_number, expected_head_sha=expected_head_sha
):
return False
prior = list(lock.get("live_mutations") or [])
if not prior:
@@ -108,10 +143,14 @@ def prior_live_mutations_block_boundary(
if not isinstance(m, dict):
return True
m_pr = m.get("pr_number")
if m_pr is None:
return True # fail closed — unscoped mutation
if int(m_pr) != int(pr_number):
# #693: foreign-PR history on the shared durable lock does not block.
continue
m_head = mutation_head_sha(m, lock)
if (
m_pr == pr_number
and target
target
and m_head
and not heads_equal(m_head, target)
):
@@ -128,23 +167,34 @@ def terminal_boundary_allows_fresh_decision(
expected_head_sha: str | None,
operation: str,
) -> bool:
"""Whether #332 hard-stop should yield for a new PR+head boundary (#620).
"""Whether #332 hard-stop should yield for a new PR+head boundary (#620/#693).
Only for ``mark_ready`` / ``review`` / ``resume`` on the **same PR** when
the requested head differs from the last terminal's head. Merge is never
reopened by head change (approved head merge path is separate).
For ``mark_ready`` / ``review`` / ``resume``:
* **same PR, different head** — allowed (#620)
* **different PR than last terminal** — allowed (#693 cross-PR isolation)
* **same PR, same head** — not allowed (unless scoped correction)
Merge is never reopened by head change or cross-PR isolation (approved
head merge path is separate).
"""
if operation not in ("mark_ready", "review", "resume"):
return False
if not lock:
return False
if lock.get("correction_authorized"):
if correction_applies(
lock, pr_number=pr_number, expected_head_sha=expected_head_sha
):
return True
last = last_terminal_mutation(lock)
if last is None:
return True
if last.get("pr_number") != pr_number:
last_pr = last.get("pr_number")
if last_pr is None:
return False
if int(last_pr) != int(pr_number):
# #693: last terminal belongs to another PR — do not hard-stop this PR.
return True
locked_head = mutation_head_sha(last, lock)
target = normalize_head_sha(expected_head_sha)
if not locked_head or not target:
@@ -440,6 +490,400 @@ def format_cleanup_audit_comment(audit: dict[str, Any]) -> str:
return "\n".join(lines)
# --- #693 classification / recovery / correction audit -----------------------
# Deterministic classification labels for diagnostics and recovery routing.
CLASS_NO_LOCK = "no_lock"
CLASS_READY_FOR_TARGET = "ready_for_target"
CLASS_IN_PROGRESS_SAME_HEAD = "in_progress_same_head"
CLASS_TERMINAL_ACTIVE_SAME_HEAD = "terminal_active_same_head"
CLASS_STALE_SUPERSEDED_HEAD = "stale_superseded_head"
CLASS_FOREIGN_PR_TERMINAL = "foreign_pr_terminal"
CLASS_MOOT_MERGED_CLOSED = "moot_merged_closed"
CLASS_CORRECTION_ACTIVE = "correction_active"
CLASS_SESSION_OR_PROFILE_MISMATCH = "session_or_profile_mismatch"
CLASS_AMBIGUOUS = "ambiguous"
def classify_review_decision_lock(
lock: dict | None,
*,
target_pr_number: int | None = None,
target_head_sha: str | None = None,
pr_live: dict | None = None,
pr_lookup_error: str | None = None,
active_profile_identity: str | None = None,
session_blockers: list[str] | None = None,
) -> dict[str, Any]:
"""Deterministic review-decision-lock classification (#693).
Returns classification, exact next_action, owner/evidence fields, and
whether mark_final / cleanup / correction are appropriate.
"""
summary = lock_summary(lock)
target_head = normalize_head_sha(target_head_sha)
result: dict[str, Any] = {
"classification": CLASS_NO_LOCK,
"has_lock": lock is not None,
"lock_summary": summary,
"target_pr_number": target_pr_number,
"target_head_sha": target_head,
"last_terminal_pr": None,
"last_terminal_action": None,
"locked_head_sha": None,
"owner_profile_identity": (summary or {}).get("profile_identity"),
"session_profile": (summary or {}).get("session_profile"),
"updated_at": (summary or {}).get("updated_at"),
"correction_authorized": bool((lock or {}).get("correction_authorized")),
"correction_pr_number": (lock or {}).get("correction_pr_number"),
"correction_head_sha": normalize_head_sha(
(lock or {}).get("correction_head_sha")
),
"mark_final_allowed": True,
"cleanup_allowed": False,
"correction_eligible": False,
"next_action": "gitea_resolve_task_capability(task='review_pr') then mark_final",
"reasons": [],
"evidence": {},
}
if lock is None:
result["reasons"].append("no review decision lock present")
return result
session_blockers = list(session_blockers or [])
if session_blockers:
result["classification"] = CLASS_SESSION_OR_PROFILE_MISMATCH
result["mark_final_allowed"] = False
result["next_action"] = (
"reconnect matching prgs-reviewer session or wait for lock TTL; "
"do not use gitea_authorize_review_correction as unlock; "
"do not delete session-state files"
)
result["reasons"].extend(session_blockers)
return result
stored = (
(lock.get("profile_identity") or lock.get("session_profile_lock") or "")
.strip()
)
active = (active_profile_identity or "").strip()
if active and stored and active != stored:
result["classification"] = CLASS_SESSION_OR_PROFILE_MISMATCH
result["mark_final_allowed"] = False
result["next_action"] = (
f"switch to profile '{stored}' or wait for moot cleanup; "
"do not use correction authorization as a generic unlock"
)
result["reasons"].append(
f"lock profile '{stored}' does not match active '{active}'"
)
return result
last = last_terminal_mutation(lock)
if last is None:
ready_pr = lock.get("ready_pr_number")
ready_head = normalize_head_sha(lock.get("ready_expected_head_sha"))
if (
target_pr_number is not None
and ready_pr == target_pr_number
and ready_head
and target_head
and heads_equal(ready_head, target_head)
and lock.get("final_review_decision_ready")
):
result["classification"] = CLASS_IN_PROGRESS_SAME_HEAD
result["mark_final_allowed"] = True # idempotent re-mark
result["next_action"] = (
"gitea_submit_pr_review with final_review_decision_ready=true "
"for the ready PR/head"
)
result["reasons"].append(
"final decision already marked ready for this PR/head (idempotent)"
)
return result
result["classification"] = CLASS_READY_FOR_TARGET
result["next_action"] = "gitea_mark_final_review_decision then submit"
result["reasons"].append("no terminal live mutations; mark_final allowed")
return result
last_pr = last.get("pr_number")
last_action = last.get("action")
locked_head = mutation_head_sha(last, lock)
result["last_terminal_pr"] = last_pr
result["last_terminal_action"] = last_action
result["locked_head_sha"] = locked_head
result["evidence"] = {
"last_review_id": last.get("review_id"),
"live_mutations_count": len(lock.get("live_mutations") or []),
}
if correction_applies(
lock, pr_number=target_pr_number, expected_head_sha=target_head
):
result["classification"] = CLASS_CORRECTION_ACTIVE
result["mark_final_allowed"] = True
result["next_action"] = (
"gitea_mark_final_review_decision for the correction-scoped PR/head, "
"then submit once"
)
result["reasons"].append(
f"scoped correction active for PR #{lock.get('correction_pr_number')}"
)
return result
# Live state of last terminal PR when provided.
live = None
if pr_lookup_error:
result["classification"] = CLASS_AMBIGUOUS
result["mark_final_allowed"] = False
result["next_action"] = (
"retry diagnosis after live PR state is available; "
"fail closed under ambiguous evidence"
)
result["reasons"].append(f"live PR lookup failed: {pr_lookup_error}")
return result
if pr_live is not None:
live = classify_pr_live_state(pr_live)
result["evidence"]["last_terminal_pr_state"] = live.get("pr_state")
result["evidence"]["last_terminal_pr_merged"] = live.get("pr_merged")
if live.get("pr_merged_or_closed"):
result["classification"] = CLASS_MOOT_MERGED_CLOSED
result["cleanup_allowed"] = True
result["mark_final_allowed"] = target_pr_number is not None and (
int(last_pr) != int(target_pr_number)
if last_pr is not None and target_pr_number is not None
else True
)
result["next_action"] = (
"gitea_cleanup_stale_review_decision_lock(apply=true, "
f"expected_terminal_pr={last_pr}) then mark_final on the target PR"
)
result["reasons"].append(
f"terminal on PR #{last_pr} is moot (merged/closed); cleanup allowed"
)
return result
if target_pr_number is None:
result["classification"] = CLASS_AMBIGUOUS
result["mark_final_allowed"] = False
result["next_action"] = "re-run diagnosis with target_pr_number"
result["reasons"].append("target_pr_number required for open-PR classification")
return result
if last_pr is not None and int(last_pr) != int(target_pr_number):
# Cross-PR isolation: foreign terminal is history, not a block.
result["classification"] = CLASS_FOREIGN_PR_TERMINAL
result["mark_final_allowed"] = True
result["correction_eligible"] = False # must not use correction to "unlock"
result["next_action"] = (
f"gitea_mark_final_review_decision(pr_number={target_pr_number}, ...) "
f"— foreign terminal on PR #{last_pr} does not block (#693); "
"do not call gitea_authorize_review_correction for cross-PR unlock"
)
result["reasons"].append(
f"last terminal is {last_action} on foreign open/closed PR #{last_pr}; "
f"target PR #{target_pr_number} is isolated (#693)"
)
return result
# Same PR as target.
if (
locked_head
and target_head
and not heads_equal(locked_head, target_head)
):
result["classification"] = CLASS_STALE_SUPERSEDED_HEAD
result["mark_final_allowed"] = True
result["next_action"] = (
f"gitea_mark_final_review_decision with expected_head_sha="
f"{target_head} (#620 same-PR new head)"
)
result["reasons"].append(
f"terminal {last_action} on head {locked_head[:12]}…; target head "
f"{target_head[:12]}… — fresh formal review allowed without cleanup"
)
return result
# Same PR + same head (or missing head comparison) — active terminal.
result["classification"] = CLASS_TERMINAL_ACTIVE_SAME_HEAD
result["mark_final_allowed"] = False
result["correction_eligible"] = True
result["next_action"] = (
"if the prior verdict was mistaken: gitea_authorize_review_correction "
f"with prior_review_id={last.get('review_id')}, "
f"prior_review_state={last_action}, target_pr_number={target_pr_number}, "
"operator_authorized=true, then re-mark once; "
"if the prior verdict is correct: stop and hand off (do not duplicate)"
)
result["reasons"].append(
f"terminal {last_action} already recorded for PR #{target_pr_number} "
f"at this head; same-head duplicate blocked (#332/#620)"
)
return result
def build_precise_mark_final_failure(
classification: dict[str, Any],
) -> dict[str, Any]:
"""One precise recovery instruction + durable issue handoff payload (#693 AC4/8)."""
cls = classification.get("classification")
next_action = classification.get("next_action") or (
"stop and create a durable Gitea issue with lock evidence"
)
reasons = list(classification.get("reasons") or [])
reasons.append(f"exact_next_action: {next_action}")
handoff = {
"required": cls
in (
CLASS_AMBIGUOUS,
CLASS_SESSION_OR_PROFILE_MISMATCH,
CLASS_TERMINAL_ACTIVE_SAME_HEAD,
)
and not classification.get("mark_final_allowed"),
"title_hint": (
"Review-decision-lock recovery failed during formal review"
),
"classification": cls,
"exact_next_action": next_action,
"owner_profile_identity": classification.get("owner_profile_identity"),
"last_terminal_pr": classification.get("last_terminal_pr"),
"last_terminal_action": classification.get("last_terminal_action"),
"locked_head_sha": classification.get("locked_head_sha"),
"target_pr_number": classification.get("target_pr_number"),
"target_head_sha": classification.get("target_head_sha"),
"evidence": classification.get("evidence") or {},
"instruction": (
"If recovery cannot complete with the exact_next_action above, "
"create or update a durable Gitea issue with this payload and stop; "
"do not delete session-state files; do not misuse correction as unlock."
),
}
return {
"reasons": reasons,
"classification": cls,
"exact_next_action": next_action,
"durable_issue_handoff": handoff,
}
def build_correction_audit_record(
*,
prior_review_id: int | None,
prior_review_state: str | None,
target_pr_number: int | None,
target_head_sha: str | None,
reason: str | None,
actor_username: str | None,
profile_name: str | None,
authorized: bool,
) -> dict[str, Any]:
"""Structured durable audit for review-correction authorization (#693)."""
return {
"event": "review_correction_authorization",
"issue_ref": "#693",
"authorized": bool(authorized),
"timestamp": datetime.now(timezone.utc).isoformat(),
"actor_username": actor_username,
"profile_name": profile_name,
"prior_review_id": prior_review_id,
"prior_review_state": prior_review_state,
"target_pr_number": target_pr_number,
"target_head_sha": normalize_head_sha(target_head_sha),
"reason": reason,
"scope": "same_pr_head_only",
}
def format_correction_audit_comment(audit: dict[str, Any]) -> str:
"""Markdown body for a thread-visible correction authorization audit."""
status = "AUTHORIZED" if audit.get("authorized") else "DENIED"
lines = [
"## Review correction authorization audit (#693)",
"",
f"Status: **{status}**",
"",
f"- actor: `{audit.get('actor_username')}`",
f"- profile: `{audit.get('profile_name')}`",
f"- timestamp: `{audit.get('timestamp')}`",
f"- prior_review_id: `{audit.get('prior_review_id')}`",
f"- prior_review_state: `{audit.get('prior_review_state')}`",
f"- target_pr: `#{audit.get('target_pr_number')}`",
f"- target_head_sha: `{audit.get('target_head_sha')}`",
f"- reason: {audit.get('reason')}",
f"- scope: `{audit.get('scope')}` (cannot unlock a different PR)",
"",
"This is **not** a generic decision-lock unlock. Cross-PR recovery "
"uses isolation (#693) or moot cleanup (#594), never correction.",
]
return "\n".join(lines)
def assess_open_pr_decision_lock_recovery(
lock: dict | None,
*,
target_pr_number: int,
target_head_sha: str | None,
last_terminal_pr_live: dict | None = None,
pr_lookup_error: str | None = None,
active_profile_identity: str | None = None,
session_blockers: list[str] | None = None,
controller_recovery_authorized: bool = False,
) -> dict[str, Any]:
"""Assess guarded recovery for decision locks affecting an open target PR (#693).
Recovery here means *workflow recovery routing*, not necessarily lock wipe.
Foreign-PR terminals are recoverable by isolation (mark_final allowed).
Moot terminals use #594 cleanup. Same-head active terminals refuse wipe.
"""
classification = classify_review_decision_lock(
lock,
target_pr_number=target_pr_number,
target_head_sha=target_head_sha,
pr_live=last_terminal_pr_live,
pr_lookup_error=pr_lookup_error,
active_profile_identity=active_profile_identity,
session_blockers=session_blockers,
)
cls = classification["classification"]
recovery_allowed = False
recovery_mode = None
if cls == CLASS_FOREIGN_PR_TERMINAL:
recovery_allowed = True
recovery_mode = "cross_pr_isolation"
elif cls == CLASS_STALE_SUPERSEDED_HEAD:
recovery_allowed = True
recovery_mode = "same_pr_new_head"
elif cls == CLASS_MOOT_MERGED_CLOSED:
recovery_allowed = True
recovery_mode = "moot_cleanup"
elif cls == CLASS_READY_FOR_TARGET:
recovery_allowed = True
recovery_mode = "none_required"
elif cls == CLASS_CORRECTION_ACTIVE:
recovery_allowed = True
recovery_mode = "scoped_correction"
elif cls == CLASS_TERMINAL_ACTIVE_SAME_HEAD:
recovery_allowed = False
recovery_mode = "correction_only_if_mistake"
else:
recovery_allowed = False
recovery_mode = "fail_closed"
if recovery_mode == "moot_cleanup" and not controller_recovery_authorized:
# Cleanup still needs apply + capability; assess reports path.
pass
return {
**classification,
"recovery_allowed": recovery_allowed,
"recovery_mode": recovery_mode,
"controller_recovery_authorized": bool(controller_recovery_authorized),
"manual_file_delete_forbidden": True,
"correction_as_generic_unlock_forbidden": True,
}
# ---------------------------------------------------------------------------
# #709 cross-profile cleanup, overwrite protection, recovery provenance
# ---------------------------------------------------------------------------
task_capability_map.py
+38
View File
@@ -60,6 +60,15 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
"permission": "gitea.pr.close",
"role": "author",
},
# Non-closing PR metadata edits (title/body/base). Closing uses close_pr.
"edit_pr": {
"permission": "gitea.pr.create",
"role": "author",
},
"gitea_edit_pr": {
"permission": "gitea.pr.create",
"role": "author",
},
"address_pr_change_requests": {
"permission": "gitea.branch.push",
"role": "author",
@@ -68,10 +77,22 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
"permission": "gitea.pr.review",
"role": "reviewer",
},
"submit_pr_review": {
"permission": "gitea.pr.review",
"role": "reviewer",
},
"merge_pr": {
"permission": "gitea.pr.merge",
"role": "merger",
},
"acquire_reviewer_pr_lease": {
"permission": "gitea.pr.comment",
"role": "reviewer",
},
"gitea_acquire_reviewer_pr_lease": {
"permission": "gitea.pr.comment",
"role": "reviewer",
},
# #695 AC8: controller quarantine of contaminated formal reviews.
# Apply path posts an append-only forensic audit comment (pr.comment).
"quarantine_contaminated_review": {
@@ -127,6 +148,23 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
"permission": "gitea.pr.review",
"role": "reviewer",
},
# #693: read-only classification of durable decision locks (incl. open PRs).
"diagnose_review_decision_lock": {
"permission": "gitea.read",
"role": "reviewer",
},
"gitea_diagnose_review_decision_lock": {
"permission": "gitea.read",
"role": "reviewer",
},
"authorize_review_correction": {
"permission": "gitea.pr.review",
"role": "reviewer",
},
"gitea_authorize_review_correction": {
"permission": "gitea.pr.review",
"role": "reviewer",
},
# #709: truthful absence-of-proof recovery (server-side auth + record + consume).
# Dedicated mutation capability — gitea.read is insufficient (review 434 F1).
"issue_irrecoverable_provenance_authorization": {
tests/test_anti_stomp_preflight.py
+1032
View File
File diff suppressed because it is too large Load Diff
tests/test_head_scoped_review_decision_lock.py
+11 -4
View File
@@ -24,6 +24,10 @@ HEAD_C = "c" * 40
def _lock(mutations=None, correction=False, ready_pr=None, ready_head=None, ready_action=None):
mutations = list(mutations or [])
corr_pr = None
if correction and mutations:
corr_pr = mutations[-1].get("pr_number")
return {
"task": "review_pr",
"remote": "prgs",
@@ -40,9 +44,11 @@ def _lock(mutations=None, correction=False, ready_pr=None, ready_head=None, read
"ready_remote": "prgs" if ready_pr else None,
"ready_org": "Scaled-Tech-Consulting" if ready_pr else None,
"ready_repo": "Gitea-Tools" if ready_pr else None,
"live_mutations": list(mutations or []),
"live_mutations": mutations,
"correction_authorized": correction,
"correction_reason": None,
"correction_reason": "test" if correction else None,
"correction_pr_number": corr_pr,
"correction_head_sha": None,
}
@@ -174,12 +180,13 @@ class TestHardStopHeadScope(unittest.TestCase):
self.assertTrue(reasons)
self.assertIn("#332", reasons[0])
def test_rc_head_a_blocks_other_pr(self):
def test_rc_head_a_allows_other_pr_via_cross_pr_isolation(self):
"""#693: foreign-PR terminal must not hard-stop mark_ready on another PR."""
_seed([RC_619_HEAD_A], ready_pr=619, ready_head=HEAD_A)
reasons = mcp_server.terminal_review_hard_stop_reasons(
700, "mark_ready", expected_head_sha=HEAD_B
)
self.assertTrue(reasons)
self.assertEqual(reasons, [])
def test_legacy_619_ledger_allows_new_head(self):
"""PR #619-style durable lock without mutation head_sha."""
tests/test_issue_693_review_decision_lock_recovery.py
+427
View File
@@ -0,0 +1,427 @@
"""Review-decision-lock recovery / cross-PR isolation (#693).
Reproduces the PR #688 → PR #692 formal-review sequence:
* durable lock holds REQUEST_CHANGES on open PR #688 @ head A (review_id 425)
* reviewer leases PR #692 @ head B (6d86db…)
* mark_final for #692 must succeed without correction unlock
* cleanup of open #688 terminal remains forbidden (#594)
* authorize_review_correction for #688 cannot unlock #692
* eventual #692 APPROVE is unique and head-bound (review_id 426)
* mark_final is idempotent for same PR/action/head
* diagnosis returns one exact next_action + durable handoff payload
"""
from __future__ import annotations
import os
import unittest
from unittest.mock import patch
import mcp_server
import review_workflow_load
import stale_review_decision_lock as srdl
# PR #688 / #692 incident fixtures (sanitized reconstruction from issue #693)
HEAD_688 = "c7a444eb4b41cf916fdbd20a4999ffd78af496d0"
HEAD_692 = "6d86db793bbdfaed16bd54d93171f1dd66f234ca"
PR_A = 688
PR_B = 692
REVIEW_ID_A = 425
REVIEW_ID_B = 426
RC_688 = {
"pr_number": PR_A,
"action": "request_changes",
"review_id": REVIEW_ID_A,
"review_state": "request_changes",
"head_sha": HEAD_688,
}
APPROVE_692 = {
"pr_number": PR_B,
"action": "approve",
"review_id": REVIEW_ID_B,
"review_state": "approve",
"head_sha": HEAD_692,
}
def _lock(mutations=None, **kwargs):
from datetime import datetime, timezone
now = datetime.now(timezone.utc).isoformat()
base = {
"task": "review_pr",
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools",
"session_pid": os.getpid(),
"session_profile": "prgs-reviewer",
"session_profile_lock": "prgs-reviewer",
"profile_identity": "prgs-reviewer",
"final_review_decision_ready": False,
"ready_pr_number": None,
"ready_action": None,
"ready_expected_head_sha": None,
"ready_remote": None,
"ready_org": None,
"ready_repo": None,
"live_mutations": list(mutations or []),
"correction_authorized": False,
"correction_reason": None,
"correction_pr_number": None,
"correction_head_sha": None,
"updated_at": now,
"recorded_at": now,
"writer_pid": os.getpid(),
}
base.update(kwargs)
# Ensure TTL fields stay fresh unless the caller is testing expiry.
if "recorded_at" not in kwargs:
base["recorded_at"] = now
if "updated_at" not in kwargs:
base["updated_at"] = now
return base
REVIEWER_ENV = {
"GITEA_PROFILE_NAME": "prgs-reviewer",
"GITEA_ALLOWED_OPERATIONS": (
"gitea.read,gitea.pr.review,gitea.pr.approve,"
"gitea.pr.request_changes,gitea.pr.comment"
),
"GITEA_SESSION_PROFILE_LOCK": "prgs-reviewer",
}
REVIEWER_PROFILE = {
"profile_name": "prgs-reviewer",
"allowed_operations": [
"gitea.read",
"gitea.pr.review",
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.comment",
],
"forbidden_operations": ["gitea.pr.merge"],
}
def _seed(mutations=None, **kwargs):
review_workflow_load.record_review_workflow_load(mcp_server.PROJECT_ROOT)
mcp_server._save_review_decision_lock(_lock(mutations, **kwargs))
mcp_server.gitea_load_review_workflow()
def _no_lease():
return {"block": False, "reasons": [], "mutation_allowed": True}
def _open_pr(number, head):
return {
"number": number,
"state": "open",
"merged": False,
"merged_at": None,
"head": {"sha": head},
}
class TestCrossPrIsolationPure(unittest.TestCase):
def test_foreign_terminal_does_not_block_mark_boundary(self):
lock = _lock([RC_688])
self.assertFalse(
srdl.prior_live_mutations_block_boundary(
lock, pr_number=PR_B, expected_head_sha=HEAD_692
)
)
self.assertTrue(
srdl.prior_live_mutations_block_boundary(
lock, pr_number=PR_A, expected_head_sha=HEAD_688
)
)
def test_terminal_boundary_allows_fresh_decision_on_other_pr(self):
lock = _lock([RC_688])
self.assertTrue(
srdl.terminal_boundary_allows_fresh_decision(
lock,
pr_number=PR_B,
expected_head_sha=HEAD_692,
operation="mark_ready",
)
)
self.assertFalse(
srdl.terminal_boundary_allows_fresh_decision(
lock,
pr_number=PR_A,
expected_head_sha=HEAD_688,
operation="mark_ready",
)
)
def test_unscoped_correction_does_not_apply(self):
lock = _lock([RC_688], correction_authorized=True, correction_reason="x")
# Missing correction_pr_number → fail closed (#693)
self.assertFalse(
srdl.correction_applies(lock, pr_number=PR_A, expected_head_sha=HEAD_688)
)
def test_scoped_correction_only_for_named_pr(self):
lock = _lock(
[RC_688],
correction_authorized=True,
correction_reason="mistake",
correction_pr_number=PR_A,
correction_head_sha=HEAD_688,
)
self.assertTrue(
srdl.correction_applies(lock, pr_number=PR_A, expected_head_sha=HEAD_688)
)
self.assertFalse(
srdl.correction_applies(lock, pr_number=PR_B, expected_head_sha=HEAD_692)
)
class TestClassification(unittest.TestCase):
def test_foreign_pr_terminal_classification(self):
lock = _lock([RC_688])
c = srdl.classify_review_decision_lock(
lock,
target_pr_number=PR_B,
target_head_sha=HEAD_692,
pr_live=_open_pr(PR_A, HEAD_688),
active_profile_identity="prgs-reviewer",
)
self.assertEqual(c["classification"], srdl.CLASS_FOREIGN_PR_TERMINAL)
self.assertTrue(c["mark_final_allowed"])
self.assertFalse(c["correction_eligible"])
self.assertIn("do not call gitea_authorize_review_correction", c["next_action"])
def test_same_head_terminal_classification(self):
lock = _lock([RC_688])
c = srdl.classify_review_decision_lock(
lock,
target_pr_number=PR_A,
target_head_sha=HEAD_688,
pr_live=_open_pr(PR_A, HEAD_688),
)
self.assertEqual(c["classification"], srdl.CLASS_TERMINAL_ACTIVE_SAME_HEAD)
self.assertFalse(c["mark_final_allowed"])
self.assertTrue(c["correction_eligible"])
def test_precise_failure_includes_durable_handoff(self):
lock = _lock([RC_688])
c = srdl.classify_review_decision_lock(
lock, target_pr_number=PR_A, target_head_sha=HEAD_688
)
fail = srdl.build_precise_mark_final_failure(c)
self.assertIn("exact_next_action", fail)
self.assertIn("durable_issue_handoff", fail)
self.assertTrue(fail["durable_issue_handoff"]["required"])
class TestPr692Sequence(unittest.TestCase):
def setUp(self):
self._env = patch.dict(os.environ, REVIEWER_ENV, clear=False)
self._env.start()
self._prof = patch.object(
mcp_server, "get_profile", return_value=REVIEWER_PROFILE
)
self._prof.start()
def tearDown(self):
self._prof.stop()
self._env.stop()
mcp_server._save_review_decision_lock(None)
review_workflow_load.clear_review_workflow_load()
def test_mark_final_692_succeeds_with_open_688_terminal(self):
_seed([RC_688], writer_pid=25883, session_pid=60615)
with patch("mcp_server._list_pr_lease_comments", return_value=[]), patch(
"mcp_server._pr_work_lease_reviewer_block", return_value=_no_lease()
), patch.object(
mcp_server,
"gitea_check_pr_eligibility",
return_value={"head_sha": HEAD_692, "eligible": True},
):
result = mcp_server.gitea_mark_final_review_decision(
pr_number=PR_B,
action="approve",
expected_head_sha=HEAD_692,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertTrue(result["marked_ready"], result.get("reasons"))
lock = mcp_server._load_review_decision_lock()
self.assertEqual(lock["ready_pr_number"], PR_B)
self.assertEqual(
srdl.normalize_head_sha(lock["ready_expected_head_sha"]), HEAD_692
)
# Foreign terminal history preserved (non-destructive isolation).
self.assertEqual(lock["live_mutations"][0]["pr_number"], PR_A)
def test_mark_final_idempotent_same_binding(self):
_seed(
[RC_688],
final_review_decision_ready=True,
ready_pr_number=PR_B,
ready_action="approve",
ready_expected_head_sha=HEAD_692,
ready_remote="prgs",
ready_org="Scaled-Tech-Consulting",
ready_repo="Gitea-Tools",
)
with patch("mcp_server._list_pr_lease_comments", return_value=[]), patch(
"mcp_server._pr_work_lease_reviewer_block", return_value=_no_lease()
), patch.object(
mcp_server,
"gitea_check_pr_eligibility",
return_value={"head_sha": HEAD_692, "eligible": True},
):
result = mcp_server.gitea_mark_final_review_decision(
pr_number=PR_B,
action="approve",
expected_head_sha=HEAD_692,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertTrue(result["marked_ready"])
self.assertTrue(result.get("idempotent"))
def test_cleanup_still_forbidden_while_688_open(self):
_seed([RC_688])
assessment = srdl.assess_stale_review_decision_lock(
mcp_server._load_review_decision_lock(),
pr_live=_open_pr(PR_A, HEAD_688),
active_profile_identity="prgs-reviewer",
)
self.assertFalse(assessment["cleanup_allowed"])
self.assertFalse(assessment["is_moot"])
def test_correction_cannot_unlock_different_pr(self):
_seed([RC_688])
r = mcp_server.gitea_authorize_review_correction(
prior_review_id=REVIEW_ID_A,
prior_review_state="request_changes",
reason="try unlock 692",
operator_authorized=True,
target_pr_number=PR_B,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
post_audit_comment=False,
)
self.assertFalse(r["authorized"])
self.assertTrue(any("different PR" in x for x in r["reasons"]))
def test_scoped_correction_for_same_pr_posts_audit(self):
_seed([RC_688])
with patch(
"mcp_server._authenticated_username", return_value="sysadmin"
), patch(
"mcp_server.get_auth_header", return_value="Basic dGVzdDp0ZXN0"
), patch(
"mcp_server._auth", return_value="Basic dGVzdDp0ZXN0"
), patch("mcp_server.api_request", return_value={"id": 99901}):
r = mcp_server.gitea_authorize_review_correction(
prior_review_id=REVIEW_ID_A,
prior_review_state="request_changes",
reason="mistaken RC wording",
operator_authorized=True,
target_pr_number=PR_A,
expected_head_sha=HEAD_688,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
post_audit_comment=True,
)
self.assertTrue(r["authorized"], r.get("reasons"))
self.assertEqual(r.get("correction_pr_number"), PR_A)
self.assertEqual(
r.get("audit_comment_id"),
99901,
r.get("audit_comment_skipped") or r.get("audit_comment_error") or r,
)
def test_diagnose_foreign_terminal_next_action(self):
_seed([RC_688])
with patch(
"mcp_server._authenticated_username", return_value="sysadmin"
), patch(
"mcp_server.get_auth_header", return_value="Basic dGVzdDp0ZXN0"
), patch(
"mcp_server._auth", return_value="Basic dGVzdDp0ZXN0"
), patch("mcp_server.api_request", return_value=_open_pr(PR_A, HEAD_688)):
d = mcp_server.gitea_diagnose_review_decision_lock(
target_pr_number=PR_B,
expected_head_sha=HEAD_692,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertTrue(d["success"], d.get("reasons"))
self.assertEqual(d["classification"], srdl.CLASS_FOREIGN_PR_TERMINAL)
self.assertTrue(d["mark_final_allowed"])
self.assertIn("mark_final", d["exact_next_action"])
self.assertTrue(d["correction_as_generic_unlock_forbidden"])
def test_approval_ledger_unique_and_bound_after_sequence(self):
"""After mark_final + record mutation, ledger binds unique approve to 692 head."""
_seed([RC_688])
with patch("mcp_server._list_pr_lease_comments", return_value=[]), patch(
"mcp_server._pr_work_lease_reviewer_block", return_value=_no_lease()
), patch.object(
mcp_server,
"gitea_check_pr_eligibility",
return_value={"head_sha": HEAD_692, "eligible": True},
):
marked = mcp_server.gitea_mark_final_review_decision(
pr_number=PR_B,
action="approve",
expected_head_sha=HEAD_692,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertTrue(marked["marked_ready"])
mcp_server.record_live_review_mutation(PR_B, "approve", REVIEW_ID_B)
lock = mcp_server._load_review_decision_lock()
terminals = [
m
for m in lock["live_mutations"]
if m.get("action") in srdl.TERMINAL_REVIEW_ACTIONS
and m.get("pr_number") == PR_B
]
self.assertEqual(len(terminals), 1)
self.assertEqual(terminals[0]["review_id"], REVIEW_ID_B)
self.assertEqual(
srdl.normalize_head_sha(terminals[0].get("head_sha")), HEAD_692
)
# Same-head duplicate still blocked.
self.assertTrue(
srdl.prior_live_mutations_block_boundary(
lock, pr_number=PR_B, expected_head_sha=HEAD_692
)
)
class TestSessionRestartWriterPid(unittest.TestCase):
def tearDown(self):
mcp_server._save_review_decision_lock(None)
review_workflow_load.clear_review_workflow_load()
def test_writer_pid_change_does_not_inherit_foreign_block(self):
# session_pid 60615 origin, writer later 25883 — still foreign isolation.
_seed([RC_688], session_pid=60615, writer_pid=25883)
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(
PR_B, "mark_ready", expected_head_sha=HEAD_692
),
[],
)
if __name__ == "__main__":
unittest.main()
tests/test_mcp_server.py
+24 -6
View File
@@ -2567,15 +2567,18 @@ class TestSubmitPrReview(unittest.TestCase):
lock = _load_review_decision_lock() or {}
lock["live_mutations"] = [{"pr_number": 8, "action": "approve", "review_id": 42, "review_state": "approve"}]
_save_review_decision_lock(lock)
r = gitea_authorize_review_correction(
prior_review_id=42,
prior_review_state="approve",
reason="typo",
operator_authorized=True,
)
with patch("mcp_server.api_request", return_value={"id": 9001}):
r = gitea_authorize_review_correction(
prior_review_id=42,
prior_review_state="approve",
reason="typo",
operator_authorized=True,
target_pr_number=8,
)
self.assertTrue(r["authorized"])
lock = _load_review_decision_lock()
self.assertTrue(lock["correction_authorized"])
self.assertEqual(lock["correction_pr_number"], 8)
def test_authorize_review_correction_mismatch(self):
from mcp_server import _load_review_decision_lock, _save_review_decision_lock
@@ -2589,6 +2592,7 @@ class TestSubmitPrReview(unittest.TestCase):
prior_review_state="approve",
reason="typo",
operator_authorized=True,
target_pr_number=8,
)
self.assertFalse(r["authorized"])
self.assertTrue(any("prior review ID" in x for x in r["reasons"]))
@@ -2599,10 +2603,22 @@ class TestSubmitPrReview(unittest.TestCase):
prior_review_state="request_changes",
reason="typo",
operator_authorized=True,
target_pr_number=8,
)
self.assertFalse(r["authorized"])
self.assertTrue(any("prior review state" in x for x in r["reasons"]))
# Cross-PR unlock refused (#693)
r = gitea_authorize_review_correction(
prior_review_id=42,
prior_review_state="approve",
reason="unlock other pr",
operator_authorized=True,
target_pr_number=692,
)
self.assertFalse(r["authorized"])
self.assertTrue(any("different PR" in x for x in r["reasons"]))
def test_authorize_review_correction_requires_operator_auth(self):
from mcp_server import _load_review_decision_lock, _save_review_decision_lock
lock = _load_review_decision_lock() or {}
@@ -2615,6 +2631,7 @@ class TestSubmitPrReview(unittest.TestCase):
prior_review_state="approve",
reason="typo",
operator_authorized=False,
target_pr_number=8,
)
self.assertFalse(r["authorized"])
self.assertTrue(any("operator authorization" in x for x in r["reasons"]))
@@ -2672,6 +2689,7 @@ class TestSubmitPrReview(unittest.TestCase):
prior_review_state="approve",
reason="operator approved correcting mistaken approve",
operator_authorized=True,
target_pr_number=8,
)
_mark_request_changes_ready(remote="prgs")
second = gitea_submit_pr_review(
tests/test_stale_review_decision_lock_cleanup.py
+21 -9
View File
@@ -188,20 +188,32 @@ class TestActiveHardStopPreserved(unittest.TestCase):
mcp_server._save_review_decision_lock(None)
mcp_server.review_workflow_load.clear_review_workflow_load()
def test_open_approve_still_blocks_other_pr_mark_ready(self):
_seed([APPROVED_OPEN])
reasons = mcp_server.terminal_review_hard_stop_reasons(592, "mark_ready")
self.assertTrue(reasons)
self.assertIn("#332", reasons[0])
self.assertIn("gitea_cleanup_stale_review_decision_lock", reasons[0])
def test_open_approve_blocks_same_pr_not_other_pr_mark_ready(self):
_seed([APPROVED_OPEN]) # approve on PR #700
# Same PR still hard-stopped for mark_ready.
reasons_same = mcp_server.terminal_review_hard_stop_reasons(
700, "mark_ready"
)
self.assertTrue(reasons_same)
self.assertIn("#332", reasons_same[0])
# #693: other PR may mark_ready without cleanup.
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(592, "mark_ready"),
[],
)
def test_request_changes_still_blocks_everything(self):
_seed([RC_OPEN])
def test_request_changes_still_blocks_same_pr(self):
_seed([RC_OPEN]) # request_changes on PR #701
for op in ("merge", "mark_ready", "review"):
self.assertTrue(
mcp_server.terminal_review_hard_stop_reasons(592, op),
mcp_server.terminal_review_hard_stop_reasons(701, op),
msg=op,
)
# Foreign PR review path is open (#693).
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(592, "mark_ready"),
[],
)
# --------------------------------------------------------------------------- #
tests/test_terminal_review_hard_stop.py
+54 -22
View File
@@ -1,10 +1,10 @@
"""Tests for the session hard-stop after a terminal review mutation (#332).
"""Tests for the session hard-stop after a terminal review mutation (#332/#693).
Extends the single-terminal-decision machinery (#211): after a live
REQUEST_CHANGES the run must stop; after an APPROVE only the merge sequence
for that same PR may continue; a different PR can never be reviewed or
merged in the same run; duplicate REQUEST_CHANGES at an unchanged head is
suppressed.
REQUEST_CHANGES on a PR head the same-head path must stop; after an APPROVE
only the merge sequence for that same PR may continue; #693 allows a
*different* PR to be mark_ready/review in the same durable lock (cross-PR
isolation); duplicate REQUEST_CHANGES at an unchanged head is suppressed.
"""
import os
import unittest
@@ -15,7 +15,10 @@ import mcp_server
HEAD_SHA = "a" * 40
def _lock(mutations=None, correction=False):
def _lock(mutations=None, correction=False, correction_pr=None):
mutations = list(mutations or [])
if correction and correction_pr is None and mutations:
correction_pr = mutations[-1].get("pr_number")
return {
"task": "review_pr",
"remote": "prgs",
@@ -29,9 +32,11 @@ def _lock(mutations=None, correction=False):
"ready_remote": None,
"ready_org": None,
"ready_repo": None,
"live_mutations": list(mutations or []),
"live_mutations": mutations,
"correction_authorized": correction,
"correction_reason": None,
"correction_reason": "test" if correction else None,
"correction_pr_number": correction_pr if correction else None,
"correction_head_sha": None,
}
@@ -64,25 +69,35 @@ class TestTerminalHardStopReasons(unittest.TestCase):
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(5, "merge"), [])
def test_request_changes_blocks_everything(self):
def test_request_changes_blocks_same_pr_allows_other_pr_review(self):
_seed([RC_A])
# Same PR: still hard-stopped for mark_ready/review/merge.
for op in ("merge", "mark_ready", "review"):
for pr in (5, 6):
reasons = mcp_server.terminal_review_hard_stop_reasons(pr, op)
self.assertTrue(reasons, f"{op}/{pr}")
self.assertIn("request_changes on PR #5", reasons[0])
self.assertIn("#332", reasons[0])
reasons = mcp_server.terminal_review_hard_stop_reasons(5, op)
self.assertTrue(reasons, f"{op}/5")
self.assertIn("request_changes on PR #5", reasons[0])
self.assertIn("#332", reasons[0])
# #693: different PR may mark_ready / review (not merge via hard-stop alone).
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(6, "mark_ready"), [])
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(6, "review"), [])
# Merge of an unapproved other PR remains blocked by hard-stop path
# (last terminal is not approve of PR 6).
self.assertTrue(
mcp_server.terminal_review_hard_stop_reasons(6, "merge"))
def test_approve_allows_same_pr_merge_only(self):
def test_approve_allows_same_pr_merge_and_other_pr_review(self):
_seed([APPROVED_A])
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(5, "merge"), [])
self.assertTrue(
mcp_server.terminal_review_hard_stop_reasons(6, "merge"))
self.assertTrue(
mcp_server.terminal_review_hard_stop_reasons(6, "mark_ready"))
self.assertTrue(
mcp_server.terminal_review_hard_stop_reasons(6, "review"))
# #693 cross-PR isolation: other PR formal review path is open.
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(6, "mark_ready"), [])
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(6, "review"), [])
def test_correction_reopens_review_path_only(self):
_seed([RC_A], correction=True)
@@ -90,9 +105,11 @@ class TestTerminalHardStopReasons(unittest.TestCase):
mcp_server.terminal_review_hard_stop_reasons(5, "mark_ready"), [])
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(5, "review"), [])
# correction never opens a cross-PR merge
# scoped correction never opens a cross-PR merge
self.assertTrue(
mcp_server.terminal_review_hard_stop_reasons(6, "merge"))
# unscoped/cross-PR correction must not lift PR 6 via correction alone
# (PR 6 is allowed by isolation, not by correction scope)
class TestMergeHardStopWiring(unittest.TestCase):
@@ -126,14 +143,29 @@ class TestMarkFinalHardStopWiring(unittest.TestCase):
mcp_server._save_review_decision_lock(None)
mcp_server.review_workflow_load.clear_review_workflow_load()
def test_mark_ready_blocked_after_terminal_mutation(self):
def test_mark_ready_same_pr_blocked_after_terminal_mutation(self):
_seed([RC_A])
result = mcp_server.gitea_mark_final_review_decision(
pr_number=6, action="approve", remote="prgs")
pr_number=5, action="approve", remote="prgs",
expected_head_sha=HEAD_SHA)
self.assertFalse(result["marked_ready"])
self.assertTrue(
any("#332" in r for r in result["reasons"]), result["reasons"])
def test_mark_ready_other_pr_allowed_after_foreign_terminal(self):
"""#693: foreign open-PR terminal must not block mark_final on PR B."""
_seed([RC_A])
no_lease = {"block": False, "reasons": [], "mutation_allowed": True}
with patch("mcp_server._list_pr_lease_comments", return_value=[]), \
patch("mcp_server._pr_work_lease_reviewer_block",
return_value=no_lease), \
patch.object(mcp_server, "gitea_check_pr_eligibility",
return_value={"head_sha": HEAD_SHA, "eligible": True}):
result = mcp_server.gitea_mark_final_review_decision(
pr_number=6, action="approve", remote="prgs",
expected_head_sha=HEAD_SHA)
self.assertTrue(result["marked_ready"], result.get("reasons"))
def _feedback(blocking, stale=False, success=True):
return {