Compare commits

..
Author SHA1 Message Date
sysadminandClaude Opus 4.8 8b61c4b41f test: fix reviewer lease gate regressions in MCP integration tests (#407)
Align merge/review/audit mocks with the per-PR reviewer lease gate introduced
in #407: install owned session leases after init_review_decision_lock, stub
_fetch_pr_comments and _authenticated_username to preserve mock ordering, and
update head-SHA mismatch expectations for lease-first fail-closed paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 16:32:23 -04:00
sysadminandClaude Opus 4.8 a9d6a9c863 feat: add per-PR reviewer leases for parallel review (Closes #407)
Structured PR-thread leases with acquire/heartbeat MCP tools and
mutation-time enforcement so reviewers cannot race the same PR queue.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 13:07:22 -04:00
12 changed files with 1093 additions and 519 deletions
-198
View File
@@ -1,198 +0,0 @@
"""Early duplicate-work detection for author work-issue flows (#400)."""
from __future__ import annotations
import re
from typing import Any
from issue_claim_heartbeat import (
_linked_open_pr,
_matching_branch_names,
classify_issue_claim,
)
STAGES = (
"claim",
"lock",
"worktree",
"edit",
"commit",
"push",
"create_pr",
)
ELIGIBILITY_OPEN_PR_EXISTS = "OPEN_PR_EXISTS"
ELIGIBILITY_DUPLICATE_BRANCH_EXISTS = "DUPLICATE_BRANCH_EXISTS"
ELIGIBILITY_ACTIVE_CLAIM = "ACTIVE_CLAIM_BY_OTHER"
ELIGIBILITY_CLEAR = "CLEAR"
def assess_author_duplicate_work(
issue_number: int,
*,
stage: str,
open_prs: list[dict] | None = None,
branch_names: list[str] | None = None,
claim_entry: dict | None = None,
matching_branches: list[str] | None = None,
allow_stale_takeover: bool = False,
) -> dict[str, Any]:
"""Fail closed when duplicate work is detected before author mutations (#400)."""
stage_norm = (stage or "").strip().lower()
if stage_norm not in STAGES:
return {
"allowed": False,
"block": True,
"eligibility_class": "INVALID_STAGE",
"stage": stage_norm or None,
"reasons": [f"unknown duplicate-work stage {stage!r}"],
"safe_next_action": f"use one of: {', '.join(STAGES)}",
}
prs = list(open_prs or [])
linked_pr = _linked_open_pr(int(issue_number), prs)
branches = list(
matching_branches
if matching_branches is not None
else _matching_branch_names(int(issue_number), list(branch_names or []))
)
reasons: list[str] = []
eligibility = ELIGIBILITY_CLEAR
if linked_pr:
eligibility = ELIGIBILITY_OPEN_PR_EXISTS
reasons.append(
f"open PR #{linked_pr.get('number')} already covers issue "
f"#{issue_number}"
)
if branches and stage_norm in {"claim", "lock", "worktree", "edit"}:
if eligibility == ELIGIBILITY_CLEAR:
eligibility = ELIGIBILITY_DUPLICATE_BRANCH_EXISTS
reasons.append(
f"remote branch(es) already exist for issue #{issue_number}: "
f"{', '.join(branches)}"
)
entry = claim_entry or {}
claim_status = (entry.get("status") or "").strip()
if claim_status in {"active", "awaiting_review"} and stage_norm == "claim":
if entry.get("reclaimable") and allow_stale_takeover:
pass
elif claim_status == "active" and not entry.get("reclaimable"):
if eligibility == ELIGIBILITY_CLEAR:
eligibility = ELIGIBILITY_ACTIVE_CLAIM
reasons.append(
f"issue #{issue_number} has active claim "
f"(status={claim_status})"
)
elif claim_status == "awaiting_review" and stage_norm == "claim":
if not linked_pr:
reasons.append(
f"issue #{issue_number} is awaiting_review but no linked open PR "
"was supplied for duplicate-work proof"
)
allowed = not reasons
outcome = "duplicate_work_prevented" if not allowed else "clear"
if stage_norm == "create_pr" and not allowed:
outcome = "duplicate_pr_prevented"
elif stage_norm in {"commit", "push"} and not allowed:
outcome = "duplicate_push_prevented" if stage_norm == "push" else "duplicate_commit_prevented"
return {
"allowed": allowed,
"block": not allowed,
"eligibility_class": eligibility if not allowed else ELIGIBILITY_CLEAR,
"stage": stage_norm,
"linked_open_pr": linked_pr.get("number") if linked_pr else None,
"matching_branches": branches,
"claim_status": claim_status or None,
"outcome": outcome,
"reasons": reasons,
"safe_next_action": (
"stop without edits/commit/push/PR; produce reconciliation handoff "
"preserving local work only"
if not allowed
else "proceed"
),
}
def build_claim_entry_from_classification(classification: dict) -> dict:
"""Map ``classify_issue_claim`` output to gate claim metadata."""
return {
"status": classification.get("status"),
"reclaimable": classification.get("reclaimable"),
"linked_open_pr": classification.get("linked_open_pr"),
}
_DUPLICATE_OUTCOME_RE = re.compile(
r"(duplicate\s+(?:pr|branch|commit|push)\s+prevented|"
r"duplicate\s+work\s+not\s+prevented|reconciliation\s+handoff)",
re.IGNORECASE,
)
def assess_work_issue_duplicate_prevention_report(report_text: str) -> dict:
"""#400: work-issue reports must state duplicate-work prevention outcome."""
text = report_text or ""
if "duplicate work" not in text.lower() and "duplicate pr" not in text.lower():
return {
"proven": True,
"block": False,
"reasons": [],
"safe_next_action": "proceed",
}
if _DUPLICATE_OUTCOME_RE.search(text):
return {
"proven": True,
"block": False,
"reasons": [],
"safe_next_action": "proceed",
}
return {
"proven": False,
"block": True,
"reasons": [
"duplicate-work discussion must name a prevention outcome "
"(duplicate PR/branch/commit/push prevented, or duplicate work "
"not prevented, or reconciliation handoff)"
],
"safe_next_action": "state exact duplicate-work prevention class in final report",
}
def classify_and_assess(
issue: dict,
*,
stage: str,
comments: list[dict] | None = None,
open_prs: list[dict] | None = None,
branch_names: list[str] | None = None,
allow_stale_takeover: bool = False,
) -> dict[str, Any]:
"""Combine claim classification with duplicate-work gate assessment."""
issue_number = int(issue.get("number") or 0)
claim = classify_issue_claim(
issue=issue,
comments=comments or [],
open_prs=open_prs or [],
branch_names=branch_names or [],
)
assessment = assess_author_duplicate_work(
issue_number,
stage=stage,
open_prs=open_prs,
branch_names=branch_names,
claim_entry=build_claim_entry_from_classification(claim),
matching_branches=claim.get("matching_branches"),
allow_stale_takeover=allow_stale_takeover,
)
return {
"issue_number": issue_number,
"claim": claim,
"duplicate_work": assessment,
}
+325 -158
View File
@@ -503,7 +503,7 @@ import issue_lock_worktree # noqa: E402
import already_landed_reconcile # noqa: E402
import author_mutation_worktree # noqa: E402
import issue_claim_heartbeat # noqa: E402
import author_duplicate_work_gate # noqa: E402
import reviewer_pr_lease # noqa: E402
import merged_cleanup_reconcile # noqa: E402
import reconciler_profile # noqa: E402
import reconciliation_workflow # noqa: E402
@@ -1050,76 +1050,6 @@ def gitea_create_issue(
return _with_optional_url({"number": data["number"]}, data.get("html_url"))
def _list_repo_branch_names(h: str, o: str, r: str, auth: str, *, limit: int = 200) -> list[str]:
branches = api_get_all(f"{repo_api_url(h, o, r)}/branches", auth, limit=limit)
return [_branch_entry_name(branch) for branch in branches]
def _gather_author_duplicate_work_context(
issue_number: int,
*,
h: str,
o: str,
r: str,
auth: str,
exclude_branch_name: str | None = None,
) -> dict:
base = repo_api_url(h, o, r)
issue = api_request("GET", f"{base}/issues/{issue_number}", auth)
comments = api_request("GET", f"{base}/issues/{issue_number}/comments", auth) or []
open_prs = api_get_all(f"{base}/pulls?state=open", auth)
branch_names = _list_repo_branch_names(h, o, r, auth)
if exclude_branch_name:
branch_names = [
name for name in branch_names
if name != exclude_branch_name
]
return {
"issue": issue,
"comments": comments,
"open_prs": open_prs,
"branch_names": branch_names,
}
def _enforce_author_duplicate_work_gate(
issue_number: int,
stage: str,
*,
h: str,
o: str,
r: str,
auth: str,
allow_stale_takeover: bool = False,
exclude_branch_name: str | None = None,
) -> dict:
"""Fail closed when duplicate work is detected (#400)."""
ctx = _gather_author_duplicate_work_context(
issue_number,
h=h,
o=o,
r=r,
auth=auth,
exclude_branch_name=exclude_branch_name,
)
result = author_duplicate_work_gate.classify_and_assess(
ctx["issue"],
stage=stage,
comments=ctx["comments"],
open_prs=ctx["open_prs"],
branch_names=ctx["branch_names"],
allow_stale_takeover=allow_stale_takeover,
)
assessment = result.get("duplicate_work") or {}
if assessment.get("block"):
reasons = "; ".join(assessment.get("reasons") or ["duplicate work detected"])
raise RuntimeError(
f"Author duplicate-work gate (#400) blocked at stage '{stage}': "
f"{reasons} (fail closed)"
)
return result
@mcp.tool()
def gitea_lock_issue(
issue_number: int,
@@ -1182,17 +1112,48 @@ def gitea_lock_issue(
issue_lock_worktree.format_issue_lock_worktree_error(lock_assessment)
)
# 2. Check if the issue already has an open PR (reuse protection)
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
_enforce_author_duplicate_work_gate(
issue_number,
"lock",
h=h,
o=o,
r=r,
auth=auth,
exclude_branch_name=branch_name,
)
url = f"{repo_api_url(h, o, r)}/pulls?state=open"
try:
prs = api_get_all(url, auth)
except Exception as e:
raise RuntimeError(f"Could not list open PRs to verify issue lock: {e}")
for pr in prs:
pr_head = pr.get("head", {}).get("ref", "")
pr_title = pr.get("title", "")
pr_body = pr.get("body", "")
if expected_pattern in pr_head:
raise ValueError(
f"Issue #{issue_number} is already tied to an open PR (PR #{pr.get('number')}, branch '{pr_head}') (fail closed)"
)
patterns = [
f"closes #{issue_number}",
f"fixes #{issue_number}",
]
text_to_check = f"{pr_title} {pr_body}".lower()
if any(p in text_to_check for p in patterns):
raise ValueError(
f"Issue #{issue_number} is already tied to an open PR (PR #{pr.get('number')}) via Closes/Fixes reference (fail closed)"
)
branch_url = f"{repo_api_url(h, o, r)}/branches"
try:
branches = api_get_all(branch_url, auth)
except Exception as e:
raise RuntimeError(f"Could not list branches to verify issue lock: {e}")
for branch in branches:
name = _branch_entry_name(branch)
if expected_pattern in name:
raise ValueError(
f"Issue #{issue_number} already has matching branch '{name}' "
"(fail closed)"
)
work_lease = _build_author_issue_work_lease(
issue_number=issue_number,
@@ -1313,17 +1274,6 @@ def gitea_create_pr(
f"PR head branch '{head}' does not match locked branch '{locked_branch}' (fail closed)"
)
auth = _auth(h)
_enforce_author_duplicate_work_gate(
int(locked_issue),
"create_pr",
h=h,
o=o,
r=r,
auth=auth,
exclude_branch_name=locked_branch,
)
# Check for forbidden terms anywhere in title/body
forbidden_terms = ["equivalent", "related", "same as"]
text_to_check = f"{title} {body}".lower()
@@ -1340,6 +1290,7 @@ def gitea_create_pr(
f"PR title or body must contain 'Closes #{locked_issue}' or 'Fixes #{locked_issue}' exactly to ensure durable tracking (fail closed)"
)
auth = _auth(h)
url = f"{repo_api_url(h, o, r)}/pulls"
payload = {"title": title, "body": body, "head": head, "base": base}
meta = {"title": title, "head": head, "base": base}
@@ -1878,6 +1829,7 @@ def init_review_decision_lock(remote: str | None, task: str | None):
(os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip()
or profile_name
)
reviewer_pr_lease.clear_session_lease()
_save_review_decision_lock({
"task": task,
"remote": remote,
@@ -2320,6 +2272,20 @@ def _evaluate_pr_review_submission(
result["permission_report"] = elig["permission_report"]
return result
if live:
reasons.extend(_reviewer_pr_lease_gate(
pr_number=pr_number,
remote=remote,
host=host,
org=org,
repo=repo,
mutation=action,
live_head_sha=result.get("head_sha"),
pinned_head_sha=expected_head_sha,
))
if reasons:
return result
auth_user = result["authenticated_user"]
pr_author = result["pr_author"]
if action == "approve" and auth_user and pr_author and auth_user == pr_author:
@@ -3105,6 +3071,19 @@ def gitea_merge_pr(
result["permission_report"] = elig["permission_report"]
return result
reasons.extend(_reviewer_pr_lease_gate(
pr_number=pr_number,
remote=remote,
host=host,
org=org,
repo=repo,
mutation="merge",
live_head_sha=result.get("head_sha"),
pinned_head_sha=expected_head_sha,
))
if reasons:
return result
# Gate 4 — head SHA must match if the caller pinned a reviewed SHA.
actual_sha = result["head_sha"]
if expected_head_sha and actual_sha and expected_head_sha != actual_sha:
@@ -4404,6 +4383,261 @@ def _namespace_mutation_block(mutation_task: str, **extra_fields) -> dict | None
return blocked
def _fetch_pr_comments(
pr_number: int,
*,
remote: str,
host: str | None,
org: str | None,
repo: str | None,
) -> list[dict]:
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
api = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
return api_request("GET", api, auth) or []
def _reviewer_pr_lease_gate(
*,
pr_number: int,
remote: str,
host: str | None,
org: str | None,
repo: str | None,
mutation: str,
live_head_sha: str | None,
pinned_head_sha: str | None,
) -> list[str]:
"""Return block reasons when the session lacks an owned PR reviewer lease."""
session = reviewer_pr_lease.get_session_lease()
session_id = (session or {}).get("session_id")
identity = _authenticated_username(remote) or ""
try:
comments = _fetch_pr_comments(
pr_number, remote=remote, host=host, org=org, repo=repo)
except Exception as exc:
return [f"cannot fetch PR comments for lease gate: {_redact(str(exc))}"]
assessment = reviewer_pr_lease.assess_mutation_lease_gate(
pr_number=pr_number,
comments=comments,
reviewer_identity=identity,
session_id=session_id,
mutation=mutation,
live_head_sha=live_head_sha,
pinned_head_sha=pinned_head_sha,
)
return list(assessment.get("reasons") or []) if assessment.get("block") else []
@mcp.tool()
def gitea_acquire_reviewer_pr_lease(
pr_number: int,
worktree: str,
candidate_head: str | None = None,
target_branch: str = "master",
target_branch_sha: str | None = None,
issue_number: int | None = None,
session_id: str | None = None,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Acquire a per-PR reviewer lease before review/merge mutations (#407)."""
read_block = _profile_operation_gate("gitea.read")
if read_block:
return {
"success": False,
"acquired": False,
"reasons": read_block,
"permission_report": _permission_block_report("gitea.read"),
}
comment_block = _profile_operation_gate("gitea.pr.comment")
if comment_block:
return {
"success": False,
"acquired": False,
"reasons": comment_block,
"permission_report": _permission_block_report("gitea.pr.comment"),
}
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
profile = get_profile()
identity = _authenticated_username(remote) or profile.get("username") or ""
sid = (session_id or "").strip() or reviewer_pr_lease.new_session_id()
repo_label = f"{o}/{r}"
comments = _fetch_pr_comments(
pr_number, remote=remote, host=host, org=org, repo=repo)
assessment = reviewer_pr_lease.assess_acquire_lease(
comments,
pr_number=pr_number,
reviewer_identity=identity,
profile=profile.get("profile_name") or "unknown",
session_id=sid,
repo=repo_label,
issue_number=issue_number,
worktree=worktree,
candidate_head=candidate_head,
target_branch=target_branch,
target_branch_sha=target_branch_sha,
)
if not assessment.get("acquire_allowed"):
return {
"success": False,
"acquired": False,
"reasons": assessment.get("reasons") or [],
"existing_lease": assessment.get("existing_lease"),
}
body = assessment["lease_body"]
comment_url = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
with _audited(
"comment_pr",
host=h,
remote=remote,
org=o,
repo=r,
pr_number=pr_number,
request_metadata={"source": "acquire_reviewer_pr_lease"},
):
posted = api_request("POST", comment_url, auth, {"body": body})
session_lease = reviewer_pr_lease.record_session_lease({
"pr_number": pr_number,
"issue_number": issue_number,
"session_id": sid,
"reviewer_identity": identity,
"profile": profile.get("profile_name"),
"worktree": worktree,
"phase": "claimed",
"candidate_head": candidate_head,
"target_branch": target_branch,
"target_branch_sha": target_branch_sha,
"repo": repo_label,
"comment_id": posted.get("id"),
})
return {
"success": True,
"acquired": True,
"pr_number": pr_number,
"session_id": sid,
"comment_id": posted.get("id"),
"session_lease": session_lease,
"reasons": [],
}
@mcp.tool()
def gitea_heartbeat_reviewer_pr_lease(
pr_number: int,
phase: str,
worktree: str | None = None,
candidate_head: str | None = None,
target_branch_sha: str | None = None,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Post a reviewer lease heartbeat / phase update on the PR thread (#407)."""
comment_block = _profile_operation_gate("gitea.pr.comment")
if comment_block:
return {
"success": False,
"posted": False,
"reasons": comment_block,
"permission_report": _permission_block_report("gitea.pr.comment"),
}
session = reviewer_pr_lease.get_session_lease()
if not session or session.get("pr_number") != pr_number:
return {
"success": False,
"posted": False,
"reasons": [
f"no in-session lease for PR #{pr_number}; acquire first "
"(fail closed)"
],
}
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
body = reviewer_pr_lease.format_lease_body(
repo=f"{o}/{r}",
pr_number=pr_number,
issue_number=session.get("issue_number"),
reviewer_identity=session.get("reviewer_identity") or "",
profile=session.get("profile") or "unknown",
session_id=session.get("session_id") or reviewer_pr_lease.new_session_id(),
worktree=worktree or session.get("worktree") or "",
phase=phase,
candidate_head=candidate_head or session.get("candidate_head"),
target_branch=session.get("target_branch") or "master",
target_branch_sha=target_branch_sha or session.get("target_branch_sha"),
)
comment_url = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
with _audited(
"comment_pr",
host=h,
remote=remote,
org=o,
repo=r,
pr_number=pr_number,
request_metadata={"source": "heartbeat_reviewer_pr_lease", "phase": phase},
):
posted = api_request("POST", comment_url, auth, {"body": body})
updated = reviewer_pr_lease.record_session_lease({
**session,
"phase": phase,
"worktree": worktree or session.get("worktree"),
"candidate_head": candidate_head or session.get("candidate_head"),
"target_branch_sha": target_branch_sha or session.get("target_branch_sha"),
"last_comment_id": posted.get("id"),
})
return {
"success": True,
"posted": True,
"pr_number": pr_number,
"phase": phase,
"comment_id": posted.get("id"),
"session_lease": updated,
"reasons": [],
}
@mcp.tool()
def gitea_assess_reviewer_pr_lease(
pr_number: int,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Read-only: assess active reviewer lease state for a PR (#407)."""
read_block = _profile_operation_gate("gitea.read")
if read_block:
return {
"success": False,
"reasons": read_block,
"permission_report": _permission_block_report("gitea.read"),
}
comments = _fetch_pr_comments(
pr_number, remote=remote, host=host, org=org, repo=repo)
active = reviewer_pr_lease.find_active_reviewer_lease(
comments, pr_number=pr_number)
return {
"success": True,
"pr_number": pr_number,
"active_lease": active,
"session_lease": reviewer_pr_lease.get_session_lease(),
"reasons": [],
}
@mcp.tool()
def gitea_list_issue_comments(
issue_number: int,
@@ -6053,14 +6287,6 @@ def gitea_mark_issue(
)
if action == "start":
_enforce_author_duplicate_work_gate(
issue_number,
"claim",
h=h,
o=o,
r=r,
auth=auth,
)
with _audited("label_issue", host=h, remote=remote, org=o, repo=r,
issue_number=issue_number,
request_metadata={"op": "add", "label": "status:in-progress"}):
@@ -6142,65 +6368,6 @@ def gitea_post_heartbeat(
)
@mcp.tool()
def gitea_assess_author_duplicate_work(
issue_number: int,
stage: str,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
allow_stale_takeover: bool = False,
exclude_branch_name: str | None = None,
) -> dict:
"""Read-only: assess duplicate-work risk before author mutations (#400).
Call at claim, lock, worktree, edit, commit, push, and create_pr stages.
"""
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)
ctx = _gather_author_duplicate_work_context(
issue_number,
h=h,
o=o,
r=r,
auth=auth,
exclude_branch_name=exclude_branch_name,
)
result = author_duplicate_work_gate.classify_and_assess(
ctx["issue"],
stage=stage,
comments=ctx["comments"],
open_prs=ctx["open_prs"],
branch_names=ctx["branch_names"],
allow_stale_takeover=allow_stale_takeover,
)
assessment = result.get("duplicate_work") or {}
return {
"success": True,
"performed": False,
"issue_number": issue_number,
"stage": stage,
"allowed": assessment.get("allowed"),
"block": assessment.get("block"),
"eligibility_class": assessment.get("eligibility_class"),
"outcome": assessment.get("outcome"),
"linked_open_pr": assessment.get("linked_open_pr"),
"matching_branches": assessment.get("matching_branches"),
"claim_status": assessment.get("claim_status"),
"reasons": assessment.get("reasons"),
"safe_next_action": assessment.get("safe_next_action"),
"claim": result.get("claim"),
}
@mcp.tool()
def gitea_reconcile_issue_claims(
state: str = "open",
+1 -19
View File
@@ -3622,33 +3622,18 @@ def assess_work_issue_mode_isolation(report_text: str) -> dict:
}
def assess_work_issue_duplicate_prevention_report(report_text, **kwargs):
"""#400: work-issue reports must classify duplicate-work prevention."""
from author_duplicate_work_gate import (
assess_work_issue_duplicate_prevention_report as _assess,
)
return _assess(report_text, **kwargs)
def assess_work_issue_final_report(report_text: str) -> dict:
"""#139: composite verifier for work-issue final reports."""
checks = {
"workflow_source": assess_work_issue_workflow_source(report_text),
"mode_isolation": assess_work_issue_mode_isolation(report_text),
"duplicate_prevention": assess_work_issue_duplicate_prevention_report(
report_text
),
}
reasons = []
downgraded = False
for name, result in checks.items():
verdict = result.get("verdict")
if result.get("block"):
downgraded = True
reasons.extend(result.get("reasons") or [])
elif verdict in ("missing", "incomplete"):
if verdict in ("missing", "incomplete"):
downgraded = True
reasons.extend(result.get("reasons") or [])
elif result.get("downgraded") or not result.get("complete", True):
@@ -3656,9 +3641,6 @@ def assess_work_issue_final_report(report_text: str) -> dict:
reasons.extend(
f"{name}: {r}" for r in (result.get("reasons") or [])
)
elif result.get("proven") is False:
downgraded = True
reasons.extend(result.get("reasons") or [])
grade = "A" if not downgraded else "downgraded"
return {
+382
View File
@@ -0,0 +1,382 @@
"""Per-PR reviewer leases for safe parallel review sessions (#407)."""
from __future__ import annotations
import os
import re
import uuid
from datetime import datetime, timedelta, timezone
from typing import Any
MARKER = "<!-- mcp-review-lease:v1 -->"
_FIELD_RE = re.compile(
r"^\s*([a-z_]+)\s*:\s*(.+?)\s*$",
re.IGNORECASE | re.MULTILINE,
)
_FULL_SHA = re.compile(r"^[0-9a-f]{40}$", re.IGNORECASE)
_TERMINAL_PHASES = frozenset({"done", "released", "blocked"})
_ACTIVE_PHASES = frozenset({
"claimed",
"validating",
"approved",
"request-changes",
"merging",
})
DEFAULT_LEASE_TTL_MINUTES = 120
STALE_WARNING_MINUTES = 30
RECLAIMABLE_MINUTES = 60
_SESSION_LEASE: dict[str, Any] | None = None
def _parse_timestamp(value: str | None) -> datetime | None:
if not value:
return None
text = value.strip()
if text.endswith("Z"):
text = text[:-1] + "+00:00"
try:
parsed = datetime.fromisoformat(text)
except ValueError:
return None
if parsed.tzinfo is None:
return parsed.replace(tzinfo=timezone.utc)
return parsed.astimezone(timezone.utc)
def _normalize_sha(value: str | None) -> str | None:
text = (value or "").strip().lower()
return text if text and _FULL_SHA.match(text) else None
def _parse_pr_ref(value: str | None) -> int | None:
digits = re.sub(r"[^\d]", "", value or "")
return int(digits) if digits.isdigit() else None
def new_session_id() -> str:
return f"{os.getpid()}-{uuid.uuid4().hex[:12]}"
def format_lease_body(
*,
repo: str,
pr_number: int,
issue_number: int | None,
reviewer_identity: str,
profile: str,
session_id: str,
worktree: str,
phase: str,
candidate_head: str | None,
target_branch: str,
target_branch_sha: str | None,
last_activity: datetime | None = None,
expires_at: datetime | None = None,
blocker: str = "none",
) -> str:
now = last_activity or datetime.now(timezone.utc)
expires = expires_at or (now + timedelta(minutes=DEFAULT_LEASE_TTL_MINUTES))
last_text = now.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace(
"+00:00", "Z"
)
expires_text = expires.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace(
"+00:00", "Z"
)
issue_text = f"#{issue_number}" if issue_number else "none"
lines = [
MARKER,
f"repo: {repo}",
f"pr: #{pr_number}",
f"issue: {issue_text}",
f"reviewer_identity: {reviewer_identity}",
f"profile: {profile}",
f"session_id: {session_id}",
f"worktree: {worktree}",
f"phase: {phase}",
f"candidate_head: {candidate_head or 'none'}",
f"target_branch: {target_branch}",
f"target_branch_sha: {target_branch_sha or 'none'}",
f"last_activity: {last_text}",
f"expires_at: {expires_text}",
f"blocker: {blocker}",
]
return "\n".join(lines)
def parse_lease_comment(body: str) -> dict[str, Any] | None:
text = body or ""
if MARKER not in text:
return None
fields: dict[str, str] = {}
for match in _FIELD_RE.finditer(text):
fields[match.group(1).strip().lower()] = match.group(2).strip()
if not fields:
return None
return {
"repo": fields.get("repo"),
"pr_number": _parse_pr_ref(fields.get("pr")),
"issue_number": _parse_pr_ref(fields.get("issue")),
"reviewer_identity": fields.get("reviewer_identity"),
"profile": fields.get("profile"),
"session_id": fields.get("session_id"),
"worktree": fields.get("worktree"),
"phase": (fields.get("phase") or "").strip().lower() or None,
"candidate_head": _normalize_sha(fields.get("candidate_head")),
"target_branch": fields.get("target_branch"),
"target_branch_sha": _normalize_sha(fields.get("target_branch_sha")),
"last_activity": fields.get("last_activity"),
"expires_at": fields.get("expires_at"),
"blocker": fields.get("blocker"),
"raw_fields": fields,
}
def _lease_entries(comments: list[dict], *, pr_number: int) -> list[dict]:
entries: list[dict] = []
for comment in comments or []:
parsed = parse_lease_comment(comment.get("body") or "")
if not parsed:
continue
if parsed.get("pr_number") not in (None, pr_number):
continue
entries.append({
**parsed,
"comment_id": comment.get("id"),
"author": (comment.get("user") or {}).get("login") or comment.get("author"),
"created_at": comment.get("created_at"),
"updated_at": comment.get("updated_at"),
})
return entries
def _lease_expired(lease: dict, *, now: datetime) -> bool:
expires_at = _parse_timestamp(lease.get("expires_at"))
return bool(expires_at and expires_at <= now)
def _minutes_since_activity(lease: dict, *, now: datetime) -> float | None:
last = _parse_timestamp(lease.get("last_activity"))
if not last:
return None
return (now - last).total_seconds() / 60.0
def classify_lease_freshness(lease: dict, *, now: datetime | None = None) -> str:
"""Return active, stale_warning, reclaimable, expired, or terminal."""
now = now or datetime.now(timezone.utc)
phase = (lease.get("phase") or "").strip().lower()
if phase in _TERMINAL_PHASES:
return "terminal"
if _lease_expired(lease, now=now):
return "expired"
minutes = _minutes_since_activity(lease, now=now)
if minutes is None:
return "active"
if minutes >= RECLAIMABLE_MINUTES:
return "reclaimable"
if minutes >= STALE_WARNING_MINUTES:
return "stale_warning"
return "active"
def find_active_reviewer_lease(
comments: list[dict],
*,
pr_number: int,
now: datetime | None = None,
) -> dict[str, Any] | None:
"""Newest non-terminal, unexpired lease for *pr_number*."""
now = now or datetime.now(timezone.utc)
for lease in reversed(_lease_entries(comments, pr_number=pr_number)):
phase = (lease.get("phase") or "").strip().lower()
if phase in _TERMINAL_PHASES:
continue
if _lease_expired(lease, now=now):
continue
if phase in _ACTIVE_PHASES or phase:
lease = dict(lease)
lease["freshness"] = classify_lease_freshness(lease, now=now)
return lease
return None
def assess_acquire_lease(
comments: list[dict],
*,
pr_number: int,
reviewer_identity: str,
profile: str,
session_id: str,
repo: str,
issue_number: int | None,
worktree: str,
candidate_head: str | None,
target_branch: str,
target_branch_sha: str | None,
now: datetime | None = None,
) -> dict[str, Any]:
"""Fail closed when another session holds an active lease."""
now = now or datetime.now(timezone.utc)
reasons: list[str] = []
existing = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
if existing:
owner_session = (existing.get("session_id") or "").strip()
freshness = existing.get("freshness") or classify_lease_freshness(existing, now=now)
if owner_session and owner_session != session_id and freshness in {
"active", "stale_warning"
}:
reasons.append(
f"PR #{pr_number} already has active reviewer lease "
f"(session_id={owner_session}, phase={existing.get('phase')})"
)
elif owner_session and owner_session != session_id and freshness == "reclaimable":
reasons.append(
f"PR #{pr_number} lease is reclaimable but still held by "
f"session_id={owner_session}; explicit reclaim not implemented "
"(fail closed)"
)
if not (reviewer_identity or "").strip():
reasons.append("reviewer identity required for lease acquisition")
if not (session_id or "").strip():
reasons.append("session_id required for lease acquisition")
if not (worktree or "").strip():
reasons.append("worktree path required for lease acquisition")
allowed = not reasons
body = None
if allowed:
body = format_lease_body(
repo=repo,
pr_number=pr_number,
issue_number=issue_number,
reviewer_identity=reviewer_identity,
profile=profile,
session_id=session_id,
worktree=worktree,
phase="claimed",
candidate_head=candidate_head,
target_branch=target_branch,
target_branch_sha=target_branch_sha,
last_activity=now,
)
return {
"acquire_allowed": allowed,
"reasons": reasons,
"existing_lease": existing,
"lease_body": body,
"session_id": session_id,
}
def record_session_lease(lease: dict[str, Any]) -> dict[str, Any]:
global _SESSION_LEASE
_SESSION_LEASE = dict(lease)
return dict(_SESSION_LEASE)
def clear_session_lease() -> None:
global _SESSION_LEASE
_SESSION_LEASE = None
def get_session_lease() -> dict[str, Any] | None:
return dict(_SESSION_LEASE) if _SESSION_LEASE else None
def assess_mutation_lease_gate(
*,
pr_number: int,
comments: list[dict],
reviewer_identity: str,
session_id: str | None,
mutation: str,
live_head_sha: str | None,
pinned_head_sha: str | None,
now: datetime | None = None,
) -> dict[str, Any]:
"""Reviewer mutations require an owned, current PR lease."""
now = now or datetime.now(timezone.utc)
reasons: list[str] = []
session = get_session_lease()
active = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
if not session:
reasons.append(
f"no in-session reviewer lease recorded; acquire via "
f"gitea_acquire_reviewer_pr_lease before {mutation}"
)
elif session.get("pr_number") != pr_number:
reasons.append(
f"session lease is for PR #{session.get('pr_number')}, not #{pr_number}"
)
elif (session.get("session_id") or "") != (session_id or session.get("session_id")):
reasons.append("session lease session_id mismatch (fail closed)")
if active:
owner = (active.get("session_id") or "").strip()
if owner and session_id and owner != session_id:
reasons.append(
f"active PR lease owned by session_id={owner}; current session "
f"cannot {mutation}"
)
pinned = _normalize_sha(pinned_head_sha)
live = _normalize_sha(live_head_sha)
lease_head = active.get("candidate_head")
if pinned and live and pinned != live:
reasons.append(
"PR head changed during lease; stop and re-validate before "
f"reviewer {mutation}"
)
if lease_head and live and lease_head != live:
reasons.append(
"live PR head differs from lease candidate_head; refresh lease "
f"before {mutation}"
)
freshness = active.get("freshness") or classify_lease_freshness(active, now=now)
if freshness in {"expired", "reclaimable"}:
reasons.append(f"reviewer lease freshness is '{freshness}' (fail closed)")
else:
reasons.append(f"no active reviewer lease found on PR #{pr_number}")
allowed = not reasons
return {
"mutation_allowed": allowed,
"block": not allowed,
"reasons": reasons,
"active_lease": active,
"session_lease": session,
}
def assess_lease_inventory(
comments_by_pr: dict[int, list[dict]],
*,
now: datetime | None = None,
) -> dict[str, Any]:
"""Summarize lease states across PR comment threads."""
now = now or datetime.now(timezone.utc)
active: list[dict] = []
stale: list[dict] = []
reclaimable: list[dict] = []
for pr_number, comments in (comments_by_pr or {}).items():
lease = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
if not lease:
continue
freshness = lease.get("freshness") or classify_lease_freshness(lease, now=now)
entry = {"pr_number": pr_number, "session_id": lease.get("session_id"), "freshness": freshness}
if freshness == "stale_warning":
stale.append(entry)
elif freshness == "reclaimable":
reclaimable.append(entry)
else:
active.append(entry)
return {
"active_review_leases": active,
"stale_review_leases": stale,
"reclaimable_review_leases": reclaimable,
}
@@ -732,6 +732,26 @@ The final report must identify:
* whether same-PR merge continuation was allowed
* whether the run stopped as required
## 26B. Per-PR reviewer lease (#407)
Parallel reviewer sessions are allowed only when each session holds a distinct,
live PR lease.
Before validation or review mutation on a selected PR:
1. Call `gitea_acquire_reviewer_pr_lease` with worktree path, candidate head SHA,
and target branch SHA.
2. Post heartbeats via `gitea_heartbeat_reviewer_pr_lease` before validation,
after validation, before review mutation, and before merge.
3. Do not approve, request changes, or merge unless the in-session lease
matches the selected PR.
If PR head or target branch advances during the lease, stop and refresh
inventory before continuing.
Final reports must include lease session id, acquisition proof, heartbeat
status, and release/blocked status.
## 27. Merge rules
Before merge, rerun fresh live checks:
@@ -277,24 +277,6 @@ Do not select an issue based only on memory from a previous session.
Before claiming or working on an issue, check whether there is already an open PR, branch, or active claim for that issue.
Run `gitea_assess_author_duplicate_work` at these stages and stop when `block` is true:
* `claim` — before `gitea_mark_issue`
* `lock` — before `gitea_lock_issue`
* `worktree` / `edit` — before creating a worktree or editing files
* `commit` — immediately before `git commit`
* `push` — immediately before `git push`
* `create_pr` — immediately before `gitea_create_pr` (also enforced server-side)
`gitea_mark_issue`, `gitea_lock_issue`, and `gitea_create_pr` enforce the same gate server-side and fail closed.
If a concurrent open PR appears after work begins:
* before commit or push — stop and preserve local work without pushing
* after push but before PR creation — produce a reconciliation handoff instead of opening a PR
Final reports must name the duplicate-work outcome (`duplicate PR prevented`, `duplicate branch prevented`, `duplicate commit prevented`, `duplicate push prevented`, `duplicate work not prevented`, or `reconciliation handoff`).
If an open PR already exists for the issue, do not implement duplicate work.
Classify the issue as:
+29 -3
View File
@@ -286,6 +286,21 @@ class TestSimpleToolAudit(_AuditWiringBase):
class TestGatedToolAudit(_AuditWiringBase):
def setUp(self):
super().setUp()
from tests.test_mcp_server import _install_owned_reviewer_lease
import reviewer_pr_lease
self._lease_patch = _install_owned_reviewer_lease(8)
self._lease_patch.start()
self._auth_identity_patch = patch(
"mcp_server._authenticated_username", return_value="reviewer-bot"
)
self._auth_identity_patch.start()
self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def _pr(self, author, state="open", sha="abc123", mergeable=True):
return {"user": {"login": author}, "state": state,
"head": {"sha": sha}, "mergeable": mergeable}
@@ -344,11 +359,22 @@ class TestGatedToolAudit(_AuditWiringBase):
GITEA_ALLOWED_OPERATIONS="read,review,approve")
with patch.dict(os.environ, env, clear=True):
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
from tests.test_mcp_server import _install_owned_reviewer_lease
import reviewer_pr_lease
init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(8, "approve", remote="prgs")
r = gitea_submit_pr_review(pr_number=8, action="approve",
body="LGTM", remote="prgs",
final_review_decision_ready=True)
lease_patch = _install_owned_reviewer_lease(8)
lease_patch.start()
try:
r = gitea_submit_pr_review(
pr_number=8, action="approve",
body="LGTM", remote="prgs",
final_review_decision_ready=True,
)
finally:
lease_patch.stop()
reviewer_pr_lease.clear_session_lease()
self.assertTrue(r["performed"])
recs = self._records()
self.assertEqual(len(recs), 1)
-102
View File
@@ -1,102 +0,0 @@
"""Tests for early author duplicate-work gate (#400)."""
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from author_duplicate_work_gate import ( # noqa: E402
ELIGIBILITY_OPEN_PR_EXISTS,
assess_author_duplicate_work,
assess_work_issue_duplicate_prevention_report,
)
def _open_pr(number: int = 397, issue: int = 395) -> dict:
return {
"number": number,
"head": {"ref": f"feat/issue-{issue}-example"},
"title": f"feat: example (Closes #{issue})",
"body": f"Closes #{issue}",
}
class TestAuthorDuplicateWorkGate(unittest.TestCase):
def test_clear_when_no_duplicates(self):
result = assess_author_duplicate_work(
400,
stage="claim",
open_prs=[],
branch_names=["master", "feat/issue-399-other"],
)
self.assertTrue(result["allowed"])
self.assertFalse(result["block"])
def test_open_pr_blocks_claim(self):
result = assess_author_duplicate_work(
395,
stage="claim",
open_prs=[_open_pr()],
branch_names=[],
)
self.assertFalse(result["allowed"])
self.assertEqual(result["eligibility_class"], ELIGIBILITY_OPEN_PR_EXISTS)
def test_matching_branch_blocks_lock_not_create_pr(self):
branches = ["feat/issue-400-early-duplicate-work-gate"]
lock = assess_author_duplicate_work(
400,
stage="lock",
open_prs=[],
branch_names=branches,
)
self.assertFalse(lock["allowed"])
create_pr = assess_author_duplicate_work(
400,
stage="create_pr",
open_prs=[],
branch_names=branches,
matching_branches=[],
)
self.assertTrue(create_pr["allowed"])
def test_open_pr_blocks_create_pr_stage(self):
result = assess_author_duplicate_work(
395,
stage="create_pr",
open_prs=[_open_pr()],
branch_names=["feat/issue-395-proof-backed-review-handoff"],
)
self.assertFalse(result["allowed"])
self.assertEqual(result["outcome"], "duplicate_pr_prevented")
def test_push_stage_blocks_on_concurrent_pr(self):
result = assess_author_duplicate_work(
395,
stage="push",
open_prs=[_open_pr()],
branch_names=[],
)
self.assertFalse(result["allowed"])
self.assertEqual(result["outcome"], "duplicate_push_prevented")
def test_duplicate_prevention_report_requires_outcome(self):
bad = assess_work_issue_duplicate_prevention_report(
"Duplicate work detected for issue #395."
)
self.assertFalse(bad["proven"])
good = assess_work_issue_duplicate_prevention_report(
"Duplicate PR prevented; reconciliation handoff produced."
)
self.assertTrue(good["proven"])
def test_exported_from_review_proofs(self):
from review_proofs import assess_work_issue_duplicate_prevention_report as exported
self.assertTrue(callable(exported))
if __name__ == "__main__":
unittest.main()
-6
View File
@@ -95,12 +95,6 @@ def test_create_issue_workflow_contract():
assert "## 9. Duplicate search before mutation" in text
def test_author_duplicate_work_gate_exported():
from review_proofs import assess_work_issue_duplicate_prevention_report
assert callable(assess_work_issue_duplicate_prevention_report)
def test_work_issue_workflow_contract():
text = (SKILL_DIR / "workflows" / "work-issue.md").read_text(encoding="utf-8")
assert "canonical: true" in text
+127 -12
View File
@@ -79,6 +79,64 @@ def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"):
return [_formal_review(reviewer, "APPROVED", sha=sha)]
_DEFAULT_LEASE_SESSION = "mcp-test-reviewer-lease"
def _reviewer_lease_comment(
pr_number,
*,
session_id=_DEFAULT_LEASE_SESSION,
head_sha="abc123",
reviewer="reviewer-bot",
):
from datetime import datetime, timezone
import reviewer_pr_lease
body = reviewer_pr_lease.format_lease_body(
repo="Scaled-Tech-Consulting/Gitea-Tools",
pr_number=pr_number,
issue_number=407,
reviewer_identity=reviewer,
profile="gitea-reviewer",
session_id=session_id,
worktree="branches/review-test",
phase="claimed",
candidate_head=head_sha,
target_branch="master",
target_branch_sha="b" * 40,
last_activity=datetime.now(timezone.utc),
)
return {"id": 9001, "body": body, "user": {"login": reviewer}}
def _install_owned_reviewer_lease(
pr_number,
*,
session_id=_DEFAULT_LEASE_SESSION,
head_sha="abc123",
):
import reviewer_pr_lease
reviewer_pr_lease.clear_session_lease()
reviewer_pr_lease.record_session_lease({
"pr_number": pr_number,
"session_id": session_id,
"candidate_head": head_sha,
"target_branch": "master",
})
return patch(
"mcp_server._fetch_pr_comments",
return_value=[
_reviewer_lease_comment(
pr_number,
session_id=session_id,
head_sha=head_sha,
)
],
)
# Issue-write tools are profile-gated (#69).
ISSUE_WRITE_ENV = {
"GITEA_ALLOWED_OPERATIONS": (
@@ -512,6 +570,19 @@ class TestViewPR(unittest.TestCase):
class TestMergePR(unittest.TestCase):
"""Gated merge workflow (#16). gitea_merge_pr is the only merge path."""
def setUp(self):
import reviewer_pr_lease
self._lease_patch = _install_owned_reviewer_lease(8)
self._lease_patch.start()
self._auth_identity_patch = patch(
"mcp_server._authenticated_username", return_value="reviewer-bot"
)
self._auth_identity_patch.start()
self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def _pr(self, author, state="open", sha="abc123", mergeable=True):
return {
"user": {"login": author},
@@ -772,9 +843,11 @@ class TestMergePR(unittest.TestCase):
pr_number=8, confirmation=self._confirm(8),
expected_head_sha="deadbeef", remote="prgs")
self.assertFalse(r["performed"])
self.assertIn(
"expected head SHA does not match current PR head (fail closed)",
r["reasons"])
self.assertTrue(any(
"expected head SHA does not match current PR head (fail closed)" in reason
or "PR head changed during lease" in reason
for reason in r["reasons"]
))
self._assert_no_merge_call(mock_api)
@patch("mcp_server.api_request")
@@ -1653,7 +1726,20 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
}
def setUp(self):
import reviewer_pr_lease
init_review_decision_lock("prgs", "review_pr")
self._lease_patch = _install_owned_reviewer_lease(
self.PR, head_sha=self.SHA,
)
self._lease_patch.start()
self._auth_identity_patch = patch(
"mcp_server._authenticated_username", return_value="reviewer-bot"
)
self._auth_identity_patch.start()
self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def _env(self):
return patch.dict(os.environ, {
@@ -1748,8 +1834,19 @@ class TestSubmitPrReview(unittest.TestCase):
"""Gated review-mutation tool (#15)."""
def setUp(self):
import reviewer_pr_lease
init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(8, "approve", remote="prgs")
self._lease_patch = _install_owned_reviewer_lease(8)
self._lease_patch.start()
self._auth_identity_patch = patch(
"mcp_server._authenticated_username", return_value="reviewer-bot"
)
self._auth_identity_patch.start()
self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def _pr(self, author, state="open", sha="abc123", mergeable=True):
return {
@@ -1987,9 +2084,11 @@ class TestSubmitPrReview(unittest.TestCase):
final_review_decision_ready=True,
)
self.assertFalse(r["performed"])
self.assertIn(
"expected head SHA does not match current PR head (fail closed)",
r["reasons"])
self.assertTrue(any(
"expected head SHA does not match current PR head (fail closed)" in reason
or "PR head changed during lease" in reason
for reason in r["reasons"]
))
self._assert_no_mutation(mock_api)
def test_head_sha_match_allows(self):
@@ -2052,9 +2151,9 @@ class TestSubmitPrReview(unittest.TestCase):
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
with patch.dict(os.environ, env, clear=True):
gitea_mark_final_review_decision(5, "approve", remote="prgs")
gitea_mark_final_review_decision(8, "approve", remote="prgs")
r = gitea_submit_pr_review(
pr_number=5, action="approve", remote="prgs",
pr_number=8, action="approve", remote="prgs",
final_review_decision_ready=True,
)
self.assertFalse(r["performed"])
@@ -2273,6 +2372,13 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
self.assertEqual(res["cleanup_status"].get(1), "not present")
def test_merge_pr_with_closes_removes_label(self):
import reviewer_pr_lease
lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123")
lease_patch.start()
self.addCleanup(lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def api_side_effect(method, url, auth, payload=None):
if method == "GET" and "/user" in url:
return {"login": "merger"}
@@ -2307,6 +2413,13 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
self.assertEqual(res["cleanup_status"].get(123), "released")
def test_merge_pr_with_branch_name_removes_label(self):
import reviewer_pr_lease
lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123")
lease_patch.start()
self.addCleanup(lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def api_side_effect(method, url, auth, payload=None):
if method == "GET" and "/user" in url:
return {"login": "merger"}
@@ -2988,10 +3101,12 @@ class TestVerifyMutationAuthority(unittest.TestCase):
# profile; the active profile resolves as reviewer — side-channel
# override rejected even with a matching in-process authority.
self._authority()
with patch.dict(os.environ,
{"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}):
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_mutation_authority("prgs")
with patch("mcp_server.gitea_config.is_runtime_switching_enabled",
return_value=False):
with patch.dict(os.environ,
{"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}):
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_mutation_authority("prgs")
self.assertIn("side-channel override rejected", str(ctx.exception))
def test_foreign_pid_authority_is_not_trusted(self):
+16 -3
View File
@@ -156,9 +156,22 @@ class TestPRQueueInventory(unittest.TestCase):
]
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(1, "approve", remote="prgs")
result = gitea_review_pr(pr_number=1, event="APPROVE", remote="prgs", final_review_decision_ready=True)
from tests.test_mcp_server import _install_owned_reviewer_lease
import reviewer_pr_lease
with patch("mcp_server._authenticated_username", return_value="reviewer1"):
init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(1, "approve", remote="prgs")
lease_patch = _install_owned_reviewer_lease(
1, head_sha="abc1", session_id="inventory-review-lease",
)
lease_patch.start()
self.addCleanup(lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
result = gitea_review_pr(
pr_number=1, event="APPROVE", remote="prgs",
final_review_decision_ready=True,
)
self.assertTrue(result["success"])
self.assertIn("=== PR Queue Inventory ===", result["message"])
self.assertIn("Repository:", result["message"])
+193
View File
@@ -0,0 +1,193 @@
"""Tests for per-PR reviewer leases (#407)."""
import sys
import unittest
from datetime import datetime, timedelta, timezone
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import reviewer_pr_lease as leases
def _lease_comment(
pr_number: int,
session_id: str,
*,
phase: str = "claimed",
minutes_ago: int = 0,
candidate_head: str = "a" * 40,
) -> dict:
now = datetime.now(timezone.utc) - timedelta(minutes=minutes_ago)
body = leases.format_lease_body(
repo="Scaled-Tech-Consulting/Gitea-Tools",
pr_number=pr_number,
issue_number=295,
reviewer_identity="rev1",
profile="prgs-reviewer",
session_id=session_id,
worktree="branches/review-pr382",
phase=phase,
candidate_head=candidate_head,
target_branch="master",
target_branch_sha="b" * 40,
last_activity=now,
)
return {"id": 1, "body": body, "user": {"login": "rev1"}}
class TestReviewerLeaseAcquire(unittest.TestCase):
def setUp(self):
leases.clear_session_lease()
def test_two_reviewers_cannot_lease_same_pr(self):
comments = [_lease_comment(382, "session-a")]
result = leases.assess_acquire_lease(
comments,
pr_number=382,
reviewer_identity="rev2",
profile="prgs-reviewer",
session_id="session-b",
repo="Scaled-Tech-Consulting/Gitea-Tools",
issue_number=295,
worktree="branches/review-pr382-b",
candidate_head="c" * 40,
target_branch="master",
target_branch_sha="d" * 40,
)
self.assertFalse(result["acquire_allowed"])
self.assertTrue(any("already has active" in r for r in result["reasons"]))
def test_two_reviewers_can_lease_different_prs(self):
comments = [_lease_comment(382, "session-a")]
result = leases.assess_acquire_lease(
comments,
pr_number=383,
reviewer_identity="rev2",
profile="prgs-reviewer",
session_id="session-b",
repo="Scaled-Tech-Consulting/Gitea-Tools",
issue_number=296,
worktree="branches/review-pr383",
candidate_head="c" * 40,
target_branch="master",
target_branch_sha="d" * 40,
)
self.assertTrue(result["acquire_allowed"])
self.assertIsNotNone(result["lease_body"])
class TestReviewerLeaseFreshness(unittest.TestCase):
def test_stale_warning_after_30_minutes(self):
lease = leases.parse_lease_comment(
_lease_comment(382, "session-a", minutes_ago=35)["body"]
)
self.assertEqual(
leases.classify_lease_freshness(lease),
"stale_warning",
)
def test_reclaimable_after_60_minutes(self):
lease = leases.parse_lease_comment(
_lease_comment(382, "session-a", minutes_ago=65)["body"]
)
self.assertEqual(
leases.classify_lease_freshness(lease),
"reclaimable",
)
class TestReviewerLeaseMutationGate(unittest.TestCase):
def setUp(self):
leases.clear_session_lease()
def test_reviewer_without_lease_cannot_mutate(self):
head = "f" * 40
comments = [_lease_comment(382, "other-session", candidate_head=head)]
result = leases.assess_mutation_lease_gate(
pr_number=382,
comments=comments,
reviewer_identity="rev1",
session_id="my-session",
mutation="approve",
live_head_sha=head,
pinned_head_sha=head,
)
self.assertTrue(result["block"])
def test_owned_lease_allows_mutation(self):
head = "f" * 40
comments = [_lease_comment(382, "my-session", candidate_head=head)]
leases.record_session_lease({
"pr_number": 382,
"session_id": "my-session",
"candidate_head": head,
"target_branch": "master",
})
result = leases.assess_mutation_lease_gate(
pr_number=382,
comments=comments,
reviewer_identity="rev1",
session_id="my-session",
mutation="approve",
live_head_sha=head,
pinned_head_sha=head,
)
self.assertFalse(result["block"])
def test_head_change_invalidates_lease(self):
reviewed = "f" * 40
live = "e" * 40
comments = [_lease_comment(382, "my-session", candidate_head=reviewed)]
leases.record_session_lease({
"pr_number": 382,
"session_id": "my-session",
"candidate_head": reviewed,
})
result = leases.assess_mutation_lease_gate(
pr_number=382,
comments=comments,
reviewer_identity="rev1",
session_id="my-session",
mutation="merge",
live_head_sha=live,
pinned_head_sha=reviewed,
)
self.assertTrue(result["block"])
self.assertTrue(any("head" in r.lower() for r in result["reasons"]))
class TestReviewerLeaseMcpGate(unittest.TestCase):
def setUp(self):
leases.clear_session_lease()
patch("mcp_server.verify_preflight_purity").start()
patch("gitea_audit.audit_enabled", return_value=False).start()
mcp_server = __import__("mcp_server")
mcp_server._IDENTITY_CACHE.clear()
mcp_server.init_review_decision_lock("prgs", "review_pr")
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", "reviewer")
def tearDown(self):
patch.stopall()
leases.clear_session_lease()
def test_reviewer_pr_lease_gate_helper_blocks_without_session(self):
import mcp_server
head = "a" * 40
with patch("mcp_server._fetch_pr_comments", return_value=[]):
reasons = mcp_server._reviewer_pr_lease_gate(
pr_number=382,
remote="prgs",
host=None,
org=None,
repo=None,
mutation="approve",
live_head_sha=head,
pinned_head_sha=head,
)
self.assertTrue(any("lease" in r.lower() for r in reasons))
if __name__ == "__main__":
unittest.main()