fix: resolve conflicts for PR #422

This commit is contained in:
2026-07-08 02:20:04 -04:00
34 changed files with 3871 additions and 352 deletions
+127
View File
@@ -7,8 +7,11 @@ project's ``branches/`` directory, never from the stable control checkout.
from __future__ import annotations from __future__ import annotations
import os import os
import subprocess
BASE_BRANCHES = frozenset({"master", "main", "dev"}) BASE_BRANCHES = frozenset({"master", "main", "dev"})
ACTIVE_WORKTREE_ENV = "GITEA_ACTIVE_WORKTREE"
AUTHOR_WORKTREE_ENV = "GITEA_AUTHOR_WORKTREE"
def _normalize_path(path: str) -> str: def _normalize_path(path: str) -> str:
@@ -48,6 +51,130 @@ def resolve_mutation_workspace(
return os.path.realpath(project_root) return os.path.realpath(project_root)
def _realpath_git_common_dir(workspace_path: str, common_dir: str) -> str:
"""Resolve ``git rev-parse --git-common-dir`` relative to *workspace_path*."""
raw = (common_dir or "").strip()
if not raw:
return raw
if os.path.isabs(raw):
return os.path.realpath(raw)
return os.path.realpath(os.path.join(workspace_path, raw))
def resolve_canonical_repo_root(workspace_path: str, fallback_project_root: str) -> str:
"""Return the stable repository root for *workspace_path* via git metadata (#460)."""
path = (workspace_path or "").strip()
fallback = os.path.realpath(fallback_project_root)
if not path:
return fallback
try:
res = subprocess.run(
["git", "-C", path, "rev-parse", "--git-common-dir"],
capture_output=True,
text=True,
check=True,
)
common = _realpath_git_common_dir(path, res.stdout)
except Exception:
return fallback
if common.endswith(f"{os.sep}.git"):
return os.path.dirname(common)
if os.path.basename(common) == ".git":
return os.path.dirname(common)
return fallback
def resolve_author_mutation_context(
worktree_path: str | None,
process_project_root: str,
*,
active_worktree_env: str | None = None,
author_worktree_env: str | None = None,
) -> dict:
"""Shared workspace resolution for runtime_context and mutation guards (#460)."""
workspace = resolve_mutation_workspace(
worktree_path,
process_project_root,
active_worktree_env=active_worktree_env,
author_worktree_env=author_worktree_env,
)
process_root = os.path.realpath(process_project_root)
# Canonical repository identity comes from the MCP process checkout (#460),
# not from the declared task workspace being validated.
canonical_root = resolve_canonical_repo_root(process_root, process_root)
return {
"workspace_path": workspace,
"process_project_root": process_root,
"canonical_repo_root": canonical_root,
"roots_aligned": canonical_root == process_root,
}
def assess_workspace_repo_membership(
*,
workspace_path: str,
canonical_repo_root: str,
) -> dict:
"""Fail closed when *workspace_path* is not a git worktree of *canonical_repo_root*."""
workspace = os.path.realpath(workspace_path)
root = os.path.realpath(canonical_repo_root)
reasons: list[str] = []
if not os.path.exists(workspace):
reasons.append(f"worktree path '{workspace}' does not exist")
return _membership_assessment(False, reasons, workspace, root, None)
if not os.path.isdir(workspace):
reasons.append(f"worktree path '{workspace}' is not a directory")
return _membership_assessment(False, reasons, workspace, root, None)
try:
res = subprocess.run(
["git", "-C", workspace, "rev-parse", "--git-common-dir"],
capture_output=True,
text=True,
check=True,
)
common_dir = _realpath_git_common_dir(workspace, res.stdout)
except Exception:
reasons.append(f"worktree '{workspace}' is not a valid git repository")
return _membership_assessment(False, reasons, workspace, root, None)
expected_dir = os.path.realpath(os.path.join(root, ".git"))
if common_dir != expected_dir:
reasons.append(
f"worktree '{workspace}' does not belong to the target repository '{root}'"
)
return _membership_assessment(not reasons, reasons, workspace, root, common_dir)
def _membership_assessment(
proven: bool,
reasons: list[str],
workspace: str,
root: str,
common_dir: str | None,
) -> dict:
return {
"proven": proven,
"block": not proven,
"reasons": reasons,
"workspace_path": workspace,
"canonical_repo_root": root,
"git_common_dir": common_dir,
}
def format_workspace_repo_membership_error(assessment: dict) -> str:
workspace = assessment.get("workspace_path") or "(unknown)"
root = assessment.get("canonical_repo_root") or "(unknown)"
reasons = "; ".join(assessment.get("reasons") or ["unknown repository membership violation"])
return (
f"Branches-only mutation guard (#274): {reasons} (fail closed). "
f"canonical repository root: {root}; workspace: {workspace}."
)
def assess_author_mutation_worktree( def assess_author_mutation_worktree(
*, *,
workspace_path: str, workspace_path: str,
+18 -6
View File
@@ -274,12 +274,24 @@ 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.
Each lock payload includes issue number, optional PR number, branch, worktree
path, claimant identity/profile, created timestamp, expiry timestamp, and last
heartbeat timestamp. An active same-issue/same-operation lease blocks duplicate
work. An expired lease still blocks takeover until a recovery review records why
the prior work is abandoned, completed, or unsafe to continue.
**Do not manually seed `/tmp/gitea_issue_lock.json` or any lock file as a normal
recovery path.** That global slot is deprecated and can clobber unrelated live
leases (#438). After an MCP restart, call `gitea_lock_issue` again — own-branch
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
matching `head` branch without unsafe manual seeding.
**Issue-lock recovery (#447):** Do not manually seed, restore, or delete **Issue-lock recovery (#447):** Do not manually seed, restore, or delete
`/tmp/gitea_issue_lock.json` as a normal recovery path. That file is global `/tmp/gitea_issue_lock.json` as a normal recovery path. That file is global
+71
View File
@@ -21,6 +21,7 @@ from review_proofs import (
assess_review_mutation_final_report, assess_review_mutation_final_report,
assess_validation_report, assess_validation_report,
) )
from validation_status_vocabulary import assess_validation_status_vocabulary
FINAL_REPORT_TASK_KINDS = frozenset({ FINAL_REPORT_TASK_KINDS = frozenset({
"review_pr", "review_pr",
@@ -507,6 +508,45 @@ def _rule_reviewer_validation_failure_history(
] ]
def _rule_reviewer_stale_head_proof(report_text: str) -> list[dict[str, str]]:
from pr_work_lease import assess_reviewer_stale_head_final_report
result = assess_reviewer_stale_head_final_report(report_text)
if result.get("proven"):
return []
return _findings_from_reasons(
"reviewer.stale_head_proof",
result.get("reasons") or [],
field="Stale-head proof",
severity="block",
safe_next_action=(
"state reviewed head SHA, live head before approval/merge, and "
"whether any push occurred during validation"
),
)
def _rule_conflict_fix_push_proof(report_text: str) -> list[dict[str, str]]:
from pr_work_lease import assess_conflict_fix_final_report
text = report_text or ""
if "conflict-fix" not in text.lower() and "conflict fix" not in text.lower():
return []
result = assess_conflict_fix_final_report(text)
if result.get("proven"):
return []
return _findings_from_reasons(
"author.conflict_fix_push_proof",
result.get("reasons") or [],
field="Conflict-fix push proof",
severity="block",
safe_next_action=(
"state branch head before/after push, reviewer lease status, "
"fast-forward status, and whether any reviewer was active"
),
)
def _rule_reviewer_validation_command(report_text: str) -> list[dict[str, str]]: def _rule_reviewer_validation_command(report_text: str) -> list[dict[str, str]]:
text = report_text or "" text = report_text or ""
if not _BARE_PYTEST_RE.search(text): if not _BARE_PYTEST_RE.search(text):
@@ -615,6 +655,34 @@ def _rule_reviewer_main_checkout_baseline(report_text: str) -> list[dict[str, st
] ]
def _rule_reviewer_validation_status_vocabulary(
report_text: str,
*,
action_log: list[dict] | None = None,
) -> list[dict[str, str]]:
text = report_text or ""
if not re.search(
r"validation status|pr-head validation status|official validation status",
text,
re.IGNORECASE,
):
return []
result = assess_validation_status_vocabulary(
text,
command_log=action_log,
)
if not result.get("block"):
return []
return _findings_from_reasons(
"reviewer.validation_status_vocabulary",
result.get("reasons") or [],
field="Validation status",
severity="block",
safe_next_action=result.get("safe_next_action")
or "use a validation status that matches the proof path executed",
)
def _rule_reviewer_main_checkout_path(report_text: str) -> list[dict[str, str]]: def _rule_reviewer_main_checkout_path(report_text: str) -> list[dict[str, str]]:
text = report_text or "" text = report_text or ""
if "baseline worktree path" not in text.lower(): if "baseline worktree path" not in text.lower():
@@ -1022,6 +1090,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
_rule_reviewer_validation_structured, _rule_reviewer_validation_structured,
_rule_reviewer_linked_issue, _rule_reviewer_linked_issue,
_rule_reviewer_baseline_on_failure, _rule_reviewer_baseline_on_failure,
_rule_reviewer_validation_status_vocabulary,
_rule_reviewer_main_checkout_baseline, _rule_reviewer_main_checkout_baseline,
_rule_reviewer_main_checkout_path, _rule_reviewer_main_checkout_path,
_rule_reviewer_already_landed_eligible, _rule_reviewer_already_landed_eligible,
@@ -1030,6 +1099,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
_rule_reviewer_workflow_load_boundary, _rule_reviewer_workflow_load_boundary,
_rule_reviewer_mutation_ledger, _rule_reviewer_mutation_ledger,
_rule_reviewer_review_mutation, _rule_reviewer_review_mutation,
_rule_reviewer_stale_head_proof,
], ],
"reconcile_already_landed": [ "reconcile_already_landed": [
_rule_reconcile_controller_handoff, _rule_reconcile_controller_handoff,
@@ -1054,6 +1124,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
_rule_shared_email_disclosure, _rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES, *_SHARED_ISSUE_LOCK_RULES,
_rule_reviewer_vague_mutations_none, _rule_reviewer_vague_mutations_none,
_rule_conflict_fix_push_proof,
], ],
"issue_filing": [ "issue_filing": [
_rule_shared_controller_handoff, _rule_shared_controller_handoff,
+438 -87
View File
@@ -190,14 +190,22 @@ def _ensure_process_start_porcelain() -> str:
def _resolve_preflight_workspace_path(worktree_path: str | None = None) -> str: def _resolve_preflight_workspace_path(worktree_path: str | None = None) -> str:
"""Resolve the workspace root inspected by pre-flight guards.""" """Resolve the workspace root inspected by pre-flight guards."""
path = (worktree_path or "").strip() return author_mutation_worktree.resolve_mutation_workspace(
if not path: worktree_path,
path = (os.environ.get(ACTIVE_WORKTREE_ENV) or "").strip() PROJECT_ROOT,
if not path: active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV),
path = (os.environ.get(AUTHOR_WORKTREE_ENV) or "").strip() author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV),
if not path: )
path = PROJECT_ROOT
return os.path.realpath(os.path.abspath(path))
def _resolve_author_mutation_context(worktree_path: str | None = None) -> dict:
"""Canonical workspace + repository root for runtime_context and guards (#460)."""
return author_mutation_worktree.resolve_author_mutation_context(
worktree_path,
PROJECT_ROOT,
active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV),
author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV),
)
def _get_git_root(path: str) -> str | None: def _get_git_root(path: str) -> str | None:
@@ -267,21 +275,31 @@ def _format_preflight_files(files: list[str]) -> str:
def _preflight_workspace_details(worktree_path: str | None, dirty_files: list[str]) -> dict: def _preflight_workspace_details(worktree_path: str | None, dirty_files: list[str]) -> dict:
workspace = _resolve_preflight_workspace_path(worktree_path) ctx = _resolve_author_mutation_context(worktree_path)
workspace = ctx["workspace_path"]
inspected_root = _get_git_root(workspace) inspected_root = _get_git_root(workspace)
control_root = os.path.realpath(PROJECT_ROOT) process_root = ctx["process_project_root"]
canonical_root = ctx["canonical_repo_root"]
active_root = os.path.realpath(inspected_root or workspace) active_root = os.path.realpath(inspected_root or workspace)
if active_root == control_root: if active_root == canonical_root:
dirty_scope = "control checkout" dirty_scope = "control checkout"
else: else:
dirty_scope = "active task workspace" dirty_scope = "active task workspace"
return { details = {
"mcp_server_process_root": control_root, "mcp_server_process_root": process_root,
"canonical_repository_root": canonical_root,
"active_task_workspace_root": active_root, "active_task_workspace_root": active_root,
"inspected_git_root": inspected_root, "inspected_git_root": inspected_root,
"dirty_files": list(dirty_files), "dirty_files": list(dirty_files),
"dirty_scope": dirty_scope, "dirty_scope": dirty_scope,
"workspace_roots_aligned": ctx["roots_aligned"],
} }
if not ctx["roots_aligned"]:
details["workspace_root_mismatch"] = (
"runtime_context and mutation guard use canonical repository root "
f"'{canonical_root}' instead of MCP process root '{process_root}'"
)
return details
def _format_preflight_workspace_details(details: dict) -> str: def _format_preflight_workspace_details(details: dict) -> str:
@@ -402,16 +420,12 @@ def _enforce_branches_only_author_mutation(worktree_path: str | None = None) ->
"""#274: author mutations must run from a branches/ session worktree.""" """#274: author mutations must run from a branches/ session worktree."""
if _preflight_resolved_role == "reviewer": if _preflight_resolved_role == "reviewer":
return return
workspace = author_mutation_worktree.resolve_mutation_workspace( ctx = _resolve_author_mutation_context(worktree_path)
worktree_path, workspace = ctx["workspace_path"]
PROJECT_ROOT,
active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV),
author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV),
)
git_state = issue_lock_worktree.read_worktree_git_state(workspace) git_state = issue_lock_worktree.read_worktree_git_state(workspace)
assessment = author_mutation_worktree.assess_author_mutation_worktree( assessment = author_mutation_worktree.assess_author_mutation_worktree(
workspace_path=workspace, workspace_path=workspace,
project_root=PROJECT_ROOT, project_root=ctx["canonical_repo_root"],
current_branch=git_state.get("current_branch"), current_branch=git_state.get("current_branch"),
) )
if assessment["block"]: if assessment["block"]:
@@ -440,43 +454,23 @@ def verify_preflight_purity(remote: str | None = None, worktree_path: str | None
"Pre-flight order violation: Task capability (gitea_resolve_task_capability) has not been resolved (fail closed)" "Pre-flight order violation: Task capability (gitea_resolve_task_capability) has not been resolved (fail closed)"
) )
workspace = author_mutation_worktree.resolve_mutation_workspace( ctx = _resolve_author_mutation_context(worktree_path)
worktree_path, workspace = ctx["workspace_path"]
PROJECT_ROOT, canonical_root = ctx["canonical_repo_root"]
active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV), process_root = ctx["process_project_root"]
author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV),
)
real_workspace = os.path.realpath(workspace) real_workspace = os.path.realpath(workspace)
real_root = os.path.realpath(PROJECT_ROOT)
if real_workspace != real_root: if real_workspace != process_root:
if not _preflight_in_test_mode(): if not _preflight_in_test_mode():
if not os.path.exists(real_workspace): membership = author_mutation_worktree.assess_workspace_repo_membership(
workspace_path=workspace,
canonical_repo_root=canonical_root,
)
if membership["block"]:
raise RuntimeError( raise RuntimeError(
f"Branches-only mutation guard (#274): worktree path '{workspace}' does not exist (fail closed)" author_mutation_worktree.format_workspace_repo_membership_error(
) membership
if not os.path.isdir(real_workspace):
raise RuntimeError(
f"Branches-only mutation guard (#274): worktree path '{workspace}' is not a directory (fail closed)"
)
try:
res = subprocess.run(
["git", "-C", real_workspace, "rev-parse", "--git-common-dir"],
capture_output=True,
text=True,
check=True,
)
common_dir = os.path.realpath(res.stdout.strip())
expected_dir = os.path.realpath(os.path.join(real_root, ".git"))
if common_dir != expected_dir:
raise RuntimeError(
f"Branches-only mutation guard (#274): worktree '{workspace}' does not belong to the target repository '{PROJECT_ROOT}' (fail closed)"
) )
except Exception as e:
if isinstance(e, RuntimeError):
raise e
raise RuntimeError(
f"Branches-only mutation guard (#274): worktree '{workspace}' is not a valid git repository (fail closed)"
) )
dirty_files = sorted(_parse_porcelain_entries(_get_workspace_porcelain(workspace))) dirty_files = sorted(_parse_porcelain_entries(_get_workspace_porcelain(workspace)))
@@ -541,6 +535,9 @@ import review_workflow_load # noqa: E402
import agent_temp_artifacts import agent_temp_artifacts
import issue_lock_worktree # noqa: E402 import issue_lock_worktree # noqa: E402
import issue_lock_provenance # noqa: E402 import issue_lock_provenance # noqa: E402
import issue_lock_store # noqa: E402
import issue_lock_adoption # noqa: E402
import merge_approval_gate # 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
@@ -550,11 +547,13 @@ 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
import review_merge_state_machine # noqa: E402 import review_merge_state_machine # noqa: E402
import pr_work_lease # 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"
@@ -586,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:
@@ -797,6 +840,19 @@ def _enforce_locked_issue_duplicate_recheck(
return None return None
def _branch_entry_commit_sha(branch: dict | str) -> str | None:
"""Best-effort head SHA for a Gitea branch entry (None when absent)."""
if not isinstance(branch, dict):
return None
commit = branch.get("commit")
if isinstance(commit, dict):
sha = commit.get("id") or commit.get("sha")
if sha:
return str(sha)
sha = branch.get("commit_sha")
return str(sha) if sha else None
def _reveal_endpoints() -> bool: def _reveal_endpoints() -> bool:
"""Admin/debug opt-in (#120): include endpoint URLs and token source """Admin/debug opt-in (#120): include endpoint URLs and token source
names in tool output. Off by default so normal LLM-facing responses names in tool output. Off by default so normal LLM-facing responses
@@ -1244,8 +1300,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,
@@ -1269,7 +1326,6 @@ 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)
auth = _auth(h) auth = _auth(h)
duplicate_gate = _assess_issue_duplicate_gate( duplicate_gate = _assess_issue_duplicate_gate(
issue_number, issue_number,
@@ -1285,6 +1341,30 @@ def gitea_lock_issue(
f"duplicate work gate blocked issue #{issue_number} (fail closed)" f"duplicate work gate blocked issue #{issue_number} (fail closed)"
])) ]))
branch_url = f"{repo_api_url(h, o, r)}/branches"
try:
branches = api_get_all(branch_url, auth)
except Exception as e:
raise RuntimeError(f"Could not list branches to verify issue lock: {e}")
existing_branch_entries = [
{
"name": _branch_entry_name(branch),
"commit_sha": _branch_entry_commit_sha(branch),
}
for branch in branches
]
adoption = issue_lock_adoption.assess_own_branch_adoption(
issue_number=issue_number,
requested_branch=branch_name,
existing_branches=existing_branch_entries,
)
if adoption["block"]:
competing = ", ".join(adoption["competing_branches"])
raise ValueError(
f"Issue #{issue_number} already has matching branch '{competing}' "
"that is not the requested branch (fail closed)"
)
work_lease = _build_author_issue_work_lease( work_lease = _build_author_issue_work_lease(
issue_number=issue_number, issue_number=issue_number,
branch_name=branch_name, branch_name=branch_name,
@@ -1305,11 +1385,20 @@ def gitea_lock_issue(
), ),
} }
try: lock_file_path = _save_issue_lock(data)
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: lock_record = issue_lock_store.read_lock_file(lock_file_path) or data
json.dump(data, f) freshness = issue_lock_store.assess_lock_freshness(lock_record)
except Exception as e: competing = [
raise RuntimeError(f"Could not write issue lock file: {e}") entry
for entry in issue_lock_store.list_live_locks()
if entry.get("issue_number") != issue_number
]
lock_proof = issue_lock_store.format_lock_proof(
lock_record,
freshness=freshness,
competing_live_locks=competing,
released=False,
)
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 ""
@@ -1324,7 +1413,24 @@ 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,
"lock_freshness": freshness,
"lock_proof": lock_proof,
} }
if adoption["adopt"]:
result["adoption"] = issue_lock_adoption.build_adoption_proof(
issue_number=issue_number,
branch_name=branch_name,
assessment=adoption,
open_pr_checked=True,
competing_lock_checked=True,
lock_file_path=lock_file_path,
lock_file_status="written",
)
result["message"] = (
f"Adopted existing branch '{branch_name}' and locked issue "
f"#{issue_number} for recovery (fail-closed check complete)."
)
if agent_artifacts: if agent_artifacts:
result["warnings"] = [ result["warnings"] = [
"Agent temp artifacts at repo root (delete before implementation): " "Agent temp artifacts at repo root (delete before implementation): "
@@ -1416,15 +1522,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_provenance_check = issue_lock_provenance.assess_lock_file_for_create_pr(
lock_data lock_data
@@ -1449,6 +1548,15 @@ def gitea_create_pr(
f"PR head branch '{head}' does not match locked branch '{locked_branch}' (fail closed)" f"PR head branch '{head}' does not match locked branch '{locked_branch}' (fail closed)"
) )
ownership = issue_lock_store.verify_lock_for_mutation(
lock_data,
issue_number=locked_issue,
branch_name=head,
worktree_path=worktree_path,
)
if ownership["block"]:
raise ValueError(ownership["reasons"][0])
# Check for forbidden terms anywhere in title/body # Check for forbidden terms anywhere in title/body
forbidden_terms = ["equivalent", "related", "same as"] forbidden_terms = ["equivalent", "related", "same as"]
text_to_check = f"{title} {body}".lower() text_to_check = f"{title} {body}".lower()
@@ -2377,6 +2485,10 @@ def gitea_get_pr_review_feedback(
e for e in latest_by_reviewer.values() e for e in latest_by_reviewer.values()
if e["verdict"] == "APPROVED" and not e["dismissed"] if e["verdict"] == "APPROVED" and not e["dismissed"]
] ]
approval_head = merge_approval_gate.assess_merge_approval_head(
current_head_sha=current_head,
latest_by_reviewer=latest_by_reviewer,
)
return { return {
"success": True, "success": True,
"pr_number": pr_number, "pr_number": pr_number,
@@ -2387,6 +2499,9 @@ def gitea_get_pr_review_feedback(
login: e["verdict"] for login, e in latest_by_reviewer.items()}, login: e["verdict"] for login, e in latest_by_reviewer.items()},
"has_blocking_change_requests": bool(blocking), "has_blocking_change_requests": bool(blocking),
"approval_visible": bool(approvals), "approval_visible": bool(approvals),
"approval_at_current_head": approval_head["approval_at_current_head"],
"latest_approved_head_sha": approval_head["latest_approved_head_sha"],
"stale_approval_block_reason": approval_head["stale_approval_block_reason"],
"latest_reviewed_head_sha": latest_reviewed_head, "latest_reviewed_head_sha": latest_reviewed_head,
"review_feedback_stale": bool( "review_feedback_stale": bool(
latest_reviewed_head and current_head latest_reviewed_head and current_head
@@ -2398,6 +2513,50 @@ def gitea_get_pr_review_feedback(
} }
def _list_pr_lease_comments(
pr_number: int,
*,
remote: str,
host: str | None,
org: str | None,
repo: str | None,
limit: int = 100,
) -> list[dict]:
"""Fetch PR/issue thread comments used for reviewer/conflict-fix leases."""
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
api = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
comments = api_request("GET", api, auth) or []
return list(comments[:limit])
def _pr_work_lease_reviewer_block(
*,
pr_number: int,
reviewed_head_sha: str | None,
live_head_sha: str | None,
mutation: str,
remote: str,
host: str | None,
org: str | None,
repo: str | None,
) -> dict:
comments = _list_pr_lease_comments(
pr_number,
remote=remote,
host=host,
org=org,
repo=repo,
)
return pr_work_lease.assess_reviewer_mutation_blocked(
pr_number=pr_number,
comments=comments,
reviewed_head_sha=reviewed_head_sha,
live_head_sha=live_head_sha,
mutation=mutation,
)
def _evaluate_pr_review_submission( def _evaluate_pr_review_submission(
pr_number: int, pr_number: int,
action: str, action: str,
@@ -2501,6 +2660,11 @@ def _evaluate_pr_review_submission(
lock = _load_review_decision_lock() or {} lock = _load_review_decision_lock() or {}
if live and lock.get("ready_expected_head_sha"): if live and lock.get("ready_expected_head_sha"):
pinned_sha = lock.get("ready_expected_head_sha") pinned_sha = lock.get("ready_expected_head_sha")
if live and not pinned_sha:
reasons.append(
"reviewed head SHA required before live review mutation (fail closed, #399)"
)
return result
if pinned_sha and actual_sha and pinned_sha != actual_sha: if pinned_sha and actual_sha and pinned_sha != actual_sha:
reasons.append( reasons.append(
"expected head SHA does not match current PR head (fail closed)" "expected head SHA does not match current PR head (fail closed)"
@@ -2510,6 +2674,21 @@ def _evaluate_pr_review_submission(
reasons.append("PR head SHA unavailable (fail closed)") reasons.append("PR head SHA unavailable (fail closed)")
return result return result
lease_block = _pr_work_lease_reviewer_block(
pr_number=pr_number,
reviewed_head_sha=pinned_sha,
live_head_sha=actual_sha,
mutation=action,
remote=remote,
host=host,
org=org,
repo=repo,
)
if lease_block.get("block"):
reasons.extend(lease_block.get("reasons") or [])
result["pr_work_lease"] = lease_block
return result
result["would_perform"] = True result["would_perform"] = True
if not live: if not live:
reasons.append( reasons.append(
@@ -2659,6 +2838,39 @@ def gitea_mark_final_review_decision(
f"{sorted(_REVIEW_ACTIONS)}" f"{sorted(_REVIEW_ACTIONS)}"
], ],
} }
if not (expected_head_sha or "").strip():
return {
"marked_ready": False,
"reasons": [
"expected_head_sha required before marking final review "
"decision (fail closed, #399)"
],
}
elig = gitea_check_pr_eligibility(
pr_number=pr_number,
action="review",
remote=remote,
host=None,
org=org,
repo=repo,
)
live_head = elig.get("head_sha")
lease_block = _pr_work_lease_reviewer_block(
pr_number=pr_number,
reviewed_head_sha=expected_head_sha,
live_head_sha=live_head,
mutation="mark_ready",
remote=remote,
host=None,
org=org,
repo=repo,
)
if lease_block.get("block"):
return {
"marked_ready": False,
"reasons": lease_block.get("reasons") or [],
"pr_work_lease": lease_block,
}
if action == "request_changes": if action == "request_changes":
# Duplicate request-changes suppression (#332): an unresolved # Duplicate request-changes suppression (#332): an unresolved
# REQUEST_CHANGES at the current head must not be duplicated. # REQUEST_CHANGES at the current head must not be duplicated.
@@ -2979,13 +3191,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:
@@ -3314,8 +3520,27 @@ def gitea_merge_pr(
if reasons: if reasons:
return result return result
# Gate 4 — head SHA must match if the caller pinned a reviewed SHA. # Gate 4 — reviewed head SHA is mandatory and must match live PR head (#399).
actual_sha = result["head_sha"] actual_sha = result["head_sha"]
if not (expected_head_sha or "").strip():
reasons.append(
"expected_head_sha required before merge (fail closed, #399)"
)
return result
lease_block = _pr_work_lease_reviewer_block(
pr_number=pr_number,
reviewed_head_sha=expected_head_sha,
live_head_sha=actual_sha,
mutation="merge",
remote=remote,
host=host,
org=org,
repo=repo,
)
if lease_block.get("block"):
reasons.extend(lease_block.get("reasons") or [])
result["pr_work_lease"] = lease_block
return result
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:
reasons.append( reasons.append(
"expected head SHA does not match current PR head (fail closed)" "expected head SHA does not match current PR head (fail closed)"
@@ -3370,6 +3595,9 @@ def gitea_merge_pr(
result["permission_report"] = feedback["permission_report"] result["permission_report"] = feedback["permission_report"]
return result return result
result["approval_visible"] = feedback.get("approval_visible") result["approval_visible"] = feedback.get("approval_visible")
result["approval_at_current_head"] = feedback.get("approval_at_current_head")
result["latest_approved_head_sha"] = feedback.get("latest_approved_head_sha")
result["review_feedback_stale"] = feedback.get("review_feedback_stale")
result["has_blocking_change_requests"] = feedback.get( result["has_blocking_change_requests"] = feedback.get(
"has_blocking_change_requests") "has_blocking_change_requests")
if feedback.get("has_blocking_change_requests"): if feedback.get("has_blocking_change_requests"):
@@ -3383,6 +3611,16 @@ def gitea_merge_pr(
"completed before merge (fail closed)" "completed before merge (fail closed)"
) )
return result return result
if not feedback.get("approval_at_current_head"):
reasons.append(
feedback.get("stale_approval_block_reason")
or (
"approval does not apply to current PR head SHA "
"(fail closed); required next action: re-review PR at "
"current head before merge"
)
)
return result
# Gate 8 — in-process mutation authority (#199): the last check before # Gate 8 — in-process mutation authority (#199): the last check before
# the merge mutation, using the identity the eligibility gate proved. # the merge mutation, using the identity the eligibility gate proved.
@@ -6676,6 +6914,110 @@ def gitea_post_heartbeat(
) )
@mcp.tool()
def gitea_acquire_conflict_fix_lease(
pr_number: int,
branch: str,
worktree_path: str,
head_before: str,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Acquire a conflict-fix lease on a PR branch before pushing (#399)."""
blocked = _profile_permission_block(
task_capability_map.required_permission("comment_issue"))
if blocked:
return blocked
verify_preflight_purity(remote, worktree_path=worktree_path)
comments = _list_pr_lease_comments(
pr_number,
remote=remote,
host=host,
org=org,
repo=repo,
)
reviewer_lease = pr_work_lease.find_active_reviewer_lease(
comments, pr_number=pr_number)
if reviewer_lease:
return {
"acquired": False,
"reasons": [
f"active reviewer lease on PR #{pr_number}; cannot acquire "
"conflict-fix lease (fail closed)"
],
"active_reviewer_lease": reviewer_lease,
}
profile_name = get_profile().get("profile_name") or "unknown"
body = pr_work_lease.format_conflict_fix_lease_body(
pr_number=pr_number,
branch=branch,
worktree=worktree_path,
profile=profile_name,
head_before=head_before,
reviewer_active=bool(reviewer_lease),
)
posted = _post_structured_issue_comment(
issue_number=pr_number,
body=body,
remote=remote,
host=host,
org=org,
repo=repo,
audit_op="conflict_fix_lease_acquire",
)
return {
"acquired": posted.get("success", False),
"pr_number": pr_number,
"branch": branch,
"worktree_path": worktree_path,
"head_before": head_before,
"comment_id": posted.get("comment_id"),
"active_reviewer_lease": reviewer_lease,
"reasons": [] if posted.get("success") else ["lease comment post failed"],
}
@mcp.tool()
def gitea_assess_conflict_fix_push(
pr_number: int,
branch_head_before: str,
branch_head_after: str,
worktree_path: str,
push_cwd: str,
is_fast_forward: bool = True,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Read-only pre-push gate for author conflict-fix sessions (#399)."""
read_block = _profile_operation_gate("gitea.read")
if read_block:
return {
"push_allowed": False,
"reasons": read_block,
"permission_report": _permission_block_report("gitea.read"),
}
comments = _list_pr_lease_comments(
pr_number,
remote=remote,
host=host,
org=org,
repo=repo,
)
return pr_work_lease.assess_conflict_fix_push(
pr_number=pr_number,
comments=comments,
branch_head_before=branch_head_before,
branch_head_after=branch_head_after,
worktree_path=worktree_path,
push_cwd=push_cwd,
is_fast_forward=is_fast_forward,
)
@mcp.tool() @mcp.tool()
def gitea_reconcile_issue_claims( def gitea_reconcile_issue_claims(
state: str = "open", state: str = "open",
@@ -6721,6 +7063,15 @@ def gitea_reconcile_issue_claims(
heartbeat_lease_minutes=heartbeat_lease_minutes, heartbeat_lease_minutes=heartbeat_lease_minutes,
reclaim_after_minutes=reclaim_after_minutes, reclaim_after_minutes=reclaim_after_minutes,
) )
live_locks = issue_lock_store.list_live_locks()
inventory["live_issue_locks"] = live_locks
inventory["live_issue_lock_numbers"] = sorted(
{
int(entry["issue_number"])
for entry in live_locks
if entry.get("issue_number") is not None
}
)
inventory["cleanup_plan"] = issue_claim_heartbeat.build_cleanup_plan(inventory) inventory["cleanup_plan"] = issue_claim_heartbeat.build_cleanup_plan(inventory)
inventory["success"] = True inventory["success"] = True
inventory["performed"] = False inventory["performed"] = False
+156
View File
@@ -0,0 +1,156 @@
"""Own-branch lock adoption / recovery for ``gitea_lock_issue`` (#442 / #443).
When an issue's own already-pushed branch exists, lock reacquisition must be
allowed (adoption) instead of being treated as #400 duplicate competing work.
This module isolates the pure decision so it can be unit-tested apart from the
MCP server's live Gitea calls.
Adoption is granted only for the issue's *exact* requested branch. Any other
branch that merely contains the same ``issue-<n>`` marker is competing work and
stays fail-closed. Open-PR, competing-live-lock, capability, and worktree
safety checks are enforced by the caller before this decision is consulted;
this module additionally records whether they passed for proof purposes.
"""
from __future__ import annotations
import re
ADOPT = "adopt_existing_branch"
BLOCK_COMPETING = "block_competing_branch"
NO_MATCH = "no_matching_branch"
def _branch_name(entry) -> str:
if isinstance(entry, dict):
return str(entry.get("name") or "")
return str(entry or "")
def _branch_sha(entry) -> str | None:
if isinstance(entry, dict):
sha = entry.get("commit_sha")
if sha:
return str(sha)
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(
*,
issue_number: int,
requested_branch: str,
existing_branches,
) -> dict:
"""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()
matches: list[tuple[str, str | None]] = []
for entry in existing_branches or []:
name = _branch_name(entry).strip()
if _branch_carries_issue_marker(name, issue_number):
matches.append((name, _branch_sha(entry)))
competing = sorted({name for name, _ 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:
return {
"outcome": BLOCK_COMPETING,
"adopt": False,
"block": True,
"reason": (
f"issue #{issue_number} already has matching branch(es) "
f"{competing} that are not the requested branch "
f"'{requested}' (fail closed)"
),
"matched_branch": None,
"matched_head_sha": None,
"competing_branches": competing,
}
if exact:
name, sha = exact[0]
return {
"outcome": ADOPT,
"adopt": True,
"block": False,
"reason": (
f"existing branch '{name}' is the exact requested branch for "
f"issue #{issue_number}; adopting it for lock recovery"
),
"matched_branch": name,
"matched_head_sha": sha,
"competing_branches": [],
}
return {
"outcome": NO_MATCH,
"adopt": False,
"block": False,
"reason": f"no existing branch matches issue #{issue_number}",
"matched_branch": None,
"matched_head_sha": None,
"competing_branches": [],
}
def build_adoption_proof(
*,
issue_number: int,
branch_name: str,
assessment: dict,
open_pr_checked: bool,
competing_lock_checked: bool,
lock_file_path: str,
lock_file_status: str,
) -> dict:
"""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 {
"issue_number": issue_number,
"branch_name": branch_name,
"branch_head_commit": assessment.get("matched_head_sha"),
"adoption_reason": assessment.get("reason"),
"no_existing_pr_proof": bool(open_pr_checked),
"no_competing_live_lock_proof": bool(competing_lock_checked),
"lock_file_path": lock_file_path,
"lock_file_status": lock_file_status,
}
+612
View File
@@ -0,0 +1,612 @@
"""Keyed, persistent issue-lock storage (#443) with flock hardening (#438).
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. Acquisition is serialized per issue with ``fcntl.flock``.
"""
from __future__ import annotations
import errno
import fcntl
import json
import os
import re
import tempfile
from contextlib import contextmanager
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._+-]+")
class LockContentionError(RuntimeError):
"""Raised when an exclusive per-issue lock cannot be acquired."""
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 flock_path(json_path: str) -> str:
return f"{json_path}.lock"
def is_process_alive(pid: int | None) -> bool:
if not pid or pid <= 0:
return False
try:
os.kill(int(pid), 0)
return True
except OSError as exc:
return exc.errno != errno.ESRCH
except (TypeError, ValueError):
return False
@contextmanager
def _exclusive_file_lock(lock_path: str):
os.makedirs(os.path.dirname(lock_path) or ".", exist_ok=True)
fd = os.open(lock_path, os.O_CREAT | os.O_RDWR, 0o600)
try:
try:
fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
except BlockingIOError as exc:
raise LockContentionError(
f"could not acquire exclusive lock on '{lock_path}'"
) from exc
yield fd
finally:
try:
fcntl.flock(fd, fcntl.LOCK_UN)
finally:
os.close(fd)
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()
record.setdefault("pid", os.getpid())
pointer = {
"pid": os.getpid(),
"lock_file_path": path,
"issue_number": issue_number,
"branch_name": record.get("branch_name"),
"remote": remote,
"org": org,
"repo": repo,
}
sentinel = flock_path(path)
try:
with _exclusive_file_lock(sentinel):
existing = read_lock_file(path)
overwrite_block = assess_foreign_lock_overwrite(existing, record)
if overwrite_block:
raise RuntimeError(overwrite_block)
lease_block = assess_same_issue_lease_conflict(
existing,
issue_number=issue_number,
branch_name=str(record.get("branch_name") or ""),
worktree_path=str(record.get("worktree_path") or ""),
)
if lease_block:
raise RuntimeError(lease_block)
save_lock_file(path, record)
save_lock_file(session_pointer_path(root), pointer)
except LockContentionError as exc:
competing = read_lock_file(path)
if competing:
owner_pid = competing.get("session_pid") or competing.get("pid")
raise RuntimeError(
f"Issue #{issue_number} lock contention: {exc}; competing owner "
f"pid={owner_pid} (fail closed)"
) from exc
raise RuntimeError(f"Issue #{issue_number} lock contention: {exc} (fail closed)") from exc
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:
return assess_lock_freshness(lock, now=now)["live"]
def assess_lock_freshness(
lock_data: dict[str, Any] | None,
*,
now: datetime | None = None,
) -> dict[str, Any]:
"""Classify a lock as live, expired, stale, or absent."""
current = _lease_now(now)
if not lock_data:
return {
"status": "absent",
"live": False,
"stale": False,
"reason": "no lock record",
}
expires_at = lease_expires_at(lock_data)
lease = lock_data.get("work_lease")
heartbeat_at = _parse_lease_timestamp(lock_data.get("last_heartbeat_at"))
if heartbeat_at is None and isinstance(lease, dict):
heartbeat_at = _parse_lease_timestamp(lease.get("last_heartbeat_at"))
pid = lock_data.get("session_pid")
if pid is None:
pid = lock_data.get("pid")
pid_alive = is_process_alive(pid) if pid is not None else False
if expires_at and expires_at <= current:
return {
"status": "expired",
"live": False,
"stale": True,
"reason": f"lease expired at {expires_at.isoformat()}",
"pid_alive": pid_alive,
}
if pid is not None and not pid_alive:
return {
"status": "stale",
"live": False,
"stale": True,
"reason": f"owner pid {pid} is not alive",
"pid_alive": False,
}
return {
"status": "live",
"live": True,
"stale": False,
"reason": "lock heartbeat and lease are fresh",
"pid_alive": pid_alive,
"heartbeat_at": heartbeat_at.isoformat() if heartbeat_at else None,
"expires_at": expires_at.isoformat() if expires_at else None,
}
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
def verify_lock_for_mutation(
lock_data: dict[str, Any] | None,
*,
issue_number: int | None = None,
branch_name: str | None = None,
worktree_path: str | None = None,
) -> dict[str, Any]:
"""Re-check lock ownership immediately before a mutation (#438)."""
reasons: list[str] = []
if not lock_data:
return {"proven": False, "block": True, "reasons": ["issue lock is missing (fail closed)"]}
freshness = assess_lock_freshness(lock_data)
if not freshness["live"]:
reasons.append(f"issue lock is not live: {freshness['reason']} (fail closed)")
if issue_number is not None and lock_data.get("issue_number") != issue_number:
reasons.append(
f"issue lock targets #{lock_data.get('issue_number')}, expected #{issue_number} (fail closed)"
)
if branch_name is not None and lock_data.get("branch_name") != branch_name:
reasons.append(
f"issue lock branch '{lock_data.get('branch_name')}' does not match "
f"'{branch_name}' (fail closed)"
)
if worktree_path is not None:
locked = os.path.realpath(str(lock_data.get("worktree_path") or ""))
declared = os.path.realpath(worktree_path)
if locked != declared:
reasons.append(
f"issue lock worktree '{locked}' does not match declared '{declared}' (fail closed)"
)
return {
"proven": not reasons,
"block": bool(reasons),
"reasons": reasons,
"freshness": freshness,
"lock_proof": format_lock_proof(lock_data, freshness=freshness),
}
def list_live_locks(
*,
lock_dir: str | None = None,
now: datetime | None = None,
) -> list[dict[str, Any]]:
"""Return live per-issue locks for queue visibility."""
live: list[dict[str, Any]] = []
for path in iter_lock_files(lock_dir):
record = read_lock_file(path)
if not record:
continue
freshness = assess_lock_freshness(record, now=now)
if not freshness["live"]:
continue
live.append(
{
"issue_number": record.get("issue_number"),
"branch_name": record.get("branch_name"),
"remote": record.get("remote"),
"org": record.get("org"),
"repo": record.get("repo"),
"worktree_path": record.get("worktree_path"),
"pid": record.get("session_pid") or record.get("pid"),
"claimant": (
record.get("claimant")
or (record.get("work_lease") or {}).get("claimant")
),
"freshness": freshness,
"lock_path": record.get("lock_file_path") or path,
}
)
return live
def format_lock_proof(
lock_data: dict[str, Any] | None,
*,
freshness: dict[str, Any] | None = None,
competing_live_locks: list[dict[str, Any]] | None = None,
released: bool | None = None,
) -> str:
"""Canonical issue-lock proof string for final reports."""
if not lock_data:
return "issue lock proof: not acquired"
fresh = freshness or assess_lock_freshness(lock_data)
owner = lock_data.get("claimant") or {}
if not owner and isinstance(lock_data.get("work_lease"), dict):
owner = lock_data["work_lease"].get("claimant") or {}
parts = [
"issue lock proof:",
f"acquired issue #{lock_data.get('issue_number')}",
f"branch {lock_data.get('branch_name')}",
f"owner {owner.get('profile') or 'unknown'}",
f"pid {lock_data.get('session_pid') or lock_data.get('pid')}",
f"freshness {fresh.get('status')}",
]
if competing_live_locks is not None:
parts.append(
"no competing live lock"
if not competing_live_locks
else f"competing live locks {len(competing_live_locks)}"
)
if released is True:
parts.append("lock released")
elif released is False:
parts.append("lock retained")
return "; ".join(parts)
+61
View File
@@ -0,0 +1,61 @@
"""Merge approval must pin the current PR head SHA (#471).
Formal APPROVED reviews that predate the live PR head must not satisfy
``gitea_merge_pr`` eligibility. Pure assessment helpers are isolated here
for hermetic unit tests apart from MCP HTTP calls.
"""
from __future__ import annotations
def assess_merge_approval_head(
*,
current_head_sha: str | None,
latest_by_reviewer: dict,
) -> dict:
"""Return whether a visible approval applies to the live PR head.
Args:
current_head_sha: Current PR head commit SHA.
latest_by_reviewer: Map of reviewer login → review entry dicts with
``verdict``, ``dismissed``, and ``reviewed_head_sha`` keys.
Returns:
dict with ``approval_at_current_head``, ``latest_approved_head_sha``,
and ``stale_approval_block_reason`` (set when merge must fail closed).
"""
current = (current_head_sha or "").strip()
approved_entries = [
entry
for entry in (latest_by_reviewer or {}).values()
if (entry.get("verdict") or "").upper() == "APPROVED"
and not entry.get("dismissed")
]
at_current = any(
(entry.get("reviewed_head_sha") or "").strip() == current
for entry in approved_entries
if current
)
latest_approved = None
if approved_entries:
latest_entry = sorted(
approved_entries,
key=lambda entry: (
entry.get("submitted_at") or "",
entry.get("reviewed_head_sha") or "",
),
)[-1]
latest_approved = (latest_entry.get("reviewed_head_sha") or "").strip() or None
reason = None
if approved_entries and not at_current:
reason = (
f"stale approval: approved SHA '{latest_approved}' does not match "
f"current live PR head SHA '{current or '(unknown)'}' (fail closed); "
"required next action: re-review PR at current head before merge"
)
return {
"approval_at_current_head": at_current,
"latest_approved_head_sha": latest_approved,
"stale_approval_block_reason": reason,
}
+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]:
+482
View File
@@ -0,0 +1,482 @@
"""Conflict-fix and reviewer PR work leases (#399, #407 reader).
Structured PR/issue comments prove exclusive phases so author conflict-fix
pushes cannot race reviewer validation/approval/merge on the same head.
"""
from __future__ import annotations
import re
from datetime import datetime, timedelta, timezone
from typing import Any
REVIEWER_LEASE_MARKER = "<!-- mcp-review-lease:v1 -->"
CONFLICT_FIX_LEASE_MARKER = "<!-- mcp-conflict-fix-lease:v1 -->"
_FULL_SHA = re.compile(r"^[0-9a-f]{40}$", re.IGNORECASE)
_FIELD_RE = re.compile(
r"^\s*([a-z_]+)\s*:\s*(.+?)\s*$",
re.IGNORECASE | re.MULTILINE,
)
_TERMINAL_REVIEWER_PHASES = frozenset({"done", "released", "blocked"})
_ACTIVE_REVIEWER_PHASES = frozenset({
"claimed",
"validating",
"approved",
"request-changes",
"merging",
})
_TERMINAL_CONFLICT_FIX_PHASES = frozenset({"released", "blocked", "done"})
_ACTIVE_CONFLICT_FIX_PHASES = frozenset({"claimed", "pushing", "pushed"})
DEFAULT_CONFLICT_FIX_TTL_MINUTES = 120
DEFAULT_REVIEWER_LEASE_TTL_MINUTES = 120
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()
if not text:
return None
return text if _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 _parse_marker_comment(body: str, marker: str) -> dict[str, str] | 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()
return fields or None
def parse_reviewer_lease_comment(body: str) -> dict[str, Any] | None:
fields = _parse_marker_comment(body, REVIEWER_LEASE_MARKER)
if not fields:
return None
return {
"lease_kind": "reviewer",
"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 parse_conflict_fix_lease_comment(body: str) -> dict[str, Any] | None:
fields = _parse_marker_comment(body, CONFLICT_FIX_LEASE_MARKER)
if not fields:
return None
ff = (fields.get("fast_forward") or "").strip().lower()
reviewer_active = (fields.get("reviewer_active") or "").strip().lower()
return {
"lease_kind": "conflict_fix",
"pr_number": _parse_pr_ref(fields.get("pr")),
"branch": fields.get("branch"),
"worktree": fields.get("worktree"),
"profile": fields.get("profile"),
"session_id": fields.get("session_id"),
"phase": (fields.get("phase") or "").strip().lower() or None,
"head_before": _normalize_sha(fields.get("head_before")),
"head_after": _normalize_sha(fields.get("head_after")),
"expires_at": fields.get("expires_at"),
"reviewer_active": reviewer_active in {"yes", "true", "1"},
"fast_forward": ff in {"yes", "true", "1"},
"raw_fields": fields,
}
def _comment_entries(comments: list[dict], *, pr_number: int | None) -> list[dict]:
entries: list[dict] = []
for comment in comments or []:
body = comment.get("body") or ""
for parser in (parse_reviewer_lease_comment, parse_conflict_fix_lease_comment):
parsed = parser(body)
if not parsed:
continue
if pr_number is not None and 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"),
})
break
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 _lease_phase_active(lease: dict, *, active_phases: frozenset[str]) -> bool:
phase = (lease.get("phase") or "").strip().lower()
if phase in _TERMINAL_REVIEWER_PHASES or phase in _TERMINAL_CONFLICT_FIX_PHASES:
return False
return phase in active_phases or bool(phase and phase not in (
_TERMINAL_REVIEWER_PHASES | _TERMINAL_CONFLICT_FIX_PHASES
))
def find_active_reviewer_lease(
comments: list[dict],
*,
pr_number: int,
now: datetime | None = None,
) -> dict[str, Any] | None:
"""Return the newest unexpired reviewer lease for *pr_number*, if any."""
now = now or datetime.now(timezone.utc)
candidates = [
entry for entry in _comment_entries(comments, pr_number=pr_number)
if entry.get("lease_kind") == "reviewer"
]
for lease in reversed(candidates):
if _lease_expired(lease, now=now):
continue
phase = (lease.get("phase") or "").strip().lower()
if phase in _TERMINAL_REVIEWER_PHASES:
continue
if phase in _ACTIVE_REVIEWER_PHASES or phase:
return lease
return None
def find_active_conflict_fix_lease(
comments: list[dict],
*,
pr_number: int,
now: datetime | None = None,
) -> dict[str, Any] | None:
"""Return the newest unexpired conflict-fix lease for *pr_number*, if any."""
now = now or datetime.now(timezone.utc)
candidates = [
entry for entry in _comment_entries(comments, pr_number=pr_number)
if entry.get("lease_kind") == "conflict_fix"
]
for lease in reversed(candidates):
if _lease_expired(lease, now=now):
continue
phase = (lease.get("phase") or "").strip().lower()
if phase in _TERMINAL_CONFLICT_FIX_PHASES:
continue
if phase in _ACTIVE_CONFLICT_FIX_PHASES or phase:
return lease
return None
def format_conflict_fix_lease_body(
*,
pr_number: int,
branch: str,
worktree: str,
profile: str,
head_before: str,
phase: str = "claimed",
session_id: str = "unknown",
expires_at: datetime | None = None,
reviewer_active: bool = False,
) -> str:
expires = expires_at or (
datetime.now(timezone.utc) + timedelta(minutes=DEFAULT_CONFLICT_FIX_TTL_MINUTES)
)
expires_text = expires.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace(
"+00:00", "Z"
)
lines = [
CONFLICT_FIX_LEASE_MARKER,
f"pr: #{pr_number}",
f"branch: {branch}",
f"worktree: {worktree}",
f"profile: {profile}",
f"session_id: {session_id}",
f"phase: {phase}",
f"head_before: {head_before}",
f"expires_at: {expires_text}",
f"reviewer_active: {'yes' if reviewer_active else 'no'}",
]
return "\n".join(lines)
def assess_head_sha_equality(
reviewed_head_sha: str | None,
live_head_sha: str | None,
) -> dict[str, Any]:
"""Fail closed when reviewed and live PR heads differ."""
reviewed = _normalize_sha(reviewed_head_sha)
live = _normalize_sha(live_head_sha)
reasons: list[str] = []
if not reviewed or not live:
reasons.append(
"reviewed/live head SHA missing or not full 40-hex; fail closed"
)
elif reviewed != live:
reasons.append(
"PR head changed after validation; re-pin and re-validate before "
"approval or merge"
)
proven = not reasons
return {
"proven": proven,
"block": not proven,
"reasons": reasons,
"reviewed_head_sha": reviewed,
"live_head_sha": live,
"head_changed": bool(reviewed and live and reviewed != live),
}
def assess_conflict_fix_push(
*,
pr_number: int,
comments: list[dict],
branch_head_before: str | None,
branch_head_after: str | None,
worktree_path: str | None,
push_cwd: str | None,
is_fast_forward: bool | None,
now: datetime | None = None,
) -> dict[str, Any]:
"""Author pre-push gate: block when a reviewer holds an active lease."""
now = now or datetime.now(timezone.utc)
reasons: list[str] = []
reviewer_lease = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
conflict_lease = find_active_conflict_fix_lease(comments, pr_number=pr_number, now=now)
if reviewer_lease:
reasons.append(
f"active reviewer lease on PR #{pr_number} "
f"(phase={reviewer_lease.get('phase')}); author push blocked"
)
head_before = _normalize_sha(branch_head_before)
head_after = _normalize_sha(branch_head_after)
if not head_before:
reasons.append("branch head before push missing or invalid SHA")
if head_after and head_before and head_before == head_after:
reasons.append("branch head unchanged; no push to perform")
worktree = (worktree_path or "").strip()
cwd = (push_cwd or "").strip()
if not worktree:
reasons.append("worktree path required for conflict-fix push proof")
elif cwd and worktree and not cwd.rstrip("/").endswith(worktree.rstrip("/").split("/")[-1]):
if worktree not in cwd:
reasons.append(
f"push cwd '{cwd}' does not match session worktree '{worktree}'"
)
if is_fast_forward is False:
reasons.append("non-fast-forward push rejected for conflict-fix (fail closed)")
if conflict_lease and conflict_lease.get("phase") == "pushing":
owner = conflict_lease.get("worktree")
if owner and worktree and owner != worktree:
reasons.append(
f"sibling conflict-fix lease active from worktree '{owner}'"
)
push_allowed = not reasons
return {
"push_allowed": push_allowed,
"block": not push_allowed,
"reasons": reasons,
"active_reviewer_lease": reviewer_lease,
"active_conflict_fix_lease": conflict_lease,
"branch_head_before": head_before,
"branch_head_after": head_after,
"reviewer_was_active": bool(reviewer_lease),
"fast_forward": is_fast_forward,
}
def assess_reviewer_mutation_blocked(
*,
pr_number: int,
comments: list[dict],
reviewed_head_sha: str | None,
live_head_sha: str | None,
mutation: str,
now: datetime | None = None,
) -> dict[str, Any]:
"""Reviewer gate: block when conflict-fix lease active or head moved."""
now = now or datetime.now(timezone.utc)
reasons: list[str] = []
conflict_lease = find_active_conflict_fix_lease(comments, pr_number=pr_number, now=now)
if conflict_lease and (conflict_lease.get("phase") or "") in _ACTIVE_CONFLICT_FIX_PHASES:
reasons.append(
f"active conflict-fix lease on PR #{pr_number} "
f"(phase={conflict_lease.get('phase')}); reviewer {mutation} blocked"
)
head_check = assess_head_sha_equality(reviewed_head_sha, live_head_sha)
if head_check["block"]:
reasons.extend(head_check["reasons"])
if not _normalize_sha(reviewed_head_sha):
reasons.append(
f"reviewed head SHA required before reviewer {mutation} (fail closed)"
)
allowed = not reasons
return {
"mutation_allowed": allowed,
"block": not allowed,
"reasons": reasons,
"active_conflict_fix_lease": conflict_lease,
"head_check": head_check,
"reviewed_head_sha": head_check.get("reviewed_head_sha"),
"live_head_sha": head_check.get("live_head_sha"),
"push_during_validation": bool(
conflict_lease and conflict_lease.get("phase") in {"pushing", "pushed"}
),
}
_REVIEWED_HEAD_RE = re.compile(
r"reviewed head sha\s*:\s*([0-9a-f]{40})",
re.IGNORECASE,
)
_LIVE_HEAD_BEFORE_APPROVAL_RE = re.compile(
r"(?:live head sha before approval|final live head sha before approval)\s*:\s*([0-9a-f]{40})",
re.IGNORECASE,
)
_LIVE_HEAD_BEFORE_MERGE_RE = re.compile(
r"(?:live head sha before merge|final live head sha before merge)\s*:\s*([0-9a-f]{40})",
re.IGNORECASE,
)
_PUSH_DURING_VALIDATION_RE = re.compile(
r"push(?:es)? occurred during validation\s*:\s*(yes|no|true|false)",
re.IGNORECASE,
)
_CONFLICT_HEAD_BEFORE_RE = re.compile(
r"branch head before push\s*:\s*([0-9a-f]{40})",
re.IGNORECASE,
)
_CONFLICT_HEAD_AFTER_RE = re.compile(
r"branch head after push\s*:\s*([0-9a-f]{40})",
re.IGNORECASE,
)
_REVIEWER_LEASE_STATUS_RE = re.compile(
r"active reviewer lease status\s*:\s*(.+)$",
re.IGNORECASE | re.MULTILINE,
)
_FAST_FORWARD_RE = re.compile(
r"whether push was fast-forward\s*:\s*(yes|no|true|false)",
re.IGNORECASE,
)
_REVIEWER_ACTIVE_RE = re.compile(
r"whether any reviewer was active\s*:\s*(yes|no|true|false)",
re.IGNORECASE,
)
def assess_reviewer_stale_head_final_report(report_text: str) -> dict[str, Any]:
"""Final-report proof for reviewed vs live head SHAs (#399 AC 6)."""
text = report_text or ""
reasons: list[str] = []
reviewed = _normalize_sha(_REVIEWED_HEAD_RE.search(text).group(1) if _REVIEWED_HEAD_RE.search(text) else None)
live_approval = _normalize_sha(
_LIVE_HEAD_BEFORE_APPROVAL_RE.search(text).group(1)
if _LIVE_HEAD_BEFORE_APPROVAL_RE.search(text)
else None
)
live_merge = _normalize_sha(
_LIVE_HEAD_BEFORE_MERGE_RE.search(text).group(1)
if _LIVE_HEAD_BEFORE_MERGE_RE.search(text)
else None
)
push_during = _PUSH_DURING_VALIDATION_RE.search(text)
if not reviewed:
reasons.append("reviewed head SHA not stated in final report")
if not live_approval:
reasons.append("final live head SHA before approval not stated")
if not live_merge:
reasons.append("final live head SHA before merge not stated")
if not push_during:
reasons.append("whether push occurred during validation not stated")
elif reviewed and live_approval and reviewed != live_approval:
reasons.append("live head before approval differs from reviewed head SHA")
elif reviewed and live_merge and reviewed != live_merge:
reasons.append("live head before merge differs from reviewed head SHA")
proven = not reasons
return {
"proven": proven,
"block": not proven,
"reasons": reasons,
"reviewed_head_sha": reviewed,
"live_head_sha_before_approval": live_approval,
"live_head_sha_before_merge": live_merge,
"push_during_validation": (push_during.group(1).lower() if push_during else None),
}
def assess_conflict_fix_final_report(report_text: str) -> dict[str, Any]:
"""Final-report proof for conflict-fix push sessions (#399 AC 7)."""
text = report_text or ""
reasons: list[str] = []
head_before = _normalize_sha(
_CONFLICT_HEAD_BEFORE_RE.search(text).group(1)
if _CONFLICT_HEAD_BEFORE_RE.search(text)
else None
)
head_after = _normalize_sha(
_CONFLICT_HEAD_AFTER_RE.search(text).group(1)
if _CONFLICT_HEAD_AFTER_RE.search(text)
else None
)
if not head_before:
reasons.append("branch head before push not stated")
if not head_after:
reasons.append("branch head after push not stated")
if not _REVIEWER_LEASE_STATUS_RE.search(text):
reasons.append("active reviewer lease status not stated")
if not _FAST_FORWARD_RE.search(text):
reasons.append("whether push was fast-forward not stated")
if not _REVIEWER_ACTIVE_RE.search(text):
reasons.append("whether any reviewer was active not stated")
proven = not reasons
return {
"proven": proven,
"block": not proven,
"reasons": reasons,
"branch_head_before": head_before,
"branch_head_after": head_after,
}
+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"
@@ -606,6 +606,28 @@ If the cause is unknown, do not erase the earlier failure with plain
`gitea_validate_review_final_report` rejects reports that omit known earlier `gitea_validate_review_final_report` rejects reports that omit known earlier
validation failures when `validation_session.observed_failures` is supplied. validation failures when `validation_session.observed_failures` is supplied.
## 21B. Validation status taxonomy (#406)
When the final report summarizes how validation concluded, use one of these
**validation status** labels (distinct from per-command pass/fail entries):
* `passed` — raw PR-head validation passed on the unmodified head.
* `failed` — raw PR-head validation failed and no allowed resolution path
was proven.
* `baseline-equivalent failure accepted` — only when a clean baseline
worktree under `branches/` proves matching failure signatures on the target
branch (baseline path, target SHA, exact commands, failure lists, and
`failure signatures match: true`).
* `raw-head failure resolved by merge simulation` — raw PR-head validation
failed, but merge simulation into the current target passed cleanly; report
merge simulation under `Worktree/index mutations` with full #317 proof.
* `passed after transient failure investigation` — a later run passed after an
earlier failure in the same session; document the failure history (#396).
Do not use `baseline-equivalent failure accepted` when only merge simulation
resolved the failure. Do not use bare `passed` when raw PR-head validation
failed unless one of the resolution statuses above applies.
## 22. Baseline validation rule ## 22. Baseline validation rule
Do not run tests in the main checkout. Do not run tests in the main checkout.
@@ -790,6 +812,24 @@ inventory before continuing.
Final reports must include lease session id, acquisition proof, heartbeat Final reports must include lease session id, acquisition proof, heartbeat
status, and release/blocked status. status, and release/blocked status.
## 26C. Conflict-fix lease and stale-head protection (#399)
Before validating, approving, or merging a PR:
1. Check for an active conflict-fix lease on the PR; stop if one is active.
2. Pin `expected_head_sha` before validation and pass it to
`gitea_mark_final_review_decision`, `gitea_submit_pr_review`, and
`gitea_merge_pr`.
3. Re-fetch live PR head immediately before approval and merge; refuse when
live head differs from the reviewed SHA.
Final reports must state:
* reviewed head SHA
* final live head SHA before approval
* final live head SHA before merge
* whether any push occurred during validation
## 27. Merge rules ## 27. Merge rules
Before merge, rerun fresh live checks: Before merge, rerun fresh live checks:
@@ -800,6 +840,7 @@ Before merge, rerun fresh live checks:
* author safety * author safety
* PR re-fetch * PR re-fetch
* reviewed head SHA unchanged * reviewed head SHA unchanged
* visible APPROVED review applies to the **current live PR head SHA** (`approval_at_current_head`); if the head moved after approval, re-review at the new head before merge (#471)
* target branch freshly fetched * target branch freshly fetched
* PR still open * PR still open
* PR still mergeable * PR still mergeable
@@ -816,6 +857,7 @@ Do not merge if:
* capability state is stale * capability state is stale
* worktree is dirty * worktree is dirty
* PR head changed * PR head changed
* approval is stale (approved SHA ≠ current live head SHA)
* validation failed * validation failed
* inventory was incomplete * inventory was incomplete
* PR is already landed * PR is already landed
@@ -607,6 +607,29 @@ After push, report:
If push fails, stop and produce a recovery handoff. If push fails, stop and produce a recovery handoff.
## 20A. Conflict-fix lease and push gate (#399)
When pushing to an existing PR branch to resolve merge conflicts:
1. Call `gitea_acquire_conflict_fix_lease` before any push.
2. Call `gitea_assess_conflict_fix_push` immediately before `git push` with:
* branch head before push
* branch head after push (local)
* session worktree path
* push cwd
* whether the push is fast-forward
3. Do not push when a reviewer holds an active lease on the same PR.
4. Do not force-push.
5. Do not push from the main checkout or wrong cwd.
Conflict-fix final reports must state:
* branch head before push
* branch head after push
* active reviewer lease status
* whether push was fast-forward
* whether any reviewer was active
## 21. PR creation rules ## 21. PR creation rules
Create a PR only if implementation and validation pass, unless project policy explicitly allows draft PRs with documented validation failures. Create a PR only if implementation and validation pass, unless project policy explicitly allows draft PRs with documented validation failures.
+6 -3
View File
@@ -74,21 +74,24 @@ 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( @patch(
"mcp_server.issue_duplicate_context_fetcher", "mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}), return_value=([], [], {"status": "not_claimed"}),
) )
@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",
+30 -17
View File
@@ -284,21 +284,38 @@ class TestSimpleToolAudit(_AuditWiringBase):
self.assertEqual(result["number"], 9) self.assertEqual(result["number"], 9)
_NO_PR_WORK_LEASE_BLOCK = {"block": False, "reasons": [], "mutation_allowed": True}
class TestGatedToolAudit(_AuditWiringBase): class TestGatedToolAudit(_AuditWiringBase):
def setUp(self): def setUp(self):
super().setUp() super().setUp()
from mcp_server import init_review_decision_lock
from tests.test_mcp_server import _install_owned_reviewer_lease from tests.test_mcp_server import _install_owned_reviewer_lease
import reviewer_pr_lease import reviewer_pr_lease
# init_review_decision_lock clears any prior session lease (#407).
init_review_decision_lock("prgs", "review_pr")
self._lease_patch = _install_owned_reviewer_lease(8) self._lease_patch = _install_owned_reviewer_lease(8)
self._lease_patch.start() self._lease_patch.start()
self._auth_identity_patch = patch( self._auth_identity_patch = patch(
"mcp_server._authenticated_username", return_value="reviewer-bot" "mcp_server._authenticated_username", return_value="reviewer-bot"
) )
self._auth_identity_patch.start() self._auth_identity_patch.start()
self._pr_lease_comments_patch = patch(
"mcp_server._list_pr_lease_comments", return_value=[]
)
self._pr_lease_comments_patch.start()
self._pr_work_lease_patch = patch(
"mcp_server._pr_work_lease_reviewer_block",
return_value=dict(_NO_PR_WORK_LEASE_BLOCK),
)
self._pr_work_lease_patch.start()
self.addCleanup(self._auth_identity_patch.stop) self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop) self.addCleanup(self._lease_patch.stop)
self.addCleanup(self._pr_lease_comments_patch.stop)
self.addCleanup(self._pr_work_lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease) 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):
@@ -313,7 +330,8 @@ class TestGatedToolAudit(_AuditWiringBase):
{"login": "merger-bot"}, self._pr("author-bot"), {"login": "merger-bot"}, self._pr("author-bot"),
self._pr("author-bot"), self._pr("author-bot"),
[{"id": 1, "user": {"login": "reviewer-bot"}, "state": "APPROVED", [{"id": 1, "user": {"login": "reviewer-bot"}, "state": "APPROVED",
"submitted_at": "2026-07-06T10:00:00Z", "dismissed": False}], "commit_id": "abc123", "submitted_at": "2026-07-06T10:00:00Z",
"dismissed": False}],
{}, {"merged_commit_sha": "c1"}, {}, {"merged_commit_sha": "c1"},
] ]
env = self._env(GITEA_PROFILE_NAME="gitea-merger", env = self._env(GITEA_PROFILE_NAME="gitea-merger",
@@ -349,7 +367,9 @@ class TestGatedToolAudit(_AuditWiringBase):
@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)
def test_submit_review_success_audited(self, _auth, mock_api): def test_submit_review_success_audited(self, _auth, mock_api):
# mark_final_review_decision and submit each run eligibility (user + PR).
mock_api.side_effect = [ mock_api.side_effect = [
{"login": "reviewer-bot"}, self._pr("author-bot"),
{"login": "reviewer-bot"}, self._pr("author-bot"), {"login": "reviewer-bot"}, self._pr("author-bot"),
{"id": 7, "state": "APPROVED"}, {"id": 7, "state": "APPROVED"},
[{"id": 7, "user": {"login": "reviewer-bot"}, "state": "APPROVED", [{"id": 7, "user": {"login": "reviewer-bot"}, "state": "APPROVED",
@@ -358,23 +378,16 @@ class TestGatedToolAudit(_AuditWiringBase):
env = self._env(GITEA_PROFILE_NAME="gitea-reviewer", env = self._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):
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision from mcp_server import gitea_mark_final_review_decision
from tests.test_mcp_server import _install_owned_reviewer_lease
import reviewer_pr_lease
init_review_decision_lock("prgs", "review_pr") gitea_mark_final_review_decision(
gitea_mark_final_review_decision(8, "approve", remote="prgs") 8, "approve", expected_head_sha="abc123", remote="prgs",
lease_patch = _install_owned_reviewer_lease(8) )
lease_patch.start() r = gitea_submit_pr_review(
try: pr_number=8, action="approve",
r = gitea_submit_pr_review( body="LGTM", remote="prgs",
pr_number=8, action="approve", final_review_decision_ready=True,
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 -5
View File
@@ -66,9 +66,12 @@ 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 import issue_lock_provenance
self._lock_dir = tempfile.TemporaryDirectory()
os.environ["GITEA_ISSUE_LOCK_DIR"] = self._lock_dir.name
work_lease = { work_lease = {
"operation_type": "author_issue_work", "operation_type": "author_issue_work",
"issue_number": 263, "issue_number": 263,
@@ -89,8 +92,7 @@ class TestCommitPayloads(unittest.TestCase):
claimant=work_lease.get("claimant"), 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
@@ -114,8 +116,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 +125,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")
+22 -10
View File
@@ -106,20 +106,32 @@ class TestCreateIssueWorkspaceGuard(unittest.TestCase):
@patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", return_value={"current_branch": "feat/issue-1"}) @patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", return_value={"current_branch": "feat/issue-1"})
@patch("os.path.exists", return_value=True) @patch("os.path.exists", return_value=True)
@patch("os.path.isdir", return_value=True) @patch("os.path.isdir", return_value=True)
@patch("author_mutation_worktree.subprocess.run")
@patch("subprocess.run") @patch("subprocess.run")
def test_create_issue_wrong_repo_fails_closed(self, mock_run, mock_isdir, mock_exists, _git, _get_all, mock_api, _role, _ns, _prof, _auth): def test_create_issue_wrong_repo_fails_closed(self, mock_run, mock_amw_run, mock_isdir, mock_exists, _git, _get_all, mock_api, _role, _ns, _prof, _auth):
# Mock subprocess.run for git --git-common-dir to return a different path
mock_res = MagicMock()
mock_res.stdout = "/Users/jasonwalker/Development/some-other-repo/.git\n"
mock_run.return_value = mock_res
wrong_repo_path = os.path.join(CONTROL_CHECKOUT_ROOT, "branches", "feat-issue-1") wrong_repo_path = os.path.join(CONTROL_CHECKOUT_ROOT, "branches", "feat-issue-1")
def _subprocess_side_effect(cmd, *args, **kwargs):
mock_res = MagicMock(returncode=0)
if "--git-common-dir" in cmd:
cwd = cmd[cmd.index("-C") + 1] if "-C" in cmd else ""
if cwd == wrong_repo_path:
mock_res.stdout = "/Users/jasonwalker/Development/some-other-repo/.git\n"
else:
mock_res.stdout = f"{CONTROL_CHECKOUT_ROOT}/.git\n"
else:
mock_res.stdout = ""
return mock_res
mock_run.side_effect = _subprocess_side_effect
mock_amw_run.side_effect = _subprocess_side_effect
with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT):
with self.assertRaises(RuntimeError) as ctx: with patch("gitea_mcp_server._get_workspace_porcelain", return_value=""):
srv.gitea_create_issue( with self.assertRaises(RuntimeError) as ctx:
title="Test issue", body="body", worktree_path=wrong_repo_path srv.gitea_create_issue(
) title="Test issue", body="body", worktree_path=wrong_repo_path
)
self.assertIn("does not belong to the target repository", str(ctx.exception)) self.assertIn("does not belong to the target repository", str(ctx.exception))
@patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH)
+4
View File
@@ -41,6 +41,10 @@ def _review_handoff(**overrides):
"- Selected PR: #203", "- Selected PR: #203",
"- Reviewer eligibility: eligible", "- Reviewer eligibility: eligible",
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", "- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before approval: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before merge: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Push occurred during validation: no",
"- Worktree path: branches/review-203", "- Worktree path: branches/review-203",
"- Worktree dirty: clean", "- Worktree dirty: clean",
"- Scratch worktree used: yes (branches/review-203)", "- Scratch worktree used: yes (branches/review-203)",
+138
View File
@@ -0,0 +1,138 @@
"""Unit tests for own-branch lock adoption decision (#442 / #443)."""
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from issue_lock_adoption import ( # noqa: E402
ADOPT,
BLOCK_COMPETING,
NO_MATCH,
assess_own_branch_adoption,
build_adoption_proof,
)
REQ = "feat/issue-420-server-code-parity"
class TestAssessOwnBranchAdoption(unittest.TestCase):
def test_exact_own_branch_is_adopted(self):
result = assess_own_branch_adoption(
issue_number=420,
requested_branch=REQ,
existing_branches=[{"name": REQ, "commit_sha": "934688a"}],
)
self.assertEqual(result["outcome"], 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):
result = assess_own_branch_adoption(
issue_number=420,
requested_branch=REQ,
existing_branches=[{"name": "feat/issue-420-other-work"}],
)
self.assertEqual(result["outcome"], BLOCK_COMPETING)
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):
result = assess_own_branch_adoption(
issue_number=420,
requested_branch=REQ,
existing_branches=[{"name": "feat/issue-999-unrelated"}],
)
self.assertEqual(result["outcome"], NO_MATCH)
self.assertFalse(result["block"])
self.assertFalse(result["adopt"])
def test_empty_branch_list_is_normal_path(self):
result = assess_own_branch_adoption(
issue_number=420, requested_branch=REQ, existing_branches=[]
)
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):
def test_proof_has_all_required_fields(self):
assessment = assess_own_branch_adoption(
issue_number=420,
requested_branch=REQ,
existing_branches=[{"name": REQ, "commit_sha": "934688a"}],
)
proof = build_adoption_proof(
issue_number=420,
branch_name=REQ,
assessment=assessment,
open_pr_checked=True,
competing_lock_checked=True,
lock_file_path="/tmp/example-lock.json",
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.assertTrue(proof["no_existing_pr_proof"])
self.assertTrue(proof["no_competing_live_lock_proof"])
if __name__ == "__main__":
unittest.main()
+256
View File
@@ -0,0 +1,256 @@
"""Unit tests for keyed issue-lock storage (#443) and flock hardening (#438)."""
import json
import os
import sys
import tempfile
import threading
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)
def test_concurrent_bind_same_issue_only_one_wins(self):
barrier = threading.Barrier(2)
results: list[str | Exception] = []
def worker():
barrier.wait()
try:
ils.bind_session_lock(
_lock_record(worktree_path=f"/tmp/wt-{threading.get_ident()}")
)
results.append("ok")
except Exception as exc: # noqa: BLE001
results.append(exc)
threads = [threading.Thread(target=worker) for _ in range(2)]
for thread in threads:
thread.start()
for thread in threads:
thread.join()
successes = [item for item in results if item == "ok"]
failures = [item for item in results if isinstance(item, Exception)]
self.assertEqual(len(successes), 1)
self.assertEqual(len(failures), 1)
failure_text = str(failures[0]).lower()
self.assertTrue(
"active" in failure_text or "lock contention" in failure_text,
failures[0],
)
def test_verify_lock_for_mutation_blocks_stale_lock(self):
record = _lock_record(
work_lease=_lease("2000-01-01T00:00:00Z"),
)
record["pid"] = 999999
record["session_pid"] = 999999
result = ils.verify_lock_for_mutation(
record,
issue_number=420,
branch_name="feat/issue-420-server-code-parity",
worktree_path="/tmp/wt-420",
)
self.assertTrue(result["block"])
self.assertIn("not live", result["reasons"][0])
def test_list_live_locks_excludes_stale_records(self):
live_path = ils.lock_file_path(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
issue_number=420,
)
ils.save_lock_file(
live_path,
_lock_record(worktree_path="/tmp/wt-420"),
)
stale_path = ils.lock_file_path(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
issue_number=440,
)
ils.save_lock_file(
stale_path,
_lock_record(
issue_number=440,
branch_name="feat/issue-440-recovery",
work_lease=_lease("2000-01-01T00:00:00Z"),
worktree_path="/tmp/wt-440",
),
)
live = ils.list_live_locks(lock_dir=self.lock_dir)
self.assertEqual([entry["issue_number"] for entry in live], [420])
if __name__ == "__main__":
unittest.main()
+34 -26
View File
@@ -1,5 +1,4 @@
"""Tests for early duplicate-work detection (#400).""" """Tests for early duplicate-work detection (#400)."""
import json
import os import os
import sys import sys
import tempfile import tempfile
@@ -10,6 +9,7 @@ from unittest.mock import patch
sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import issue_lock_provenance import issue_lock_provenance
import issue_lock_store
import issue_work_duplicate_gate as dup_gate import issue_work_duplicate_gate as dup_gate
import mcp_server import mcp_server
from issue_work_duplicate_gate import ( from issue_work_duplicate_gate import (
@@ -123,8 +123,9 @@ class TestDuplicateReportOutcome(unittest.TestCase):
class TestInjectableDuplicateFetcher(unittest.TestCase): class TestInjectableDuplicateFetcher(unittest.TestCase):
@patch("mcp_server.api_get_all", return_value=[])
@patch("mcp_server.get_auth_header", return_value="token x") @patch("mcp_server.get_auth_header", return_value="token x")
def test_lock_issue_uses_injected_fetcher(self, _auth): def test_lock_issue_uses_injected_fetcher(self, _auth, _api):
seen = {} seen = {}
def fetcher(h, o, r, auth, issue_number): def fetcher(h, o, r, auth, issue_number):
@@ -141,26 +142,29 @@ class TestInjectableDuplicateFetcher(unittest.TestCase):
"porcelain_status": "", "porcelain_status": "",
"base_equivalent": True, "base_equivalent": True,
}, },
), patch.dict(os.environ, { ):
"GITEA_ALLOWED_OPERATIONS": "gitea.issue.comment", with tempfile.TemporaryDirectory() as lock_dir:
}, clear=True): with patch.dict(os.environ, {
with patch.object(mcp_server, "ISSUE_LOCK_FILE", tempfile.mktemp()): "GITEA_ALLOWED_OPERATIONS": "gitea.issue.comment",
mcp_server.gitea_lock_issue( "GITEA_ISSUE_LOCK_DIR": lock_dir,
issue_number=400, }, clear=True):
branch_name="feat/issue-400-duplicate-work-preflight", mcp_server.gitea_lock_issue(
remote="prgs", issue_number=400,
) branch_name="feat/issue-400-duplicate-work-preflight",
remote="prgs",
)
self.assertEqual(seen["issue_number"], 400) self.assertEqual(seen["issue_number"], 400)
class TestMcpDuplicateRecheck(unittest.TestCase): class TestMcpDuplicateRecheck(unittest.TestCase):
def setUp(self): def setUp(self):
self._dir = tempfile.TemporaryDirectory() self._dir = tempfile.TemporaryDirectory()
self.lock_path = os.path.join(self._dir.name, "gitea_issue_lock.json") self._env_patch = patch.dict(
self._lock_patch = patch.object( os.environ,
mcp_server, "ISSUE_LOCK_FILE", self.lock_path {"GITEA_ISSUE_LOCK_DIR": self._dir.name},
clear=False,
) )
self._lock_patch.start() self._env_patch.start()
self._remotes = patch.dict(mcp_server.REMOTES, { self._remotes = patch.dict(mcp_server.REMOTES, {
"prgs": {"host": "gitea.example.com", "org": "Example-Org", "prgs": {"host": "gitea.example.com", "org": "Example-Org",
"repo": "Example-Repo"}, "repo": "Example-Repo"},
@@ -173,6 +177,7 @@ class TestMcpDuplicateRecheck(unittest.TestCase):
self._dir.cleanup() self._dir.cleanup()
def _write_lock(self, issue_number=400, branch="feat/issue-400-x"): def _write_lock(self, issue_number=400, branch="feat/issue-400-x"):
worktree_path = os.path.realpath(os.getcwd())
work_lease = { work_lease = {
"operation_type": "author_issue_work", "operation_type": "author_issue_work",
"issue_number": issue_number, "issue_number": issue_number,
@@ -180,17 +185,19 @@ class TestMcpDuplicateRecheck(unittest.TestCase):
"claimant": {"username": "test-user", "profile": "test-author"}, "claimant": {"username": "test-user", "profile": "test-author"},
"expires_at": "2999-01-01T00:00:00Z", "expires_at": "2999-01-01T00:00:00Z",
} }
with open(self.lock_path, "w", encoding="utf-8") as fh: issue_lock_store.bind_session_lock({
json.dump({ "issue_number": issue_number,
"issue_number": issue_number, "branch_name": branch,
"branch_name": branch, "remote": "prgs",
"remote": "prgs", "org": "Example-Org",
"work_lease": work_lease, "repo": "Example-Repo",
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance( "worktree_path": worktree_path,
tool="gitea_lock_issue", "work_lease": work_lease,
claimant=work_lease.get("claimant"), "lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
), tool="gitea_lock_issue",
}, fh) claimant=work_lease.get("claimant"),
),
})
@patch("mcp_server._assess_issue_duplicate_gate") @patch("mcp_server._assess_issue_duplicate_gate")
@patch("mcp_server.get_profile", return_value={ @patch("mcp_server.get_profile", return_value={
@@ -254,6 +261,7 @@ class TestMcpDuplicateRecheck(unittest.TestCase):
base="master", base="master",
body="Closes #400", body="Closes #400",
remote="prgs", remote="prgs",
worktree_path=os.path.realpath(os.getcwd()),
) )
self.assertFalse(result["success"]) self.assertFalse(result["success"])
self.assertIsNone(result.get("number")) self.assertIsNone(result.get("number"))
+8 -5
View File
@@ -123,16 +123,19 @@ class TestShaCannotBypassSelfReview(unittest.TestCase):
@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)
def test_review_tool_refuses_self_approval_despite_sha(self, _auth, mock_api, mock_get_all): def test_review_tool_refuses_self_approval_despite_sha(self, _auth, mock_api, mock_get_all):
mock_get_all.return_value = [{"number": 9, "title": "PR 9", "state": "open", "head": {"ref": "branch9", "sha": "abc1234"}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "jcwalker3"}}] from mcp_server import init_review_decision_lock
from tests.test_mcp_server import FULL_HEAD_SHA, _seed_ready_review_decision
head_sha = FULL_HEAD_SHA
mock_get_all.return_value = [{"number": 9, "title": "PR 9", "state": "open", "head": {"ref": "branch9", "sha": head_sha}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "jcwalker3"}}]
mock_api.side_effect = [ mock_api.side_effect = [
{"login": "jcwalker3"}, # /user (inventory) {"login": "jcwalker3"}, # /user (inventory)
{"login": "jcwalker3"}, # /user (submit eligibility) {"login": "jcwalker3"}, # /user (submit eligibility)
{"user": {"login": "jcwalker3"}, "state": "open", "head": {"sha": "abc1234"}, "mergeable": True}, # /pulls/9 {"user": {"login": "jcwalker3"}, "state": "open", "head": {"sha": head_sha}, "mergeable": True}, # /pulls/9
] ]
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
init_review_decision_lock("prgs", "review_pr") init_review_decision_lock("prgs", "review_pr")
mcp_server.gitea_load_review_workflow() with patch("mcp_server._list_pr_lease_comments", return_value=[]):
gitea_mark_final_review_decision(9, "approve", remote="prgs") _seed_ready_review_decision(9, "approve", sha=head_sha, remote="prgs")
env = self._env(SHA_WOULD_BE_REVIEWER, "reviewer") env = self._env(SHA_WOULD_BE_REVIEWER, "reviewer")
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
r = gitea_review_pr( r = gitea_review_pr(
+398 -135
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,8 +46,10 @@ 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"
FULL_HEAD_SHA = "a" * 40
_NO_BLOCKER_FEEDBACK = { _NO_BLOCKER_FEEDBACK = {
"success": True, "success": True,
@@ -66,6 +69,7 @@ def _mark_request_changes_ready(pr_number=8, **kwargs):
suppression feedback fetch stubbed to 'no existing blocker'.""" suppression feedback fetch stubbed to 'no existing blocker'."""
with patch("mcp_server.gitea_get_pr_review_feedback", with patch("mcp_server.gitea_get_pr_review_feedback",
return_value=dict(_NO_BLOCKER_FEEDBACK)): return_value=dict(_NO_BLOCKER_FEEDBACK)):
kwargs.setdefault("expected_head_sha", "abc123")
return gitea_mark_final_review_decision( return gitea_mark_final_review_decision(
pr_number, "request_changes", **kwargs) pr_number, "request_changes", **kwargs)
@@ -86,6 +90,7 @@ def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"):
_DEFAULT_LEASE_SESSION = "mcp-test-reviewer-lease" _DEFAULT_LEASE_SESSION = "mcp-test-reviewer-lease"
_NO_PR_WORK_LEASE_BLOCK = {"block": False, "reasons": [], "mutation_allowed": True}
def _reviewer_lease_comment( def _reviewer_lease_comment(
@@ -143,6 +148,46 @@ def _install_owned_reviewer_lease(
) )
def _seed_ready_review_decision(
pr_number,
action,
*,
sha=FULL_HEAD_SHA,
remote="prgs",
org=None,
repo=None,
):
"""Mark the review-decision lock ready without mark_final API calls (#399)."""
import mcp_server as _m
resolved_org, resolved_repo = org, repo
if remote in _m.REMOTES:
_, resolved_org, resolved_repo = _m._resolve(remote, None, org, repo)
profile_name = (_m.get_profile().get("profile_name") or "").strip()
session_lock = (
(os.environ.get(_m.SESSION_PROFILE_LOCK_ENV) or "").strip()
or profile_name
)
_m._save_review_decision_lock({
"task": "review_pr",
"remote": remote,
"session_pid": os.getpid(),
"session_profile": profile_name,
"session_profile_lock": session_lock,
"final_review_decision_ready": True,
"ready_pr_number": pr_number,
"ready_action": action,
"ready_expected_head_sha": sha,
"ready_remote": remote,
"ready_org": resolved_org,
"ready_repo": resolved_repo,
"live_mutations": [],
"correction_authorized": False,
"correction_reason": None,
})
_m.gitea_load_review_workflow()
# 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": (
@@ -180,6 +225,7 @@ def _sample_issue_lock(issue_number=123, branch_name="feat/x", **overrides):
"remote": "dadeschools", "remote": "dadeschools",
"org": "Scaled-Tech-Consulting", "org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools", "repo": "Gitea-Tools",
"worktree_path": "/tmp/test-worktree",
"work_lease": work_lease, "work_lease": work_lease,
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance( "lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
tool="gitea_lock_issue", tool="gitea_lock_issue",
@@ -195,6 +241,17 @@ def _clear_duplicate_context_fetcher(*_args, **_kwargs):
return [], [], {"status": "not_claimed"} return [], [], {"status": "not_claimed"}
def _bind_test_lock(**overrides) -> str:
remote = overrides.get("remote", "dadeschools")
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)
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Create Issue # Create Issue
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -257,18 +314,21 @@ class TestCreatePR(unittest.TestCase):
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, _dup_fetcher):
@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")
@@ -282,30 +342,42 @@ class TestCreatePR(unittest.TestCase):
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, _dup_fetcher):
@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")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -616,6 +688,17 @@ class TestMergePR(unittest.TestCase):
self.addCleanup(self._auth_identity_patch.stop) self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop) self.addCleanup(self._lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease) self.addCleanup(reviewer_pr_lease.clear_session_lease)
self._pr_lease_comments_patch = patch(
"mcp_server._list_pr_lease_comments", return_value=[]
)
self._pr_lease_comments_patch.start()
self.addCleanup(self._pr_lease_comments_patch.stop)
self._pr_work_lease_patch = patch(
"mcp_server._pr_work_lease_reviewer_block",
return_value=dict(_NO_PR_WORK_LEASE_BLOCK),
)
self._pr_work_lease_patch.start()
self.addCleanup(self._pr_work_lease_patch.stop)
def _pr(self, author, state="open", sha="abc123", mergeable=True): def _pr(self, author, state="open", sha="abc123", mergeable=True):
return { return {
@@ -689,7 +772,8 @@ class TestMergePR(unittest.TestCase):
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr( r = gitea_merge_pr(
pr_number=8, confirmation=self._confirm(8), pr_number=8, confirmation=self._confirm(8),
expected_changed_files=["b.py", "a.py"], remote="prgs") expected_changed_files=["b.py", "a.py"],
expected_head_sha="abc123", remote="prgs")
self.assertTrue(r["performed"]) self.assertTrue(r["performed"])
# -- read-back / cleanup surfacing (#98) ----------------------------------- # -- read-back / cleanup surfacing (#98) -----------------------------------
@@ -707,8 +791,10 @@ class TestMergePR(unittest.TestCase):
env = {"GITEA_PROFILE_NAME": "gitea-merger", env = {"GITEA_PROFILE_NAME": "gitea-merger",
"GITEA_ALLOWED_OPERATIONS": "read,merge"} "GITEA_ALLOWED_OPERATIONS": "read,merge"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr(pr_number=8, confirmation=self._confirm(8), r = gitea_merge_pr(
remote="prgs") pr_number=8, confirmation=self._confirm(8),
expected_head_sha="abc123", remote="prgs",
)
# The merge itself is still reported performed/successful. # The merge itself is still reported performed/successful.
self.assertTrue(r["performed"]) self.assertTrue(r["performed"])
self.assertEqual(r["merge_result"], "PR #8 merged via 'merge'.") self.assertEqual(r["merge_result"], "PR #8 merged via 'merge'.")
@@ -736,8 +822,10 @@ class TestMergePR(unittest.TestCase):
env = {"GITEA_PROFILE_NAME": "gitea-merger", env = {"GITEA_PROFILE_NAME": "gitea-merger",
"GITEA_ALLOWED_OPERATIONS": "read,merge"} "GITEA_ALLOWED_OPERATIONS": "read,merge"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr(pr_number=8, confirmation=self._confirm(8), r = gitea_merge_pr(
remote="prgs") pr_number=8, confirmation=self._confirm(8),
expected_head_sha="abc123", remote="prgs",
)
self.assertTrue(r["performed"]) self.assertTrue(r["performed"])
self.assertEqual(r["merge_commit"], "c9") self.assertEqual(r["merge_commit"], "c9")
self.assertTrue(r["cleanup_status"].startswith("skipped (cleanup error:")) self.assertTrue(r["cleanup_status"].startswith("skipped (cleanup error:"))
@@ -751,7 +839,7 @@ class TestMergePR(unittest.TestCase):
env = {"GITEA_PROFILE_NAME": "gitea-merger", env = {"GITEA_PROFILE_NAME": "gitea-merger",
"GITEA_ALLOWED_OPERATIONS": "read,merge"} "GITEA_ALLOWED_OPERATIONS": "read,merge"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr(pr_number=8, confirmation="", remote="prgs") r = gitea_merge_pr(pr_number=8, confirmation="", expected_head_sha="abc123", remote="prgs")
self.assertFalse(r["performed"]) self.assertFalse(r["performed"])
self.assertTrue(any("explicit confirmation required" in x for x in r["reasons"])) self.assertTrue(any("explicit confirmation required" in x for x in r["reasons"]))
mock_api.assert_not_called() mock_api.assert_not_called()
@@ -762,7 +850,7 @@ class TestMergePR(unittest.TestCase):
env = {"GITEA_PROFILE_NAME": "gitea-merger", env = {"GITEA_PROFILE_NAME": "gitea-merger",
"GITEA_ALLOWED_OPERATIONS": "read,merge"} "GITEA_ALLOWED_OPERATIONS": "read,merge"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr(pr_number=8, confirmation="MERGE PR 9", remote="prgs") r = gitea_merge_pr(pr_number=8, confirmation="MERGE PR 9", expected_head_sha="abc123", remote="prgs")
self.assertFalse(r["performed"]) self.assertFalse(r["performed"])
mock_api.assert_not_called() mock_api.assert_not_called()
@@ -896,7 +984,8 @@ class TestMergePR(unittest.TestCase):
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr( r = gitea_merge_pr(
pr_number=8, confirmation=self._confirm(8), pr_number=8, confirmation=self._confirm(8),
expected_changed_files=["a.py", "b.py"], remote="prgs") expected_changed_files=["a.py", "b.py"],
expected_head_sha="abc123", remote="prgs")
self.assertFalse(r["performed"]) self.assertFalse(r["performed"])
self.assertIn( self.assertIn(
"PR changed files do not match expected_changed_files (fail closed)", "PR changed files do not match expected_changed_files (fail closed)",
@@ -909,7 +998,7 @@ class TestMergePR(unittest.TestCase):
def test_invalid_merge_method_rejected(self, mock_api): def test_invalid_merge_method_rejected(self, mock_api):
with patch.dict(os.environ, {}, clear=True): with patch.dict(os.environ, {}, clear=True):
r = gitea_merge_pr( r = gitea_merge_pr(
pr_number=8, confirmation="MERGE PR 8", do="octopus", remote="prgs") pr_number=8, confirmation="MERGE PR 8", do="octopus", expected_head_sha="abc123", remote="prgs")
self.assertFalse(r["performed"]) self.assertFalse(r["performed"])
self.assertTrue(any("unknown merge method" in x for x in r["reasons"])) self.assertTrue(any("unknown merge method" in x for x in r["reasons"]))
mock_api.assert_not_called() mock_api.assert_not_called()
@@ -927,7 +1016,9 @@ class TestMergePR(unittest.TestCase):
"GITEA_TOKEN": "super-secret-token"} "GITEA_TOKEN": "super-secret-token"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr( r = gitea_merge_pr(
pr_number=8, confirmation=self._confirm(8), remote="prgs") pr_number=8, confirmation=self._confirm(8),
expected_head_sha="abc123", remote="prgs",
)
blob = repr(r).lower() blob = repr(r).lower()
for secret in ("super-secret-token", "authorization", "basic ", FAKE_AUTH.lower()): for secret in ("super-secret-token", "authorization", "basic ", FAKE_AUTH.lower()):
self.assertNotIn(secret, blob) self.assertNotIn(secret, blob)
@@ -944,12 +1035,37 @@ class TestMergePR(unittest.TestCase):
"GITEA_ALLOWED_OPERATIONS": "read,merge"} "GITEA_ALLOWED_OPERATIONS": "read,merge"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr( r = gitea_merge_pr(
pr_number=8, confirmation=self._confirm(8), remote="prgs") pr_number=8, confirmation=self._confirm(8),
expected_head_sha="abc123", remote="prgs",
)
self.assertFalse(r["performed"]) self.assertFalse(r["performed"])
blob = repr(r) blob = repr(r)
self.assertIn("[REDACTED]", blob) self.assertIn("[REDACTED]", blob)
self.assertNotIn("abc-secret-xyz", blob) self.assertNotIn("abc-secret-xyz", blob)
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_merge_blocked_on_stale_approval_head(self, _auth, mock_api):
old_sha = "8b61c4b41f1b49b271ed3b99657431cf06eeda3e"
new_sha = "3e4b721d60e97147ba0704773cf57cd0d42cbe31"
mock_api.side_effect = [
{"login": "merger-bot"}, self._pr("author-bot", sha=new_sha),
self._pr("author-bot", sha=new_sha),
[_formal_review("reviewer-bot", "APPROVED", sha=old_sha)],
]
env = {"GITEA_PROFILE_NAME": "gitea-merger",
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr(
pr_number=8, confirmation=self._confirm(8), remote="prgs")
self.assertFalse(r["performed"])
self.assertTrue(r.get("approval_visible"))
self.assertFalse(r.get("approval_at_current_head"))
self.assertTrue(any("stale approval" in x for x in r["reasons"]))
self.assertTrue(any(old_sha in x for x in r["reasons"]))
self.assertTrue(any(new_sha in x for x in r["reasons"]))
self._assert_no_merge_call(mock_api)
@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)
def test_merge_blocked_without_visible_approval(self, _auth, mock_api): def test_merge_blocked_without_visible_approval(self, _auth, mock_api):
@@ -962,7 +1078,9 @@ class TestMergePR(unittest.TestCase):
"GITEA_ALLOWED_OPERATIONS": "read,merge"} "GITEA_ALLOWED_OPERATIONS": "read,merge"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr( r = gitea_merge_pr(
pr_number=8, confirmation=self._confirm(8), remote="prgs") pr_number=8, confirmation=self._confirm(8),
expected_head_sha="abc123", remote="prgs",
)
self.assertFalse(r["performed"]) self.assertFalse(r["performed"])
self.assertFalse(r.get("approval_visible")) self.assertFalse(r.get("approval_visible"))
self.assertTrue(any("no visible APPROVED review" in x for x in r["reasons"])) self.assertTrue(any("no visible APPROVED review" in x for x in r["reasons"]))
@@ -983,7 +1101,9 @@ class TestMergePR(unittest.TestCase):
"GITEA_ALLOWED_OPERATIONS": "read,merge"} "GITEA_ALLOWED_OPERATIONS": "read,merge"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr( r = gitea_merge_pr(
pr_number=8, confirmation=self._confirm(8), remote="prgs") pr_number=8, confirmation=self._confirm(8),
expected_head_sha="abc123", remote="prgs",
)
self.assertFalse(r["performed"]) self.assertFalse(r["performed"])
self.assertTrue(r.get("has_blocking_change_requests")) self.assertTrue(r.get("has_blocking_change_requests"))
self.assertTrue(any("REQUEST_CHANGES" in x for x in r["reasons"])) self.assertTrue(any("REQUEST_CHANGES" in x for x in r["reasons"]))
@@ -1084,15 +1204,18 @@ class TestReviewPR(unittest.TestCase):
"forbidden_operations": [], "forbidden_operations": [],
"base_url": None, "base_url": None,
} }
mock_get_all.return_value = [{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": "abc1234"}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "jcwalker3"}}] head_sha = FULL_HEAD_SHA
mock_get_all.return_value = [{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": head_sha}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "jcwalker3"}}]
# mock_api responses: 1) /user (inventory), 2) /user (eligibility), 3) /pulls/1 (eligibility) # mock_api responses: 1) /user (inventory), 2) /user (eligibility), 3) /pulls/1 (eligibility)
mock_api.side_effect = [ mock_api.side_effect = [
{"login": "jcwalker3"}, # /api/v1/user (inventory) {"login": "jcwalker3"}, # /api/v1/user (inventory)
{"login": "jcwalker3"}, # /api/v1/user (submit eligibility) {"login": "jcwalker3"}, # /api/v1/user (submit eligibility)
{"user": {"login": "jcwalker3"}, "state": "open", "head": {"sha": "abc1234"}, "mergeable": True}, # /pulls/1 {"user": {"login": "jcwalker3"}, "state": "open", "head": {"sha": head_sha}, "mergeable": True}, # /pulls/1
] ]
_init_reviewer_session("prgs") from mcp_server import init_review_decision_lock
gitea_mark_final_review_decision(1, "approve", remote="prgs") init_review_decision_lock("prgs", "review_pr")
with patch("mcp_server._list_pr_lease_comments", return_value=[]):
_seed_ready_review_decision(1, "approve", sha=head_sha, remote="prgs")
result = gitea_review_pr( result = gitea_review_pr(
pr_number=1, pr_number=1,
event="APPROVE", event="APPROVE",
@@ -1748,7 +1871,7 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
"""Block incidental live review mutations during validation.""" """Block incidental live review mutations during validation."""
PR = 203 PR = 203
SHA = "abc123" SHA = FULL_HEAD_SHA
def _pr(self, author, sha=SHA): def _pr(self, author, sha=SHA):
return { return {
@@ -1773,6 +1896,18 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
self.addCleanup(self._auth_identity_patch.stop) self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop) self.addCleanup(self._lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease) self.addCleanup(reviewer_pr_lease.clear_session_lease)
self._pr_lease_comments_patch = patch(
"mcp_server._list_pr_lease_comments", return_value=[]
)
self._pr_lease_comments_patch.start()
self._pr_work_lease_patch = patch(
"mcp_server._pr_work_lease_reviewer_block",
return_value=dict(_NO_PR_WORK_LEASE_BLOCK),
)
self._pr_work_lease_patch.start()
self.addCleanup(self._pr_lease_comments_patch.stop)
self.addCleanup(self._pr_work_lease_patch.stop)
def _env(self): def _env(self):
return patch.dict(os.environ, { return patch.dict(os.environ, {
@@ -1870,15 +2005,28 @@ class TestSubmitPrReview(unittest.TestCase):
import reviewer_pr_lease import reviewer_pr_lease
_init_reviewer_session("prgs") _init_reviewer_session("prgs")
gitea_mark_final_review_decision(8, "approve", remote="prgs")
self._lease_patch = _install_owned_reviewer_lease(8) self._lease_patch = _install_owned_reviewer_lease(8)
self._lease_patch.start() self._lease_patch.start()
self._auth_identity_patch = patch( self._auth_identity_patch = patch(
"mcp_server._authenticated_username", return_value="reviewer-bot" "mcp_server._authenticated_username", return_value="reviewer-bot"
) )
self._auth_identity_patch.start() self._auth_identity_patch.start()
self._pr_lease_comments_patch = patch(
"mcp_server._list_pr_lease_comments", return_value=[]
)
self._pr_lease_comments_patch.start()
self._pr_work_lease_patch = patch(
"mcp_server._pr_work_lease_reviewer_block",
return_value=dict(_NO_PR_WORK_LEASE_BLOCK),
)
self._pr_work_lease_patch.start()
gitea_mark_final_review_decision(
8, "approve", remote="prgs", expected_head_sha="abc123",
)
self.addCleanup(self._auth_identity_patch.stop) self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop) self.addCleanup(self._lease_patch.stop)
self.addCleanup(self._pr_lease_comments_patch.stop)
self.addCleanup(self._pr_work_lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease) 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):
@@ -2039,10 +2187,11 @@ class TestSubmitPrReview(unittest.TestCase):
@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)
def test_comment_succeeds_when_review_eligible(self, _auth, mock_api): def test_comment_succeeds_when_review_eligible(self, _auth, mock_api):
gitea_mark_final_review_decision(8, "comment", remote="prgs")
mock_api.side_effect = [ mock_api.side_effect = [
{"login": "reviewer-bot"}, self._pr("author-bot"),
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 3}, {"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 3},
] ]
gitea_mark_final_review_decision(8, "comment", remote="prgs", expected_head_sha="abc123")
env = {"GITEA_PROFILE_NAME": "gitea-reviewer", env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
"GITEA_ALLOWED_OPERATIONS": "read,review"} "GITEA_ALLOWED_OPERATIONS": "read,review"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
@@ -2058,12 +2207,15 @@ class TestSubmitPrReview(unittest.TestCase):
with patch("mcp_server.get_auth_header", return_value=FAKE_AUTH), \ with patch("mcp_server.get_auth_header", return_value=FAKE_AUTH), \
patch("mcp_server.api_request") as mock_api: patch("mcp_server.api_request") as mock_api:
mock_api.side_effect = [ mock_api.side_effect = [
{"login": "jcwalker3"}, self._pr("jcwalker3"),
{"login": "jcwalker3"}, self._pr("jcwalker3"), {"id": 4}, {"login": "jcwalker3"}, self._pr("jcwalker3"), {"id": 4},
] ]
env = {"GITEA_PROFILE_NAME": "gitea-reviewer", env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
"GITEA_ALLOWED_OPERATIONS": "read,review"} "GITEA_ALLOWED_OPERATIONS": "read,review"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
gitea_mark_final_review_decision(8, "comment", remote="prgs") gitea_mark_final_review_decision(
8, "comment", remote="prgs", expected_head_sha="abc123",
)
r = gitea_submit_pr_review( r = gitea_submit_pr_review(
pr_number=8, action="comment", body="note", remote="prgs", pr_number=8, action="comment", body="note", remote="prgs",
final_review_decision_ready=True, final_review_decision_ready=True,
@@ -2106,14 +2258,14 @@ class TestSubmitPrReview(unittest.TestCase):
with patch("mcp_server.get_auth_header", return_value=FAKE_AUTH), \ with patch("mcp_server.get_auth_header", return_value=FAKE_AUTH), \
patch("mcp_server.api_request") as mock_api: patch("mcp_server.api_request") as mock_api:
mock_api.side_effect = [ mock_api.side_effect = [
{"login": "reviewer-bot"}, self._pr("author-bot", sha="abc123"), {"login": "reviewer-bot"}, self._pr("author-bot", sha="deadbeef"),
] ]
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):
r = gitea_submit_pr_review( r = gitea_submit_pr_review(
pr_number=8, action="approve", pr_number=8, action="approve",
expected_head_sha="deadbeef", remote="prgs", expected_head_sha="abc123", remote="prgs",
final_review_decision_ready=True, final_review_decision_ready=True,
) )
self.assertFalse(r["performed"]) self.assertFalse(r["performed"])
@@ -2164,7 +2316,7 @@ class TestSubmitPrReview(unittest.TestCase):
"GITEA_ALLOWED_OPERATIONS": "read,review,approve", "GITEA_ALLOWED_OPERATIONS": "read,review,approve",
"GITEA_TOKEN": "super-secret-token"} "GITEA_TOKEN": "super-secret-token"}
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
gitea_mark_final_review_decision(5, "approve", remote="prgs") gitea_mark_final_review_decision(5, "approve", remote="prgs", expected_head_sha="abc123")
r = gitea_submit_pr_review( r = gitea_submit_pr_review(
pr_number=5, action="approve", remote="prgs", pr_number=5, action="approve", remote="prgs",
final_review_decision_ready=True, final_review_decision_ready=True,
@@ -2184,7 +2336,6 @@ 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")
r = gitea_submit_pr_review( r = gitea_submit_pr_review(
pr_number=8, action="approve", remote="prgs", pr_number=8, action="approve", remote="prgs",
final_review_decision_ready=True, final_review_decision_ready=True,
@@ -2274,7 +2425,7 @@ class TestSubmitPrReview(unittest.TestCase):
def test_mark_final_decision_rejects_remote_mismatch(self): def test_mark_final_decision_rejects_remote_mismatch(self):
_init_reviewer_session("prgs") _init_reviewer_session("prgs")
r = gitea_mark_final_review_decision(8, "approve", remote="dadeschools") r = gitea_mark_final_review_decision(8, "approve", remote="dadeschools", expected_head_sha="abc123")
self.assertFalse(r["marked_ready"]) self.assertFalse(r["marked_ready"])
self.assertTrue(any("does not match locked remote" in x for x in r["reasons"])) self.assertTrue(any("does not match locked remote" in x for x in r["reasons"]))
@@ -2286,28 +2437,30 @@ class TestSubmitPrReview(unittest.TestCase):
{"id": 42, "state": "APPROVED"}, {"id": 42, "state": "APPROVED"},
[_formal_review("reviewer-bot", "APPROVED", review_id=42)], [_formal_review("reviewer-bot", "APPROVED", review_id=42)],
{"login": "reviewer-bot"}, self._pr("author-bot"), {"login": "reviewer-bot"}, self._pr("author-bot"),
{"login": "reviewer-bot"}, self._pr("author-bot"),
{"id": 43, "state": "REQUEST_CHANGES"}, {"id": 43, "state": "REQUEST_CHANGES"},
[_formal_review("reviewer-bot", "REQUEST_CHANGES", review_id=43)], [_formal_review("reviewer-bot", "REQUEST_CHANGES", review_id=43)],
] ]
env = {"GITEA_PROFILE_NAME": "gitea-reviewer", env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
"GITEA_ALLOWED_OPERATIONS": "read,review,approve,request_changes"} "GITEA_ALLOWED_OPERATIONS": "read,review,approve,request_changes"}
gitea_mark_final_review_decision(8, "approve", remote="prgs") with patch("mcp_server.gitea_get_pr_review_feedback",
with patch.dict(os.environ, env, clear=True): return_value=dict(_NO_BLOCKER_FEEDBACK)):
first = gitea_submit_pr_review( with patch.dict(os.environ, env, clear=True):
pr_number=8, action="approve", remote="prgs", first = gitea_submit_pr_review(
final_review_decision_ready=True, pr_number=8, action="approve", remote="prgs",
) final_review_decision_ready=True,
auth = gitea_authorize_review_correction( )
prior_review_id=42, auth = gitea_authorize_review_correction(
prior_review_state="approve", prior_review_id=42,
reason="operator approved correcting mistaken approve", prior_review_state="approve",
operator_authorized=True, reason="operator approved correcting mistaken approve",
) operator_authorized=True,
_mark_request_changes_ready(remote="prgs") )
second = gitea_submit_pr_review( _mark_request_changes_ready(remote="prgs")
pr_number=8, action="request_changes", remote="prgs", second = gitea_submit_pr_review(
final_review_decision_ready=True, pr_number=8, action="request_changes", remote="prgs",
) final_review_decision_ready=True,
)
self.assertTrue(first["performed"]) self.assertTrue(first["performed"])
self.assertTrue(auth["authorized"]) self.assertTrue(auth["authorized"])
self.assertTrue(second["performed"]) self.assertTrue(second["performed"])
@@ -2319,7 +2472,7 @@ class TestSubmitPrReview(unittest.TestCase):
"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):
# Mark decision for specific remote, org, repo # Mark decision for specific remote, org, repo
gitea_mark_final_review_decision(8, "approve", remote="prgs", org="MyOrg", repo="MyRepo") gitea_mark_final_review_decision(8, "approve", remote="prgs", expected_head_sha="abc123", org="MyOrg", repo="MyRepo")
# Mismatched remote # Mismatched remote
r = gitea_submit_pr_review( r = gitea_submit_pr_review(
@@ -2362,6 +2515,11 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
self.mock_audit = patch("gitea_audit.write_event").start() self.mock_audit = patch("gitea_audit.write_event").start()
# gitea.pr.close: closing a PR via gitea_edit_pr is capability-gated (#216). # gitea.pr.close: closing a PR via gitea_edit_pr is capability-gated (#216).
patch("mcp_server.get_profile", return_value={"profile_name": "test", "allowed_operations": ["read", "merge", "edit", "close", "gitea.pr.close", "gitea.issue.close"], "audit_label": "test", "forbidden_operations": []}).start() patch("mcp_server.get_profile", return_value={"profile_name": "test", "allowed_operations": ["read", "merge", "edit", "close", "gitea.pr.close", "gitea.issue.close"], "audit_label": "test", "forbidden_operations": []}).start()
patch("mcp_server._list_pr_lease_comments", return_value=[]).start()
patch(
"mcp_server._pr_work_lease_reviewer_block",
return_value=dict(_NO_PR_WORK_LEASE_BLOCK),
).start()
def tearDown(self): def tearDown(self):
patch.stopall() patch.stopall()
@@ -2408,7 +2566,8 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
def test_merge_pr_with_closes_removes_label(self): def test_merge_pr_with_closes_removes_label(self):
import reviewer_pr_lease import reviewer_pr_lease
lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123") head_sha = FULL_HEAD_SHA
lease_patch = _install_owned_reviewer_lease(1, head_sha=head_sha)
lease_patch.start() lease_patch.start()
self.addCleanup(lease_patch.stop) self.addCleanup(lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease) self.addCleanup(reviewer_pr_lease.clear_session_lease)
@@ -2417,12 +2576,12 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
if method == "GET" and "/user" in url: if method == "GET" and "/user" in url:
return {"login": "merger"} return {"login": "merger"}
if method == "GET" and url.endswith("/reviews"): if method == "GET" and url.endswith("/reviews"):
return [_formal_review("reviewer", "APPROVED", sha="sha123")] return [_formal_review("reviewer", "APPROVED", sha=head_sha)]
if method == "GET" and "pulls/1" in url and "/files" not in url: if method == "GET" and "pulls/1" in url and "/files" not in url:
return { return {
"user": {"login": "author"}, "user": {"login": "author"},
"state": "open", "state": "open",
"head": {"sha": "sha123", "ref": "feat/my-branch"}, "head": {"sha": head_sha, "ref": "feat/my-branch"},
"base": {"ref": "main"}, "base": {"ref": "main"},
"mergeable": True, "mergeable": True,
"merged_commit_sha": "merge123", "merged_commit_sha": "merge123",
@@ -2442,14 +2601,18 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
return {} return {}
self.mock_api.side_effect = api_side_effect self.mock_api.side_effect = api_side_effect
res = gitea_merge_pr(pr_number=1, confirmation="MERGE PR 1", do="merge") res = gitea_merge_pr(
pr_number=1, confirmation="MERGE PR 1", do="merge",
expected_head_sha=head_sha,
)
self.assertTrue(res["performed"]) self.assertTrue(res["performed"])
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 import reviewer_pr_lease
lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123") head_sha = FULL_HEAD_SHA
lease_patch = _install_owned_reviewer_lease(1, head_sha=head_sha)
lease_patch.start() lease_patch.start()
self.addCleanup(lease_patch.stop) self.addCleanup(lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease) self.addCleanup(reviewer_pr_lease.clear_session_lease)
@@ -2458,12 +2621,12 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
if method == "GET" and "/user" in url: if method == "GET" and "/user" in url:
return {"login": "merger"} return {"login": "merger"}
if method == "GET" and url.endswith("/reviews"): if method == "GET" and url.endswith("/reviews"):
return [_formal_review("reviewer", "APPROVED", sha="sha123")] return [_formal_review("reviewer", "APPROVED", sha=head_sha)]
if method == "GET" and "pulls/1" in url and "/files" not in url: if method == "GET" and "pulls/1" in url and "/files" not in url:
return { return {
"user": {"login": "author"}, "user": {"login": "author"},
"state": "open", "state": "open",
"head": {"sha": "sha123", "ref": "fix/issue-123-slug"}, "head": {"sha": head_sha, "ref": "fix/issue-123-slug"},
"base": {"ref": "main"}, "base": {"ref": "main"},
"mergeable": True, "mergeable": True,
"merged_commit_sha": "merge123", "merged_commit_sha": "merge123",
@@ -2483,7 +2646,10 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
return {} return {}
self.mock_api.side_effect = api_side_effect self.mock_api.side_effect = api_side_effect
res = gitea_merge_pr(pr_number=1, confirmation="MERGE PR 1", do="merge") res = gitea_merge_pr(
pr_number=1, confirmation="MERGE PR 1", do="merge",
expected_head_sha=head_sha,
)
self.assertTrue(res["performed"]) self.assertTrue(res["performed"])
self.assertEqual(res["cleanup_status"].get(123), "released") self.assertEqual(res["cleanup_status"].get(123), "released")
@@ -3191,7 +3357,12 @@ 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( self._dup_fetcher_patcher = patch(
"mcp_server.issue_duplicate_context_fetcher", "mcp_server.issue_duplicate_context_fetcher",
@@ -3202,15 +3373,21 @@ class TestIssueLocking(unittest.TestCase):
def tearDown(self): def tearDown(self):
self._dup_fetcher_patcher.stop() 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.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_success(self, _auth, _git_state): def test_lock_issue_success(self, _auth, _api, _git_state):
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")
@@ -3220,9 +3397,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)
@@ -3278,32 +3454,81 @@ class TestIssueLocking(unittest.TestCase):
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("remote branch(es) already match issue pattern", str(ctx.exception)) self.assertIn("remote branch(es) already match issue pattern", str(ctx.exception))
def test_lock_issue_blocks_active_same_operation_lease(self): @patch(
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: "mcp_server.issue_lock_worktree.read_worktree_git_state",
json.dump({ return_value=_clean_master_git_state_for_lock(),
)
@patch("mcp_server.api_get_all")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_adopts_exact_own_branch(self, _auth, mock_api, _git_state):
branch = "feat/issue-196-mutations"
self.mock_dup_fetcher.return_value = ([], [branch], {"status": "not_claimed"})
mock_api.return_value = [{"name": branch, "commit": {"id": "abc123"}}]
res = gitea_lock_issue(issue_number=196, branch_name=branch, remote="prgs")
self.assertTrue(res["success"])
self.assertIn("adoption", res)
self.assertEqual(res["adoption"]["branch_head_commit"], "abc123")
@patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_master_git_state_for_lock(),
)
@patch("mcp_server.api_get_all", return_value=[])
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_blocks_active_same_operation_lease(self, _auth, _api, _git_state):
prgs_repo = mcp_server.REMOTES["prgs"]["repo"]
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))
def test_lock_issue_blocks_expired_same_operation_lease_for_recovery(self): @patch(
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: "mcp_server.issue_lock_worktree.read_worktree_git_state",
json.dump({ return_value=_clean_master_git_state_for_lock(),
)
@patch("mcp_server.api_get_all", return_value=[])
@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):
prgs_repo = mcp_server.REMOTES["prgs"]["repo"]
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))
@@ -3370,9 +3595,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))
@@ -3381,37 +3604,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",
@@ -3419,13 +3669,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",
@@ -3439,22 +3689,35 @@ 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_manual_lock_seed_blocked(self, _auth, _role): def test_create_pr_manual_lock_seed_blocked(self, _auth, _role):
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: worktree = os.path.realpath(os.getcwd())
json.dump( with tempfile.TemporaryDirectory() as lock_dir:
env = {**self._create_pr_env(), "GITEA_ISSUE_LOCK_DIR": lock_dir}
with patch.dict(os.environ, env, clear=True):
issue_lock_store.save_lock_file(
issue_lock_store.lock_file_path(
remote="prgs",
org="Scaled-Tech-Consulting",
repo=mcp_server.REMOTES["prgs"]["repo"],
issue_number=447,
lock_dir=lock_dir,
),
_sample_issue_lock( _sample_issue_lock(
issue_number=447, issue_number=447,
branch_name="feat/issue-447-lock-provenance", branch_name="feat/issue-447-lock-provenance",
remote="prgs",
org="Scaled-Tech-Consulting",
repo=mcp_server.REMOTES["prgs"]["repo"],
worktree_path=worktree,
lock_provenance=None, 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",
) )
with self.assertRaises(RuntimeError) as ctx:
gitea_create_pr(
title="feat: lock provenance Closes #447",
head="feat/issue-447-lock-provenance",
remote="prgs",
worktree_path=worktree,
)
self.assertIn("lock provenance", str(ctx.exception).lower()) self.assertIn("lock provenance", str(ctx.exception).lower())
@patch("mcp_server.api_request") @patch("mcp_server.api_request")
@@ -3464,13 +3727,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",
+59
View File
@@ -0,0 +1,59 @@
"""Hermetic tests for merge approval head pinning (#471)."""
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from merge_approval_gate import assess_merge_approval_head # noqa: E402
HEAD_OLD = "8b61c4b41f1b49b271ed3b99657431cf06eeda3e"
HEAD_NEW = "3e4b721d60e97147ba0704773cf57cd0d42cbe31"
class TestMergeApprovalGate(unittest.TestCase):
def test_fresh_approval_at_current_head(self):
result = assess_merge_approval_head(
current_head_sha=HEAD_NEW,
latest_by_reviewer={
"reviewer1": {
"verdict": "APPROVED",
"dismissed": False,
"reviewed_head_sha": HEAD_NEW,
"submitted_at": "2026-07-06T12:00:00Z",
}
},
)
self.assertTrue(result["approval_at_current_head"])
self.assertIsNone(result["stale_approval_block_reason"])
def test_stale_approval_after_rebase(self):
result = assess_merge_approval_head(
current_head_sha=HEAD_NEW,
latest_by_reviewer={
"reviewer1": {
"verdict": "APPROVED",
"dismissed": False,
"reviewed_head_sha": HEAD_OLD,
"submitted_at": "2026-07-06T10:00:00Z",
}
},
)
self.assertFalse(result["approval_at_current_head"])
self.assertEqual(result["latest_approved_head_sha"], HEAD_OLD)
self.assertIn("stale approval", result["stale_approval_block_reason"])
self.assertIn(HEAD_OLD, result["stale_approval_block_reason"])
self.assertIn(HEAD_NEW, result["stale_approval_block_reason"])
self.assertIn("re-review PR at current head", result["stale_approval_block_reason"])
def test_no_approval_entries(self):
result = assess_merge_approval_head(
current_head_sha=HEAD_NEW,
latest_by_reviewer={},
)
self.assertFalse(result["approval_at_current_head"])
self.assertIsNone(result["latest_approved_head_sha"])
if __name__ == "__main__":
unittest.main()
+6 -4
View File
@@ -230,8 +230,9 @@ class TestEligibilityDenialReport(PermissionReportBase):
return PR_PAYLOAD return PR_PAYLOAD
mock_api.side_effect = fake_api mock_api.side_effect = fake_api
mcp_server.init_review_decision_lock("prgs", "review_pr") mcp_server.init_review_decision_lock("prgs", "review_pr")
mcp_server.gitea_load_review_workflow() from tests.test_mcp_server import _seed_ready_review_decision
mcp_server.gitea_mark_final_review_decision(42, "approve", remote="prgs")
_seed_ready_review_decision(42, "approve", remote="prgs")
with patch.dict(os.environ, self._env("author-profile")): with patch.dict(os.environ, self._env("author-profile")):
res = mcp_server.gitea_submit_pr_review( res = mcp_server.gitea_submit_pr_review(
pr_number=42, action="approve", body="lgtm", remote="prgs", pr_number=42, action="approve", body="lgtm", remote="prgs",
@@ -273,8 +274,9 @@ class TestReviewCommentPathUsesCanonicalOp(PermissionReportBase):
return PR_PAYLOAD return PR_PAYLOAD
mock_api.side_effect = fake_api mock_api.side_effect = fake_api
mcp_server.init_review_decision_lock("prgs", "review_pr") mcp_server.init_review_decision_lock("prgs", "review_pr")
mcp_server.gitea_load_review_workflow() from tests.test_mcp_server import _seed_ready_review_decision
mcp_server.gitea_mark_final_review_decision(42, "comment", remote="prgs")
_seed_ready_review_decision(42, "comment", remote="prgs")
with patch.dict(os.environ, self._env("author-profile")): with patch.dict(os.environ, self._env("author-profile")):
res = mcp_server.gitea_submit_pr_review( res = mcp_server.gitea_submit_pr_review(
pr_number=42, action="comment", body="finding", remote="prgs", pr_number=42, action="comment", body="finding", remote="prgs",
+20 -12
View File
@@ -128,18 +128,29 @@ class TestPRQueueInventory(unittest.TestCase):
"forbidden_operations": [], "forbidden_operations": [],
"base_url": None, "base_url": None,
} }
from mcp_server import init_review_decision_lock
from tests.test_mcp_server import (
FULL_HEAD_SHA,
_install_owned_reviewer_lease,
_seed_ready_review_decision,
)
import reviewer_pr_lease
head_sha = FULL_HEAD_SHA
mock_fetch.return_value = _final_page_fetch([ mock_fetch.return_value = _final_page_fetch([
{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": "abc1"}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "other_user"}} {"number": 1, "title": "PR 1", "state": "open",
"head": {"ref": "branch1", "sha": head_sha},
"base": {"ref": "master"}, "mergeable": True,
"user": {"login": "other_user"}}
]) ])
# mock_api: inventory whoami, eligibility whoami, eligibility PR,
# POST review (#244: state + visible-verdict GET reviews).
mock_api.side_effect = [ mock_api.side_effect = [
{"login": "reviewer1"}, {"login": "reviewer1"},
{"login": "reviewer1"}, {"login": "reviewer1"},
{ {
"user": {"login": "other_user"}, "user": {"login": "other_user"},
"state": "open", "state": "open",
"head": {"sha": "abc1"}, "head": {"sha": head_sha},
"mergeable": True, "mergeable": True,
}, },
{"id": 100, "state": "APPROVED"}, {"id": 100, "state": "APPROVED"},
@@ -148,22 +159,19 @@ class TestPRQueueInventory(unittest.TestCase):
"id": 100, "id": 100,
"user": {"login": "reviewer1"}, "user": {"login": "reviewer1"},
"state": "APPROVED", "state": "APPROVED",
"commit_id": "abc1", "commit_id": head_sha,
"submitted_at": "2026-07-06T10:00:00Z", "submitted_at": "2026-07-06T10:00:00Z",
"dismissed": False, "dismissed": False,
} }
], ],
] ]
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision with patch("mcp_server._authenticated_username", return_value="reviewer1"), \
from tests.test_mcp_server import _install_owned_reviewer_lease patch("mcp_server._list_pr_lease_comments", return_value=[]):
import reviewer_pr_lease
with patch("mcp_server._authenticated_username", return_value="reviewer1"):
init_review_decision_lock("prgs", "review_pr") init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(1, "approve", remote="prgs") _seed_ready_review_decision(1, "approve", sha=head_sha, remote="prgs")
lease_patch = _install_owned_reviewer_lease( lease_patch = _install_owned_reviewer_lease(
1, head_sha="abc1", session_id="inventory-review-lease", 1, head_sha=head_sha, session_id="inventory-review-lease",
) )
lease_patch.start() lease_patch.start()
self.addCleanup(lease_patch.stop) self.addCleanup(lease_patch.stop)
+207
View File
@@ -0,0 +1,207 @@
#!/usr/bin/env python3
"""Regression tests for conflict-fix and reviewer PR work leases (#399)."""
from __future__ import annotations
import os
import sys
import unittest
from datetime import datetime, timedelta, timezone
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
from pr_work_lease import ( # noqa: E402
CONFLICT_FIX_LEASE_MARKER,
REVIEWER_LEASE_MARKER,
assess_conflict_fix_final_report,
assess_conflict_fix_push,
assess_head_sha_equality,
assess_reviewer_mutation_blocked,
assess_reviewer_stale_head_final_report,
format_conflict_fix_lease_body,
parse_conflict_fix_lease_comment,
parse_reviewer_lease_comment,
)
HEAD_A = "a" * 40
HEAD_B = "b" * 40
NOW = datetime(2026, 7, 7, 15, 0, tzinfo=timezone.utc)
def _reviewer_lease_body(*, phase: str = "validating", expires_minutes: int = 60) -> str:
expires = (NOW + timedelta(minutes=expires_minutes)).isoformat().replace("+00:00", "Z")
return "\n".join([
REVIEWER_LEASE_MARKER,
"pr: #376",
"phase: " + phase,
f"candidate_head: {HEAD_A}",
f"expires_at: {expires}",
"profile: prgs-reviewer",
])
def _conflict_fix_body(*, phase: str = "claimed", worktree: str = "branches/fix-376") -> str:
expires = (NOW + timedelta(minutes=60)).isoformat().replace("+00:00", "Z")
return "\n".join([
CONFLICT_FIX_LEASE_MARKER,
"pr: #376",
f"phase: {phase}",
f"worktree: {worktree}",
f"head_before: {HEAD_A}",
f"expires_at: {expires}",
"profile: prgs-author",
])
class TestLeaseParsing(unittest.TestCase):
def test_parse_reviewer_lease(self):
parsed = parse_reviewer_lease_comment(_reviewer_lease_body())
self.assertEqual(parsed["pr_number"], 376)
self.assertEqual(parsed["phase"], "validating")
self.assertEqual(parsed["candidate_head"], HEAD_A)
def test_parse_conflict_fix_lease(self):
parsed = parse_conflict_fix_lease_comment(_conflict_fix_body())
self.assertEqual(parsed["pr_number"], 376)
self.assertEqual(parsed["phase"], "claimed")
class TestConflictFixPushGate(unittest.TestCase):
def test_blocks_push_during_active_reviewer_lease(self):
comments = [{"body": _reviewer_lease_body()}]
result = assess_conflict_fix_push(
pr_number=376,
comments=comments,
branch_head_before=HEAD_A,
branch_head_after=HEAD_B,
worktree_path="branches/fix-376",
push_cwd="/proj/branches/fix-376",
is_fast_forward=True,
now=NOW,
)
self.assertFalse(result["push_allowed"])
self.assertTrue(any("reviewer lease" in r for r in result["reasons"]))
def test_rejects_non_fast_forward(self):
result = assess_conflict_fix_push(
pr_number=376,
comments=[],
branch_head_before=HEAD_A,
branch_head_after=HEAD_B,
worktree_path="branches/fix-376",
push_cwd="/proj/branches/fix-376",
is_fast_forward=False,
now=NOW,
)
self.assertFalse(result["push_allowed"])
self.assertTrue(any("non-fast-forward" in r for r in result["reasons"]))
def test_wrong_cwd_push_attempt(self):
result = assess_conflict_fix_push(
pr_number=376,
comments=[],
branch_head_before=HEAD_A,
branch_head_after=HEAD_B,
worktree_path="branches/fix-376",
push_cwd="/proj/master",
is_fast_forward=True,
now=NOW,
)
self.assertFalse(result["push_allowed"])
self.assertTrue(any("cwd" in r.lower() for r in result["reasons"]))
def test_sibling_conflict_fix_collision(self):
comments = [{"body": _conflict_fix_body(phase="pushing", worktree="branches/other")}]
result = assess_conflict_fix_push(
pr_number=376,
comments=comments,
branch_head_before=HEAD_A,
branch_head_after=HEAD_B,
worktree_path="branches/fix-376",
push_cwd="/proj/branches/fix-376",
is_fast_forward=True,
now=NOW,
)
self.assertFalse(result["push_allowed"])
self.assertTrue(any("sibling conflict-fix" in r for r in result["reasons"]))
class TestReviewerMutationGate(unittest.TestCase):
def test_blocks_review_during_conflict_fix(self):
comments = [{"body": _conflict_fix_body(phase="pushing")}]
result = assess_reviewer_mutation_blocked(
pr_number=376,
comments=comments,
reviewed_head_sha=HEAD_A,
live_head_sha=HEAD_A,
mutation="approve",
now=NOW,
)
self.assertFalse(result["mutation_allowed"])
self.assertTrue(any("conflict-fix lease" in r for r in result["reasons"]))
def test_stale_head_blocks_approval(self):
result = assess_reviewer_mutation_blocked(
pr_number=376,
comments=[],
reviewed_head_sha=HEAD_A,
live_head_sha=HEAD_B,
mutation="merge",
now=NOW,
)
self.assertFalse(result["mutation_allowed"])
self.assertTrue(result["head_check"]["head_changed"])
def test_head_equality_required_fields(self):
result = assess_head_sha_equality(HEAD_A, HEAD_B)
self.assertFalse(result["proven"])
self.assertTrue(result["head_changed"])
class TestFinalReportProof(unittest.TestCase):
def test_reviewer_stale_head_report_requires_fields(self):
result = assess_reviewer_stale_head_final_report("no head proof here")
self.assertFalse(result["proven"])
def test_reviewer_stale_head_report_passes(self):
report = "\n".join([
f"Reviewed head SHA: {HEAD_A}",
f"Final live head SHA before approval: {HEAD_A}",
f"Final live head SHA before merge: {HEAD_A}",
"Push occurred during validation: no",
])
result = assess_reviewer_stale_head_final_report(report)
self.assertTrue(result["proven"])
def test_conflict_fix_report_requires_fields(self):
result = assess_conflict_fix_final_report("incomplete")
self.assertFalse(result["proven"])
def test_conflict_fix_report_passes(self):
report = "\n".join([
f"Branch head before push: {HEAD_A}",
f"Branch head after push: {HEAD_B}",
"Active reviewer lease status: none",
"Whether push was fast-forward: yes",
"Whether any reviewer was active: no",
])
result = assess_conflict_fix_final_report(report)
self.assertTrue(result["proven"])
class TestFormatLease(unittest.TestCase):
def test_format_conflict_fix_lease_includes_marker(self):
body = format_conflict_fix_lease_body(
pr_number=376,
branch="feat/x",
worktree="branches/fix-376",
profile="prgs-author",
head_before=HEAD_A,
)
self.assertIn(CONFLICT_FIX_LEASE_MARKER, body)
parsed = parse_conflict_fix_lease_comment(body)
self.assertEqual(parsed["pr_number"], 376)
if __name__ == "__main__":
unittest.main()
+16
View File
@@ -115,6 +115,22 @@ class TestPRReviewFeedbackDiscovery(unittest.TestCase):
result["latest_review_state_by_reviewer"], {"reviewer1": "APPROVED"}) result["latest_review_state_by_reviewer"], {"reviewer1": "APPROVED"})
self.assertFalse(result["has_blocking_change_requests"]) self.assertFalse(result["has_blocking_change_requests"])
self.assertTrue(result["approval_visible"]) self.assertTrue(result["approval_visible"])
self.assertTrue(result["approval_at_current_head"])
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
@patch("mcp_server.get_profile")
def test_stale_approval_not_at_current_head(self, mock_get_profile, _auth, mock_api):
mock_get_profile.return_value = self._profile()
mock_api.side_effect = [
_pr_details(head_sha="newhead3"),
[_review("reviewer1", "APPROVED", commit_id="oldhead1")],
]
result = gitea_get_pr_review_feedback(pr_number=5, remote="prgs")
self.assertTrue(result["approval_visible"])
self.assertFalse(result["approval_at_current_head"])
self.assertEqual(result["latest_approved_head_sha"], "oldhead1")
self.assertIn("stale approval", result["stale_approval_block_reason"])
@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)
+6 -2
View File
@@ -19,7 +19,11 @@ def _minimal_review_report(**overrides):
"- Issue/PR: #182 / PR #203", "- Issue/PR: #182 / PR #203",
"- Branch/SHA: feat/x @ 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", "- Branch/SHA: feat/x @ 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Files changed: review_proofs.py", "- Files changed: review_proofs.py",
"- Validation: pytest tests/test_review_proofs.py -q in branches/review-203", "- Validation: pass: pytest tests/test_review_proofs.py -q in branches/review-203",
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before approval: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before merge: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Push occurred during validation: no",
"- Mutations: review only", "- Mutations: review only",
"- File edits by reviewer: none", "- File edits by reviewer: none",
"- Worktree/index mutations: none", "- Worktree/index mutations: none",
@@ -80,7 +84,7 @@ class TestReviewFinalReportSchema(unittest.TestCase):
def test_reviewed_head_without_validation_blocks(self): def test_reviewed_head_without_validation_blocks(self):
report = _minimal_review_report().replace( report = _minimal_review_report().replace(
"- Validation: pytest tests/test_review_proofs.py -q in branches/review-203", "- Validation: pass: pytest tests/test_review_proofs.py -q in branches/review-203",
"- Validation: not run", "- Validation: not run",
) )
report += "\n- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9" report += "\n- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9"
+3 -1
View File
@@ -130,7 +130,9 @@ class TestReviewWorkflowLoadGates(unittest.TestCase):
self.assertTrue(blockers) self.assertTrue(blockers)
def test_dry_run_allowed_without_load(self): def test_dry_run_allowed_without_load(self):
with patch("mcp_server.gitea_check_pr_eligibility") as elig: with patch("mcp_server.get_auth_header", return_value="Basic dGVzdA=="), \
patch("mcp_server._list_pr_lease_comments", return_value=[]), \
patch("mcp_server.gitea_check_pr_eligibility") as elig:
elig.return_value = { elig.return_value = {
"eligible": True, "eligible": True,
"authenticated_user": "rev", "authenticated_user": "rev",
+18 -11
View File
@@ -12,6 +12,8 @@ from unittest.mock import patch
import mcp_server import mcp_server
HEAD_SHA = "a" * 40
def _lock(mutations=None, correction=False): def _lock(mutations=None, correction=False):
return { return {
@@ -127,10 +129,20 @@ def _feedback(blocking, stale=False, success=True):
"success": success, "success": success,
"has_blocking_change_requests": blocking, "has_blocking_change_requests": blocking,
"review_feedback_stale": stale, "review_feedback_stale": stale,
"current_head_sha": "abc123", "current_head_sha": HEAD_SHA,
} }
def _mark(action, pr_number=6, **kwargs):
kwargs.setdefault("expected_head_sha", HEAD_SHA)
no_lease_block = {"block": False, "reasons": [], "mutation_allowed": True}
with patch("mcp_server._list_pr_lease_comments", return_value=[]), \
patch("mcp_server._pr_work_lease_reviewer_block", return_value=no_lease_block):
return mcp_server.gitea_mark_final_review_decision(
pr_number=pr_number, action=action, remote="prgs", **kwargs
)
class TestDuplicateRequestChangesSuppression(unittest.TestCase): class TestDuplicateRequestChangesSuppression(unittest.TestCase):
def tearDown(self): def tearDown(self):
mcp_server._save_review_decision_lock(None) mcp_server._save_review_decision_lock(None)
@@ -139,8 +151,7 @@ class TestDuplicateRequestChangesSuppression(unittest.TestCase):
_seed() _seed()
with patch.object(mcp_server, "gitea_get_pr_review_feedback", with patch.object(mcp_server, "gitea_get_pr_review_feedback",
return_value=_feedback(blocking=True, stale=False)): return_value=_feedback(blocking=True, stale=False)):
result = mcp_server.gitea_mark_final_review_decision( result = _mark("request_changes")
pr_number=6, action="request_changes", remote="prgs")
self.assertFalse(result["marked_ready"]) self.assertFalse(result["marked_ready"])
self.assertTrue( self.assertTrue(
any("duplicate" in r for r in result["reasons"]), any("duplicate" in r for r in result["reasons"]),
@@ -150,16 +161,14 @@ class TestDuplicateRequestChangesSuppression(unittest.TestCase):
_seed() _seed()
with patch.object(mcp_server, "gitea_get_pr_review_feedback", with patch.object(mcp_server, "gitea_get_pr_review_feedback",
return_value=_feedback(blocking=True, stale=True)): return_value=_feedback(blocking=True, stale=True)):
result = mcp_server.gitea_mark_final_review_decision( result = _mark("request_changes")
pr_number=6, action="request_changes", remote="prgs")
self.assertTrue(result["marked_ready"], result.get("reasons")) self.assertTrue(result["marked_ready"], result.get("reasons"))
def test_request_changes_allowed_when_no_blocker(self): def test_request_changes_allowed_when_no_blocker(self):
_seed() _seed()
with patch.object(mcp_server, "gitea_get_pr_review_feedback", with patch.object(mcp_server, "gitea_get_pr_review_feedback",
return_value=_feedback(blocking=False)): return_value=_feedback(blocking=False)):
result = mcp_server.gitea_mark_final_review_decision( result = _mark("request_changes")
pr_number=6, action="request_changes", remote="prgs")
self.assertTrue(result["marked_ready"], result.get("reasons")) self.assertTrue(result["marked_ready"], result.get("reasons"))
def test_request_changes_fails_closed_when_feedback_unavailable(self): def test_request_changes_fails_closed_when_feedback_unavailable(self):
@@ -167,8 +176,7 @@ class TestDuplicateRequestChangesSuppression(unittest.TestCase):
with patch.object(mcp_server, "gitea_get_pr_review_feedback", with patch.object(mcp_server, "gitea_get_pr_review_feedback",
return_value=_feedback(blocking=False, return_value=_feedback(blocking=False,
success=False)): success=False)):
result = mcp_server.gitea_mark_final_review_decision( result = _mark("request_changes")
pr_number=6, action="request_changes", remote="prgs")
self.assertFalse(result["marked_ready"]) self.assertFalse(result["marked_ready"])
self.assertTrue( self.assertTrue(
any("could not verify" in r for r in result["reasons"]), any("could not verify" in r for r in result["reasons"]),
@@ -178,8 +186,7 @@ class TestDuplicateRequestChangesSuppression(unittest.TestCase):
_seed() _seed()
with patch.object(mcp_server, "gitea_get_pr_review_feedback", with patch.object(mcp_server, "gitea_get_pr_review_feedback",
side_effect=AssertionError("must not be called")): side_effect=AssertionError("must not be called")):
result = mcp_server.gitea_mark_final_review_decision( result = _mark("approve")
pr_number=6, action="approve", remote="prgs")
self.assertTrue(result["marked_ready"], result.get("reasons")) self.assertTrue(result["marked_ready"], result.get("reasons"))
+198
View File
@@ -0,0 +1,198 @@
"""Tests for validation status vocabulary (#406)."""
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from final_report_validator import assess_final_report_validator # noqa: E402
from validation_status_vocabulary import ( # noqa: E402
STATUS_BASELINE_EQUIVALENT,
STATUS_FAILED,
STATUS_MERGE_SIM_RESOLVED,
STATUS_PASSED,
STATUS_TRANSIENT_PASS,
assess_validation_status_vocabulary,
)
def _handoff(**extra):
fields = {
"Task": "review PR #386",
"Validation status": STATUS_PASSED,
"Raw PR-head validation result": "passed",
"Merge simulation result": "not run",
"Baseline worktree used": "none",
}
fields.update(extra)
lines = ["## Controller Handoff", ""]
lines.extend(f"- {key}: {value}" for key, value in fields.items())
return "\n".join(lines)
class TestValidationStatusVocabulary(unittest.TestCase):
def test_raw_head_pass_status(self):
report = _handoff()
result = assess_validation_status_vocabulary(report)
self.assertFalse(result["block"])
self.assertEqual(result["status_claimed"], STATUS_PASSED)
def test_raw_head_failure_with_baseline_match(self):
report = _handoff(
**{
"Validation status": STATUS_BASELINE_EQUIVALENT,
"Raw PR-head validation result": "failed",
"Baseline worktree used": "branches/baseline-master-pr386",
"Baseline target SHA": "a" * 40,
"Baseline failures": "test_foo failed",
"PR failures": "test_foo failed",
"Failure signatures match": "true",
}
)
result = assess_validation_status_vocabulary(report)
self.assertFalse(result["block"])
self.assertTrue(result["baseline_proof_complete"])
def test_baseline_equivalent_without_baseline_proof_blocked(self):
report = _handoff(
**{
"Validation status": STATUS_BASELINE_EQUIVALENT,
"Raw PR-head validation result": "failed",
}
)
result = assess_validation_status_vocabulary(report)
self.assertTrue(result["block"])
self.assertIn("baseline-equivalent", result["reasons"][0])
def test_merge_simulation_resolution_passes(self):
report = "\n".join([
_handoff(
**{
"Validation status": STATUS_MERGE_SIM_RESOLVED,
"Raw PR-head validation result": "failed",
"Merge simulation result": "passed",
}
),
"Worktree/index mutations: merge simulation in branches/review-pr386",
"Worktree path: branches/review-pr386",
"Pre-simulation clean status: clean",
"Merge result: clean merge",
"Abort command: git merge --abort",
"Post-abort clean status: clean",
])
command_log = [
{"command": "git merge --no-commit prgs/master"},
{"command": "git merge --abort"},
]
result = assess_validation_status_vocabulary(
report, command_log=command_log
)
self.assertFalse(result["block"])
self.assertTrue(result["merge_simulation_passed"])
def test_merge_simulation_failure_stays_failed(self):
report = _handoff(
**{
"Validation status": STATUS_FAILED,
"Raw PR-head validation result": "failed",
"Merge simulation result": "failed",
}
)
result = assess_validation_status_vocabulary(report)
self.assertFalse(result["block"])
def test_failed_status_with_passing_merge_sim_blocked(self):
report = "\n".join([
_handoff(
**{
"Validation status": STATUS_FAILED,
"Raw PR-head validation result": "failed",
"Merge simulation result": "passed",
}
),
"Worktree/index mutations: merge simulation",
"Worktree path: branches/review-pr386",
"Pre-simulation clean status: clean",
"Merge result: clean",
"Abort command: git merge --abort",
"Post-abort clean status: clean",
])
result = assess_validation_status_vocabulary(
report,
command_log=[{"command": "git merge --no-commit prgs/master"}],
)
self.assertTrue(result["block"])
def test_transient_failure_then_pass(self):
report = _handoff(
**{
"Validation status": STATUS_TRANSIENT_PASS,
"Raw PR-head validation result": "passed",
"Transient validation failure history": (
"first run failed with infra flake; rerun passed"
),
}
)
result = assess_validation_status_vocabulary(report)
self.assertFalse(result["block"])
def test_transient_pass_without_history_blocked(self):
report = _handoff(
**{
"Validation status": STATUS_TRANSIENT_PASS,
"Raw PR-head validation result": "passed",
}
)
result = assess_validation_status_vocabulary(report)
self.assertTrue(result["block"])
def test_bare_passed_after_raw_failure_blocked(self):
report = _handoff(
**{
"Validation status": STATUS_PASSED,
"Raw PR-head validation result": "failed",
}
)
result = assess_validation_status_vocabulary(report)
self.assertTrue(result["block"])
def test_wrong_baseline_label_when_merge_sim_used_blocked(self):
report = "\n".join([
_handoff(
**{
"Validation status": STATUS_BASELINE_EQUIVALENT,
"Raw PR-head validation result": "failed",
"Merge simulation result": "passed",
}
),
"Worktree/index mutations: merge simulation",
"Worktree path: branches/review-pr386",
"Pre-simulation clean status: clean",
"Merge result: clean",
"Abort command: git merge --abort",
"Post-abort clean status: clean",
])
result = assess_validation_status_vocabulary(
report,
command_log=[{"command": "git merge --no-commit prgs/master"}],
)
self.assertTrue(result["block"])
joined = " ".join(result["reasons"]).lower()
self.assertTrue(
"misleading" in joined or "baseline-equivalent" in joined
)
def test_final_report_validator_integration_blocks_misleading_label(self):
report = _handoff(
**{
"Validation status": STATUS_BASELINE_EQUIVALENT,
"Raw PR-head validation result": "failed",
}
)
result = assess_final_report_validator(report, "review_pr")
blocked_ids = {f["rule_id"] for f in result["findings"]}
self.assertIn("reviewer.validation_status_vocabulary", blocked_ids)
if __name__ == "__main__":
unittest.main()
+153
View File
@@ -0,0 +1,153 @@
"""Tests for runtime_context / mutation-guard workspace alignment (#460)."""
from __future__ import annotations
import os
import sys
import unittest
from pathlib import Path
from unittest import mock
from unittest.mock import MagicMock
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import author_mutation_worktree as amw # noqa: E402
import gitea_mcp_server as srv # noqa: E402
CONTROL_ROOT = str(Path(__file__).resolve().parents[3])
BRANCHES_WORKTREE = str(Path(__file__).resolve().parents[1])
MCP_PROCESS_ROOT = BRANCHES_WORKTREE
class TestCanonicalRepoRoot(unittest.TestCase):
@mock.patch("subprocess.run")
def test_resolves_main_repo_from_branches_worktree(self, mock_run):
mock_run.return_value = MagicMock(
returncode=0,
stdout=f"{CONTROL_ROOT}/.git\n",
)
root = amw.resolve_canonical_repo_root(BRANCHES_WORKTREE, MCP_PROCESS_ROOT)
self.assertEqual(root, CONTROL_ROOT)
def test_falls_back_when_git_unavailable(self):
root = amw.resolve_canonical_repo_root("/missing/path", MCP_PROCESS_ROOT)
self.assertEqual(root, os.path.realpath(MCP_PROCESS_ROOT))
class TestWorkspaceRepoMembership(unittest.TestCase):
@mock.patch("os.path.isdir", return_value=True)
@mock.patch("os.path.exists", return_value=True)
@mock.patch("subprocess.run")
def test_valid_branches_worktree_accepted(self, mock_run, *_exists):
mock_run.return_value = MagicMock(
returncode=0,
stdout=f"{CONTROL_ROOT}/.git\n",
)
result = amw.assess_workspace_repo_membership(
workspace_path=BRANCHES_WORKTREE,
canonical_repo_root=CONTROL_ROOT,
)
self.assertTrue(result["proven"])
self.assertFalse(result["block"])
@mock.patch("os.path.isdir", return_value=True)
@mock.patch("os.path.exists", return_value=True)
@mock.patch("subprocess.run")
def test_wrong_repo_rejected(self, mock_run, *_exists):
mock_run.return_value = MagicMock(
returncode=0,
stdout="/other/repo/.git\n",
)
result = amw.assess_workspace_repo_membership(
workspace_path=BRANCHES_WORKTREE,
canonical_repo_root=CONTROL_ROOT,
)
self.assertTrue(result["block"])
self.assertIn("does not belong", result["reasons"][0])
@mock.patch("os.path.exists", return_value=False)
def test_missing_worktree_rejected(self, *_exists):
result = amw.assess_workspace_repo_membership(
workspace_path=f"{CONTROL_ROOT}/branches/missing-worktree",
canonical_repo_root=CONTROL_ROOT,
)
self.assertTrue(result["block"])
self.assertIn("does not exist", result["reasons"][0])
class TestRuntimeContextGuardAlignment(unittest.TestCase):
def setUp(self):
srv._preflight_whoami_called = True
srv._preflight_capability_called = True
srv._preflight_resolved_role = "author"
self._orig_in_test = srv._preflight_in_test_mode
srv._preflight_in_test_mode = lambda: False
self._env_patch = mock.patch.dict(
os.environ,
{},
clear=False,
)
self._env_patch.start()
os.environ.pop("GITEA_ACTIVE_WORKTREE", None)
os.environ.pop("GITEA_AUTHOR_WORKTREE", None)
def tearDown(self):
srv._preflight_in_test_mode = self._orig_in_test
self._env_patch.stop()
def test_runtime_context_and_guard_share_resolved_workspace(self):
with mock.patch.object(srv, "PROJECT_ROOT", MCP_PROCESS_ROOT):
ctx = srv._resolve_author_mutation_context(BRANCHES_WORKTREE)
status = srv.assess_preflight_status(worktree_path=BRANCHES_WORKTREE)
self.assertEqual(ctx["workspace_path"], os.path.realpath(BRANCHES_WORKTREE))
self.assertEqual(ctx["canonical_repo_root"], CONTROL_ROOT)
self.assertFalse(ctx["roots_aligned"])
self.assertEqual(
status["preflight_workspace"]["active_task_workspace_root"],
os.path.realpath(BRANCHES_WORKTREE),
)
self.assertEqual(
status["preflight_workspace"]["canonical_repository_root"],
CONTROL_ROOT,
)
self.assertIn("workspace_root_mismatch", status["preflight_workspace"])
@mock.patch("subprocess.run")
@mock.patch("os.path.isdir", return_value=True)
@mock.patch("os.path.exists", return_value=True)
def test_declared_branches_worktree_passes_when_mcp_root_differs(
self, _exists, _isdir, mock_run
):
mock_run.return_value = MagicMock(
returncode=0,
stdout=f"{CONTROL_ROOT}/.git\n",
)
with mock.patch.object(srv, "PROJECT_ROOT", MCP_PROCESS_ROOT):
with mock.patch.dict("os.environ", {"GITEA_TEST_PORCELAIN": ""}, clear=False):
srv.verify_preflight_purity(worktree_path=BRANCHES_WORKTREE)
def test_stable_checkout_still_rejected(self):
with mock.patch.object(srv, "PROJECT_ROOT", CONTROL_ROOT):
with mock.patch.dict("os.environ", {"GITEA_TEST_PORCELAIN": ""}, clear=False):
with self.assertRaises(RuntimeError) as ctx:
srv.verify_preflight_purity()
self.assertIn("stable control checkout", str(ctx.exception))
@mock.patch("os.path.isdir", return_value=True)
@mock.patch("os.path.exists", return_value=True)
@mock.patch("subprocess.run")
def test_non_branches_worktree_rejected(self, mock_run, *_exists):
outside = "/tmp/outside-repo-checkout"
mock_run.return_value = MagicMock(
returncode=0,
stdout=f"{CONTROL_ROOT}/.git\n",
)
with mock.patch.object(srv, "PROJECT_ROOT", CONTROL_ROOT):
with mock.patch.dict("os.environ", {"GITEA_TEST_PORCELAIN": ""}, clear=False):
with self.assertRaises(RuntimeError) as ctx:
srv.verify_preflight_purity(worktree_path=outside)
self.assertIn("not under", str(ctx.exception))
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):
+205
View File
@@ -0,0 +1,205 @@
"""Precise validation-status vocabulary for reviewer final reports (#406)."""
from __future__ import annotations
import re
from typing import Any
from reviewer_merge_simulation import assess_merge_simulation_report
STATUS_PASSED = "passed"
STATUS_FAILED = "failed"
STATUS_BASELINE_EQUIVALENT = "baseline-equivalent failure accepted"
STATUS_MERGE_SIM_RESOLVED = "raw-head failure resolved by merge simulation"
STATUS_TRANSIENT_PASS = "passed after transient failure investigation"
ALLOWED_VALIDATION_STATUSES = frozenset({
STATUS_PASSED,
STATUS_FAILED,
STATUS_BASELINE_EQUIVALENT,
STATUS_MERGE_SIM_RESOLVED,
STATUS_TRANSIENT_PASS,
})
_STATUS_FIELD_RE = re.compile(
r"^\s*[-*]?\s*(?:validation status|pr-head validation status|"
r"official validation status)\s*:\s*(.+?)\s*$",
re.IGNORECASE | re.MULTILINE,
)
_RAW_HEAD_RESULT_RE = re.compile(
r"^\s*[-*]?\s*raw pr-head validation result\s*:\s*(.+?)\s*$",
re.IGNORECASE | re.MULTILINE,
)
_MERGE_SIM_RESULT_RE = re.compile(
r"^\s*[-*]?\s*merge simulation result\s*:\s*(.+?)\s*$",
re.IGNORECASE | re.MULTILINE,
)
_BASELINE_WORKTREE_USED_RE = re.compile(
r"^\s*[-*]?\s*baseline (?:validation )?worktree(?: used)?\s*:\s*(.+?)\s*$",
re.IGNORECASE | re.MULTILINE,
)
_BASELINE_TARGET_SHA_RE = re.compile(
r"^\s*[-*]?\s*baseline target sha\s*:\s*([0-9a-f]{7,40})\s*$",
re.IGNORECASE | re.MULTILINE,
)
_FAILURE_SIGNATURE_RE = re.compile(
r"failure signatures match\s*:\s*(true|yes)",
re.IGNORECASE,
)
_BASELINE_FAILURES_RE = re.compile(
r"baseline failures\s*:",
re.IGNORECASE,
)
_TRANSIENT_HISTORY_RE = re.compile(
r"(?:transient validation failure|earlier validation failure|"
r"prior failure|failure history|failed then passed)",
re.IGNORECASE,
)
_FULL_SHA = re.compile(r"^[0-9a-f]{40}$", re.IGNORECASE)
def _first_match(pattern: re.Pattern[str], text: str) -> str:
match = pattern.search(text or "")
return (match.group(1).strip() if match else "")
def _normalize_status_label(raw: str) -> str:
text = (raw or "").strip().lower()
for status in ALLOWED_VALIDATION_STATUSES:
if text == status.lower():
return status
return raw.strip()
def _baseline_proof_complete(text: str, baseline_proof: dict | None) -> bool:
proof = baseline_proof or {}
worktree = (
(proof.get("worktree_path") or "").strip()
or _first_match(_BASELINE_WORKTREE_USED_RE, text)
).lower()
if not worktree or worktree in {"none", "n/a", "not used", "not applicable"}:
return False
if "branches/" not in worktree and not worktree.startswith("branches/"):
return False
target_sha = (proof.get("baseline_target_sha") or "").strip()
if not target_sha:
target_sha = _first_match(_BASELINE_TARGET_SHA_RE, text)
if not _FULL_SHA.match(target_sha or ""):
return False
if proof.get("failure_signatures_match") is True:
return True
if _FAILURE_SIGNATURE_RE.search(text) and _BASELINE_FAILURES_RE.search(text):
return True
return False
def _merge_simulation_passed(text: str, command_log: list | None) -> bool:
merge_result = _first_match(_MERGE_SIM_RESULT_RE, text).lower()
if merge_result in {"passed", "pass", "clean", "succeeded", "success"}:
sim = assess_merge_simulation_report(text, command_log=command_log)
return sim.get("proven") and not sim.get("block")
if "pass" in merge_result and "fail" not in merge_result:
sim = assess_merge_simulation_report(text, command_log=command_log)
return sim.get("proven") and not sim.get("block")
return False
def _raw_head_failed(text: str) -> bool:
raw = _first_match(_RAW_HEAD_RESULT_RE, text).lower()
if raw in {"failed", "fail", "failure"}:
return True
if "fail" in raw and "pass" not in raw:
return True
return bool(re.search(r"\bfailed\b.*pr-head validation", text, re.IGNORECASE))
def _raw_head_passed(text: str) -> bool:
raw = _first_match(_RAW_HEAD_RESULT_RE, text).lower()
return raw in {"passed", "pass", "success"}
def assess_validation_status_vocabulary(
report_text: str,
*,
command_log: list | None = None,
baseline_proof: dict | None = None,
) -> dict[str, Any]:
"""Bind validation-status labels to the proof path that actually ran (#406)."""
text = report_text or ""
reasons: list[str] = []
status_raw = _first_match(_STATUS_FIELD_RE, text)
status = _normalize_status_label(status_raw) if status_raw else ""
if status_raw and status not in ALLOWED_VALIDATION_STATUSES:
reasons.append(
f"unknown validation status {status_raw!r}; use one of "
f"{sorted(ALLOWED_VALIDATION_STATUSES)}"
)
if status == STATUS_BASELINE_EQUIVALENT:
if not _baseline_proof_complete(text, baseline_proof):
reasons.append(
"baseline-equivalent failure accepted requires baseline "
"worktree path, baseline target SHA, and matching failure "
"signatures (#406)"
)
if status == STATUS_MERGE_SIM_RESOLVED:
if not _raw_head_failed(text):
reasons.append(
"raw-head failure resolved by merge simulation requires "
"raw PR-head validation result: failed (#406)"
)
if not _merge_simulation_passed(text, command_log):
reasons.append(
"raw-head failure resolved by merge simulation requires "
"passing merge simulation with worktree/index mutation proof "
"(#317/#406)"
)
if status == STATUS_TRANSIENT_PASS:
if not _TRANSIENT_HISTORY_RE.search(text):
reasons.append(
"passed after transient failure investigation requires "
"documented earlier validation failure history (#396/#406)"
)
if status == STATUS_PASSED and _raw_head_failed(text):
reasons.append(
"validation status passed contradicts raw PR-head validation "
"failure; use a precise status (#406)"
)
if status == STATUS_BASELINE_EQUIVALENT and _merge_simulation_passed(
text, command_log
) and not _baseline_proof_complete(text, baseline_proof):
reasons.append(
"baseline-equivalent failure accepted is misleading when only "
"merge simulation resolved the failure; use "
"'raw-head failure resolved by merge simulation' (#406)"
)
if status == STATUS_FAILED and _merge_simulation_passed(text, command_log):
reasons.append(
"validation status failed contradicts passing merge simulation; "
"report the precise resolution status (#406)"
)
block = bool(reasons)
return {
"block": block,
"proven": not block,
"status_claimed": status or None,
"raw_status_label": status_raw or None,
"raw_head_failed": _raw_head_failed(text),
"raw_head_passed": _raw_head_passed(text),
"merge_simulation_passed": _merge_simulation_passed(text, command_log),
"baseline_proof_complete": _baseline_proof_complete(text, baseline_proof),
"reasons": reasons,
"safe_next_action": (
"use a validation status that matches the proof path executed "
"(baseline worktree, merge simulation, or transient history)"
if reasons
else "proceed"
),
}