Compare commits

..
Author SHA1 Message Date
sysadminandClaude Opus 4.8 80aa295635 feat: non-destructive branch recovery after lost issue locks (Closes #440)
Implements the #420 recovery umbrella: keyed persistent issue locks with
own-branch adoption, structured branch ownership parsing, create_pr lock
resolution after MCP restart, and workflow docs forbidding destructive
branch deletion as the default recovery path.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 16:42:23 -04:00
sysadminandClaude Opus 4.8 6b97544ff6 feat: replace global issue lock with keyed persistent store (Closes #443)
Store per remote/org/repo/issue locks under GITEA_ISSUE_LOCK_DIR with
atomic writes and per-session binding. Integrate own-branch adoption for
lock recovery, update worktree-start and cleanup reconcile, and add tests
documenting the ban on manual global lock seeding.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 16:27:10 -04:00
29 changed files with 1193 additions and 2671 deletions
+18 -19
View File
@@ -274,27 +274,26 @@ is proven abandoned and the takeover is recorded.
Gitea-Tools lease gates: `gitea_lock_issue` (fail-closed before author Gitea-Tools lease gates: `gitea_lock_issue` (fail-closed before author
mutations), `status:in-progress`, and claim comments. `gitea_lock_issue` mutations), `status:in-progress`, and claim comments. `gitea_lock_issue`
records an `author_issue_work` lease in the issue-lock payload with issue records an `author_issue_work` lease in a keyed lock file under
number, optional PR number, branch, worktree path, claimant identity/profile, `GITEA_ISSUE_LOCK_DIR` (default `~/.cache/gitea-tools/issue-locks`), one file
created timestamp, expiry timestamp, and last heartbeat timestamp. An active per `remote` + `org` + `repo` + `issue_number`. The current MCP session binds
same-issue/same-operation lease blocks duplicate work. An expired lease still its active lock through a per-process pointer so concurrent repos/issues never
blocks takeover until a recovery review records why the prior work is abandoned, share one overwrite-prone slot (#443).
completed, or unsafe to continue.
**Issue-lock recovery (#447):** Do not manually seed, restore, or delete Each lock payload includes issue number, optional PR number, branch, worktree
`/tmp/gitea_issue_lock.json` as a normal recovery path. That file is global path, claimant identity/profile, created timestamp, expiry timestamp, and last
shared state and manual writes can clobber another session's live lease. Use heartbeat timestamp. An active same-issue/same-operation lease blocks duplicate
`sanctioned recovery` instead: work. An expired lease still blocks takeover until a recovery review records why
the prior work is abandoned, completed, or unsafe to continue.
1. `gitea_lock_issue` on a clean `branches/` worktree (normal path). **Do not manually seed `/tmp/gitea_issue_lock.json` or any lock file as a normal
2. Own-branch adoption via #442 when the issue's exact branch is already pushed. recovery path.** That global slot is deprecated and can clobber unrelated live
3. Operator override only when explicitly authorized — record leases (#438). After an MCP restart, call `gitea_lock_issue` again — own-branch
`External-state mutations` and `operator override proof` in the final report. adoption rebinds the session when the issue's exact branch already exists (#442).
`gitea_create_pr` resolves the durable keyed lock by session pointer or by
`gitea_create_pr` rejects lock files that lack sanctioned `lock_provenance` matching `head` branch without unsafe manual seeding (#440). Branch ownership
metadata. Final-report validation blocks handoffs that hide lock read/write/delete uses structured ``(fix|feat|docs|chore)/issue-<n>-`` parsing — not broad
under `External-state mutations: none` or mix author PR creation with reviewer substring matches like ``issue-420`` inside ``issue-4200``.
approval in one run. See also #438 (global lock redesign).
Remote branches matching the issue number are also treated as active work unless Remote branches matching the issue number are also treated as active work unless
the recovery review proves the branch is abandoned or superseded. Never delete the recovery review proves the branch is abandoned or superseded. Never delete
-62
View File
@@ -11,7 +11,6 @@ import inspect
import re import re
from typing import Any, Callable from typing import Any, Callable
import issue_lock_provenance
from review_proofs import ( from review_proofs import (
HANDOFF_HEADING, HANDOFF_HEADING,
assess_controller_handoff, assess_controller_handoff,
@@ -871,54 +870,6 @@ def _rule_reviewer_mutation_ledger(
) )
def _rule_shared_issue_lock_external_state(report_text: str) -> list[dict[str, str]]:
result = issue_lock_provenance.assess_issue_lock_external_state_report(report_text)
if result.get("proven"):
return []
return _findings_from_reasons(
"shared.issue_lock_external_state",
result.get("reasons") or [],
field="External-state mutations",
severity="block",
safe_next_action=(
"disclose gitea_issue_lock.json read/write/delete under "
"External-state mutations; never claim none after lock seeding"
),
)
def _rule_shared_manual_lock_pr_override(report_text: str) -> list[dict[str, str]]:
result = issue_lock_provenance.assess_manual_lock_pr_without_override(report_text)
if result.get("proven"):
return []
return _findings_from_reasons(
"shared.manual_lock_pr_override",
result.get("reasons") or [],
field="External-state mutations",
severity="block",
safe_next_action=(
"use gitea_lock_issue or #442 adoption instead of manual lock seeding; "
"if operator override was authorized, cite override proof"
),
)
def _rule_shared_author_reviewer_same_run(report_text: str) -> list[dict[str, str]]:
result = issue_lock_provenance.assess_author_reviewer_same_run_report(report_text)
if result.get("proven"):
return []
return _findings_from_reasons(
"shared.author_reviewer_same_run",
result.get("reasons") or [],
field="Review mutations",
severity="block",
safe_next_action=(
"split author PR creation and reviewer approval across separate "
"sessions and handoffs"
),
)
def _rule_reviewer_review_mutation( def _rule_reviewer_review_mutation(
report_text: str, report_text: str,
*, *,
@@ -938,17 +889,10 @@ def _rule_reviewer_review_mutation(
) )
_SHARED_ISSUE_LOCK_RULES = (
_rule_shared_issue_lock_external_state,
_rule_shared_manual_lock_pr_override,
_rule_shared_author_reviewer_same_run,
)
_RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = { _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
"review_pr": [ "review_pr": [
_rule_shared_controller_handoff, _rule_shared_controller_handoff,
_rule_shared_email_disclosure, _rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
_rule_reviewer_legacy_workspace_mutations, _rule_reviewer_legacy_workspace_mutations,
_rule_reviewer_vague_mutations_none, _rule_reviewer_vague_mutations_none,
_rule_reviewer_mutation_categories, _rule_reviewer_mutation_categories,
@@ -969,7 +913,6 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
"reconcile_already_landed": [ "reconcile_already_landed": [
_rule_reconcile_controller_handoff, _rule_reconcile_controller_handoff,
_rule_shared_email_disclosure, _rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
_rule_reconcile_stale_author_fields, _rule_reconcile_stale_author_fields,
_rule_reconcile_eligible_reviewed, _rule_reconcile_eligible_reviewed,
_rule_reconcile_linked_issue_live, _rule_reconcile_linked_issue_live,
@@ -981,30 +924,25 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
"author_issue": [ "author_issue": [
_rule_shared_controller_handoff, _rule_shared_controller_handoff,
_rule_shared_email_disclosure, _rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
_rule_reviewer_vague_mutations_none, _rule_reviewer_vague_mutations_none,
], ],
"work_issue": [ "work_issue": [
_rule_shared_controller_handoff, _rule_shared_controller_handoff,
_rule_shared_email_disclosure, _rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
_rule_reviewer_vague_mutations_none, _rule_reviewer_vague_mutations_none,
], ],
"issue_filing": [ "issue_filing": [
_rule_shared_controller_handoff, _rule_shared_controller_handoff,
_rule_shared_email_disclosure, _rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
], ],
"inventory": [ "inventory": [
_rule_shared_controller_handoff, _rule_shared_controller_handoff,
_rule_shared_email_disclosure, _rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
_rule_reconcile_pagination_proof, _rule_reconcile_pagination_proof,
], ],
"issue_selection": [ "issue_selection": [
_rule_shared_controller_handoff, _rule_shared_controller_handoff,
_rule_shared_email_disclosure, _rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
], ],
} }
+94 -523
View File
@@ -537,14 +537,13 @@ import role_namespace_gate # noqa: E402
import task_capability_map # noqa: E402 import task_capability_map # noqa: E402
import review_proofs # noqa: E402 import review_proofs # noqa: E402
import agent_temp_artifacts import agent_temp_artifacts
import issue_branch_ownership # noqa: E402
import issue_lock_worktree # noqa: E402 import issue_lock_worktree # noqa: E402
import issue_lock_provenance # noqa: E402 import issue_lock_store # noqa: E402
import issue_lock_adoption # noqa: E402
import already_landed_reconcile # noqa: E402 import already_landed_reconcile # noqa: E402
import author_mutation_worktree # noqa: E402 import author_mutation_worktree # noqa: E402
import issue_claim_heartbeat # noqa: E402 import issue_claim_heartbeat # noqa: E402
import issue_work_duplicate_gate # noqa: E402
import issue_lock_adoption # noqa: E402
import reviewer_pr_lease # noqa: E402
import merged_cleanup_reconcile # noqa: E402 import merged_cleanup_reconcile # noqa: E402
import reconciler_profile # noqa: E402 import reconciler_profile # noqa: E402
import reconciliation_workflow # noqa: E402 import reconciliation_workflow # noqa: E402
@@ -552,8 +551,9 @@ import review_merge_state_machine # noqa: E402
import native_mcp_preference # noqa: E402 import native_mcp_preference # noqa: E402
# Fail-closed exact-issue-lock file (#204): written by gitea_lock_issue, # Keyed issue-lock storage (#443): per remote/org/repo/issue files under
# consumed by gitea_create_pr and scripts/worktree-start. # GITEA_ISSUE_LOCK_DIR, bound to the current MCP session via a per-PID pointer.
# Legacy global path retained only for test/doc references — do not seed manually.
ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json" ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json"
WORK_LEASE_TTL_HOURS = 4 WORK_LEASE_TTL_HOURS = 4
AUTHOR_ISSUE_WORK_LEASE = "author_issue_work" AUTHOR_ISSUE_WORK_LEASE = "author_issue_work"
@@ -585,15 +585,59 @@ def _parse_work_lease_timestamp(value: str | None) -> datetime | None:
return None return None
def _load_existing_issue_lock() -> dict | None: def _load_existing_issue_lock(
if not os.path.exists(ISSUE_LOCK_FILE): *,
return None remote: str | None = None,
org: str | None = None,
repo: str | None = None,
issue_number: int | None = None,
) -> dict | None:
if remote and org and repo and issue_number is not None:
return issue_lock_store.load_issue_lock(
remote=remote,
org=org,
repo=repo,
issue_number=issue_number,
)
return issue_lock_store.read_session_issue_lock()
def _resolve_issue_lock_for_pr(
*,
remote: str,
org: str,
repo: str,
head: str,
) -> dict:
lock_data = issue_lock_store.read_session_issue_lock()
if not lock_data:
lock_data = issue_lock_store.find_lock_for_branch(
remote=remote,
org=org,
repo=repo,
branch_name=head,
)
if not lock_data:
raise RuntimeError(
"Issue lock is missing (fail closed). Call gitea_lock_issue first."
)
return lock_data
def _save_issue_lock(data: dict) -> str:
existing = issue_lock_store.load_issue_lock(
remote=str(data.get("remote") or ""),
org=str(data.get("org") or ""),
repo=str(data.get("repo") or ""),
issue_number=int(data.get("issue_number") or 0),
)
overwrite_block = issue_lock_store.assess_foreign_lock_overwrite(existing, data)
if overwrite_block:
raise RuntimeError(overwrite_block)
try: try:
with open(ISSUE_LOCK_FILE, "r", encoding="utf-8") as f: return issue_lock_store.bind_session_lock(data)
data = json.load(f) except Exception as e:
return data if isinstance(data, dict) else None raise RuntimeError(f"Could not write issue lock file: {e}") from e
except Exception:
return None
def _work_lease_claimant(host: str | None) -> dict: def _work_lease_claimant(host: str | None) -> dict:
@@ -683,119 +727,6 @@ def _branch_entry_name(branch: dict | str) -> str:
return str(branch.get("name") or branch.get("ref") or "") return str(branch.get("name") or branch.get("ref") or "")
def _live_fetch_issue_duplicate_context(
h: str,
o: str,
r: str,
auth: str,
issue_number: int,
) -> tuple[list[dict], list[str], dict]:
"""Live open PRs, remote branch names, and claim state for one issue."""
base = repo_api_url(h, o, r)
open_prs = api_get_all(f"{base}/pulls?state=open", auth)
branches = api_get_all(f"{base}/branches", auth)
branch_names = [_branch_entry_name(b) for b in branches]
issue = api_request("GET", f"{base}/issues/{issue_number}", auth) or {}
comments = api_request(
"GET", f"{base}/issues/{issue_number}/comments", auth
) or []
claim_entry = issue_claim_heartbeat.classify_issue_claim(
issue=issue,
comments=comments,
open_prs=open_prs,
branch_names=branch_names,
)
return open_prs, branch_names, claim_entry
# Injectable duplicate-work context fetcher (#400). Production uses the live
# Gitea API path above; unit tests patch this symbol instead of hitting the
# network.
issue_duplicate_context_fetcher = _live_fetch_issue_duplicate_context
def _collect_issue_duplicate_context(
h: str,
o: str,
r: str,
auth: str,
issue_number: int,
) -> tuple[list[dict], list[str], dict]:
return issue_duplicate_context_fetcher(h, o, r, auth, issue_number)
def _assess_issue_duplicate_gate(
issue_number: int,
*,
h: str,
o: str,
r: str,
auth: str,
locked_branch: str | None = None,
phase: str,
) -> dict:
open_prs, branch_names, claim_entry = _collect_issue_duplicate_context(
h, o, r, auth, issue_number
)
return issue_work_duplicate_gate.assess_work_issue_duplicate_gate(
issue_number,
open_prs=open_prs,
branch_names=branch_names,
claim_entry=claim_entry,
locked_branch=locked_branch,
phase=phase,
)
def _duplicate_gate_block_response(gate: dict, **extra) -> dict:
out = {
"success": False,
"performed": False,
"reasons": list(gate.get("reasons") or []),
"duplicate_gate": gate,
"safe_next_action": gate.get("safe_next_action"),
}
out.update(extra)
return out
def _enforce_locked_issue_duplicate_recheck(
remote: str,
phase: str,
*,
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict | None:
"""Re-check duplicate-work gates for the locked issue (#400)."""
lock_data = _load_existing_issue_lock()
if not lock_data:
return None
issue_number = int(lock_data.get("issue_number") or 0)
locked_branch = lock_data.get("branch_name")
if not issue_number:
return None
h, o, r = _resolve(
remote or lock_data.get("remote") or "dadeschools",
host or lock_data.get("host"),
org or lock_data.get("org"),
repo or lock_data.get("repo"),
)
auth = _auth(h)
gate = _assess_issue_duplicate_gate(
issue_number,
h=h,
o=o,
r=r,
auth=auth,
locked_branch=locked_branch,
phase=phase,
)
if gate.get("block"):
return gate
return None
def _branch_entry_commit_sha(branch: dict | str) -> str | None: def _branch_entry_commit_sha(branch: dict | str) -> str | None:
"""Best-effort head SHA for a Gitea branch entry (None when absent).""" """Best-effort head SHA for a Gitea branch entry (None when absent)."""
if not isinstance(branch, dict): if not isinstance(branch, dict):
@@ -1256,8 +1187,9 @@ def gitea_lock_issue(
resolved_worktree = issue_lock_worktree.resolve_author_worktree_path( resolved_worktree = issue_lock_worktree.resolve_author_worktree_path(
worktree_path, PROJECT_ROOT worktree_path, PROJECT_ROOT
) )
active_lease_block = _active_work_lease_block( h, o, r = _resolve(remote, host, org, repo)
_load_existing_issue_lock(), active_lease_block = issue_lock_store.assess_same_issue_lease_conflict(
_load_existing_issue_lock(remote=remote, org=o, repo=r, issue_number=issue_number),
issue_number=issue_number, issue_number=issue_number,
branch_name=branch_name, branch_name=branch_name,
worktree_path=resolved_worktree, worktree_path=resolved_worktree,
@@ -1281,21 +1213,34 @@ def gitea_lock_issue(
issue_lock_worktree.format_issue_lock_worktree_error(lock_assessment) issue_lock_worktree.format_issue_lock_worktree_error(lock_assessment)
) )
h, o, r = _resolve(remote, host, org, repo) # 2. Check if the issue already has an open PR (reuse protection)
auth = _auth(h) auth = _auth(h)
duplicate_gate = _assess_issue_duplicate_gate( url = f"{repo_api_url(h, o, r)}/pulls?state=open"
issue_number,
h=h, try:
o=o, prs = api_get_all(url, auth)
r=r, except Exception as e:
auth=auth, raise RuntimeError(f"Could not list open PRs to verify issue lock: {e}")
locked_branch=branch_name,
phase=issue_work_duplicate_gate.PHASE_LOCK, for pr in prs:
) pr_head = pr.get("head", {}).get("ref", "")
if duplicate_gate.get("block"): pr_title = pr.get("title", "")
raise ValueError("; ".join(duplicate_gate.get("reasons") or [ pr_body = pr.get("body", "")
f"duplicate work gate blocked issue #{issue_number} (fail closed)"
])) if issue_branch_ownership.branch_tracks_issue(pr_head, issue_number):
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" branch_url = f"{repo_api_url(h, o, r)}/branches"
try: try:
@@ -1335,17 +1280,9 @@ def gitea_lock_issue(
"repo": r, "repo": r,
"worktree_path": resolved_worktree, "worktree_path": resolved_worktree,
"work_lease": work_lease, "work_lease": work_lease,
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
tool="gitea_lock_issue",
claimant=work_lease.get("claimant"),
),
} }
try: lock_file_path = _save_issue_lock(data)
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
json.dump(data, f)
except Exception as e:
raise RuntimeError(f"Could not write issue lock file: {e}")
agent_artifacts = agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain( agent_artifacts = agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain(
git_state.get("porcelain_status") or "" git_state.get("porcelain_status") or ""
@@ -1360,16 +1297,16 @@ def gitea_lock_issue(
"branch_name": branch_name, "branch_name": branch_name,
"worktree_path": resolved_worktree, "worktree_path": resolved_worktree,
"work_lease": work_lease, "work_lease": work_lease,
"lock_file_path": lock_file_path,
} }
if adoption["adopt"]: if adoption["adopt"]:
# #442: recovery lock over the issue's own already-pushed branch.
result["adoption"] = issue_lock_adoption.build_adoption_proof( result["adoption"] = issue_lock_adoption.build_adoption_proof(
issue_number=issue_number, issue_number=issue_number,
branch_name=branch_name, branch_name=branch_name,
assessment=adoption, assessment=adoption,
open_pr_checked=True, open_pr_checked=True,
competing_lock_checked=True, competing_lock_checked=True,
lock_file_path=ISSUE_LOCK_FILE, lock_file_path=lock_file_path,
lock_file_status="written", lock_file_status="written",
) )
result["message"] = ( result["message"] = (
@@ -1384,39 +1321,6 @@ def gitea_lock_issue(
return result return result
@mcp.tool()
def gitea_assess_work_issue_duplicate(
issue_number: int,
branch_name: str | None = None,
phase: str = issue_work_duplicate_gate.PHASE_LOCK,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Read-only duplicate-work gate for author sessions before mutations (#400)."""
read_block = _profile_operation_gate("gitea.read")
if read_block:
return {
"success": False,
"performed": False,
"reasons": read_block,
"permission_report": _permission_block_report("gitea.read"),
}
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
gate = _assess_issue_duplicate_gate(
issue_number,
h=h,
o=o,
r=r,
auth=auth,
locked_branch=branch_name,
phase=phase,
)
return {"success": not gate.get("block"), **gate}
@mcp.tool() @mcp.tool()
def gitea_create_pr( def gitea_create_pr(
title: str, title: str,
@@ -1467,23 +1371,8 @@ def gitea_create_pr(
verify_preflight_purity(remote, worktree_path=worktree_path) verify_preflight_purity(remote, worktree_path=worktree_path)
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
# ── Issue Lock Validation (Issue #194 / #196) ── # ── Issue Lock Validation (Issue #194 / #196 / #443) ──
if not os.path.exists(ISSUE_LOCK_FILE): lock_data = _resolve_issue_lock_for_pr(remote=remote, org=o, repo=r, head=head)
raise RuntimeError("Issue lock is missing (fail closed). Call gitea_lock_issue first.")
try:
with open(ISSUE_LOCK_FILE, "r", encoding="utf-8") as f:
lock_data = json.load(f)
except Exception as e:
raise RuntimeError(f"Could not read issue lock file: {e} (fail closed)")
lock_provenance_check = issue_lock_provenance.assess_lock_file_for_create_pr(
lock_data
)
if lock_provenance_check["block"]:
raise RuntimeError(
issue_lock_provenance.format_lock_provenance_error(lock_provenance_check)
)
locked_issue = lock_data.get("issue_number") locked_issue = lock_data.get("issue_number")
locked_branch = lock_data.get("branch_name") locked_branch = lock_data.get("branch_name")
@@ -1516,21 +1405,6 @@ 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)" f"PR title or body must contain 'Closes #{locked_issue}' or 'Fixes #{locked_issue}' exactly to ensure durable tracking (fail closed)"
) )
duplicate_block = _enforce_locked_issue_duplicate_recheck(
remote,
issue_work_duplicate_gate.PHASE_CREATE_PR,
host=host,
org=org,
repo=repo,
)
if duplicate_block:
return _duplicate_gate_block_response(
duplicate_block,
number=None,
issue_number=locked_issue,
branch_name=locked_branch,
)
auth = _auth(h) auth = _auth(h)
url = f"{repo_api_url(h, o, r)}/pulls" url = f"{repo_api_url(h, o, r)}/pulls"
payload = {"title": title, "body": body, "head": head, "base": base} payload = {"title": title, "body": body, "head": head, "base": base}
@@ -2070,7 +1944,6 @@ def init_review_decision_lock(remote: str | None, task: str | None):
(os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip() (os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip()
or profile_name or profile_name
) )
reviewer_pr_lease.clear_session_lease()
_save_review_decision_lock({ _save_review_decision_lock({
"task": task, "task": task,
"remote": remote, "remote": remote,
@@ -2513,20 +2386,6 @@ def _evaluate_pr_review_submission(
result["permission_report"] = elig["permission_report"] result["permission_report"] = elig["permission_report"]
return result 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"] auth_user = result["authenticated_user"]
pr_author = result["pr_author"] pr_author = result["pr_author"]
if action == "approve" and auth_user and pr_author and auth_user == pr_author: if action == "approve" and auth_user and pr_author and auth_user == pr_author:
@@ -3009,13 +2868,7 @@ def _prepare_commit_payload_files(files: list[dict]) -> tuple[list[dict], list[d
processed_files = [] processed_files = []
source_proofs = [] source_proofs = []
lock_data = {} lock_data = issue_lock_store.read_session_issue_lock() or {}
if os.path.exists(ISSUE_LOCK_FILE):
try:
with open(ISSUE_LOCK_FILE, "r", encoding="utf-8") as f:
lock_data = json.load(f)
except Exception:
pass
locked_worktree = lock_data.get("worktree_path") locked_worktree = lock_data.get("worktree_path")
if locked_worktree: if locked_worktree:
@@ -3150,20 +3003,6 @@ def gitea_commit_files(
if blocked: if blocked:
return blocked return blocked
duplicate_block = _enforce_locked_issue_duplicate_recheck(
remote,
issue_work_duplicate_gate.PHASE_COMMIT,
host=host,
org=org,
repo=repo,
)
if duplicate_block:
return _duplicate_gate_block_response(
duplicate_block,
commit="",
branch="",
)
verify_preflight_purity(remote) verify_preflight_purity(remote)
processed_files, source_proofs = _prepare_commit_payload_files(files) processed_files, source_proofs = _prepare_commit_payload_files(files)
@@ -3326,19 +3165,6 @@ def gitea_merge_pr(
result["permission_report"] = elig["permission_report"] result["permission_report"] = elig["permission_report"]
return result 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. # Gate 4 — head SHA must match if the caller pinned a reviewed SHA.
actual_sha = result["head_sha"] actual_sha = result["head_sha"]
if expected_head_sha and actual_sha and expected_head_sha != actual_sha: if expected_head_sha and actual_sha and expected_head_sha != actual_sha:
@@ -4638,261 +4464,6 @@ def _namespace_mutation_block(mutation_task: str, **extra_fields) -> dict | None
return blocked 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() @mcp.tool()
def gitea_list_issue_comments( def gitea_list_issue_comments(
issue_number: int, issue_number: int,
+31
View File
@@ -0,0 +1,31 @@
"""Structured issue/branch ownership evidence (#440).
Author branches must follow ``(fix|feat|docs|chore)/issue-<n>-<desc>``. Duplicate-work
and recovery gates use this parser instead of broad substring checks like
``issue-420`` inside unrelated names (for example ``issue-4200``).
"""
from __future__ import annotations
import re
IMPLEMENTATION_BRANCH_RE = re.compile(
r"^(?:fix|feat|docs|chore)/issue-(\d+)(?:-.+)?$"
)
def parse_tracked_issue_number(branch_name: str) -> int | None:
"""Return the issue number encoded in a canonical author branch name."""
text = (branch_name or "").strip()
if not text:
return None
match = IMPLEMENTATION_BRANCH_RE.match(text)
if not match:
return None
return int(match.group(1))
def branch_tracks_issue(branch_name: str, issue_number: int) -> bool:
"""True when ``branch_name`` structurally belongs to ``issue_number``."""
parsed = parse_tracked_issue_number(branch_name)
return parsed == issue_number if parsed is not None else False
+6 -45
View File
@@ -1,4 +1,4 @@
"""Own-branch lock adoption / recovery for ``gitea_lock_issue`` (#442). """Own-branch lock adoption / recovery for ``gitea_lock_issue`` (#442 / #443).
When an issue's own already-pushed branch exists, lock reacquisition must be When an issue's own already-pushed branch exists, lock reacquisition must be
allowed (adoption) instead of being treated as #400 duplicate competing work. allowed (adoption) instead of being treated as #400 duplicate competing work.
@@ -14,7 +14,7 @@ this module additionally records whether they passed for proof purposes.
from __future__ import annotations from __future__ import annotations
import re import issue_branch_ownership
ADOPT = "adopt_existing_branch" ADOPT = "adopt_existing_branch"
BLOCK_COMPETING = "block_competing_branch" BLOCK_COMPETING = "block_competing_branch"
@@ -35,58 +35,24 @@ def _branch_sha(entry) -> str | None:
return None return None
def _branch_carries_issue_marker(branch_name: str, issue_number: int) -> bool:
"""Return True when *branch_name* references issue *issue_number* exactly.
Uses a numeric word-boundary so ``issue-42`` does not match inside
``issue-420`` (AC6 / #440).
"""
name = (branch_name or "").strip()
if not name:
return False
pattern = rf"(?:^|/)issue-{int(issue_number)}(?![0-9])"
return re.search(pattern, name) is not None
def assess_own_branch_adoption( def assess_own_branch_adoption(
*, *,
issue_number: int, issue_number: int,
requested_branch: str, requested_branch: str,
existing_branches, existing_branches,
) -> dict: ) -> dict:
"""Decide whether an existing matching branch is adoptable. """Decide whether an existing matching branch is adoptable."""
Args:
issue_number: The tracking issue number being locked.
requested_branch: The exact branch the caller wants to lock.
existing_branches: Iterable of remote branch entries — either names or
dicts with ``name`` and optional ``commit_sha``.
Returns:
dict with:
* ``outcome`` — one of ADOPT / BLOCK_COMPETING / NO_MATCH
* ``adopt`` (bool), ``block`` (bool)
* ``reason`` (str)
* ``matched_branch`` (str | None), ``matched_head_sha`` (str | None)
* ``competing_branches`` (list[str])
ADOPT: the issue's exact branch exists and no other same-issue branch does.
BLOCK_COMPETING: at least one same-issue branch is not the requested branch.
NO_MATCH: no branch carries the issue marker — normal lock path applies.
"""
requested = (requested_branch or "").strip() requested = (requested_branch or "").strip()
matches: list[tuple[str, str | None]] = [] matches: list[tuple[str, str | None]] = []
for entry in existing_branches or []: for entry in existing_branches or []:
name = _branch_name(entry).strip() name = _branch_name(entry).strip()
if _branch_carries_issue_marker(name, issue_number): if issue_branch_ownership.branch_tracks_issue(name, issue_number):
matches.append((name, _branch_sha(entry))) matches.append((name, _branch_sha(entry)))
competing = sorted({name for name, _ in matches if name != requested}) competing = sorted({name for name, _ in matches if name != requested})
exact = [(name, sha) for name, sha in matches if name == requested] exact = [(name, sha) for name, sha in matches if name == requested]
# Fail closed whenever any non-requested same-issue branch exists, even if
# the requested branch is also present: ownership is then ambiguous.
if competing: if competing:
return { return {
"outcome": BLOCK_COMPETING, "outcome": BLOCK_COMPETING,
@@ -138,12 +104,7 @@ def build_adoption_proof(
lock_file_path: str, lock_file_path: str,
lock_file_status: str, lock_file_status: str,
) -> dict: ) -> dict:
"""Assemble the proof block returned by ``gitea_lock_issue`` on adoption. """Assemble the proof block returned by ``gitea_lock_issue`` on adoption."""
Requirement #4: adoption results must carry issue number, branch name,
branch head commit, adoption reason, no-existing-PR proof, no-competing-
live-lock proof, and lock file path/status.
"""
return { return {
"issue_number": issue_number, "issue_number": issue_number,
"branch_name": branch_name, "branch_name": branch_name,
@@ -153,4 +114,4 @@ def build_adoption_proof(
"no_competing_live_lock_proof": bool(competing_lock_checked), "no_competing_live_lock_proof": bool(competing_lock_checked),
"lock_file_path": lock_file_path, "lock_file_path": lock_file_path,
"lock_file_status": lock_file_status, "lock_file_status": lock_file_status,
} }
-269
View File
@@ -1,269 +0,0 @@
"""Issue-lock provenance and external-state disclosure (#447).
Sanctioned locks are written only by ``gitea_lock_issue`` (or adoption recovery
#442). Manual seeding of ``/tmp/gitea_issue_lock.json`` is unsafe and must be
blocked at PR creation unless explicit operator override proof is recorded.
"""
from __future__ import annotations
import os
import re
from datetime import datetime, timezone
ISSUE_LOCK_FILE = os.environ.get("GITEA_ISSUE_LOCK_FILE", "/tmp/gitea_issue_lock.json")
SOURCE_LOCK_ISSUE = "gitea_lock_issue"
SOURCE_LOCK_ADOPTION = "gitea_lock_issue_adoption"
SOURCE_OPERATOR_OVERRIDE = "operator_override"
SANCTIONED_LOCK_SOURCES = frozenset({
SOURCE_LOCK_ISSUE,
SOURCE_LOCK_ADOPTION,
SOURCE_OPERATOR_OVERRIDE,
})
_OPERATOR_OVERRIDE_ENV = "GITEA_ISSUE_LOCK_OPERATOR_OVERRIDE"
_ISSUE_LOCK_PATH_RE = re.compile(
r"(?:/tmp/)?gitea_issue_lock\.json",
re.IGNORECASE,
)
_LOCK_SEED_RE = re.compile(
r"(?:seed(?:ed|ing)?|restor(?:e|ed|ing)|wrote|written|write|programmatically|"
r"hand[- ]forg|manual(?:ly)?).{0,80}gitea_issue_lock",
re.IGNORECASE | re.DOTALL,
)
_LOCK_REMOVE_RE = re.compile(
r"(?:\brm\b|remove|deleted?|unlink).{0,80}gitea_issue_lock",
re.IGNORECASE | re.DOTALL,
)
_LOCK_READ_RE = re.compile(
r"(?:read|loaded?|parsed?).{0,80}gitea_issue_lock",
re.IGNORECASE | re.DOTALL,
)
_EXTERNAL_NONE_RE = re.compile(
r"external[- ]state mutations\s*:\s*none\b",
re.IGNORECASE,
)
_EXTERNAL_FIELD_RE = re.compile(
r"external[- ]state mutations\s*:\s*(.+)$",
re.IGNORECASE | re.MULTILINE,
)
_CLEANUP_ONLY_RE = re.compile(
r"cleanup mutations\s*:\s*(?:none|lock removed|removed issue lock)",
re.IGNORECASE,
)
_PR_CREATED_RE = re.compile(
r"(?:\bgitea_create_pr\b|PR\s*#\s*\d+\s+created|created\s+PR\s*#|opened\s+PR\s*#|"
r"PR\s+creation\s+(?:succeeded|complete))",
re.IGNORECASE,
)
_REVIEW_APPROVE_RE = re.compile(
r"(?:submitted\s+(?:['\"]approve['\"]|approve\s+review)|"
r"review decision\s*:\s*approve|approved\s+PR\s*#|gitea_review_pr.*approve)",
re.IGNORECASE,
)
_OVERRIDE_PROOF_RE = re.compile(
r"operator[- ]override\s+proof\s*:\s*(.+)$",
re.IGNORECASE | re.MULTILINE,
)
def _utc_now_iso() -> str:
return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")
def build_sanctioned_lock_provenance(
*,
tool: str,
source: str = SOURCE_LOCK_ISSUE,
claimant: dict | None = None,
adoption: dict | None = None,
) -> dict:
"""Return provenance metadata stored with a sanctioned lock write."""
entry = {
"source": source,
"written_at": _utc_now_iso(),
"written_by_tool": tool,
"lock_file_path": ISSUE_LOCK_FILE,
}
if claimant:
entry["claimant"] = claimant
if adoption:
entry["adoption"] = adoption
return entry
def operator_override_requested() -> bool:
return os.environ.get(_OPERATOR_OVERRIDE_ENV, "").strip().lower() in {
"1",
"true",
"yes",
}
def build_operator_override_provenance(*, reason: str, claimant: dict | None = None) -> dict:
text = (reason or "").strip()
if not text:
raise ValueError(
"operator override requires a non-empty override reason (fail closed)"
)
entry = build_sanctioned_lock_provenance(
tool="operator_override",
source=SOURCE_OPERATOR_OVERRIDE,
claimant=claimant,
)
entry["override_reason"] = text
return entry
def assess_lock_file_for_create_pr(lock_data: dict | None) -> dict:
"""Fail closed when lock file lacks sanctioned provenance (#447)."""
data = lock_data if isinstance(lock_data, dict) else {}
reasons: list[str] = []
provenance = data.get("lock_provenance")
if not isinstance(provenance, dict):
reasons.append(
"issue lock file lacks sanctioned lock_provenance; manual seeding is "
"not a normal recovery path — call gitea_lock_issue or use #442 adoption"
)
return _provenance_result(False, reasons, provenance)
source = str(provenance.get("source") or "").strip()
if source not in SANCTIONED_LOCK_SOURCES:
reasons.append(
f"issue lock provenance source '{source or '(missing)'}' is not sanctioned"
)
if source == SOURCE_OPERATOR_OVERRIDE and not str(
provenance.get("override_reason") or ""
).strip():
reasons.append(
"operator_override lock provenance requires override_reason proof"
)
if not data.get("work_lease"):
reasons.append("issue lock file missing work_lease metadata")
if not str(provenance.get("written_by_tool") or "").strip():
reasons.append("issue lock provenance missing written_by_tool")
proven = not reasons
return _provenance_result(proven, reasons, provenance)
def _provenance_result(proven: bool, reasons: list[str], provenance: dict | None) -> dict:
return {
"proven": proven,
"block": not proven,
"reasons": reasons,
"lock_provenance": provenance,
}
def format_lock_provenance_error(assessment: dict) -> str:
reasons = "; ".join(assessment.get("reasons") or ["unknown lock provenance violation"])
return f"Issue lock provenance guard (#447): {reasons} (fail closed)"
def _lock_activity_detected(text: str) -> dict[str, bool]:
body = text or ""
return {
"seed_or_restore": bool(_LOCK_SEED_RE.search(body)),
"remove": bool(_LOCK_REMOVE_RE.search(body)),
"read": bool(_LOCK_READ_RE.search(body)),
}
def _external_state_discloses_lock(text: str) -> bool:
match = _EXTERNAL_FIELD_RE.search(text or "")
if not match:
return False
value = (match.group(1) or "").strip().lower()
if value in {"", "none", "n/a"}:
return False
return "lock" in value or "gitea_issue_lock" in value or "issue-lock" in value
def assess_issue_lock_external_state_report(report_text: str) -> dict:
"""Require explicit external-state disclosure for issue-lock mutations (#447)."""
text = report_text or ""
activity = _lock_activity_detected(text)
if not any(activity.values()):
return {"proven": True, "block": False, "reasons": [], "activity": activity}
reasons: list[str] = []
disclosed = _external_state_discloses_lock(text)
if activity["seed_or_restore"] and _EXTERNAL_NONE_RE.search(text):
reasons.append(
"report mentions seeding/restoring gitea_issue_lock.json but claims "
"External-state mutations: none"
)
elif activity["seed_or_restore"] and not disclosed:
reasons.append(
"report mentions issue-lock file activity but External-state mutations "
"does not disclose read/write of gitea_issue_lock.json"
)
if activity["remove"]:
if _EXTERNAL_NONE_RE.search(text):
reasons.append(
"report mentions removing gitea_issue_lock.json but claims "
"External-state mutations: none"
)
elif not disclosed and _CLEANUP_ONLY_RE.search(text):
reasons.append(
"report removes issue lock but classifies it as cleanup only; "
"record under External-state mutations"
)
elif not disclosed:
reasons.append(
"report mentions deleting issue lock without External-state "
"mutation disclosure"
)
proven = not reasons
return {
"proven": proven,
"block": not proven,
"reasons": reasons,
"activity": activity,
}
def assess_manual_lock_pr_without_override(report_text: str) -> dict:
"""Block reports that created a PR via manual lock seed without override proof."""
text = report_text or ""
seeded = bool(_LOCK_SEED_RE.search(text))
created = bool(_PR_CREATED_RE.search(text))
if not (seeded and created):
return {"proven": True, "block": False, "reasons": []}
if _OVERRIDE_PROOF_RE.search(text):
return {"proven": True, "block": False, "reasons": []}
return {
"proven": False,
"block": True,
"reasons": [
"report created/opened a PR after manual issue-lock seeding without "
"operator override proof"
],
}
def assess_author_reviewer_same_run_report(report_text: str) -> dict:
"""Reviewer handoff must not create and approve the same PR in one run (#447)."""
text = report_text or ""
if not (_PR_CREATED_RE.search(text) and _REVIEW_APPROVE_RE.search(text)):
return {"proven": True, "block": False, "reasons": []}
return {
"proven": False,
"block": True,
"reasons": [
"report mixes author-side PR creation and reviewer approval in one "
"final handoff; split author and reviewer sessions"
],
}
+385
View File
@@ -0,0 +1,385 @@
"""Keyed, persistent issue-lock storage (#443).
Replaces the single global ``/tmp/gitea_issue_lock.json`` slot with per-issue
lock files under ``GITEA_ISSUE_LOCK_DIR`` (default
``~/.cache/gitea-tools/issue-locks``). Each MCP session binds its active lock
via a per-process pointer file so concurrent repos/issues never clobber each
other.
"""
from __future__ import annotations
import json
import os
import re
import tempfile
from datetime import datetime, timedelta, timezone
from typing import Any
LOCK_DIR_ENV = "GITEA_ISSUE_LOCK_DIR"
DEFAULT_LOCK_DIR = os.path.expanduser("~/.cache/gitea-tools/issue-locks")
WORK_LEASE_TTL_HOURS = 4
AUTHOR_ISSUE_WORK_LEASE = "author_issue_work"
_SAFE_SEGMENT_RE = re.compile(r"[^A-Za-z0-9._+-]+")
def default_lock_dir() -> str:
raw = (os.environ.get(LOCK_DIR_ENV) or DEFAULT_LOCK_DIR).strip()
return raw or DEFAULT_LOCK_DIR
def _sanitize_segment(value: str) -> str:
text = (value or "").strip()
if not text:
return "_"
return _SAFE_SEGMENT_RE.sub("_", text)
def lock_key(
*,
remote: str,
org: str,
repo: str,
issue_number: int,
) -> str:
return "-".join(
_sanitize_segment(part)
for part in (remote, org, repo, str(issue_number))
)
def lock_file_path(
*,
remote: str,
org: str,
repo: str,
issue_number: int,
lock_dir: str | None = None,
) -> str:
root = (lock_dir or default_lock_dir()).strip()
return os.path.join(root, f"{lock_key(remote=remote, org=org, repo=repo, issue_number=issue_number)}.json")
def session_pointer_path(lock_dir: str | None = None) -> str:
root = (lock_dir or default_lock_dir()).strip()
return os.path.join(root, f"session-{os.getpid()}.json")
def _ensure_lock_dir(lock_dir: str | None = None) -> str:
root = (lock_dir or default_lock_dir()).strip()
os.makedirs(root, mode=0o700, exist_ok=True)
return root
def read_lock_file(path: str) -> dict[str, Any] | None:
lock_path = (path or "").strip()
if not lock_path or not os.path.exists(lock_path):
return None
try:
with open(lock_path, encoding="utf-8") as handle:
data = json.load(handle)
except (OSError, json.JSONDecodeError):
return None
return data if isinstance(data, dict) else None
def save_lock_file(path: str, data: dict[str, Any]) -> None:
lock_path = (path or "").strip()
if not lock_path:
raise ValueError("lock path is required (fail closed)")
parent = os.path.dirname(lock_path) or "."
os.makedirs(parent, mode=0o700, exist_ok=True)
payload = json.dumps(data, indent=2, sort_keys=True) + "\n"
fd, temp_path = tempfile.mkstemp(prefix=".lock-", suffix=".json", dir=parent)
try:
with os.fdopen(fd, "w", encoding="utf-8") as handle:
handle.write(payload)
handle.flush()
os.fsync(handle.fileno())
os.replace(temp_path, lock_path)
finally:
if os.path.exists(temp_path):
try:
os.remove(temp_path)
except OSError:
pass
def bind_session_lock(lock_data: dict[str, Any], lock_dir: str | None = None) -> str:
"""Persist a keyed lock and bind it to the current process session."""
remote = str(lock_data.get("remote") or "")
org = str(lock_data.get("org") or "")
repo = str(lock_data.get("repo") or "")
issue_number = int(lock_data.get("issue_number") or 0)
if not remote or not org or not repo or issue_number <= 0:
raise ValueError("lock record must include remote, org, repo, and issue_number")
root = _ensure_lock_dir(lock_dir)
path = lock_file_path(
remote=remote,
org=org,
repo=repo,
issue_number=issue_number,
lock_dir=root,
)
record = dict(lock_data)
record["lock_file_path"] = path
record["session_pid"] = os.getpid()
save_lock_file(path, record)
pointer = {
"pid": os.getpid(),
"lock_file_path": path,
"issue_number": issue_number,
"branch_name": record.get("branch_name"),
"remote": remote,
"org": org,
"repo": repo,
}
save_lock_file(session_pointer_path(root), pointer)
return path
def read_session_issue_lock(lock_dir: str | None = None) -> dict[str, Any] | None:
root = (lock_dir or default_lock_dir()).strip()
pointer = read_lock_file(session_pointer_path(root))
if not pointer:
return None
lock_path = str(pointer.get("lock_file_path") or "").strip()
if not lock_path:
return None
return read_lock_file(lock_path)
def load_issue_lock(
*,
remote: str,
org: str,
repo: str,
issue_number: int,
lock_dir: str | None = None,
) -> dict[str, Any] | None:
return read_lock_file(
lock_file_path(
remote=remote,
org=org,
repo=repo,
issue_number=issue_number,
lock_dir=lock_dir,
)
)
def iter_lock_files(lock_dir: str | None = None) -> list[str]:
root = (lock_dir or default_lock_dir()).strip()
if not os.path.isdir(root):
return []
paths: list[str] = []
for name in os.listdir(root):
if not name.endswith(".json") or name.startswith("session-"):
continue
paths.append(os.path.join(root, name))
return sorted(paths)
def find_lock_for_branch(
*,
remote: str,
org: str,
repo: str,
branch_name: str,
lock_dir: str | None = None,
) -> dict[str, Any] | None:
target = (branch_name or "").strip()
if not target:
return None
for path in iter_lock_files(lock_dir):
lock = read_lock_file(path)
if not lock:
continue
if (
str(lock.get("remote") or "") == remote
and str(lock.get("org") or "") == org
and str(lock.get("repo") or "") == repo
and str(lock.get("branch_name") or "").strip() == target
):
lock = dict(lock)
lock.setdefault("lock_file_path", path)
return lock
return None
def _lease_now(now: datetime | None = None) -> datetime:
return now or datetime.now(timezone.utc)
def _parse_lease_timestamp(value: str | None) -> datetime | None:
text = (value or "").strip()
if not text:
return None
try:
return datetime.fromisoformat(text.replace("Z", "+00:00")).astimezone(timezone.utc)
except ValueError:
return None
def lease_expires_at(lock: dict[str, Any] | None) -> datetime | None:
if not lock:
return None
lease = lock.get("work_lease")
if not isinstance(lease, dict):
return None
return _parse_lease_timestamp(lease.get("expires_at"))
def is_lease_expired(lock: dict[str, Any] | None, *, now: datetime | None = None) -> bool:
expires = lease_expires_at(lock)
if expires is None:
return False
return expires <= _lease_now(now)
def is_lease_live(lock: dict[str, Any] | None, *, now: datetime | None = None) -> bool:
if not lock:
return False
lease = lock.get("work_lease")
if not isinstance(lease, dict):
return True
expires = _parse_lease_timestamp(lease.get("expires_at"))
if expires is None:
return True
return expires > _lease_now(now)
def _same_realpath(left: str | None, right: str | None) -> bool:
if not left or not right:
return False
try:
return os.path.realpath(left) == os.path.realpath(right)
except OSError:
return left == right
def assess_same_issue_lease_conflict(
existing_lock: dict[str, Any] | None,
*,
issue_number: int,
branch_name: str,
worktree_path: str,
operation_type: str = AUTHOR_ISSUE_WORK_LEASE,
now: datetime | None = None,
) -> str | None:
"""Return a fail-closed error when a competing live lease blocks acquisition."""
if not existing_lock:
return None
existing_issue = existing_lock.get("issue_number")
lease = existing_lock.get("work_lease")
existing_operation = (
lease.get("operation_type")
if isinstance(lease, dict)
else AUTHOR_ISSUE_WORK_LEASE
)
if existing_issue != issue_number or existing_operation != operation_type:
return None
existing_branch = existing_lock.get("branch_name")
existing_worktree = existing_lock.get("worktree_path")
same_owner = (
existing_branch == branch_name
and _same_realpath(str(existing_worktree or ""), worktree_path)
)
if is_lease_expired(existing_lock, now=now):
return (
f"Issue #{issue_number} has an expired {operation_type} lease on "
f"branch '{existing_branch}' from worktree '{existing_worktree}'. "
"Recovery review is required before takeover (fail closed)"
)
if same_owner:
return None
return (
f"Issue #{issue_number} already has an active {operation_type} lease on "
f"branch '{existing_branch}' from worktree '{existing_worktree}' "
"(fail closed)"
)
def assess_foreign_lock_overwrite(
existing_lock: dict[str, Any] | None,
incoming_lock: dict[str, Any],
*,
now: datetime | None = None,
) -> str | None:
"""Block writes that would clobber an unrelated live lease on the same key."""
if not existing_lock:
return None
same_issue = existing_lock.get("issue_number") == incoming_lock.get("issue_number")
same_branch = existing_lock.get("branch_name") == incoming_lock.get("branch_name")
same_worktree = _same_realpath(
str(existing_lock.get("worktree_path") or ""),
str(incoming_lock.get("worktree_path") or ""),
)
if same_issue and same_branch and same_worktree:
return None
if not is_lease_live(existing_lock, now=now):
return None
return (
"Refusing to overwrite a live foreign issue lock "
f"(issue #{existing_lock.get('issue_number')}, "
f"branch '{existing_lock.get('branch_name')}', "
f"worktree '{existing_lock.get('worktree_path')}') (fail closed)"
)
def find_live_lock_for_branch(
branch_name: str,
lock_dir: str | None = None,
) -> dict[str, Any] | None:
target = (branch_name or "").strip()
if not target:
return None
for path in iter_lock_files(lock_dir):
lock = read_lock_file(path)
if not lock:
continue
if str(lock.get("branch_name") or "").strip() != target:
continue
if not is_lease_live(lock):
continue
record = dict(lock)
record.setdefault("lock_file_path", path)
return record
return None
def resolve_locked_branch_for_session(
branch_name: str | None = None,
lock_dir: str | None = None,
) -> str:
if branch_name:
lock = find_live_lock_for_branch(branch_name, lock_dir)
if lock:
return str(lock.get("branch_name") or "")
lock = read_session_issue_lock(lock_dir)
return str((lock or {}).get("branch_name") or "")
def has_active_issue_lock(
branch: str,
*,
lock_dir: str | None = None,
) -> bool:
target = (branch or "").strip()
if not target:
return False
for path in iter_lock_files(lock_dir):
lock = read_lock_file(path)
if not lock:
continue
if str(lock.get("branch_name") or "").strip() != target:
continue
if is_lease_live(lock):
return True
return False
-180
View File
@@ -1,180 +0,0 @@
"""Early duplicate-work detection for author work-issue sessions (#400)."""
from __future__ import annotations
from typing import Any
import issue_claim_heartbeat as claim_hb
PHASE_LOCK = "lock_issue"
PHASE_COMMIT = "commit"
PHASE_PUSH = "push"
PHASE_CREATE_PR = "create_pr"
OUTCOME_DUPLICATE_PR_PREVENTED = "duplicate_pr_prevented"
OUTCOME_DUPLICATE_BRANCH_PREVENTED = "duplicate_branch_prevented"
OUTCOME_DUPLICATE_COMMIT_PREVENTED = "duplicate_commit_prevented"
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED = "duplicate_work_not_prevented"
_ACTIVE_CLAIM_STATUSES = frozenset({"active", "awaiting_review"})
def _issue_pattern(issue_number: int) -> str:
return f"issue-{int(issue_number)}"
def _linked_open_pr(issue_number: int, open_prs: list[dict]) -> dict | None:
return claim_hb._linked_open_pr(issue_number, open_prs)
def _matching_branches(
issue_number: int,
branch_names: list[str],
*,
locked_branch: str | None = None,
) -> list[str]:
pattern = _issue_pattern(issue_number)
matches = [
name for name in (branch_names or [])
if pattern in (name or "").lower()
]
if locked_branch:
locked = locked_branch.strip()
matches = [name for name in matches if name != locked]
return matches
def assess_work_issue_duplicate_gate(
issue_number: int,
*,
open_prs: list[dict] | None = None,
branch_names: list[str] | None = None,
claim_entry: dict | None = None,
locked_branch: str | None = None,
phase: str = PHASE_LOCK,
) -> dict[str, Any]:
"""Fail closed when duplicate work is already in flight for an issue."""
reasons: list[str] = []
outcome = OUTCOME_DUPLICATE_WORK_NOT_PREVENTED
prs = list(open_prs or [])
branches = list(branch_names or [])
pattern = _issue_pattern(issue_number)
linked = _linked_open_pr(issue_number, prs)
if linked:
reasons.append(
f"open PR #{linked.get('number')} already covers issue "
f"#{issue_number} (fail closed)"
)
outcome = OUTCOME_DUPLICATE_PR_PREVENTED
conflicting_branches = _matching_branches(
issue_number, branches, locked_branch=locked_branch
)
if conflicting_branches:
names = ", ".join(conflicting_branches[:5])
reasons.append(
f"remote branch(es) already match issue pattern '{pattern}': "
f"{names} (fail closed)"
)
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
entry = claim_entry or {}
if entry.get("linked_open_pr") and not linked:
reasons.append(
f"claim inventory reports open PR #{entry['linked_open_pr']} "
f"for issue #{issue_number} (fail closed)"
)
outcome = OUTCOME_DUPLICATE_PR_PREVENTED
status = (entry.get("status") or "").strip().lower()
if status in _ACTIVE_CLAIM_STATUSES and not linked:
heartbeat = entry.get("latest_heartbeat") or {}
claim_branch = (heartbeat.get("branch") or "").strip()
if locked_branch and claim_branch and claim_branch != locked_branch:
reasons.append(
f"active claim lease on branch '{claim_branch}' blocks "
f"work on '{locked_branch}' for issue #{issue_number} "
"(fail closed)"
)
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
elif not locked_branch and status == "active":
reasons.append(
f"issue #{issue_number} has an active claim lease "
"(fail closed)"
)
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
if phase in {PHASE_COMMIT, PHASE_PUSH} and reasons:
if outcome == OUTCOME_DUPLICATE_PR_PREVENTED:
outcome = OUTCOME_DUPLICATE_COMMIT_PREVENTED
elif outcome == OUTCOME_DUPLICATE_BRANCH_PREVENTED:
outcome = OUTCOME_DUPLICATE_COMMIT_PREVENTED
block = bool(reasons)
return {
"block": block,
"performed": not block,
"issue_number": issue_number,
"phase": phase,
"outcome": outcome,
"linked_open_pr": linked.get("number") if linked else entry.get("linked_open_pr"),
"conflicting_branches": conflicting_branches,
"claim_status": status or None,
"reasons": reasons,
"safe_next_action": (
"stop before mutating; preserve local work and produce a "
"reconciliation handoff if a concurrent PR appeared after push"
if block and phase == PHASE_CREATE_PR
else "stop before mutating; do not commit or push duplicate work"
if block
else "proceed"
),
}
def assess_work_issue_duplicate_report(report_text: str) -> dict[str, Any]:
"""Require explicit duplicate-work outcome wording in work-issue reports."""
text = (report_text or "").lower()
markers = {
OUTCOME_DUPLICATE_PR_PREVENTED: (
"duplicate pr prevented",
"duplicate_pr_prevented",
),
OUTCOME_DUPLICATE_BRANCH_PREVENTED: (
"duplicate branch prevented",
"duplicate_branch_prevented",
),
OUTCOME_DUPLICATE_COMMIT_PREVENTED: (
"duplicate commit prevented",
"duplicate_commit_prevented",
),
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED: (
"duplicate work not prevented",
"duplicate_work_not_prevented",
"no duplicate work",
),
}
matched = [
key for key, phrases in markers.items()
if any(phrase in text for phrase in phrases)
]
if len(matched) != 1:
return {
"complete": False,
"downgraded": True,
"reasons": [
"work-issue report must state exactly one duplicate-work "
"outcome (duplicate PR/branch/commit prevented, or "
"duplicate work not prevented)"
],
}
return {
"complete": True,
"downgraded": False,
"outcome": matched[0],
"reasons": [],
}
+10 -14
View File
@@ -13,9 +13,9 @@ import subprocess
from typing import Any from typing import Any
from reviewer_worktree import parse_dirty_tracked_files from reviewer_worktree import parse_dirty_tracked_files
import issue_lock_store
PROTECTED_BRANCHES = frozenset({"master", "main", "dev"}) PROTECTED_BRANCHES = frozenset({"master", "main", "dev"})
ISSUE_LOCK_FILE = os.environ.get("GITEA_ISSUE_LOCK_FILE", "/tmp/gitea_issue_lock.json")
CLOSES_FIXES_RE = re.compile(r"\b(?:closes|fixes)\s+#(\d+)\b", re.IGNORECASE) CLOSES_FIXES_RE = re.compile(r"\b(?:closes|fixes)\s+#(\d+)\b", re.IGNORECASE)
@@ -37,22 +37,18 @@ def resolve_worktree_path(project_root: str, branch: str) -> str:
def read_issue_lock(path: str | None = None) -> dict[str, Any] | None: def read_issue_lock(path: str | None = None) -> dict[str, Any] | None:
lock_path = (path or ISSUE_LOCK_FILE).strip() if path:
if not lock_path or not os.path.exists(lock_path): return issue_lock_store.read_lock_file(path.strip())
return None return issue_lock_store.read_session_issue_lock()
try:
with open(lock_path, encoding="utf-8") as handle:
data = json.load(handle)
except (OSError, json.JSONDecodeError):
return None
return data if isinstance(data, dict) else None
def has_active_issue_lock(branch: str, lock_path: str | None = None) -> bool: def has_active_issue_lock(branch: str, lock_path: str | None = None) -> bool:
lock = read_issue_lock(lock_path) if lock_path:
if not lock: lock = issue_lock_store.read_lock_file(lock_path.strip())
return False if not lock:
return (lock.get("branch_name") or "").strip() == (branch or "").strip() return False
return (lock.get("branch_name") or "").strip() == (branch or "").strip()
return issue_lock_store.has_active_issue_lock(branch)
def collect_open_pr_heads(open_prs: list[dict[str, Any]]) -> set[str]: def collect_open_pr_heads(open_prs: list[dict[str, Any]]) -> set[str]:
-3
View File
@@ -3624,12 +3624,9 @@ def assess_work_issue_mode_isolation(report_text: str) -> dict:
def assess_work_issue_final_report(report_text: str) -> dict: def assess_work_issue_final_report(report_text: str) -> dict:
"""#139: composite verifier for work-issue final reports.""" """#139: composite verifier for work-issue final reports."""
from issue_work_duplicate_gate import assess_work_issue_duplicate_report
checks = { checks = {
"workflow_source": assess_work_issue_workflow_source(report_text), "workflow_source": assess_work_issue_workflow_source(report_text),
"mode_isolation": assess_work_issue_mode_isolation(report_text), "mode_isolation": assess_work_issue_mode_isolation(report_text),
"duplicate_work_outcome": assess_work_issue_duplicate_report(report_text),
} }
reasons = [] reasons = []
-382
View File
@@ -1,382 +0,0 @@
"""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,
}
+11 -5
View File
@@ -38,13 +38,21 @@ fi
branch="$1" branch="$1"
start_ref="${2:-prgs/master}" start_ref="${2:-prgs/master}"
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
repo_root="$(cd "$script_dir/.." && pwd)"
# Enforce issue-linked, traceable branch names (issue → branch → worktree → PR). # Enforce issue-linked, traceable branch names (issue → branch → worktree → PR).
if [[ "$allow_unlinked" -eq 0 ]]; then if [[ "$allow_unlinked" -eq 0 ]]; then
if [[ ! -f "/tmp/gitea_issue_lock.json" ]]; then locked_branch=$(python3 -c "
echo "Error: Issue lock file '/tmp/gitea_issue_lock.json' is missing. You must lock exactly one issue before branch creation (fail closed)." >&2 import sys
sys.path.insert(0, '$repo_root')
import issue_lock_store
print(issue_lock_store.resolve_locked_branch_for_session('$branch'))
")
if [[ -z "$locked_branch" ]]; then
echo "Error: No session issue lock is bound. Call gitea_lock_issue before branch creation (fail closed)." >&2
exit 2 exit 2
fi fi
locked_branch=$(python3 -c "import json; print(json.load(open('/tmp/gitea_issue_lock.json')).get('branch_name', ''))")
if [[ "$branch" != "$locked_branch" ]]; then if [[ "$branch" != "$locked_branch" ]]; then
echo "Error: Requested branch '$branch' does not match locked branch '$locked_branch' (fail closed)." >&2 echo "Error: Requested branch '$branch' does not match locked branch '$locked_branch' (fail closed)." >&2
exit 2 exit 2
@@ -68,8 +76,6 @@ EOF
fi fi
fi fi
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
repo_root="$(cd "$script_dir/.." && pwd)"
worktree_name="${branch//\//-}" worktree_name="${branch//\//-}"
worktree_path="$repo_root/branches/$worktree_name" worktree_path="$repo_root/branches/$worktree_name"
@@ -732,26 +732,6 @@ The final report must identify:
* whether same-PR merge continuation was allowed * whether same-PR merge continuation was allowed
* whether the run stopped as required * 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 ## 27. Merge rules
Before merge, rerun fresh live checks: Before merge, rerun fresh live checks:
@@ -297,36 +297,6 @@ Report:
Do not create another branch/PR for the same issue unless the project explicitly allows taking over or updating existing work and exact capability is proven. Do not create another branch/PR for the same issue unless the project explicitly allows taking over or updating existing work and exact capability is proven.
### 10A. Duplicate-work gate phases (#400)
Before any file edits, prove duplicate-work clearance with
`gitea_assess_work_issue_duplicate` or `gitea_lock_issue` (which runs the same
gate). The gate checks live:
* open PRs linked to the issue (head branch or Closes/Fixes reference),
* remote branches matching `issue-<number>`,
* active claim leases from structured heartbeats.
Re-check immediately before:
* `gitea_commit_files` (commit),
* branch push,
* `gitea_create_pr` (PR creation).
If a concurrent open PR appears after work begins:
* before commit/push → stop and preserve local work without pushing,
* after commit but before push → stop without pushing,
* after push but before PR creation → stop and produce a reconciliation
handoff instead of opening a PR.
Final reports must state exactly one duplicate-work outcome:
* `duplicate PR prevented`
* `duplicate branch prevented`
* `duplicate commit prevented`
* `duplicate work not prevented`
## 11. Claim or lock the issue before implementation ## 11. Claim or lock the issue before implementation
Claim/lock the issue before implementation if the project provides a claim/lock mechanism. Claim/lock the issue before implementation if the project provides a claim/lock mechanism.
@@ -339,6 +309,15 @@ Do not implement unclaimed work.
If the claim/lock gates are broken, produce a recovery handoff. If the claim/lock gates are broken, produce a recovery handoff.
### Lost lock recovery after push (non-destructive)
If the MCP server restarts after the issue branch was pushed but before PR creation:
* **Do not** delete the remote branch as the normal recovery path.
* Call `gitea_lock_issue` again with the same issue number and exact branch name. Own-branch adoption rebinds the session when open-PR, competing-lock, and worktree safety checks pass.
* Call `gitea_create_pr` afterward. Durable keyed locks resolve from the session pointer or by matching the PR `head` branch without manual lock seeding.
* If adoption is blocked by an open PR, a competing live lease, or a different same-issue branch, stop and produce a recovery handoff.
Create a tooling issue only if this run is explicitly authorized to switch to issue-creation mode and exact `create_issue` capability is proven. Create a tooling issue only if this run is explicitly authorized to switch to issue-creation mode and exact `create_issue` capability is proven.
Report: Report:
@@ -747,12 +726,6 @@ Use only precise categories:
* External-state mutations: * External-state mutations:
* Read-only diagnostics: * Read-only diagnostics:
Issue-lock file (`/tmp/gitea_issue_lock.json`) read/write/delete is always an
external-state mutation. Never claim `External-state mutations: none` after
seeding, restoring, or removing that file. Manual lock seeding is not a normal
recovery path (#447); use `gitea_lock_issue` or the #442 adoption recovery path
instead. Link broader redesign: #438.
`git fetch`, `git remote update`, and any command that updates refs must be listed under `Git ref mutations`, not read-only diagnostics. `git fetch`, `git remote update`, and any command that updates refs must be listed under `Git ref mutations`, not read-only diagnostics.
If `git reset --hard`, checkout, clean, worktree add/remove, merge simulation, merge abort, or similar commands occurred, report them under `Worktree/index mutations`. If `git reset --hard`, checkout, clean, worktree add/remove, merge simulation, merge abort, or similar commands occurred, report them under `Worktree/index mutations`.
+5 -7
View File
@@ -74,22 +74,20 @@ ISSUE_WRITE_ENV = {
class TestIssueLockArtifactWarning(unittest.TestCase): class TestIssueLockArtifactWarning(unittest.TestCase):
def setUp(self): def setUp(self):
self._env_patcher = patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True) self._lock_dir = tempfile.TemporaryDirectory()
env = {**ISSUE_WRITE_ENV, "GITEA_ISSUE_LOCK_DIR": self._lock_dir.name}
self._env_patcher = patch.dict(os.environ, env, clear=True)
self._env_patcher.start() self._env_patcher.start()
def tearDown(self): def tearDown(self):
self._env_patcher.stop() self._env_patcher.stop()
self._lock_dir.cleanup()
@patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
@patch("mcp_server.api_get_all", return_value=[]) @patch("mcp_server.api_get_all", return_value=[])
@patch("mcp_server._auth", return_value="token x") @patch("mcp_server._auth", return_value="token x")
@patch("mcp_server._resolve", return_value=("h", "o", "r")) @patch("mcp_server._resolve", return_value=("h", "o", "r"))
@patch("mcp_server.ISSUE_LOCK_FILE", new_callable=lambda: tempfile.mktemp())
@patch("issue_lock_worktree.read_worktree_git_state") @patch("issue_lock_worktree.read_worktree_git_state")
def test_lock_success_includes_artifact_warning(self, mock_state, _lock_file, *_mocks): def test_lock_success_includes_artifact_warning(self, mock_state, *_mocks):
mock_state.return_value = { mock_state.return_value = {
"current_branch": "master", "current_branch": "master",
"porcelain_status": "?? _emit_payload.py\n", "porcelain_status": "?? _emit_payload.py\n",
+3 -29
View File
@@ -286,21 +286,6 @@ class TestSimpleToolAudit(_AuditWiringBase):
class TestGatedToolAudit(_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): def _pr(self, author, state="open", sha="abc123", mergeable=True):
return {"user": {"login": author}, "state": state, return {"user": {"login": author}, "state": state,
"head": {"sha": sha}, "mergeable": mergeable} "head": {"sha": sha}, "mergeable": mergeable}
@@ -359,22 +344,11 @@ class TestGatedToolAudit(_AuditWiringBase):
GITEA_ALLOWED_OPERATIONS="read,review,approve") GITEA_ALLOWED_OPERATIONS="read,review,approve")
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision 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") init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(8, "approve", remote="prgs") gitea_mark_final_review_decision(8, "approve", remote="prgs")
lease_patch = _install_owned_reviewer_lease(8) r = gitea_submit_pr_review(pr_number=8, action="approve",
lease_patch.start() body="LGTM", remote="prgs",
try: final_review_decision_ready=True)
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"]) self.assertTrue(r["performed"])
recs = self._records() recs = self._records()
self.assertEqual(len(recs), 1) self.assertEqual(len(recs), 1)
-7
View File
@@ -76,14 +76,7 @@ class CommitFilesCapabilityBase(unittest.TestCase):
with open(self.config_path, "w", encoding="utf-8") as fh: with open(self.config_path, "w", encoding="utf-8") as fh:
fh.write(json.dumps(CONFIG)) fh.write(json.dumps(CONFIG))
self._dup_fetcher_patcher = patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
self._dup_fetcher_patcher.start()
def tearDown(self): def tearDown(self):
self._dup_fetcher_patcher.stop()
self._remotes.stop() self._remotes.stop()
mcp_server._IDENTITY_CACHE.clear() mcp_server._IDENTITY_CACHE.clear()
mcp_server._preflight_whoami_called, mcp_server._preflight_capability_called = ( mcp_server._preflight_whoami_called, mcp_server._preflight_capability_called = (
+10 -25
View File
@@ -66,16 +66,10 @@ class TestCommitPayloads(unittest.TestCase):
) )
self.locked_worktree_path = os.path.realpath(self.locked_worktree_dir.name) self.locked_worktree_path = os.path.realpath(self.locked_worktree_dir.name)
self.lock_file_path = "/tmp/gitea_issue_lock.json" import issue_lock_store
import issue_lock_provenance
work_lease = { self._lock_dir = tempfile.TemporaryDirectory()
"operation_type": "author_issue_work", os.environ["GITEA_ISSUE_LOCK_DIR"] = self._lock_dir.name
"issue_number": 263,
"branch": "feat/issue-263-native-commit-payloads",
"claimant": {"username": "test-user", "profile": "test-author"},
"expires_at": "2999-01-01T00:00:00Z",
}
self.lock_data = { self.lock_data = {
"issue_number": 263, "issue_number": 263,
"branch_name": "feat/issue-263-native-commit-payloads", "branch_name": "feat/issue-263-native-commit-payloads",
@@ -83,14 +77,12 @@ class TestCommitPayloads(unittest.TestCase):
"org": "Example-Org", "org": "Example-Org",
"repo": "Example-Repo", "repo": "Example-Repo",
"worktree_path": self.locked_worktree_path, "worktree_path": self.locked_worktree_path,
"work_lease": work_lease, "work_lease": {
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance( "operation_type": "author_issue_work",
tool="gitea_lock_issue", "expires_at": "2999-01-01T00:00:00Z",
claimant=work_lease.get("claimant"), },
),
} }
with open(self.lock_file_path, "w", encoding="utf-8") as fh: self.lock_file_path = issue_lock_store.bind_session_lock(self.lock_data)
fh.write(json.dumps(self.lock_data))
# Reset preflight status to bypass/pass verification in tests # Reset preflight status to bypass/pass verification in tests
self.orig_whoami_called = mcp_server._preflight_whoami_called self.orig_whoami_called = mcp_server._preflight_whoami_called
@@ -98,14 +90,7 @@ class TestCommitPayloads(unittest.TestCase):
mcp_server._preflight_whoami_called = True mcp_server._preflight_whoami_called = True
mcp_server._preflight_capability_called = True mcp_server._preflight_capability_called = True
self._dup_fetcher_patcher = patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
self._dup_fetcher_patcher.start()
def tearDown(self): def tearDown(self):
self._dup_fetcher_patcher.stop()
self._remotes.stop() self._remotes.stop()
mcp_server._IDENTITY_CACHE.clear() mcp_server._IDENTITY_CACHE.clear()
@@ -114,8 +99,7 @@ class TestCommitPayloads(unittest.TestCase):
self._dir.cleanup() self._dir.cleanup()
self.locked_worktree_dir.cleanup() self.locked_worktree_dir.cleanup()
if os.path.exists(self.lock_file_path): self._lock_dir.cleanup()
os.remove(self.lock_file_path)
def _env(self, profile: str) -> dict: def _env(self, profile: str) -> dict:
return { return {
@@ -124,6 +108,7 @@ class TestCommitPayloads(unittest.TestCase):
"GITEA_TOKEN_AUTHOR": "author-pass", "GITEA_TOKEN_AUTHOR": "author-pass",
"GITEA_TEST_PORCELAIN": "", "GITEA_TEST_PORCELAIN": "",
"GITEA_AUTHOR_WORKTREE": self.locked_worktree_path, "GITEA_AUTHOR_WORKTREE": self.locked_worktree_path,
"GITEA_ISSUE_LOCK_DIR": self._lock_dir.name,
} }
@patch("mcp_server.api_request") @patch("mcp_server.api_request")
+35
View File
@@ -0,0 +1,35 @@
"""Unit tests for structured issue/branch ownership parsing (#440)."""
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from issue_branch_ownership import ( # noqa: E402
branch_tracks_issue,
parse_tracked_issue_number,
)
class TestIssueBranchOwnership(unittest.TestCase):
def test_parses_canonical_author_branch(self):
self.assertEqual(
parse_tracked_issue_number("feat/issue-420-server-code-parity"),
420,
)
def test_issue_4200_does_not_track_issue_420(self):
self.assertEqual(parse_tracked_issue_number("feat/issue-4200-unrelated"), 4200)
self.assertFalse(branch_tracks_issue("feat/issue-4200-unrelated", 420))
def test_branch_tracks_issue_exact_match(self):
self.assertTrue(
branch_tracks_issue("fix/issue-440-branch-recovery", 440)
)
def test_unrelated_branch_does_not_track(self):
self.assertFalse(branch_tracks_issue("feat/issue-999-other", 440))
if __name__ == "__main__":
unittest.main()
+8 -70
View File
@@ -1,4 +1,4 @@
"""Unit tests for own-branch lock adoption decision (#442).""" """Unit tests for own-branch lock adoption decision (#442 / #443)."""
import sys import sys
import unittest import unittest
from pathlib import Path from pathlib import Path
@@ -25,16 +25,6 @@ class TestAssessOwnBranchAdoption(unittest.TestCase):
) )
self.assertEqual(result["outcome"], ADOPT) self.assertEqual(result["outcome"], ADOPT)
self.assertTrue(result["adopt"]) self.assertTrue(result["adopt"])
self.assertFalse(result["block"])
self.assertEqual(result["matched_branch"], REQ)
self.assertEqual(result["matched_head_sha"], "934688a")
def test_exact_own_branch_adopted_when_sha_missing(self):
result = assess_own_branch_adoption(
issue_number=420, requested_branch=REQ, existing_branches=[REQ]
)
self.assertEqual(result["outcome"], ADOPT)
self.assertIsNone(result["matched_head_sha"])
def test_different_branch_same_issue_blocks(self): def test_different_branch_same_issue_blocks(self):
result = assess_own_branch_adoption( result = assess_own_branch_adoption(
@@ -44,20 +34,6 @@ class TestAssessOwnBranchAdoption(unittest.TestCase):
) )
self.assertEqual(result["outcome"], BLOCK_COMPETING) self.assertEqual(result["outcome"], BLOCK_COMPETING)
self.assertTrue(result["block"]) self.assertTrue(result["block"])
self.assertFalse(result["adopt"])
self.assertIn("feat/issue-420-other-work", result["competing_branches"])
self.assertIn("fail closed", result["reason"])
def test_own_branch_plus_competing_branch_blocks(self):
# Ambiguous ownership: fail closed even though the exact branch exists.
result = assess_own_branch_adoption(
issue_number=420,
requested_branch=REQ,
existing_branches=[{"name": REQ}, {"name": "feat/issue-420-rogue"}],
)
self.assertEqual(result["outcome"], BLOCK_COMPETING)
self.assertTrue(result["block"])
self.assertEqual(result["competing_branches"], ["feat/issue-420-rogue"])
def test_no_matching_branch_is_normal_path(self): def test_no_matching_branch_is_normal_path(self):
result = assess_own_branch_adoption( result = assess_own_branch_adoption(
@@ -66,44 +42,18 @@ class TestAssessOwnBranchAdoption(unittest.TestCase):
existing_branches=[{"name": "feat/issue-999-unrelated"}], existing_branches=[{"name": "feat/issue-999-unrelated"}],
) )
self.assertEqual(result["outcome"], NO_MATCH) self.assertEqual(result["outcome"], NO_MATCH)
self.assertFalse(result["block"])
self.assertFalse(result["adopt"])
def test_empty_branch_list_is_normal_path(self): def test_issue_number_substring_collision_is_ignored(self):
result = assess_own_branch_adoption( result = assess_own_branch_adoption(
issue_number=420, requested_branch=REQ, existing_branches=[] issue_number=420,
requested_branch=REQ,
existing_branches=[{"name": "feat/issue-4200-unrelated"}],
) )
self.assertEqual(result["outcome"], NO_MATCH) self.assertEqual(result["outcome"], NO_MATCH)
def test_higher_issue_number_branch_does_not_block_lower_issue_adoption(self):
# issue-420 must not be treated as competing work for issue #42.
own_branch = "feat/issue-42-widget"
result = assess_own_branch_adoption(
issue_number=42,
requested_branch=own_branch,
existing_branches=[
{"name": own_branch, "commit_sha": "abc1234"},
{"name": "feat/issue-420-server-code-parity"},
],
)
self.assertEqual(result["outcome"], ADOPT)
self.assertTrue(result["adopt"])
self.assertFalse(result["block"])
self.assertEqual(result["matched_branch"], own_branch)
def test_unrelated_higher_number_branch_is_ignored_without_own_branch(self):
result = assess_own_branch_adoption(
issue_number=42,
requested_branch="feat/issue-42-thing",
existing_branches=[{"name": "feat/issue-420-server-code-parity"}],
)
self.assertEqual(result["outcome"], NO_MATCH)
self.assertFalse(result["block"])
self.assertFalse(result["adopt"])
class TestBuildAdoptionProof(unittest.TestCase): class TestBuildAdoptionProof(unittest.TestCase):
def test_proof_has_all_required_fields(self): def test_proof_has_required_fields(self):
assessment = assess_own_branch_adoption( assessment = assess_own_branch_adoption(
issue_number=420, issue_number=420,
requested_branch=REQ, requested_branch=REQ,
@@ -115,24 +65,12 @@ class TestBuildAdoptionProof(unittest.TestCase):
assessment=assessment, assessment=assessment,
open_pr_checked=True, open_pr_checked=True,
competing_lock_checked=True, competing_lock_checked=True,
lock_file_path="/tmp/gitea_issue_lock.json", lock_file_path="/tmp/example-lock.json",
lock_file_status="written", lock_file_status="written",
) )
for key in (
"issue_number",
"branch_name",
"branch_head_commit",
"adoption_reason",
"no_existing_pr_proof",
"no_competing_live_lock_proof",
"lock_file_path",
"lock_file_status",
):
self.assertIn(key, proof)
self.assertEqual(proof["branch_head_commit"], "934688a") self.assertEqual(proof["branch_head_commit"], "934688a")
self.assertTrue(proof["no_existing_pr_proof"]) self.assertTrue(proof["no_existing_pr_proof"])
self.assertTrue(proof["no_competing_live_lock_proof"])
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
-130
View File
@@ -1,130 +0,0 @@
"""Tests for issue-lock provenance and external-state disclosure (#447)."""
from __future__ import annotations
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import issue_lock_provenance as ilp # noqa: E402
from final_report_validator import assess_final_report_validator # noqa: E402
def _sanctioned_lock(**overrides):
work_lease = {
"operation_type": "author_issue_work",
"issue_number": 447,
"branch": "feat/issue-447-lock-provenance",
"claimant": {"username": "jcwalker3", "profile": "prgs-author"},
"expires_at": "2999-01-01T00:00:00Z",
}
data = {
"issue_number": 447,
"branch_name": "feat/issue-447-lock-provenance",
"work_lease": work_lease,
"lock_provenance": ilp.build_sanctioned_lock_provenance(
tool="gitea_lock_issue",
claimant=work_lease["claimant"],
),
}
data.update(overrides)
return data
class TestLockProvenanceForCreatePr(unittest.TestCase):
def test_sanctioned_lock_passes(self):
result = ilp.assess_lock_file_for_create_pr(_sanctioned_lock())
self.assertTrue(result["proven"])
self.assertFalse(result["block"])
def test_manual_seed_without_provenance_blocked(self):
result = ilp.assess_lock_file_for_create_pr(
{"issue_number": 420, "branch_name": "feat/x", "work_lease": {}}
)
self.assertTrue(result["block"])
self.assertIn("lock_provenance", result["reasons"][0])
def test_operator_override_requires_reason(self):
result = ilp.assess_lock_file_for_create_pr(
_sanctioned_lock(
lock_provenance=ilp.build_sanctioned_lock_provenance(
tool="operator_override",
source=ilp.SOURCE_OPERATOR_OVERRIDE,
)
)
)
self.assertTrue(result["block"])
class TestExternalStateReportRules(unittest.TestCase):
def test_seed_with_external_none_blocked(self):
report = (
"Restored /tmp/gitea_issue_lock.json to unblock PR creation.\n"
"- External-state mutations: none\n"
)
result = ilp.assess_issue_lock_external_state_report(report)
self.assertTrue(result["block"])
def test_seed_with_disclosure_passes(self):
report = (
"Restored /tmp/gitea_issue_lock.json after MCP restart.\n"
"- External-state mutations: wrote /tmp/gitea_issue_lock.json\n"
)
result = ilp.assess_issue_lock_external_state_report(report)
self.assertTrue(result["proven"])
def test_remove_claimed_as_cleanup_only_blocked(self):
report = (
"rm /tmp/gitea_issue_lock.json after PR creation.\n"
"- Cleanup mutations: lock removed\n"
"- External-state mutations: none\n"
)
result = ilp.assess_issue_lock_external_state_report(report)
self.assertTrue(result["block"])
def test_manual_lock_pr_without_override_blocked(self):
report = (
"Programmatically seeded gitea_issue_lock.json then gitea_create_pr.\n"
"PR #444 created.\n"
)
result = ilp.assess_manual_lock_pr_without_override(report)
self.assertTrue(result["block"])
def test_author_reviewer_same_run_blocked(self):
report = (
"gitea_create_pr opened PR #444.\n"
"Submitted approve review on PR #444.\n"
)
result = ilp.assess_author_reviewer_same_run_report(report)
self.assertTrue(result["block"])
class TestFinalReportValidatorIntegration(unittest.TestCase):
def test_work_issue_blocks_hidden_lock_mutation(self):
report = (
"## Controller Handoff\n"
"- Task: work issue #420\n"
"- External-state mutations: none\n"
"Restored /tmp/gitea_issue_lock.json before PR creation.\n"
)
result = assess_final_report_validator(report, "work_issue")
rule_ids = {f["rule_id"] for f in result["findings"]}
self.assertIn("shared.issue_lock_external_state", rule_ids)
self.assertTrue(result["blocked"])
def test_review_pr_blocks_create_and_approve(self):
report = (
"## Controller Handoff\n"
"- Task: review PR #444\n"
"- Review decision: approve\n"
"Created PR #444 via gitea_create_pr earlier in this run.\n"
)
result = assess_final_report_validator(report, "review_pr")
rule_ids = {f["rule_id"] for f in result["findings"]}
self.assertIn("shared.author_reviewer_same_run", rule_ids)
if __name__ == "__main__":
unittest.main()
+152
View File
@@ -0,0 +1,152 @@
"""Integration tests for non-destructive lock/PR recovery (#440)."""
import os
import sys
import tempfile
import unittest
from pathlib import Path
from unittest import mock
from unittest.mock import patch
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import issue_lock_adoption # noqa: E402
import issue_lock_store as ils # noqa: E402
import mcp_server # noqa: E402
from mcp_server import gitea_create_pr, gitea_lock_issue # noqa: E402
from tests.test_mcp_server import CREATE_PR_ENV, ISSUE_WRITE_ENV # noqa: E402
FAKE_AUTH = "token fake"
BRANCH = "feat/issue-420-server-code-parity"
def _lock_record(**overrides):
record = {
"issue_number": 420,
"branch_name": BRANCH,
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": mcp_server.REMOTES["prgs"]["repo"],
"worktree_path": "/tmp/wt-420",
"work_lease": {
"operation_type": "author_issue_work",
"expires_at": "2999-01-01T00:00:00Z",
},
}
record.update(overrides)
return record
def _clean_git_state():
return {
"current_branch": BRANCH,
"porcelain_status": "",
"base_equivalent": True,
"inspected_git_root": os.getcwd(),
"base_branch": "master",
}
class TestIssueLockRecovery(unittest.TestCase):
def setUp(self):
self._tmpdir = tempfile.TemporaryDirectory()
self._lock_dir = self._tmpdir.name
self._env = {
**ISSUE_WRITE_ENV,
"GITEA_ISSUE_LOCK_DIR": self._lock_dir,
}
self._create_pr_env = {
**CREATE_PR_ENV,
"GITEA_ISSUE_LOCK_DIR": self._lock_dir,
}
def tearDown(self):
self._tmpdir.cleanup()
def test_adoption_after_restart_without_session_pointer(self):
with patch.dict(os.environ, self._env, clear=True):
with mock.patch("os.getpid", return_value=9999):
self.assertIsNone(ils.read_session_issue_lock())
with mock.patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_git_state(),
), mock.patch("mcp_server.api_get_all") as mock_api, mock.patch(
"mcp_server.get_auth_header", return_value=FAKE_AUTH
):
mock_api.side_effect = [
[],
[{"name": BRANCH, "commit": {"id": "934688a"}}],
]
res = gitea_lock_issue(
issue_number=420,
branch_name=BRANCH,
remote="prgs",
worktree_path=os.path.realpath(os.getcwd()),
)
self.assertTrue(res["success"])
self.assertIn("adoption", res)
self.assertEqual(res["adoption"]["branch_head_commit"], "934688a")
@mock.patch(
"mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []),
)
@mock.patch("mcp_server.api_request")
@mock.patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_create_pr_resolves_keyed_lock_after_restart(self, _auth, mock_api, _role):
mock_api.return_value = {"number": 501, "html_url": "https://example/pr/501"}
worktree = os.path.realpath(os.getcwd())
with patch.dict(os.environ, self._create_pr_env, clear=True):
path = ils.lock_file_path(
remote="prgs",
org=mcp_server.REMOTES["prgs"]["org"],
repo=mcp_server.REMOTES["prgs"]["repo"],
issue_number=420,
lock_dir=self._lock_dir,
)
ils.save_lock_file(path, _lock_record(worktree_path=worktree))
with mock.patch("os.getpid", return_value=4242):
self.assertIsNone(ils.read_session_issue_lock())
res = gitea_create_pr(
title="feat: recovery Closes #420",
head=BRANCH,
base="master",
remote="prgs",
worktree_path=worktree,
)
self.assertEqual(res["number"], 501)
def test_open_pr_blocks_adoption(self):
with patch.dict(os.environ, self._env, clear=True):
with mock.patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_git_state(),
), mock.patch("mcp_server.api_get_all") as mock_api, mock.patch(
"mcp_server.get_auth_header", return_value=FAKE_AUTH
):
mock_api.side_effect = [
[{"number": 99, "head": {"ref": BRANCH}, "title": "", "body": ""}],
[{"name": BRANCH, "commit": {"id": "934688a"}}],
]
with self.assertRaises(ValueError) as ctx:
gitea_lock_issue(
issue_number=420,
branch_name=BRANCH,
remote="prgs",
worktree_path=os.path.realpath(os.getcwd()),
)
self.assertIn("already tied to an open PR", str(ctx.exception))
def test_structured_ownership_ignores_issue_4200_collision(self):
result = issue_lock_adoption.assess_own_branch_adoption(
issue_number=420,
requested_branch=BRANCH,
existing_branches=[{"name": "feat/issue-4200-unrelated"}],
)
self.assertEqual(result["outcome"], issue_lock_adoption.NO_MATCH)
if __name__ == "__main__":
unittest.main()
+181
View File
@@ -0,0 +1,181 @@
"""Unit tests for keyed issue-lock storage (#443)."""
import json
import os
import sys
import tempfile
import unittest
from datetime import datetime, timedelta, timezone
from pathlib import Path
from unittest import mock
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import issue_lock_store as ils # noqa: E402
def _lease(expires_at: str) -> dict:
return {
"operation_type": ils.AUTHOR_ISSUE_WORK_LEASE,
"expires_at": expires_at,
"created_at": "2026-01-01T00:00:00Z",
"last_heartbeat_at": "2026-01-01T00:00:00Z",
}
def _lock_record(**overrides) -> dict:
record = {
"issue_number": 420,
"branch_name": "feat/issue-420-server-code-parity",
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools",
"worktree_path": "/tmp/wt-420",
"work_lease": _lease("2999-01-01T00:00:00Z"),
}
record.update(overrides)
return record
class TestIssueLockStore(unittest.TestCase):
def setUp(self):
self._dir = tempfile.TemporaryDirectory()
self.lock_dir = self._dir.name
self._env = mock.patch.dict(os.environ, {"GITEA_ISSUE_LOCK_DIR": self.lock_dir})
self._env.start()
def tearDown(self):
self._env.stop()
self._dir.cleanup()
def test_concurrent_repo_locks_do_not_overwrite(self):
lock_a = _lock_record(
issue_number=108,
branch_name="feat/issue-108-root-menu",
repo="mcp-control-plane",
worktree_path="/tmp/wt-108",
)
lock_b = _lock_record(
issue_number=420,
branch_name="feat/issue-420-server-code-parity",
repo="Gitea-Tools",
worktree_path="/tmp/wt-420",
)
path_a = ils.bind_session_lock(lock_a)
with mock.patch("os.getpid", return_value=9999):
path_b = ils.bind_session_lock(lock_b)
self.assertNotEqual(path_a, path_b)
self.assertTrue(os.path.exists(path_a))
self.assertTrue(os.path.exists(path_b))
stored_a = ils.read_lock_file(path_a)
stored_b = ils.read_lock_file(path_b)
self.assertEqual(stored_a["issue_number"], 108)
self.assertEqual(stored_b["issue_number"], 420)
def test_concurrent_issue_locks_same_repo_do_not_overwrite(self):
lock_a = _lock_record(issue_number=427, branch_name="feat/issue-427-a")
lock_b = _lock_record(issue_number=428, branch_name="feat/issue-428-b")
path_a = ils.bind_session_lock(lock_a)
with mock.patch("os.getpid", return_value=4242):
path_b = ils.bind_session_lock(lock_b)
self.assertNotEqual(path_a, path_b)
self.assertEqual(ils.read_lock_file(path_a)["issue_number"], 427)
self.assertEqual(ils.read_lock_file(path_b)["issue_number"], 428)
def test_foreign_live_lease_blocks_overwrite(self):
existing = _lock_record(
branch_name="feat/issue-420-other",
worktree_path="/tmp/other",
work_lease=_lease("2999-01-01T00:00:00Z"),
)
path = ils.lock_file_path(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
issue_number=420,
)
ils.save_lock_file(path, existing)
incoming = _lock_record(worktree_path="/tmp/mine")
block = ils.assess_foreign_lock_overwrite(existing, incoming)
self.assertIn("live foreign issue lock", block or "")
def test_expired_lease_allows_takeover_with_conflict_check(self):
existing = _lock_record(
branch_name="feat/issue-420-other",
worktree_path="/tmp/other",
work_lease=_lease("2000-01-01T00:00:00Z"),
)
incoming = _lock_record(worktree_path="/tmp/mine")
self.assertIsNone(ils.assess_foreign_lock_overwrite(existing, incoming))
block = ils.assess_same_issue_lease_conflict(
existing,
issue_number=420,
branch_name="feat/issue-420-server-code-parity",
worktree_path="/tmp/mine",
)
self.assertIn("Recovery review is required", block or "")
def test_same_owner_lease_conflict_allows_refresh(self):
worktree = "/tmp/wt-420"
existing = _lock_record(worktree_path=worktree)
block = ils.assess_same_issue_lease_conflict(
existing,
issue_number=420,
branch_name="feat/issue-420-server-code-parity",
worktree_path=worktree,
)
self.assertIsNone(block)
def test_find_lock_for_branch_after_restart(self):
record = _lock_record()
path = ils.lock_file_path(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
issue_number=420,
)
ils.save_lock_file(path, record)
with mock.patch("os.getpid", return_value=5555):
self.assertIsNone(ils.read_session_issue_lock())
found = ils.find_lock_for_branch(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
branch_name="feat/issue-420-server-code-parity",
)
self.assertEqual(found["issue_number"], 420)
def test_has_active_issue_lock_scans_keyed_store(self):
ils.bind_session_lock(_lock_record())
self.assertTrue(
ils.has_active_issue_lock("feat/issue-420-server-code-parity")
)
self.assertFalse(ils.has_active_issue_lock("feat/issue-999-other"))
def test_atomic_write_preserves_unrelated_lock(self):
path_a = ils.lock_file_path(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
issue_number=108,
)
ils.save_lock_file(path_a, _lock_record(issue_number=108, repo="mcp-control-plane"))
path_b = ils.lock_file_path(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
issue_number=420,
)
ils.save_lock_file(path_b, _lock_record())
self.assertTrue(os.path.exists(path_a))
self.assertTrue(os.path.exists(path_b))
self.assertEqual(ils.read_lock_file(path_a)["issue_number"], 108)
if __name__ == "__main__":
unittest.main()
-265
View File
@@ -1,265 +0,0 @@
"""Tests for early duplicate-work detection (#400)."""
import json
import os
import sys
import tempfile
import unittest
from pathlib import Path
from unittest.mock import patch
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import issue_lock_provenance
import issue_work_duplicate_gate as dup_gate
import mcp_server
from issue_work_duplicate_gate import (
OUTCOME_DUPLICATE_BRANCH_PREVENTED,
OUTCOME_DUPLICATE_COMMIT_PREVENTED,
OUTCOME_DUPLICATE_PR_PREVENTED,
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED,
PHASE_COMMIT,
PHASE_CREATE_PR,
PHASE_LOCK,
assess_work_issue_duplicate_gate,
assess_work_issue_duplicate_report,
)
class TestDuplicateGateAssessment(unittest.TestCase):
def test_clear_issue_passes(self):
result = assess_work_issue_duplicate_gate(
400,
open_prs=[],
branch_names=["feat/other-issue-99"],
claim_entry={"status": "not_claimed"},
locked_branch="feat/issue-400-duplicate-work-preflight",
phase=PHASE_LOCK,
)
self.assertFalse(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_WORK_NOT_PREVENTED)
def test_open_pr_blocks(self):
prs = [{
"number": 397,
"title": "feat: handoff",
"body": "Closes #395",
"head": {"ref": "feat/issue-395-proof-backed-review-handoff"},
}]
result = assess_work_issue_duplicate_gate(
395,
open_prs=prs,
branch_names=[],
phase=PHASE_LOCK,
)
self.assertTrue(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_PR_PREVENTED)
def test_conflicting_remote_branch_blocks(self):
result = assess_work_issue_duplicate_gate(
395,
open_prs=[],
branch_names=["feat/issue-395-proof-backed-handoff-claims"],
locked_branch="feat/issue-395-new-attempt",
phase=PHASE_LOCK,
)
self.assertTrue(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_BRANCH_PREVENTED)
def test_active_claim_on_other_branch_blocks(self):
result = assess_work_issue_duplicate_gate(
398,
open_prs=[],
branch_names=[],
claim_entry={
"status": "active",
"latest_heartbeat": {
"branch": "feat/issue-398-validation-cwd-proof",
},
},
locked_branch="feat/issue-398-other-branch",
phase=PHASE_LOCK,
)
self.assertTrue(result["block"])
def test_commit_phase_maps_to_commit_outcome(self):
prs = [{
"number": 411,
"title": "x",
"body": "Closes #398",
"head": {"ref": "feat/issue-398-validation-cwd-proof"},
}]
result = assess_work_issue_duplicate_gate(
398,
open_prs=prs,
branch_names=[],
locked_branch="feat/issue-398-alt",
phase=PHASE_COMMIT,
)
self.assertTrue(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_COMMIT_PREVENTED)
def test_stale_claim_does_not_block_by_status_alone(self):
result = assess_work_issue_duplicate_gate(
400,
open_prs=[],
branch_names=[],
claim_entry={"status": "reclaimable", "reasons": ["stale"]},
locked_branch="feat/issue-400-duplicate-work-preflight",
phase=PHASE_LOCK,
)
self.assertFalse(result["block"])
class TestDuplicateReportOutcome(unittest.TestCase):
def test_requires_exactly_one_outcome(self):
bad = assess_work_issue_duplicate_report("work finished")
self.assertFalse(bad["complete"])
good = assess_work_issue_duplicate_report(
"Duplicate work not prevented for issue #400."
)
self.assertTrue(good["complete"])
self.assertEqual(good["outcome"], OUTCOME_DUPLICATE_WORK_NOT_PREVENTED)
class TestInjectableDuplicateFetcher(unittest.TestCase):
@patch("mcp_server.api_get_all", return_value=[])
@patch("mcp_server.get_auth_header", return_value="token x")
def test_lock_issue_uses_injected_fetcher(self, _auth, _api):
seen = {}
def fetcher(h, o, r, auth, issue_number):
seen["issue_number"] = issue_number
return [], [], {"status": "not_claimed"}
with patch(
"mcp_server.issue_duplicate_context_fetcher",
side_effect=fetcher,
), patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value={
"current_branch": "master",
"porcelain_status": "",
"base_equivalent": True,
},
), patch.dict(os.environ, {
"GITEA_ALLOWED_OPERATIONS": "gitea.issue.comment",
}, clear=True):
with patch.object(mcp_server, "ISSUE_LOCK_FILE", tempfile.mktemp()):
mcp_server.gitea_lock_issue(
issue_number=400,
branch_name="feat/issue-400-duplicate-work-preflight",
remote="prgs",
)
self.assertEqual(seen["issue_number"], 400)
class TestMcpDuplicateRecheck(unittest.TestCase):
def setUp(self):
self._dir = tempfile.TemporaryDirectory()
self.lock_path = os.path.join(self._dir.name, "gitea_issue_lock.json")
self._lock_patch = patch.object(
mcp_server, "ISSUE_LOCK_FILE", self.lock_path
)
self._lock_patch.start()
self._remotes = patch.dict(mcp_server.REMOTES, {
"prgs": {"host": "gitea.example.com", "org": "Example-Org",
"repo": "Example-Repo"},
})
self._remotes.start()
mcp_server._IDENTITY_CACHE.clear()
def tearDown(self):
patch.stopall()
self._dir.cleanup()
def _write_lock(self, issue_number=400, branch="feat/issue-400-x"):
work_lease = {
"operation_type": "author_issue_work",
"issue_number": issue_number,
"branch": branch,
"claimant": {"username": "test-user", "profile": "test-author"},
"expires_at": "2999-01-01T00:00:00Z",
}
with open(self.lock_path, "w", encoding="utf-8") as fh:
json.dump({
"issue_number": issue_number,
"branch_name": branch,
"remote": "prgs",
"work_lease": work_lease,
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
tool="gitea_lock_issue",
claimant=work_lease.get("claimant"),
),
}, fh)
@patch("mcp_server._assess_issue_duplicate_gate")
@patch("mcp_server.get_profile", return_value={
"profile_name": "test-author",
"allowed_operations": ["gitea.read", "gitea.repo.commit"],
"forbidden_operations": [],
"audit_label": "test-author",
})
@patch("mcp_server.get_auth_header", return_value="token x")
def test_commit_files_blocked_on_recheck(self, _auth, _profile, mock_gate):
self._write_lock()
mock_gate.return_value = {
"block": True,
"reasons": ["open PR #412 already covers issue #400"],
"outcome": OUTCOME_DUPLICATE_COMMIT_PREVENTED,
"safe_next_action": "stop",
}
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", resolved_role="author")
with patch(
"mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []),
):
result = mcp_server.gitea_commit_files(
files=[{
"operation": "create",
"path": "a.txt",
"content_plain": "hi",
}],
message="test",
remote="prgs",
)
self.assertFalse(result["success"])
self.assertIn("duplicate_gate", result)
@patch("mcp_server._assess_issue_duplicate_gate")
@patch("mcp_server.get_profile", return_value={
"profile_name": "test-author",
"allowed_operations": ["gitea.read", "gitea.pr.create"],
"forbidden_operations": [],
"audit_label": "test-author",
})
@patch("mcp_server.get_auth_header", return_value="token x")
def test_create_pr_returns_handoff_on_duplicate(self, _auth, _profile, mock_gate):
self._write_lock()
mock_gate.return_value = {
"block": True,
"reasons": ["open PR #412 already covers issue #400"],
"outcome": OUTCOME_DUPLICATE_PR_PREVENTED,
"safe_next_action": "reconciliation handoff",
}
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", resolved_role="author")
with patch(
"mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []),
):
result = mcp_server.gitea_create_pr(
title="feat: x (Closes #400)",
head="feat/issue-400-x",
base="master",
body="Closes #400",
remote="prgs",
)
self.assertFalse(result["success"])
self.assertIsNone(result.get("number"))
self.assertIn("duplicate_gate", result)
if __name__ == "__main__":
unittest.main()
+206 -361
View File
@@ -6,6 +6,7 @@ the MCP protocol) with mocked API responses.
import json import json
import os import os
import sys import sys
import tempfile
import unittest import unittest
from unittest.mock import patch, MagicMock from unittest.mock import patch, MagicMock
@@ -45,6 +46,7 @@ from gitea_auth import get_profile # noqa: E402
import gitea_config # noqa: E402 import gitea_config # noqa: E402
import mcp_server import mcp_server
import issue_lock_store
FAKE_AUTH = "Basic dGVzdDp0ZXN0" FAKE_AUTH = "Basic dGVzdDp0ZXN0"
@@ -79,64 +81,6 @@ def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"):
return [_formal_review(reviewer, "APPROVED", sha=sha)] 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 tools are profile-gated (#69).
ISSUE_WRITE_ENV = { ISSUE_WRITE_ENV = {
"GITEA_ALLOWED_OPERATIONS": ( "GITEA_ALLOWED_OPERATIONS": (
@@ -155,38 +99,32 @@ CREATE_PR_ENV = {
), ),
} }
ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json"
def _sample_issue_lock(issue_number=123, branch_name="feat/x", **overrides): def _sample_issue_lock(issue_number=123, branch_name="feat/x", **overrides):
import issue_lock_provenance
work_lease = {
"operation_type": "author_issue_work",
"issue_number": issue_number,
"branch": branch_name,
"claimant": {"username": "test-user", "profile": "test-author"},
"expires_at": "2999-01-01T00:00:00Z",
}
record = { record = {
"issue_number": issue_number, "issue_number": issue_number,
"branch_name": branch_name, "branch_name": branch_name,
"remote": "dadeschools", "remote": "dadeschools",
"org": "Scaled-Tech-Consulting", "org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools", "repo": "Gitea-Tools",
"work_lease": work_lease, "worktree_path": "/tmp/test-worktree",
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance( "work_lease": {
tool="gitea_lock_issue", "operation_type": "author_issue_work",
claimant=work_lease.get("claimant"), "expires_at": "2999-01-01T00:00:00Z",
), },
} }
record.update(overrides) record.update(overrides)
return record return record
def _clear_duplicate_context_fetcher(*_args, **_kwargs): def _bind_test_lock(**overrides) -> str:
"""Default injectable duplicate-work context for lock/create_pr tests.""" remote = overrides.get("remote", "dadeschools")
return [], [], {"status": "not_claimed"} record = _sample_issue_lock(**overrides)
if remote in mcp_server.REMOTES:
profile = mcp_server.REMOTES[remote]
record.setdefault("org", profile["org"])
record.setdefault("repo", profile["repo"])
record["remote"] = remote
return issue_lock_store.bind_session_lock(record)
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -243,63 +181,70 @@ class TestCreateIssue(unittest.TestCase):
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
class TestCreatePR(unittest.TestCase): class TestCreatePR(unittest.TestCase):
@patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, [])) return_value=(True, []))
@patch("mcp_server.api_request") @patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
@patch("os.path.exists", return_value=True) def test_creates_pr(self, _auth, mock_api, _role):
@patch("builtins.open") worktree = os.path.realpath(os.getcwd())
def test_creates_pr(self, mock_open, mock_exists, _auth, mock_api, _role, _dup_fetcher):
lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x"))
mock_open.return_value.__enter__.return_value.read.return_value = lock_json
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"} mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
with patch.dict(os.environ, CREATE_PR_ENV, clear=True): with tempfile.TemporaryDirectory() as lock_dir:
result = gitea_create_pr(title="feat: X Closes #123", head="feat/x", base="main") env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir}
with patch.dict(os.environ, env, clear=True):
_bind_test_lock(issue_number=123, branch_name="feat/x", worktree_path=worktree)
result = gitea_create_pr(
title="feat: X Closes #123",
head="feat/x",
base="main",
worktree_path=worktree,
)
self.assertEqual(result["number"], 3) self.assertEqual(result["number"], 3)
self.assertNotIn("url", result) self.assertNotIn("url", result)
mock_exists.assert_called_with(ISSUE_LOCK_FILE)
mock_open.assert_called_with(ISSUE_LOCK_FILE, "r", encoding="utf-8")
payload = mock_api.call_args[0][3] payload = mock_api.call_args[0][3]
self.assertEqual(payload["head"], "feat/x") self.assertEqual(payload["head"], "feat/x")
self.assertEqual(payload["base"], "main") self.assertEqual(payload["base"], "main")
self.assertIn("Closes #123", payload["title"]) self.assertIn("Closes #123", payload["title"])
@patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, [])) return_value=(True, []))
@patch("mcp_server.api_request") @patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
@patch("os.path.exists", return_value=True) def test_create_pr_reveal_opt_in_includes_url(self, _auth, mock_api, _role):
@patch("builtins.open") worktree = os.path.realpath(os.getcwd())
def test_create_pr_reveal_opt_in_includes_url(self, mock_open, mock_exists, _auth, mock_api, _role, _dup_fetcher):
lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x"))
mock_open.return_value.__enter__.return_value.read.return_value = lock_json
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"} mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
env = {**CREATE_PR_ENV, "GITEA_MCP_REVEAL_ENDPOINTS": "1"} with tempfile.TemporaryDirectory() as lock_dir:
with patch.dict(os.environ, env, clear=True): env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir, "GITEA_MCP_REVEAL_ENDPOINTS": "1"}
result = gitea_create_pr(title="feat: X Closes #123", head="feat/x", base="main") with patch.dict(os.environ, env, clear=True):
_bind_test_lock(issue_number=123, branch_name="feat/x", worktree_path=worktree)
result = gitea_create_pr(
title="feat: X Closes #123",
head="feat/x",
base="main",
worktree_path=worktree,
)
self.assertIn("pulls/3", result["url"]) self.assertIn("pulls/3", result["url"])
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, [])) return_value=(True, []))
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
@patch("os.path.exists", return_value=True) def test_create_pr_locked_issue_mismatch_fails(self, _auth, _role):
@patch("builtins.open") worktree = os.path.realpath(os.getcwd())
def test_create_pr_locked_issue_mismatch_fails(self, mock_open, mock_exists, _auth, _role): with tempfile.TemporaryDirectory() as lock_dir:
lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x")) env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir}
mock_open.return_value.__enter__.return_value.read.return_value = lock_json with patch.dict(os.environ, env, clear=True):
with patch.dict(os.environ, CREATE_PR_ENV, clear=True): _bind_test_lock(
with self.assertRaises(ValueError) as ctx: issue_number=123,
gitea_create_pr(title="feat: X Closes #999", head="feat/x", base="main") branch_name="feat/x",
worktree_path=worktree,
)
with self.assertRaises(ValueError) as ctx:
gitea_create_pr(
title="feat: X Closes #999",
head="feat/x",
base="main",
worktree_path=worktree,
)
self.assertIn("Closes #123", str(ctx.exception)) self.assertIn("Closes #123", str(ctx.exception))
mock_open.assert_called_with(ISSUE_LOCK_FILE, "r", encoding="utf-8")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -597,19 +542,6 @@ class TestViewPR(unittest.TestCase):
class TestMergePR(unittest.TestCase): class TestMergePR(unittest.TestCase):
"""Gated merge workflow (#16). gitea_merge_pr is the only merge path.""" """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): def _pr(self, author, state="open", sha="abc123", mergeable=True):
return { return {
"user": {"login": author}, "user": {"login": author},
@@ -870,11 +802,9 @@ class TestMergePR(unittest.TestCase):
pr_number=8, confirmation=self._confirm(8), pr_number=8, confirmation=self._confirm(8),
expected_head_sha="deadbeef", remote="prgs") expected_head_sha="deadbeef", remote="prgs")
self.assertFalse(r["performed"]) self.assertFalse(r["performed"])
self.assertTrue(any( self.assertIn(
"expected head SHA does not match current PR head (fail closed)" in reason "expected head SHA does not match current PR head (fail closed)",
or "PR head changed during lease" in reason r["reasons"])
for reason in r["reasons"]
))
self._assert_no_merge_call(mock_api) self._assert_no_merge_call(mock_api)
@patch("mcp_server.api_request") @patch("mcp_server.api_request")
@@ -1753,20 +1683,7 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
} }
def setUp(self): def setUp(self):
import reviewer_pr_lease
init_review_decision_lock("prgs", "review_pr") 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): def _env(self):
return patch.dict(os.environ, { return patch.dict(os.environ, {
@@ -1861,19 +1778,8 @@ class TestSubmitPrReview(unittest.TestCase):
"""Gated review-mutation tool (#15).""" """Gated review-mutation tool (#15)."""
def setUp(self): def setUp(self):
import reviewer_pr_lease
init_review_decision_lock("prgs", "review_pr") init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(8, "approve", remote="prgs") 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): def _pr(self, author, state="open", sha="abc123", mergeable=True):
return { return {
@@ -2111,11 +2017,9 @@ class TestSubmitPrReview(unittest.TestCase):
final_review_decision_ready=True, final_review_decision_ready=True,
) )
self.assertFalse(r["performed"]) self.assertFalse(r["performed"])
self.assertTrue(any( self.assertIn(
"expected head SHA does not match current PR head (fail closed)" in reason "expected head SHA does not match current PR head (fail closed)",
or "PR head changed during lease" in reason r["reasons"])
for reason in r["reasons"]
))
self._assert_no_mutation(mock_api) self._assert_no_mutation(mock_api)
def test_head_sha_match_allows(self): def test_head_sha_match_allows(self):
@@ -2178,9 +2082,9 @@ class TestSubmitPrReview(unittest.TestCase):
env = {"GITEA_PROFILE_NAME": "gitea-reviewer", env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"} "GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
gitea_mark_final_review_decision(8, "approve", remote="prgs") gitea_mark_final_review_decision(5, "approve", remote="prgs")
r = gitea_submit_pr_review( r = gitea_submit_pr_review(
pr_number=8, action="approve", remote="prgs", pr_number=5, action="approve", remote="prgs",
final_review_decision_ready=True, final_review_decision_ready=True,
) )
self.assertFalse(r["performed"]) self.assertFalse(r["performed"])
@@ -2399,13 +2303,6 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
self.assertEqual(res["cleanup_status"].get(1), "not present") self.assertEqual(res["cleanup_status"].get(1), "not present")
def test_merge_pr_with_closes_removes_label(self): 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): def api_side_effect(method, url, auth, payload=None):
if method == "GET" and "/user" in url: if method == "GET" and "/user" in url:
return {"login": "merger"} return {"login": "merger"}
@@ -2440,13 +2337,6 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
self.assertEqual(res["cleanup_status"].get(123), "released") self.assertEqual(res["cleanup_status"].get(123), "released")
def test_merge_pr_with_branch_name_removes_label(self): 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): def api_side_effect(method, url, auth, payload=None):
if method == "GET" and "/user" in url: if method == "GET" and "/user" in url:
return {"login": "merger"} return {"login": "merger"}
@@ -3128,12 +3018,10 @@ class TestVerifyMutationAuthority(unittest.TestCase):
# profile; the active profile resolves as reviewer — side-channel # profile; the active profile resolves as reviewer — side-channel
# override rejected even with a matching in-process authority. # override rejected even with a matching in-process authority.
self._authority() self._authority()
with patch("mcp_server.gitea_config.is_runtime_switching_enabled", with patch.dict(os.environ,
return_value=False): {"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}):
with patch.dict(os.environ, with self.assertRaises(RuntimeError) as ctx:
{"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}): mcp_server.verify_mutation_authority("prgs")
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_mutation_authority("prgs")
self.assertIn("side-channel override rejected", str(ctx.exception)) self.assertIn("side-channel override rejected", str(ctx.exception))
def test_foreign_pid_authority_is_not_trusted(self): def test_foreign_pid_authority_is_not_trusted(self):
@@ -3184,27 +3072,32 @@ class TestIssueLocking(unittest.TestCase):
"""Test issue locking and PR gating constraints.""" """Test issue locking and PR gating constraints."""
def setUp(self): def setUp(self):
self._env_patcher = patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True) self._lock_dir = tempfile.TemporaryDirectory()
env = {
**ISSUE_WRITE_ENV,
"GITEA_ISSUE_LOCK_DIR": self._lock_dir.name,
}
self._env_patcher = patch.dict(os.environ, env, clear=True)
self._env_patcher.start() self._env_patcher.start()
self._dup_fetcher_patcher = patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
self.mock_dup_fetcher = self._dup_fetcher_patcher.start()
def tearDown(self): def tearDown(self):
self._dup_fetcher_patcher.stop()
self._env_patcher.stop() self._env_patcher.stop()
if os.path.exists(ISSUE_LOCK_FILE): self._lock_dir.cleanup()
os.remove(ISSUE_LOCK_FILE)
def _create_pr_env(self) -> dict:
return {
**CREATE_PR_ENV,
"GITEA_ISSUE_LOCK_DIR": self._lock_dir.name,
}
@patch( @patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state", "mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_master_git_state_for_lock(), return_value=_clean_master_git_state_for_lock(),
) )
@patch("mcp_server.api_get_all", return_value=[]) @patch("mcp_server.api_get_all")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_success(self, _auth, _api, _git_state): def test_lock_issue_success(self, _auth, mock_api, _git_state):
mock_api.return_value = [] # no open PRs
res = gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") res = gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
self.assertTrue(res["success"]) self.assertTrue(res["success"])
self.assertEqual(res["work_lease"]["operation_type"], "author_issue_work") self.assertEqual(res["work_lease"]["operation_type"], "author_issue_work")
@@ -3214,9 +3107,8 @@ class TestIssueLocking(unittest.TestCase):
self.assertIn("expires_at", res["work_lease"]) self.assertIn("expires_at", res["work_lease"])
self.assertIn("last_heartbeat_at", res["work_lease"]) self.assertIn("last_heartbeat_at", res["work_lease"])
self.assertEqual(res["work_lease"]["claimant"]["profile"], "gitea-default") self.assertEqual(res["work_lease"]["claimant"]["profile"], "gitea-default")
self.assertTrue(os.path.exists(ISSUE_LOCK_FILE)) self.assertIn("lock_file_path", res)
with open(ISSUE_LOCK_FILE, encoding="utf-8") as f: lock = issue_lock_store.read_lock_file(res["lock_file_path"])
lock = json.load(f)
self.assertIn("worktree_path", lock) self.assertIn("worktree_path", lock)
self.assertIn("work_lease", lock) self.assertIn("work_lease", lock)
@@ -3229,48 +3121,35 @@ class TestIssueLocking(unittest.TestCase):
"mcp_server.issue_lock_worktree.read_worktree_git_state", "mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_master_git_state_for_lock(), return_value=_clean_master_git_state_for_lock(),
) )
@patch("mcp_server.api_get_all")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_reused_by_open_pr_branch(self, _auth, _git_state): def test_lock_issue_reused_by_open_pr_branch(self, _auth, mock_api, _git_state):
self.mock_dup_fetcher.return_value = ([{ mock_api.return_value = [{
"number": 200, "number": 200,
"head": {"ref": "feat/issue-196-boundary"}, "head": {"ref": "feat/issue-196-boundary"},
"title": "Some PR", "title": "Some PR",
"body": "No closes ref", "body": "No closes ref"
}], [], {"status": "not_claimed"}) }]
with self.assertRaises(ValueError) as ctx: with self.assertRaises(ValueError) as ctx:
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
self.assertIn("open PR #200 already covers issue", str(ctx.exception)) self.assertIn("already tied to an open PR", str(ctx.exception))
@patch( @patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state", "mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_master_git_state_for_lock(), return_value=_clean_master_git_state_for_lock(),
) )
@patch("mcp_server.api_get_all")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, _git_state): def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, mock_api, _git_state):
self.mock_dup_fetcher.return_value = ([{ mock_api.return_value = [{
"number": 200, "number": 200,
"head": {"ref": "feat/other-branch"}, "head": {"ref": "feat/other-branch"},
"title": "Some PR", "title": "Some PR",
"body": "fixes #196", "body": "fixes #196"
}], [], {"status": "not_claimed"}) }]
with self.assertRaises(ValueError) as ctx: with self.assertRaises(ValueError) as ctx:
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
self.assertIn("open PR #200 already covers issue", str(ctx.exception)) self.assertIn("already tied to an open PR", str(ctx.exception))
@patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_master_git_state_for_lock(),
)
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_reused_by_remote_branch(self, _auth, _git_state):
self.mock_dup_fetcher.return_value = (
[],
["feat/issue-196-existing-work"],
{"status": "not_claimed"},
)
with self.assertRaises(ValueError) as ctx:
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
self.assertIn("remote branch(es) already match issue pattern", str(ctx.exception))
@patch( @patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state", "mcp_server.issue_lock_worktree.read_worktree_git_state",
@@ -3278,90 +3157,31 @@ class TestIssueLocking(unittest.TestCase):
) )
@patch("mcp_server.api_get_all") @patch("mcp_server.api_get_all")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_adopts_own_existing_branch(self, _auth, mock_api, _git_state): def test_lock_issue_reused_by_remote_branch(self, _auth, mock_api, _git_state):
# #442: the issue's own already-pushed branch is adoptable for recovery. mock_api.side_effect = [
sha = "934688a71dd19d9b46369e73fb3ac356ee0cc211" [],
[{"name": "feat/issue-196-existing-work"}],
]
with self.assertRaises(ValueError) as ctx:
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
self.assertIn("not the requested branch", str(ctx.exception))
def _api(url, *a, **k): @patch(
if "/pulls" in url: "mcp_server.issue_lock_worktree.read_worktree_git_state",
return [] return_value=_clean_master_git_state_for_lock(),
if "/branches" in url: )
return [{"name": "feat/issue-196-mutations", "commit": {"id": sha}}] @patch("mcp_server.api_get_all")
return [] @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_adopts_exact_own_branch(self, _auth, mock_api, _git_state):
mock_api.side_effect = _api branch = "feat/issue-196-mutations"
res = gitea_lock_issue( mock_api.side_effect = [
issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs" [],
) [{"name": branch, "commit": {"id": "abc123"}}],
]
res = gitea_lock_issue(issue_number=196, branch_name=branch, remote="prgs")
self.assertTrue(res["success"]) self.assertTrue(res["success"])
self.assertIn("adoption", res) self.assertIn("adoption", res)
proof = res["adoption"] self.assertEqual(res["adoption"]["branch_head_commit"], "abc123")
self.assertEqual(proof["issue_number"], 196)
self.assertEqual(proof["branch_name"], "feat/issue-196-mutations")
self.assertEqual(proof["branch_head_commit"], sha)
self.assertTrue(proof["no_existing_pr_proof"])
self.assertTrue(proof["no_competing_live_lock_proof"])
self.assertEqual(proof["lock_file_path"], ISSUE_LOCK_FILE)
self.assertIn("adopt", proof["adoption_reason"].lower())
self.assertTrue(os.path.exists(ISSUE_LOCK_FILE))
@patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_master_git_state_for_lock(),
)
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_open_pr_blocks_adoption(self, _auth, _git_state):
# Even with the exact own branch present, an open PR blocks adoption.
self.mock_dup_fetcher.return_value = ([{
"number": 200,
"head": {"ref": "feat/issue-196-mutations"},
"title": "WIP",
"body": "",
}], ["feat/issue-196-mutations"], {"status": "not_claimed"})
with self.assertRaises(ValueError) as ctx:
gitea_lock_issue(
issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs"
)
self.assertIn("open PR #200 already covers issue", str(ctx.exception))
self.assertFalse(os.path.exists(ISSUE_LOCK_FILE))
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []))
@patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_master_git_state_for_lock(),
)
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_create_pr_proceeds_after_adopted_lock(
self, _auth, mock_api, mock_req, _git_state, _role
):
# #442 recovery goal: after adopting the own branch, create_pr proceeds.
sha = "934688a71dd19d9b46369e73fb3ac356ee0cc211"
def _api(url, *a, **k):
if "/pulls" in url:
return []
if "/branches" in url:
return [{"name": "feat/issue-196-mutations", "commit": {"id": sha}}]
return []
mock_api.side_effect = _api
lock_res = gitea_lock_issue(
issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs"
)
self.assertIn("adoption", lock_res)
mock_req.return_value = {"number": 7, "html_url": "https://gitea.prgs.cc/pulls/7"}
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
pr = gitea_create_pr(
title="feat: parity gate Closes #196",
head="feat/issue-196-mutations",
base="master",
remote="prgs",
)
self.assertEqual(pr["number"], 7)
@patch( @patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state", "mcp_server.issue_lock_worktree.read_worktree_git_state",
@@ -3370,16 +3190,27 @@ class TestIssueLocking(unittest.TestCase):
@patch("mcp_server.api_get_all", return_value=[]) @patch("mcp_server.api_get_all", return_value=[])
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_blocks_active_same_operation_lease(self, _auth, _api, _git_state): def test_lock_issue_blocks_active_same_operation_lease(self, _auth, _api, _git_state):
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: prgs_repo = mcp_server.REMOTES["prgs"]["repo"]
json.dump({ issue_lock_store.save_lock_file(
issue_lock_store.lock_file_path(
remote="prgs",
org="Scaled-Tech-Consulting",
repo=prgs_repo,
issue_number=196,
),
{
"issue_number": 196, "issue_number": 196,
"branch_name": "feat/issue-196-other-work", "branch_name": "feat/issue-196-other-work",
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": prgs_repo,
"worktree_path": "/tmp/other-worktree", "worktree_path": "/tmp/other-worktree",
"work_lease": { "work_lease": {
"operation_type": "author_issue_work", "operation_type": "author_issue_work",
"expires_at": "2999-01-01T00:00:00Z", "expires_at": "2999-01-01T00:00:00Z",
}, },
}, f) },
)
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(RuntimeError) as ctx:
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
self.assertIn("already has an active author_issue_work lease", str(ctx.exception)) self.assertIn("already has an active author_issue_work lease", str(ctx.exception))
@@ -3391,16 +3222,27 @@ class TestIssueLocking(unittest.TestCase):
@patch("mcp_server.api_get_all", return_value=[]) @patch("mcp_server.api_get_all", return_value=[])
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_blocks_expired_same_operation_lease_for_recovery(self, _auth, _api, _git_state): def test_lock_issue_blocks_expired_same_operation_lease_for_recovery(self, _auth, _api, _git_state):
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: prgs_repo = mcp_server.REMOTES["prgs"]["repo"]
json.dump({ issue_lock_store.save_lock_file(
issue_lock_store.lock_file_path(
remote="prgs",
org="Scaled-Tech-Consulting",
repo=prgs_repo,
issue_number=196,
),
{
"issue_number": 196, "issue_number": 196,
"branch_name": "feat/issue-196-other-work", "branch_name": "feat/issue-196-other-work",
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": prgs_repo,
"worktree_path": "/tmp/other-worktree", "worktree_path": "/tmp/other-worktree",
"work_lease": { "work_lease": {
"operation_type": "author_issue_work", "operation_type": "author_issue_work",
"expires_at": "2000-01-01T00:00:00Z", "expires_at": "2000-01-01T00:00:00Z",
}, },
}, f) },
)
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(RuntimeError) as ctx:
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
self.assertIn("Recovery review is required before takeover", str(ctx.exception)) self.assertIn("Recovery review is required before takeover", str(ctx.exception))
@@ -3467,9 +3309,7 @@ class TestIssueLocking(unittest.TestCase):
return_value=(True, [])) return_value=(True, []))
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_create_pr_missing_lock_fails(self, _auth, _role): def test_create_pr_missing_lock_fails(self, _auth, _role):
if os.path.exists(ISSUE_LOCK_FILE): with patch.dict(os.environ, self._create_pr_env(), clear=True):
os.remove(ISSUE_LOCK_FILE)
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(RuntimeError) as ctx:
gitea_create_pr(title="feat: X Closes #196", head="feat/issue-196-mutations", remote="prgs") gitea_create_pr(title="feat: X Closes #196", head="feat/issue-196-mutations", remote="prgs")
self.assertIn("Issue lock is missing", str(ctx.exception)) self.assertIn("Issue lock is missing", str(ctx.exception))
@@ -3478,37 +3318,64 @@ class TestIssueLocking(unittest.TestCase):
return_value=(True, [])) return_value=(True, []))
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_create_pr_branch_mismatch_fails(self, _auth, _role): def test_create_pr_branch_mismatch_fails(self, _auth, _role):
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: worktree = os.path.realpath(os.getcwd())
json.dump(_sample_issue_lock( _bind_test_lock(
issue_number=196, branch_name="feat/issue-196-mutations"), f) issue_number=196,
with patch.dict(os.environ, CREATE_PR_ENV, clear=True): branch_name="feat/issue-196-mutations",
remote="prgs",
worktree_path=worktree,
)
with patch.dict(os.environ, self._create_pr_env(), clear=True):
with self.assertRaises(ValueError) as ctx: with self.assertRaises(ValueError) as ctx:
gitea_create_pr(title="feat: X Closes #196", head="feat/issue-196-different", remote="prgs") gitea_create_pr(
title="feat: X Closes #196",
head="feat/issue-196-different",
remote="prgs",
worktree_path=worktree,
)
self.assertIn("does not match locked branch", str(ctx.exception)) self.assertIn("does not match locked branch", str(ctx.exception))
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, [])) return_value=(True, []))
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_create_pr_forbidden_terms_fails(self, _auth, _role): def test_create_pr_forbidden_terms_fails(self, _auth, _role):
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: worktree = os.path.realpath(os.getcwd())
json.dump(_sample_issue_lock( _bind_test_lock(
issue_number=196, branch_name="feat/issue-196-mutations"), f) issue_number=196,
with patch.dict(os.environ, CREATE_PR_ENV, clear=True): branch_name="feat/issue-196-mutations",
remote="prgs",
worktree_path=worktree,
)
with patch.dict(os.environ, self._create_pr_env(), clear=True):
for term in ("equivalent to #196", "related to #196", "same as #196"): for term in ("equivalent to #196", "related to #196", "same as #196"):
with self.assertRaises(ValueError) as ctx: with self.assertRaises(ValueError) as ctx:
gitea_create_pr(title=f"feat: X {term}", head="feat/issue-196-mutations", remote="prgs") gitea_create_pr(
title=f"feat: X {term}",
head="feat/issue-196-mutations",
remote="prgs",
worktree_path=worktree,
)
self.assertIn("contains forbidden term", str(ctx.exception)) self.assertIn("contains forbidden term", str(ctx.exception))
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, [])) return_value=(True, []))
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_create_pr_missing_closes_ref_fails(self, _auth, _role): def test_create_pr_missing_closes_ref_fails(self, _auth, _role):
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: worktree = os.path.realpath(os.getcwd())
json.dump(_sample_issue_lock( _bind_test_lock(
issue_number=196, branch_name="feat/issue-196-mutations"), f) issue_number=196,
with patch.dict(os.environ, CREATE_PR_ENV, clear=True): branch_name="feat/issue-196-mutations",
remote="prgs",
worktree_path=worktree,
)
with patch.dict(os.environ, self._create_pr_env(), clear=True):
with self.assertRaises(ValueError) as ctx: with self.assertRaises(ValueError) as ctx:
gitea_create_pr(title="feat: X refs #196", head="feat/issue-196-mutations", remote="prgs") gitea_create_pr(
title="feat: X refs #196",
head="feat/issue-196-mutations",
remote="prgs",
worktree_path=worktree,
)
self.assertIn("must contain 'Closes #196' or 'Fixes #196' exactly", str(ctx.exception)) self.assertIn("must contain 'Closes #196' or 'Fixes #196' exactly", str(ctx.exception))
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
@@ -3516,13 +3383,13 @@ class TestIssueLocking(unittest.TestCase):
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_create_pr_worktree_mismatch_fails(self, _auth, _role): def test_create_pr_worktree_mismatch_fails(self, _auth, _role):
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-pr") scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-pr")
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: _bind_test_lock(
json.dump(_sample_issue_lock( issue_number=249,
issue_number=249, branch_name="feat/issue-249-issue-lock-scratch-worktree",
branch_name="feat/issue-249-issue-lock-scratch-worktree", worktree_path=scratch,
worktree_path=scratch, remote="prgs",
), f) )
with patch.dict(os.environ, CREATE_PR_ENV, clear=True): with patch.dict(os.environ, self._create_pr_env(), clear=True):
with self.assertRaises(ValueError) as ctx: with self.assertRaises(ValueError) as ctx:
gitea_create_pr( gitea_create_pr(
title="feat: lock scratch worktree Closes #249", title="feat: lock scratch worktree Closes #249",
@@ -3532,28 +3399,6 @@ class TestIssueLocking(unittest.TestCase):
) )
self.assertIn("does not match locked worktree", str(ctx.exception)) self.assertIn("does not match locked worktree", str(ctx.exception))
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []))
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_create_pr_manual_lock_seed_blocked(self, _auth, _role):
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
json.dump(
_sample_issue_lock(
issue_number=447,
branch_name="feat/issue-447-lock-provenance",
lock_provenance=None,
),
f,
)
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
with self.assertRaises(RuntimeError) as ctx:
gitea_create_pr(
title="feat: lock provenance Closes #447",
head="feat/issue-447-lock-provenance",
remote="prgs",
)
self.assertIn("lock provenance", str(ctx.exception).lower())
@patch("mcp_server.api_request") @patch("mcp_server.api_request")
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, [])) return_value=(True, []))
@@ -3561,13 +3406,13 @@ class TestIssueLocking(unittest.TestCase):
def test_create_pr_honors_scratch_worktree_lock(self, _auth, _role, mock_api): def test_create_pr_honors_scratch_worktree_lock(self, _auth, _role, mock_api):
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-e2e") scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-e2e")
mock_api.return_value = {"number": 250, "html_url": "https://example/pr/250"} mock_api.return_value = {"number": 250, "html_url": "https://example/pr/250"}
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: _bind_test_lock(
json.dump(_sample_issue_lock( issue_number=249,
issue_number=249, branch_name="feat/issue-249-issue-lock-scratch-worktree",
branch_name="feat/issue-249-issue-lock-scratch-worktree", worktree_path=scratch,
worktree_path=scratch, remote="prgs",
), f) )
with patch.dict(os.environ, CREATE_PR_ENV, clear=True): with patch.dict(os.environ, self._create_pr_env(), clear=True):
res = gitea_create_pr( res = gitea_create_pr(
title="feat: issue-lock scratch worktree Closes #249", title="feat: issue-lock scratch worktree Closes #249",
head="feat/issue-249-issue-lock-scratch-worktree", head="feat/issue-249-issue-lock-scratch-worktree",
+3 -16
View File
@@ -156,22 +156,9 @@ class TestPRQueueInventory(unittest.TestCase):
] ]
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
from tests.test_mcp_server import _install_owned_reviewer_lease init_review_decision_lock("prgs", "review_pr")
import reviewer_pr_lease 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)
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.assertTrue(result["success"])
self.assertIn("=== PR Queue Inventory ===", result["message"]) self.assertIn("=== PR Queue Inventory ===", result["message"])
self.assertIn("Repository:", result["message"]) self.assertIn("Repository:", result["message"])
-1
View File
@@ -2346,7 +2346,6 @@ class TestWorkIssueFinalReport(unittest.TestCase):
"- Safe next action: open PR", "- Safe next action: open PR",
"- Next: open PR", "- Next: open PR",
"- Safety statement: no review/merge", "- Safety statement: no review/merge",
"- Duplicate work outcome: duplicate work not prevented",
]) ])
def test_complete_work_issue_report_earns_a(self): def test_complete_work_issue_report_earns_a(self):
-193
View File
@@ -1,193 +0,0 @@
"""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()
+26 -9
View File
@@ -20,33 +20,50 @@ def run(script, *args):
branch = arg branch = arg
break break
lock_file = Path("/tmp/gitea_issue_lock.json") lock_dir_ctx = None
created_lock = False extra_env = os.environ.copy()
if script == "worktree-start" and branch: if script == "worktree-start" and branch:
import re import re
import json import tempfile
import issue_lock_store
m = re.search(r"issue-(\d+)", branch) m = re.search(r"issue-(\d+)", branch)
if not m: if not m:
m = re.search(r"pr-(\d+)", branch) m = re.search(r"pr-(\d+)", branch)
issue_num = int(m.group(1)) if m else 999 issue_num = int(m.group(1)) if m else 999
lock_file.write_text(json.dumps({ lock_dir_ctx = tempfile.TemporaryDirectory()
extra_env["GITEA_ISSUE_LOCK_DIR"] = lock_dir_ctx.name
record = {
"issue_number": issue_num, "issue_number": issue_num,
"branch_name": branch, "branch_name": branch,
"remote": "prgs", "remote": "prgs",
"org": "Scaled-Tech-Consulting", "org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools" "repo": "Gitea-Tools",
}), encoding="utf-8") "worktree_path": "/tmp/test-worktree",
created_lock = True "work_lease": {
"operation_type": "author_issue_work",
"expires_at": "2999-01-01T00:00:00Z",
},
}
path = issue_lock_store.lock_file_path(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
issue_number=issue_num,
lock_dir=lock_dir_ctx.name,
)
issue_lock_store.save_lock_file(path, record)
try: try:
proc = subprocess.run( proc = subprocess.run(
["bash", str(SCRIPTS / script), *args], ["bash", str(SCRIPTS / script), *args],
capture_output=True, text=True, cwd=str(REPO), capture_output=True, text=True, cwd=str(REPO),
env=extra_env,
) )
return proc.returncode, proc.stdout, proc.stderr return proc.returncode, proc.stdout, proc.stderr
finally: finally:
if created_lock and lock_file.exists(): if lock_dir_ctx is not None:
lock_file.unlink() lock_dir_ctx.cleanup()
class TestWorktreeStart(unittest.TestCase): class TestWorktreeStart(unittest.TestCase):