Compare commits

..
Author SHA1 Message Date
sysadminandClaude Opus 4.8 85532059e2 fix: use exact issue-number boundary in own-branch adoption (#442)
Replace substring issue-marker matching with a numeric word-boundary
regex so issue-42 adoption is not false-blocked by unrelated issue-420
branches. Add regression tests for the #42 vs #420 collision.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 18:05:19 -04:00
sysadmin 54a75153a9 Merge pull request 'feat: keyed issue-lock store replaces global lock file (Closes #443)' (#465) from feat/issue-443-lock-store into master 2026-07-07 16:57:54 -05:00
sysadmin 5dfc00319f Merge pull request 'feat: align mutation guard with runtime_context workspace resolution (Closes #460)' (#462) from feat/issue-460-workspace-guard-alignment into master 2026-07-07 16:53:07 -05:00
sysadmin f51dc95153 Merge pull request 'feat: harden #274 branches-only guard for gitea_create_issue (Closes #274)' (#449) from feat/issue-274-branches-only-guard-hardening into master 2026-07-07 16:48:00 -05:00
sysadminandClaude Opus 4.8 e956040b6b test: align duplicate-gate and artifact tests with keyed lock store (#443)
Update lock-issue and duplicate-recheck tests to mock branch listing,
use GITEA_ISSUE_LOCK_DIR session binding, and satisfy create_pr
provenance/worktree guards after the keyed persistent lock store landed.

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

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 17:30:35 -04:00
sysadmin f061fee18d Merge pull request 'feat: add per-PR reviewer leases for parallel review (Closes #407)' (#424) from feat/issue-407-reviewer-pr-leases into master 2026-07-07 16:27:19 -05:00
sysadminandClaude Opus 4.8 3e4b721d60 test: align duplicate gate lock fixtures with #447 provenance (#407)
Rebase onto master (#413/#463) requires sanctioned lock_provenance in
create_pr duplicate-recheck tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 17:18:00 -04:00
sysadminandClaude Opus 4.8 1830360850 test: fix reviewer lease gate regressions in MCP integration tests (#407)
Align merge/review/audit mocks with the per-PR reviewer lease gate introduced
in #407: install owned session leases after init_review_decision_lock, stub
_fetch_pr_comments and _authenticated_username to preserve mock ordering, and
update head-SHA mismatch expectations for lease-first fail-closed paths.

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

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 17:16:06 -04:00
sysadmin cc4741a4ce Merge pull request 'feat: block manual issue-lock seeding and require lock disclosure (Closes #447)' (#463) from feat/issue-447-lock-provenance into master 2026-07-07 15:46:09 -05:00
sysadmin d95ea323d7 Merge pull request 'feat: move duplicate-work detection before author mutations (Closes #400)' (#413) from feat/issue-400-duplicate-work-preflight into master 2026-07-07 15:40:23 -05:00
sysadminandClaude Opus 4.8 e4adccd82a fix: inject duplicate-work context fetcher for testable lock/create_pr paths (#400)
Expose issue_duplicate_context_fetcher on the MCP server so lock_issue,
commit_files, and create_pr duplicate rechecks avoid live Gitea calls in
unit tests. Update affected test suites to patch the fetcher without
weakening duplicate-work gate assertions.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 16:33:31 -04:00
sysadminandClaude Opus 4.8 795f544047 feat: block manual issue-lock seeding and require lock disclosure (#447)
Add lock_provenance metadata on gitea_lock_issue writes and fail closed at
gitea_create_pr when provenance is missing. Final-report validation now
requires explicit External-state mutations disclosure for issue-lock
read/write/delete and blocks mixed author PR creation with reviewer approval.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 16:18:42 -04:00
sysadminandClaude Opus 4.8 b33844d74a feat: align mutation guard with runtime_context workspace resolution (Closes #460)
Share canonical workspace/repo-root resolution between gitea_get_runtime_context
and verify_preflight_purity so valid branches/ worktrees are not rejected when
the MCP process root differs from the stable control checkout. Fix relative
git-common-dir resolution and add regression tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 16:11:23 -04:00
sysadmin a9265efa82 feat: harden #274 branches-only guard for gitea_create_issue
Follow-up on the branches-only worktree guard (#274):

- verify_preflight_purity: validate resolved worktree exists, is a directory,
  and belongs to the target repository before author mutations.
- gitea_create_issue: add worktree_path for explicit branches/ workspace proof.
- tests/test_create_issue_workspace_guard.py: six regression cases including
  stable-control-checkout rejection with explicit PROJECT_ROOT simulation.

Closes #274
2026-07-07 14:38:10 -04:00
sysadminandClaude Opus 4.8 0bad26230b fix: resolve conflicts for PR #413
Rebase onto current prgs/master and patch create_pr duplicate-gate test
with auth header mock to match post-rebase recheck path.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 13:08:53 -04:00
sysadminandClaude Opus 4.8 cf057d7829 feat: move duplicate-work detection before author mutations (Closes #400)
Extract issue_work_duplicate_gate for open PR, remote branch, and active
claim checks; wire it into lock_issue, commit_files recheck, and create_pr
recheck. Add gitea_assess_work_issue_duplicate read-only preflight, work-issue
report outcome validation, and workflow section 10A.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-07 13:08:21 -04:00
27 changed files with 2805 additions and 382 deletions
+127
View File
@@ -7,8 +7,11 @@ project's ``branches/`` directory, never from the stable control checkout.
from __future__ import annotations
import os
import subprocess
BASE_BRANCHES = frozenset({"master", "main", "dev"})
ACTIVE_WORKTREE_ENV = "GITEA_ACTIVE_WORKTREE"
AUTHOR_WORKTREE_ENV = "GITEA_AUTHOR_WORKTREE"
def _normalize_path(path: str) -> str:
@@ -48,6 +51,130 @@ def resolve_mutation_workspace(
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(
*,
workspace_path: str,
+16 -3
View File
@@ -291,9 +291,22 @@ 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 (#440). Branch ownership
uses structured ``(fix|feat|docs|chore)/issue-<n>-`` parsing — not broad
substring matches like ``issue-420`` inside ``issue-4200``.
matching `head` branch without unsafe manual seeding.
**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
shared state and manual writes can clobber another session's live lease. Use
`sanctioned recovery` instead:
1. `gitea_lock_issue` on a clean `branches/` worktree (normal path).
2. Own-branch adoption via #442 when the issue's exact branch is already pushed.
3. Operator override only when explicitly authorized — record
`External-state mutations` and `operator override proof` in the final report.
`gitea_create_pr` rejects lock files that lack sanctioned `lock_provenance`
metadata. Final-report validation blocks handoffs that hide lock read/write/delete
under `External-state mutations: none` or mix author PR creation with reviewer
approval in one run. See also #438 (global lock redesign).
Remote branches matching the issue number are also treated as active work unless
the recovery review proves the branch is abandoned or superseded. Never delete
+62
View File
@@ -11,6 +11,7 @@ import inspect
import re
from typing import Any, Callable
import issue_lock_provenance
from review_proofs import (
HANDOFF_HEADING,
assess_controller_handoff,
@@ -870,6 +871,54 @@ def _rule_reviewer_mutation_ledger(
)
def _rule_shared_issue_lock_external_state(report_text: str) -> list[dict[str, str]]:
result = issue_lock_provenance.assess_issue_lock_external_state_report(report_text)
if result.get("proven"):
return []
return _findings_from_reasons(
"shared.issue_lock_external_state",
result.get("reasons") or [],
field="External-state mutations",
severity="block",
safe_next_action=(
"disclose gitea_issue_lock.json read/write/delete under "
"External-state mutations; never claim none after lock seeding"
),
)
def _rule_shared_manual_lock_pr_override(report_text: str) -> list[dict[str, str]]:
result = issue_lock_provenance.assess_manual_lock_pr_without_override(report_text)
if result.get("proven"):
return []
return _findings_from_reasons(
"shared.manual_lock_pr_override",
result.get("reasons") or [],
field="External-state mutations",
severity="block",
safe_next_action=(
"use gitea_lock_issue or #442 adoption instead of manual lock seeding; "
"if operator override was authorized, cite override proof"
),
)
def _rule_shared_author_reviewer_same_run(report_text: str) -> list[dict[str, str]]:
result = issue_lock_provenance.assess_author_reviewer_same_run_report(report_text)
if result.get("proven"):
return []
return _findings_from_reasons(
"shared.author_reviewer_same_run",
result.get("reasons") or [],
field="Review mutations",
severity="block",
safe_next_action=(
"split author PR creation and reviewer approval across separate "
"sessions and handoffs"
),
)
def _rule_reviewer_review_mutation(
report_text: str,
*,
@@ -889,10 +938,17 @@ def _rule_reviewer_review_mutation(
)
_SHARED_ISSUE_LOCK_RULES = (
_rule_shared_issue_lock_external_state,
_rule_shared_manual_lock_pr_override,
_rule_shared_author_reviewer_same_run,
)
_RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
"review_pr": [
_rule_shared_controller_handoff,
_rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
_rule_reviewer_legacy_workspace_mutations,
_rule_reviewer_vague_mutations_none,
_rule_reviewer_mutation_categories,
@@ -913,6 +969,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
"reconcile_already_landed": [
_rule_reconcile_controller_handoff,
_rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
_rule_reconcile_stale_author_fields,
_rule_reconcile_eligible_reviewed,
_rule_reconcile_linked_issue_live,
@@ -924,25 +981,30 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
"author_issue": [
_rule_shared_controller_handoff,
_rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
_rule_reviewer_vague_mutations_none,
],
"work_issue": [
_rule_shared_controller_handoff,
_rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
_rule_reviewer_vague_mutations_none,
],
"issue_filing": [
_rule_shared_controller_handoff,
_rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
],
"inventory": [
_rule_shared_controller_handoff,
_rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
_rule_reconcile_pagination_proof,
],
"issue_selection": [
_rule_shared_controller_handoff,
_rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
],
}
+532 -80
View File
@@ -190,14 +190,22 @@ def _ensure_process_start_porcelain() -> str:
def _resolve_preflight_workspace_path(worktree_path: str | None = None) -> str:
"""Resolve the workspace root inspected by pre-flight guards."""
path = (worktree_path or "").strip()
if not path:
path = (os.environ.get(ACTIVE_WORKTREE_ENV) or "").strip()
if not path:
path = (os.environ.get(AUTHOR_WORKTREE_ENV) or "").strip()
if not path:
path = PROJECT_ROOT
return os.path.realpath(os.path.abspath(path))
return author_mutation_worktree.resolve_mutation_workspace(
worktree_path,
PROJECT_ROOT,
active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV),
author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV),
)
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:
@@ -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:
workspace = _resolve_preflight_workspace_path(worktree_path)
ctx = _resolve_author_mutation_context(worktree_path)
workspace = ctx["workspace_path"]
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)
if active_root == control_root:
if active_root == canonical_root:
dirty_scope = "control checkout"
else:
dirty_scope = "active task workspace"
return {
"mcp_server_process_root": control_root,
details = {
"mcp_server_process_root": process_root,
"canonical_repository_root": canonical_root,
"active_task_workspace_root": active_root,
"inspected_git_root": inspected_root,
"dirty_files": list(dirty_files),
"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:
@@ -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."""
if _preflight_resolved_role == "reviewer":
return
workspace = author_mutation_worktree.resolve_mutation_workspace(
worktree_path,
PROJECT_ROOT,
active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV),
author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV),
)
ctx = _resolve_author_mutation_context(worktree_path)
workspace = ctx["workspace_path"]
git_state = issue_lock_worktree.read_worktree_git_state(workspace)
assessment = author_mutation_worktree.assess_author_mutation_worktree(
workspace_path=workspace,
project_root=PROJECT_ROOT,
project_root=ctx["canonical_repo_root"],
current_branch=git_state.get("current_branch"),
)
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)"
)
workspace = author_mutation_worktree.resolve_mutation_workspace(
worktree_path,
PROJECT_ROOT,
active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV),
author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV),
)
ctx = _resolve_author_mutation_context(worktree_path)
workspace = ctx["workspace_path"]
canonical_root = ctx["canonical_repo_root"]
process_root = ctx["process_project_root"]
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 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(
f"Branches-only mutation guard (#274): worktree path '{workspace}' does not exist (fail closed)"
)
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)"
author_mutation_worktree.format_workspace_repo_membership_error(
membership
)
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)))
@@ -537,13 +531,15 @@ import role_namespace_gate # noqa: E402
import task_capability_map # noqa: E402
import review_proofs # noqa: E402
import agent_temp_artifacts
import issue_branch_ownership # noqa: E402
import issue_lock_worktree # noqa: E402
import issue_lock_provenance # noqa: E402
import issue_lock_store # noqa: E402
import issue_lock_adoption # noqa: E402
import already_landed_reconcile # noqa: E402
import author_mutation_worktree # noqa: E402
import issue_claim_heartbeat # noqa: E402
import issue_work_duplicate_gate # noqa: E402
import reviewer_pr_lease # noqa: E402
import merged_cleanup_reconcile # noqa: E402
import reconciler_profile # noqa: E402
import reconciliation_workflow # noqa: E402
@@ -727,6 +723,119 @@ def _branch_entry_name(branch: dict | str) -> str:
return str(branch.get("name") or branch.get("ref") or "")
def _live_fetch_issue_duplicate_context(
h: str,
o: str,
r: str,
auth: str,
issue_number: int,
) -> tuple[list[dict], list[str], dict]:
"""Live open PRs, remote branch names, and claim state for one issue."""
base = repo_api_url(h, o, r)
open_prs = api_get_all(f"{base}/pulls?state=open", auth)
branches = api_get_all(f"{base}/branches", auth)
branch_names = [_branch_entry_name(b) for b in branches]
issue = api_request("GET", f"{base}/issues/{issue_number}", auth) or {}
comments = api_request(
"GET", f"{base}/issues/{issue_number}/comments", auth
) or []
claim_entry = issue_claim_heartbeat.classify_issue_claim(
issue=issue,
comments=comments,
open_prs=open_prs,
branch_names=branch_names,
)
return open_prs, branch_names, claim_entry
# Injectable duplicate-work context fetcher (#400). Production uses the live
# Gitea API path above; unit tests patch this symbol instead of hitting the
# network.
issue_duplicate_context_fetcher = _live_fetch_issue_duplicate_context
def _collect_issue_duplicate_context(
h: str,
o: str,
r: str,
auth: str,
issue_number: int,
) -> tuple[list[dict], list[str], dict]:
return issue_duplicate_context_fetcher(h, o, r, auth, issue_number)
def _assess_issue_duplicate_gate(
issue_number: int,
*,
h: str,
o: str,
r: str,
auth: str,
locked_branch: str | None = None,
phase: str,
) -> dict:
open_prs, branch_names, claim_entry = _collect_issue_duplicate_context(
h, o, r, auth, issue_number
)
return issue_work_duplicate_gate.assess_work_issue_duplicate_gate(
issue_number,
open_prs=open_prs,
branch_names=branch_names,
claim_entry=claim_entry,
locked_branch=locked_branch,
phase=phase,
)
def _duplicate_gate_block_response(gate: dict, **extra) -> dict:
out = {
"success": False,
"performed": False,
"reasons": list(gate.get("reasons") or []),
"duplicate_gate": gate,
"safe_next_action": gate.get("safe_next_action"),
}
out.update(extra)
return out
def _enforce_locked_issue_duplicate_recheck(
remote: str,
phase: str,
*,
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict | None:
"""Re-check duplicate-work gates for the locked issue (#400)."""
lock_data = _load_existing_issue_lock()
if not lock_data:
return None
issue_number = int(lock_data.get("issue_number") or 0)
locked_branch = lock_data.get("branch_name")
if not issue_number:
return None
h, o, r = _resolve(
remote or lock_data.get("remote") or "dadeschools",
host or lock_data.get("host"),
org or lock_data.get("org"),
repo or lock_data.get("repo"),
)
auth = _auth(h)
gate = _assess_issue_duplicate_gate(
issue_number,
h=h,
o=o,
r=r,
auth=auth,
locked_branch=locked_branch,
phase=phase,
)
if gate.get("block"):
return gate
return None
def _branch_entry_commit_sha(branch: dict | str) -> str | None:
"""Best-effort head SHA for a Gitea branch entry (None when absent)."""
if not isinstance(branch, dict):
@@ -1213,34 +1322,20 @@ def gitea_lock_issue(
issue_lock_worktree.format_issue_lock_worktree_error(lock_assessment)
)
# 2. Check if the issue already has an open PR (reuse protection)
auth = _auth(h)
url = f"{repo_api_url(h, o, r)}/pulls?state=open"
try:
prs = api_get_all(url, auth)
except Exception as e:
raise RuntimeError(f"Could not list open PRs to verify issue lock: {e}")
for pr in prs:
pr_head = pr.get("head", {}).get("ref", "")
pr_title = pr.get("title", "")
pr_body = pr.get("body", "")
if issue_branch_ownership.branch_tracks_issue(pr_head, issue_number):
raise ValueError(
f"Issue #{issue_number} is already tied to an open PR (PR #{pr.get('number')}, branch '{pr_head}') (fail closed)"
)
patterns = [
f"closes #{issue_number}",
f"fixes #{issue_number}",
]
text_to_check = f"{pr_title} {pr_body}".lower()
if any(p in text_to_check for p in patterns):
raise ValueError(
f"Issue #{issue_number} is already tied to an open PR (PR #{pr.get('number')}) via Closes/Fixes reference (fail closed)"
)
duplicate_gate = _assess_issue_duplicate_gate(
issue_number,
h=h,
o=o,
r=r,
auth=auth,
locked_branch=branch_name,
phase=issue_work_duplicate_gate.PHASE_LOCK,
)
if duplicate_gate.get("block"):
raise ValueError("; ".join(duplicate_gate.get("reasons") or [
f"duplicate work gate blocked issue #{issue_number} (fail closed)"
]))
branch_url = f"{repo_api_url(h, o, r)}/branches"
try:
@@ -1280,6 +1375,10 @@ def gitea_lock_issue(
"repo": r,
"worktree_path": resolved_worktree,
"work_lease": work_lease,
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
tool="gitea_lock_issue",
claimant=work_lease.get("claimant"),
),
}
lock_file_path = _save_issue_lock(data)
@@ -1321,6 +1420,39 @@ def gitea_lock_issue(
return result
@mcp.tool()
def gitea_assess_work_issue_duplicate(
issue_number: int,
branch_name: str | None = None,
phase: str = issue_work_duplicate_gate.PHASE_LOCK,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Read-only duplicate-work gate for author sessions before mutations (#400)."""
read_block = _profile_operation_gate("gitea.read")
if read_block:
return {
"success": False,
"performed": False,
"reasons": read_block,
"permission_report": _permission_block_report("gitea.read"),
}
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
gate = _assess_issue_duplicate_gate(
issue_number,
h=h,
o=o,
r=r,
auth=auth,
locked_branch=branch_name,
phase=phase,
)
return {"success": not gate.get("block"), **gate}
@mcp.tool()
def gitea_create_pr(
title: str,
@@ -1374,6 +1506,14 @@ def gitea_create_pr(
# ── Issue Lock Validation (Issue #194 / #196 / #443) ──
lock_data = _resolve_issue_lock_for_pr(remote=remote, org=o, repo=r, head=head)
lock_provenance_check = issue_lock_provenance.assess_lock_file_for_create_pr(
lock_data
)
if lock_provenance_check["block"]:
raise RuntimeError(
issue_lock_provenance.format_lock_provenance_error(lock_provenance_check)
)
locked_issue = lock_data.get("issue_number")
locked_branch = lock_data.get("branch_name")
locked_worktree = lock_data.get("worktree_path")
@@ -1405,6 +1545,21 @@ def gitea_create_pr(
f"PR title or body must contain 'Closes #{locked_issue}' or 'Fixes #{locked_issue}' exactly to ensure durable tracking (fail closed)"
)
duplicate_block = _enforce_locked_issue_duplicate_recheck(
remote,
issue_work_duplicate_gate.PHASE_CREATE_PR,
host=host,
org=org,
repo=repo,
)
if duplicate_block:
return _duplicate_gate_block_response(
duplicate_block,
number=None,
issue_number=locked_issue,
branch_name=locked_branch,
)
auth = _auth(h)
url = f"{repo_api_url(h, o, r)}/pulls"
payload = {"title": title, "body": body, "head": head, "base": base}
@@ -1944,6 +2099,7 @@ def init_review_decision_lock(remote: str | None, task: str | None):
(os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip()
or profile_name
)
reviewer_pr_lease.clear_session_lease()
_save_review_decision_lock({
"task": task,
"remote": remote,
@@ -2386,6 +2542,20 @@ def _evaluate_pr_review_submission(
result["permission_report"] = elig["permission_report"]
return result
if live:
reasons.extend(_reviewer_pr_lease_gate(
pr_number=pr_number,
remote=remote,
host=host,
org=org,
repo=repo,
mutation=action,
live_head_sha=result.get("head_sha"),
pinned_head_sha=expected_head_sha,
))
if reasons:
return result
auth_user = result["authenticated_user"]
pr_author = result["pr_author"]
if action == "approve" and auth_user and pr_author and auth_user == pr_author:
@@ -3003,6 +3173,20 @@ def gitea_commit_files(
if blocked:
return blocked
duplicate_block = _enforce_locked_issue_duplicate_recheck(
remote,
issue_work_duplicate_gate.PHASE_COMMIT,
host=host,
org=org,
repo=repo,
)
if duplicate_block:
return _duplicate_gate_block_response(
duplicate_block,
commit="",
branch="",
)
verify_preflight_purity(remote)
processed_files, source_proofs = _prepare_commit_payload_files(files)
@@ -3165,6 +3349,19 @@ def gitea_merge_pr(
result["permission_report"] = elig["permission_report"]
return result
reasons.extend(_reviewer_pr_lease_gate(
pr_number=pr_number,
remote=remote,
host=host,
org=org,
repo=repo,
mutation="merge",
live_head_sha=result.get("head_sha"),
pinned_head_sha=expected_head_sha,
))
if reasons:
return result
# Gate 4 — head SHA must match if the caller pinned a reviewed SHA.
actual_sha = result["head_sha"]
if expected_head_sha and actual_sha and expected_head_sha != actual_sha:
@@ -4464,6 +4661,261 @@ def _namespace_mutation_block(mutation_task: str, **extra_fields) -> dict | None
return blocked
def _fetch_pr_comments(
pr_number: int,
*,
remote: str,
host: str | None,
org: str | None,
repo: str | None,
) -> list[dict]:
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
api = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
return api_request("GET", api, auth) or []
def _reviewer_pr_lease_gate(
*,
pr_number: int,
remote: str,
host: str | None,
org: str | None,
repo: str | None,
mutation: str,
live_head_sha: str | None,
pinned_head_sha: str | None,
) -> list[str]:
"""Return block reasons when the session lacks an owned PR reviewer lease."""
session = reviewer_pr_lease.get_session_lease()
session_id = (session or {}).get("session_id")
identity = _authenticated_username(remote) or ""
try:
comments = _fetch_pr_comments(
pr_number, remote=remote, host=host, org=org, repo=repo)
except Exception as exc:
return [f"cannot fetch PR comments for lease gate: {_redact(str(exc))}"]
assessment = reviewer_pr_lease.assess_mutation_lease_gate(
pr_number=pr_number,
comments=comments,
reviewer_identity=identity,
session_id=session_id,
mutation=mutation,
live_head_sha=live_head_sha,
pinned_head_sha=pinned_head_sha,
)
return list(assessment.get("reasons") or []) if assessment.get("block") else []
@mcp.tool()
def gitea_acquire_reviewer_pr_lease(
pr_number: int,
worktree: str,
candidate_head: str | None = None,
target_branch: str = "master",
target_branch_sha: str | None = None,
issue_number: int | None = None,
session_id: str | None = None,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Acquire a per-PR reviewer lease before review/merge mutations (#407)."""
read_block = _profile_operation_gate("gitea.read")
if read_block:
return {
"success": False,
"acquired": False,
"reasons": read_block,
"permission_report": _permission_block_report("gitea.read"),
}
comment_block = _profile_operation_gate("gitea.pr.comment")
if comment_block:
return {
"success": False,
"acquired": False,
"reasons": comment_block,
"permission_report": _permission_block_report("gitea.pr.comment"),
}
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
profile = get_profile()
identity = _authenticated_username(remote) or profile.get("username") or ""
sid = (session_id or "").strip() or reviewer_pr_lease.new_session_id()
repo_label = f"{o}/{r}"
comments = _fetch_pr_comments(
pr_number, remote=remote, host=host, org=org, repo=repo)
assessment = reviewer_pr_lease.assess_acquire_lease(
comments,
pr_number=pr_number,
reviewer_identity=identity,
profile=profile.get("profile_name") or "unknown",
session_id=sid,
repo=repo_label,
issue_number=issue_number,
worktree=worktree,
candidate_head=candidate_head,
target_branch=target_branch,
target_branch_sha=target_branch_sha,
)
if not assessment.get("acquire_allowed"):
return {
"success": False,
"acquired": False,
"reasons": assessment.get("reasons") or [],
"existing_lease": assessment.get("existing_lease"),
}
body = assessment["lease_body"]
comment_url = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
with _audited(
"comment_pr",
host=h,
remote=remote,
org=o,
repo=r,
pr_number=pr_number,
request_metadata={"source": "acquire_reviewer_pr_lease"},
):
posted = api_request("POST", comment_url, auth, {"body": body})
session_lease = reviewer_pr_lease.record_session_lease({
"pr_number": pr_number,
"issue_number": issue_number,
"session_id": sid,
"reviewer_identity": identity,
"profile": profile.get("profile_name"),
"worktree": worktree,
"phase": "claimed",
"candidate_head": candidate_head,
"target_branch": target_branch,
"target_branch_sha": target_branch_sha,
"repo": repo_label,
"comment_id": posted.get("id"),
})
return {
"success": True,
"acquired": True,
"pr_number": pr_number,
"session_id": sid,
"comment_id": posted.get("id"),
"session_lease": session_lease,
"reasons": [],
}
@mcp.tool()
def gitea_heartbeat_reviewer_pr_lease(
pr_number: int,
phase: str,
worktree: str | None = None,
candidate_head: str | None = None,
target_branch_sha: str | None = None,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Post a reviewer lease heartbeat / phase update on the PR thread (#407)."""
comment_block = _profile_operation_gate("gitea.pr.comment")
if comment_block:
return {
"success": False,
"posted": False,
"reasons": comment_block,
"permission_report": _permission_block_report("gitea.pr.comment"),
}
session = reviewer_pr_lease.get_session_lease()
if not session or session.get("pr_number") != pr_number:
return {
"success": False,
"posted": False,
"reasons": [
f"no in-session lease for PR #{pr_number}; acquire first "
"(fail closed)"
],
}
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
body = reviewer_pr_lease.format_lease_body(
repo=f"{o}/{r}",
pr_number=pr_number,
issue_number=session.get("issue_number"),
reviewer_identity=session.get("reviewer_identity") or "",
profile=session.get("profile") or "unknown",
session_id=session.get("session_id") or reviewer_pr_lease.new_session_id(),
worktree=worktree or session.get("worktree") or "",
phase=phase,
candidate_head=candidate_head or session.get("candidate_head"),
target_branch=session.get("target_branch") or "master",
target_branch_sha=target_branch_sha or session.get("target_branch_sha"),
)
comment_url = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
with _audited(
"comment_pr",
host=h,
remote=remote,
org=o,
repo=r,
pr_number=pr_number,
request_metadata={"source": "heartbeat_reviewer_pr_lease", "phase": phase},
):
posted = api_request("POST", comment_url, auth, {"body": body})
updated = reviewer_pr_lease.record_session_lease({
**session,
"phase": phase,
"worktree": worktree or session.get("worktree"),
"candidate_head": candidate_head or session.get("candidate_head"),
"target_branch_sha": target_branch_sha or session.get("target_branch_sha"),
"last_comment_id": posted.get("id"),
})
return {
"success": True,
"posted": True,
"pr_number": pr_number,
"phase": phase,
"comment_id": posted.get("id"),
"session_lease": updated,
"reasons": [],
}
@mcp.tool()
def gitea_assess_reviewer_pr_lease(
pr_number: int,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Read-only: assess active reviewer lease state for a PR (#407)."""
read_block = _profile_operation_gate("gitea.read")
if read_block:
return {
"success": False,
"reasons": read_block,
"permission_report": _permission_block_report("gitea.read"),
}
comments = _fetch_pr_comments(
pr_number, remote=remote, host=host, org=org, repo=repo)
active = reviewer_pr_lease.find_active_reviewer_lease(
comments, pr_number=pr_number)
return {
"success": True,
"pr_number": pr_number,
"active_lease": active,
"session_lease": reviewer_pr_lease.get_session_lease(),
"reasons": [],
}
@mcp.tool()
def gitea_list_issue_comments(
issue_number: int,
-31
View File
@@ -1,31 +0,0 @@
"""Structured issue/branch ownership evidence (#440).
Author branches must follow ``(fix|feat|docs|chore)/issue-<n>-<desc>``. Duplicate-work
and recovery gates use this parser instead of broad substring checks like
``issue-420`` inside unrelated names (for example ``issue-4200``).
"""
from __future__ import annotations
import re
IMPLEMENTATION_BRANCH_RE = re.compile(
r"^(?:fix|feat|docs|chore)/issue-(\d+)(?:-.+)?$"
)
def parse_tracked_issue_number(branch_name: str) -> int | None:
"""Return the issue number encoded in a canonical author branch name."""
text = (branch_name or "").strip()
if not text:
return None
match = IMPLEMENTATION_BRANCH_RE.match(text)
if not match:
return None
return int(match.group(1))
def branch_tracks_issue(branch_name: str, issue_number: int) -> bool:
"""True when ``branch_name`` structurally belongs to ``issue_number``."""
parsed = parse_tracked_issue_number(branch_name)
return parsed == issue_number if parsed is not None else False
+43 -4
View File
@@ -14,7 +14,7 @@ this module additionally records whether they passed for proof purposes.
from __future__ import annotations
import issue_branch_ownership
import re
ADOPT = "adopt_existing_branch"
BLOCK_COMPETING = "block_competing_branch"
@@ -35,24 +35,58 @@ def _branch_sha(entry) -> str | None:
return None
def _branch_carries_issue_marker(branch_name: str, issue_number: int) -> bool:
"""Return True when *branch_name* references issue *issue_number* exactly.
Uses a numeric word-boundary so ``issue-42`` does not match inside
``issue-420`` (AC6 / #440).
"""
name = (branch_name or "").strip()
if not name:
return False
pattern = rf"(?:^|/)issue-{int(issue_number)}(?![0-9])"
return re.search(pattern, name) is not None
def assess_own_branch_adoption(
*,
issue_number: int,
requested_branch: str,
existing_branches,
) -> dict:
"""Decide whether an existing matching branch is adoptable."""
"""Decide whether an existing matching branch is adoptable.
Args:
issue_number: The tracking issue number being locked.
requested_branch: The exact branch the caller wants to lock.
existing_branches: Iterable of remote branch entries — either names or
dicts with ``name`` and optional ``commit_sha``.
Returns:
dict with:
* ``outcome`` — one of ADOPT / BLOCK_COMPETING / NO_MATCH
* ``adopt`` (bool), ``block`` (bool)
* ``reason`` (str)
* ``matched_branch`` (str | None), ``matched_head_sha`` (str | None)
* ``competing_branches`` (list[str])
ADOPT: the issue's exact branch exists and no other same-issue branch does.
BLOCK_COMPETING: at least one same-issue branch is not the requested branch.
NO_MATCH: no branch carries the issue marker — normal lock path applies.
"""
requested = (requested_branch or "").strip()
matches: list[tuple[str, str | None]] = []
for entry in existing_branches or []:
name = _branch_name(entry).strip()
if issue_branch_ownership.branch_tracks_issue(name, issue_number):
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,
@@ -104,7 +138,12 @@ def build_adoption_proof(
lock_file_path: str,
lock_file_status: str,
) -> dict:
"""Assemble the proof block returned by ``gitea_lock_issue`` on adoption."""
"""Assemble the proof block returned by ``gitea_lock_issue`` on adoption.
Requirement #4: adoption results must carry issue number, branch name,
branch head commit, adoption reason, no-existing-PR proof, no-competing-
live-lock proof, and lock file path/status.
"""
return {
"issue_number": issue_number,
"branch_name": branch_name,
+269
View File
@@ -0,0 +1,269 @@
"""Issue-lock provenance and external-state disclosure (#447).
Sanctioned locks are written only by ``gitea_lock_issue`` (or adoption recovery
#442). Manual seeding of ``/tmp/gitea_issue_lock.json`` is unsafe and must be
blocked at PR creation unless explicit operator override proof is recorded.
"""
from __future__ import annotations
import os
import re
from datetime import datetime, timezone
ISSUE_LOCK_FILE = os.environ.get("GITEA_ISSUE_LOCK_FILE", "/tmp/gitea_issue_lock.json")
SOURCE_LOCK_ISSUE = "gitea_lock_issue"
SOURCE_LOCK_ADOPTION = "gitea_lock_issue_adoption"
SOURCE_OPERATOR_OVERRIDE = "operator_override"
SANCTIONED_LOCK_SOURCES = frozenset({
SOURCE_LOCK_ISSUE,
SOURCE_LOCK_ADOPTION,
SOURCE_OPERATOR_OVERRIDE,
})
_OPERATOR_OVERRIDE_ENV = "GITEA_ISSUE_LOCK_OPERATOR_OVERRIDE"
_ISSUE_LOCK_PATH_RE = re.compile(
r"(?:/tmp/)?gitea_issue_lock\.json",
re.IGNORECASE,
)
_LOCK_SEED_RE = re.compile(
r"(?:seed(?:ed|ing)?|restor(?:e|ed|ing)|wrote|written|write|programmatically|"
r"hand[- ]forg|manual(?:ly)?).{0,80}gitea_issue_lock",
re.IGNORECASE | re.DOTALL,
)
_LOCK_REMOVE_RE = re.compile(
r"(?:\brm\b|remove|deleted?|unlink).{0,80}gitea_issue_lock",
re.IGNORECASE | re.DOTALL,
)
_LOCK_READ_RE = re.compile(
r"(?:read|loaded?|parsed?).{0,80}gitea_issue_lock",
re.IGNORECASE | re.DOTALL,
)
_EXTERNAL_NONE_RE = re.compile(
r"external[- ]state mutations\s*:\s*none\b",
re.IGNORECASE,
)
_EXTERNAL_FIELD_RE = re.compile(
r"external[- ]state mutations\s*:\s*(.+)$",
re.IGNORECASE | re.MULTILINE,
)
_CLEANUP_ONLY_RE = re.compile(
r"cleanup mutations\s*:\s*(?:none|lock removed|removed issue lock)",
re.IGNORECASE,
)
_PR_CREATED_RE = re.compile(
r"(?:\bgitea_create_pr\b|PR\s*#\s*\d+\s+created|created\s+PR\s*#|opened\s+PR\s*#|"
r"PR\s+creation\s+(?:succeeded|complete))",
re.IGNORECASE,
)
_REVIEW_APPROVE_RE = re.compile(
r"(?:submitted\s+(?:['\"]approve['\"]|approve\s+review)|"
r"review decision\s*:\s*approve|approved\s+PR\s*#|gitea_review_pr.*approve)",
re.IGNORECASE,
)
_OVERRIDE_PROOF_RE = re.compile(
r"operator[- ]override\s+proof\s*:\s*(.+)$",
re.IGNORECASE | re.MULTILINE,
)
def _utc_now_iso() -> str:
return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")
def build_sanctioned_lock_provenance(
*,
tool: str,
source: str = SOURCE_LOCK_ISSUE,
claimant: dict | None = None,
adoption: dict | None = None,
) -> dict:
"""Return provenance metadata stored with a sanctioned lock write."""
entry = {
"source": source,
"written_at": _utc_now_iso(),
"written_by_tool": tool,
"lock_file_path": ISSUE_LOCK_FILE,
}
if claimant:
entry["claimant"] = claimant
if adoption:
entry["adoption"] = adoption
return entry
def operator_override_requested() -> bool:
return os.environ.get(_OPERATOR_OVERRIDE_ENV, "").strip().lower() in {
"1",
"true",
"yes",
}
def build_operator_override_provenance(*, reason: str, claimant: dict | None = None) -> dict:
text = (reason or "").strip()
if not text:
raise ValueError(
"operator override requires a non-empty override reason (fail closed)"
)
entry = build_sanctioned_lock_provenance(
tool="operator_override",
source=SOURCE_OPERATOR_OVERRIDE,
claimant=claimant,
)
entry["override_reason"] = text
return entry
def assess_lock_file_for_create_pr(lock_data: dict | None) -> dict:
"""Fail closed when lock file lacks sanctioned provenance (#447)."""
data = lock_data if isinstance(lock_data, dict) else {}
reasons: list[str] = []
provenance = data.get("lock_provenance")
if not isinstance(provenance, dict):
reasons.append(
"issue lock file lacks sanctioned lock_provenance; manual seeding is "
"not a normal recovery path — call gitea_lock_issue or use #442 adoption"
)
return _provenance_result(False, reasons, provenance)
source = str(provenance.get("source") or "").strip()
if source not in SANCTIONED_LOCK_SOURCES:
reasons.append(
f"issue lock provenance source '{source or '(missing)'}' is not sanctioned"
)
if source == SOURCE_OPERATOR_OVERRIDE and not str(
provenance.get("override_reason") or ""
).strip():
reasons.append(
"operator_override lock provenance requires override_reason proof"
)
if not data.get("work_lease"):
reasons.append("issue lock file missing work_lease metadata")
if not str(provenance.get("written_by_tool") or "").strip():
reasons.append("issue lock provenance missing written_by_tool")
proven = not reasons
return _provenance_result(proven, reasons, provenance)
def _provenance_result(proven: bool, reasons: list[str], provenance: dict | None) -> dict:
return {
"proven": proven,
"block": not proven,
"reasons": reasons,
"lock_provenance": provenance,
}
def format_lock_provenance_error(assessment: dict) -> str:
reasons = "; ".join(assessment.get("reasons") or ["unknown lock provenance violation"])
return f"Issue lock provenance guard (#447): {reasons} (fail closed)"
def _lock_activity_detected(text: str) -> dict[str, bool]:
body = text or ""
return {
"seed_or_restore": bool(_LOCK_SEED_RE.search(body)),
"remove": bool(_LOCK_REMOVE_RE.search(body)),
"read": bool(_LOCK_READ_RE.search(body)),
}
def _external_state_discloses_lock(text: str) -> bool:
match = _EXTERNAL_FIELD_RE.search(text or "")
if not match:
return False
value = (match.group(1) or "").strip().lower()
if value in {"", "none", "n/a"}:
return False
return "lock" in value or "gitea_issue_lock" in value or "issue-lock" in value
def assess_issue_lock_external_state_report(report_text: str) -> dict:
"""Require explicit external-state disclosure for issue-lock mutations (#447)."""
text = report_text or ""
activity = _lock_activity_detected(text)
if not any(activity.values()):
return {"proven": True, "block": False, "reasons": [], "activity": activity}
reasons: list[str] = []
disclosed = _external_state_discloses_lock(text)
if activity["seed_or_restore"] and _EXTERNAL_NONE_RE.search(text):
reasons.append(
"report mentions seeding/restoring gitea_issue_lock.json but claims "
"External-state mutations: none"
)
elif activity["seed_or_restore"] and not disclosed:
reasons.append(
"report mentions issue-lock file activity but External-state mutations "
"does not disclose read/write of gitea_issue_lock.json"
)
if activity["remove"]:
if _EXTERNAL_NONE_RE.search(text):
reasons.append(
"report mentions removing gitea_issue_lock.json but claims "
"External-state mutations: none"
)
elif not disclosed and _CLEANUP_ONLY_RE.search(text):
reasons.append(
"report removes issue lock but classifies it as cleanup only; "
"record under External-state mutations"
)
elif not disclosed:
reasons.append(
"report mentions deleting issue lock without External-state "
"mutation disclosure"
)
proven = not reasons
return {
"proven": proven,
"block": not proven,
"reasons": reasons,
"activity": activity,
}
def assess_manual_lock_pr_without_override(report_text: str) -> dict:
"""Block reports that created a PR via manual lock seed without override proof."""
text = report_text or ""
seeded = bool(_LOCK_SEED_RE.search(text))
created = bool(_PR_CREATED_RE.search(text))
if not (seeded and created):
return {"proven": True, "block": False, "reasons": []}
if _OVERRIDE_PROOF_RE.search(text):
return {"proven": True, "block": False, "reasons": []}
return {
"proven": False,
"block": True,
"reasons": [
"report created/opened a PR after manual issue-lock seeding without "
"operator override proof"
],
}
def assess_author_reviewer_same_run_report(report_text: str) -> dict:
"""Reviewer handoff must not create and approve the same PR in one run (#447)."""
text = report_text or ""
if not (_PR_CREATED_RE.search(text) and _REVIEW_APPROVE_RE.search(text)):
return {"proven": True, "block": False, "reasons": []}
return {
"proven": False,
"block": True,
"reasons": [
"report mixes author-side PR creation and reviewer approval in one "
"final handoff; split author and reviewer sessions"
],
}
+180
View File
@@ -0,0 +1,180 @@
"""Early duplicate-work detection for author work-issue sessions (#400)."""
from __future__ import annotations
from typing import Any
import issue_claim_heartbeat as claim_hb
PHASE_LOCK = "lock_issue"
PHASE_COMMIT = "commit"
PHASE_PUSH = "push"
PHASE_CREATE_PR = "create_pr"
OUTCOME_DUPLICATE_PR_PREVENTED = "duplicate_pr_prevented"
OUTCOME_DUPLICATE_BRANCH_PREVENTED = "duplicate_branch_prevented"
OUTCOME_DUPLICATE_COMMIT_PREVENTED = "duplicate_commit_prevented"
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED = "duplicate_work_not_prevented"
_ACTIVE_CLAIM_STATUSES = frozenset({"active", "awaiting_review"})
def _issue_pattern(issue_number: int) -> str:
return f"issue-{int(issue_number)}"
def _linked_open_pr(issue_number: int, open_prs: list[dict]) -> dict | None:
return claim_hb._linked_open_pr(issue_number, open_prs)
def _matching_branches(
issue_number: int,
branch_names: list[str],
*,
locked_branch: str | None = None,
) -> list[str]:
pattern = _issue_pattern(issue_number)
matches = [
name for name in (branch_names or [])
if pattern in (name or "").lower()
]
if locked_branch:
locked = locked_branch.strip()
matches = [name for name in matches if name != locked]
return matches
def assess_work_issue_duplicate_gate(
issue_number: int,
*,
open_prs: list[dict] | None = None,
branch_names: list[str] | None = None,
claim_entry: dict | None = None,
locked_branch: str | None = None,
phase: str = PHASE_LOCK,
) -> dict[str, Any]:
"""Fail closed when duplicate work is already in flight for an issue."""
reasons: list[str] = []
outcome = OUTCOME_DUPLICATE_WORK_NOT_PREVENTED
prs = list(open_prs or [])
branches = list(branch_names or [])
pattern = _issue_pattern(issue_number)
linked = _linked_open_pr(issue_number, prs)
if linked:
reasons.append(
f"open PR #{linked.get('number')} already covers issue "
f"#{issue_number} (fail closed)"
)
outcome = OUTCOME_DUPLICATE_PR_PREVENTED
conflicting_branches = _matching_branches(
issue_number, branches, locked_branch=locked_branch
)
if conflicting_branches:
names = ", ".join(conflicting_branches[:5])
reasons.append(
f"remote branch(es) already match issue pattern '{pattern}': "
f"{names} (fail closed)"
)
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
entry = claim_entry or {}
if entry.get("linked_open_pr") and not linked:
reasons.append(
f"claim inventory reports open PR #{entry['linked_open_pr']} "
f"for issue #{issue_number} (fail closed)"
)
outcome = OUTCOME_DUPLICATE_PR_PREVENTED
status = (entry.get("status") or "").strip().lower()
if status in _ACTIVE_CLAIM_STATUSES and not linked:
heartbeat = entry.get("latest_heartbeat") or {}
claim_branch = (heartbeat.get("branch") or "").strip()
if locked_branch and claim_branch and claim_branch != locked_branch:
reasons.append(
f"active claim lease on branch '{claim_branch}' blocks "
f"work on '{locked_branch}' for issue #{issue_number} "
"(fail closed)"
)
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
elif not locked_branch and status == "active":
reasons.append(
f"issue #{issue_number} has an active claim lease "
"(fail closed)"
)
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
if phase in {PHASE_COMMIT, PHASE_PUSH} and reasons:
if outcome == OUTCOME_DUPLICATE_PR_PREVENTED:
outcome = OUTCOME_DUPLICATE_COMMIT_PREVENTED
elif outcome == OUTCOME_DUPLICATE_BRANCH_PREVENTED:
outcome = OUTCOME_DUPLICATE_COMMIT_PREVENTED
block = bool(reasons)
return {
"block": block,
"performed": not block,
"issue_number": issue_number,
"phase": phase,
"outcome": outcome,
"linked_open_pr": linked.get("number") if linked else entry.get("linked_open_pr"),
"conflicting_branches": conflicting_branches,
"claim_status": status or None,
"reasons": reasons,
"safe_next_action": (
"stop before mutating; preserve local work and produce a "
"reconciliation handoff if a concurrent PR appeared after push"
if block and phase == PHASE_CREATE_PR
else "stop before mutating; do not commit or push duplicate work"
if block
else "proceed"
),
}
def assess_work_issue_duplicate_report(report_text: str) -> dict[str, Any]:
"""Require explicit duplicate-work outcome wording in work-issue reports."""
text = (report_text or "").lower()
markers = {
OUTCOME_DUPLICATE_PR_PREVENTED: (
"duplicate pr prevented",
"duplicate_pr_prevented",
),
OUTCOME_DUPLICATE_BRANCH_PREVENTED: (
"duplicate branch prevented",
"duplicate_branch_prevented",
),
OUTCOME_DUPLICATE_COMMIT_PREVENTED: (
"duplicate commit prevented",
"duplicate_commit_prevented",
),
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED: (
"duplicate work not prevented",
"duplicate_work_not_prevented",
"no duplicate work",
),
}
matched = [
key for key, phrases in markers.items()
if any(phrase in text for phrase in phrases)
]
if len(matched) != 1:
return {
"complete": False,
"downgraded": True,
"reasons": [
"work-issue report must state exactly one duplicate-work "
"outcome (duplicate PR/branch/commit prevented, or "
"duplicate work not prevented)"
],
}
return {
"complete": True,
"downgraded": False,
"outcome": matched[0],
"reasons": [],
}
+3
View File
@@ -3624,9 +3624,12 @@ def assess_work_issue_mode_isolation(report_text: str) -> dict:
def assess_work_issue_final_report(report_text: str) -> dict:
"""#139: composite verifier for work-issue final reports."""
from issue_work_duplicate_gate import assess_work_issue_duplicate_report
checks = {
"workflow_source": assess_work_issue_workflow_source(report_text),
"mode_isolation": assess_work_issue_mode_isolation(report_text),
"duplicate_work_outcome": assess_work_issue_duplicate_report(report_text),
}
reasons = []
+382
View File
@@ -0,0 +1,382 @@
"""Per-PR reviewer leases for safe parallel review sessions (#407)."""
from __future__ import annotations
import os
import re
import uuid
from datetime import datetime, timedelta, timezone
from typing import Any
MARKER = "<!-- mcp-review-lease:v1 -->"
_FIELD_RE = re.compile(
r"^\s*([a-z_]+)\s*:\s*(.+?)\s*$",
re.IGNORECASE | re.MULTILINE,
)
_FULL_SHA = re.compile(r"^[0-9a-f]{40}$", re.IGNORECASE)
_TERMINAL_PHASES = frozenset({"done", "released", "blocked"})
_ACTIVE_PHASES = frozenset({
"claimed",
"validating",
"approved",
"request-changes",
"merging",
})
DEFAULT_LEASE_TTL_MINUTES = 120
STALE_WARNING_MINUTES = 30
RECLAIMABLE_MINUTES = 60
_SESSION_LEASE: dict[str, Any] | None = None
def _parse_timestamp(value: str | None) -> datetime | None:
if not value:
return None
text = value.strip()
if text.endswith("Z"):
text = text[:-1] + "+00:00"
try:
parsed = datetime.fromisoformat(text)
except ValueError:
return None
if parsed.tzinfo is None:
return parsed.replace(tzinfo=timezone.utc)
return parsed.astimezone(timezone.utc)
def _normalize_sha(value: str | None) -> str | None:
text = (value or "").strip().lower()
return text if text and _FULL_SHA.match(text) else None
def _parse_pr_ref(value: str | None) -> int | None:
digits = re.sub(r"[^\d]", "", value or "")
return int(digits) if digits.isdigit() else None
def new_session_id() -> str:
return f"{os.getpid()}-{uuid.uuid4().hex[:12]}"
def format_lease_body(
*,
repo: str,
pr_number: int,
issue_number: int | None,
reviewer_identity: str,
profile: str,
session_id: str,
worktree: str,
phase: str,
candidate_head: str | None,
target_branch: str,
target_branch_sha: str | None,
last_activity: datetime | None = None,
expires_at: datetime | None = None,
blocker: str = "none",
) -> str:
now = last_activity or datetime.now(timezone.utc)
expires = expires_at or (now + timedelta(minutes=DEFAULT_LEASE_TTL_MINUTES))
last_text = now.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace(
"+00:00", "Z"
)
expires_text = expires.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace(
"+00:00", "Z"
)
issue_text = f"#{issue_number}" if issue_number else "none"
lines = [
MARKER,
f"repo: {repo}",
f"pr: #{pr_number}",
f"issue: {issue_text}",
f"reviewer_identity: {reviewer_identity}",
f"profile: {profile}",
f"session_id: {session_id}",
f"worktree: {worktree}",
f"phase: {phase}",
f"candidate_head: {candidate_head or 'none'}",
f"target_branch: {target_branch}",
f"target_branch_sha: {target_branch_sha or 'none'}",
f"last_activity: {last_text}",
f"expires_at: {expires_text}",
f"blocker: {blocker}",
]
return "\n".join(lines)
def parse_lease_comment(body: str) -> dict[str, Any] | None:
text = body or ""
if MARKER not in text:
return None
fields: dict[str, str] = {}
for match in _FIELD_RE.finditer(text):
fields[match.group(1).strip().lower()] = match.group(2).strip()
if not fields:
return None
return {
"repo": fields.get("repo"),
"pr_number": _parse_pr_ref(fields.get("pr")),
"issue_number": _parse_pr_ref(fields.get("issue")),
"reviewer_identity": fields.get("reviewer_identity"),
"profile": fields.get("profile"),
"session_id": fields.get("session_id"),
"worktree": fields.get("worktree"),
"phase": (fields.get("phase") or "").strip().lower() or None,
"candidate_head": _normalize_sha(fields.get("candidate_head")),
"target_branch": fields.get("target_branch"),
"target_branch_sha": _normalize_sha(fields.get("target_branch_sha")),
"last_activity": fields.get("last_activity"),
"expires_at": fields.get("expires_at"),
"blocker": fields.get("blocker"),
"raw_fields": fields,
}
def _lease_entries(comments: list[dict], *, pr_number: int) -> list[dict]:
entries: list[dict] = []
for comment in comments or []:
parsed = parse_lease_comment(comment.get("body") or "")
if not parsed:
continue
if parsed.get("pr_number") not in (None, pr_number):
continue
entries.append({
**parsed,
"comment_id": comment.get("id"),
"author": (comment.get("user") or {}).get("login") or comment.get("author"),
"created_at": comment.get("created_at"),
"updated_at": comment.get("updated_at"),
})
return entries
def _lease_expired(lease: dict, *, now: datetime) -> bool:
expires_at = _parse_timestamp(lease.get("expires_at"))
return bool(expires_at and expires_at <= now)
def _minutes_since_activity(lease: dict, *, now: datetime) -> float | None:
last = _parse_timestamp(lease.get("last_activity"))
if not last:
return None
return (now - last).total_seconds() / 60.0
def classify_lease_freshness(lease: dict, *, now: datetime | None = None) -> str:
"""Return active, stale_warning, reclaimable, expired, or terminal."""
now = now or datetime.now(timezone.utc)
phase = (lease.get("phase") or "").strip().lower()
if phase in _TERMINAL_PHASES:
return "terminal"
if _lease_expired(lease, now=now):
return "expired"
minutes = _minutes_since_activity(lease, now=now)
if minutes is None:
return "active"
if minutes >= RECLAIMABLE_MINUTES:
return "reclaimable"
if minutes >= STALE_WARNING_MINUTES:
return "stale_warning"
return "active"
def find_active_reviewer_lease(
comments: list[dict],
*,
pr_number: int,
now: datetime | None = None,
) -> dict[str, Any] | None:
"""Newest non-terminal, unexpired lease for *pr_number*."""
now = now or datetime.now(timezone.utc)
for lease in reversed(_lease_entries(comments, pr_number=pr_number)):
phase = (lease.get("phase") or "").strip().lower()
if phase in _TERMINAL_PHASES:
continue
if _lease_expired(lease, now=now):
continue
if phase in _ACTIVE_PHASES or phase:
lease = dict(lease)
lease["freshness"] = classify_lease_freshness(lease, now=now)
return lease
return None
def assess_acquire_lease(
comments: list[dict],
*,
pr_number: int,
reviewer_identity: str,
profile: str,
session_id: str,
repo: str,
issue_number: int | None,
worktree: str,
candidate_head: str | None,
target_branch: str,
target_branch_sha: str | None,
now: datetime | None = None,
) -> dict[str, Any]:
"""Fail closed when another session holds an active lease."""
now = now or datetime.now(timezone.utc)
reasons: list[str] = []
existing = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
if existing:
owner_session = (existing.get("session_id") or "").strip()
freshness = existing.get("freshness") or classify_lease_freshness(existing, now=now)
if owner_session and owner_session != session_id and freshness in {
"active", "stale_warning"
}:
reasons.append(
f"PR #{pr_number} already has active reviewer lease "
f"(session_id={owner_session}, phase={existing.get('phase')})"
)
elif owner_session and owner_session != session_id and freshness == "reclaimable":
reasons.append(
f"PR #{pr_number} lease is reclaimable but still held by "
f"session_id={owner_session}; explicit reclaim not implemented "
"(fail closed)"
)
if not (reviewer_identity or "").strip():
reasons.append("reviewer identity required for lease acquisition")
if not (session_id or "").strip():
reasons.append("session_id required for lease acquisition")
if not (worktree or "").strip():
reasons.append("worktree path required for lease acquisition")
allowed = not reasons
body = None
if allowed:
body = format_lease_body(
repo=repo,
pr_number=pr_number,
issue_number=issue_number,
reviewer_identity=reviewer_identity,
profile=profile,
session_id=session_id,
worktree=worktree,
phase="claimed",
candidate_head=candidate_head,
target_branch=target_branch,
target_branch_sha=target_branch_sha,
last_activity=now,
)
return {
"acquire_allowed": allowed,
"reasons": reasons,
"existing_lease": existing,
"lease_body": body,
"session_id": session_id,
}
def record_session_lease(lease: dict[str, Any]) -> dict[str, Any]:
global _SESSION_LEASE
_SESSION_LEASE = dict(lease)
return dict(_SESSION_LEASE)
def clear_session_lease() -> None:
global _SESSION_LEASE
_SESSION_LEASE = None
def get_session_lease() -> dict[str, Any] | None:
return dict(_SESSION_LEASE) if _SESSION_LEASE else None
def assess_mutation_lease_gate(
*,
pr_number: int,
comments: list[dict],
reviewer_identity: str,
session_id: str | None,
mutation: str,
live_head_sha: str | None,
pinned_head_sha: str | None,
now: datetime | None = None,
) -> dict[str, Any]:
"""Reviewer mutations require an owned, current PR lease."""
now = now or datetime.now(timezone.utc)
reasons: list[str] = []
session = get_session_lease()
active = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
if not session:
reasons.append(
f"no in-session reviewer lease recorded; acquire via "
f"gitea_acquire_reviewer_pr_lease before {mutation}"
)
elif session.get("pr_number") != pr_number:
reasons.append(
f"session lease is for PR #{session.get('pr_number')}, not #{pr_number}"
)
elif (session.get("session_id") or "") != (session_id or session.get("session_id")):
reasons.append("session lease session_id mismatch (fail closed)")
if active:
owner = (active.get("session_id") or "").strip()
if owner and session_id and owner != session_id:
reasons.append(
f"active PR lease owned by session_id={owner}; current session "
f"cannot {mutation}"
)
pinned = _normalize_sha(pinned_head_sha)
live = _normalize_sha(live_head_sha)
lease_head = active.get("candidate_head")
if pinned and live and pinned != live:
reasons.append(
"PR head changed during lease; stop and re-validate before "
f"reviewer {mutation}"
)
if lease_head and live and lease_head != live:
reasons.append(
"live PR head differs from lease candidate_head; refresh lease "
f"before {mutation}"
)
freshness = active.get("freshness") or classify_lease_freshness(active, now=now)
if freshness in {"expired", "reclaimable"}:
reasons.append(f"reviewer lease freshness is '{freshness}' (fail closed)")
else:
reasons.append(f"no active reviewer lease found on PR #{pr_number}")
allowed = not reasons
return {
"mutation_allowed": allowed,
"block": not allowed,
"reasons": reasons,
"active_lease": active,
"session_lease": session,
}
def assess_lease_inventory(
comments_by_pr: dict[int, list[dict]],
*,
now: datetime | None = None,
) -> dict[str, Any]:
"""Summarize lease states across PR comment threads."""
now = now or datetime.now(timezone.utc)
active: list[dict] = []
stale: list[dict] = []
reclaimable: list[dict] = []
for pr_number, comments in (comments_by_pr or {}).items():
lease = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
if not lease:
continue
freshness = lease.get("freshness") or classify_lease_freshness(lease, now=now)
entry = {"pr_number": pr_number, "session_id": lease.get("session_id"), "freshness": freshness}
if freshness == "stale_warning":
stale.append(entry)
elif freshness == "reclaimable":
reclaimable.append(entry)
else:
active.append(entry)
return {
"active_review_leases": active,
"stale_review_leases": stale,
"reclaimable_review_leases": reclaimable,
}
@@ -732,6 +732,26 @@ The final report must identify:
* whether same-PR merge continuation was allowed
* whether the run stopped as required
## 26B. Per-PR reviewer lease (#407)
Parallel reviewer sessions are allowed only when each session holds a distinct,
live PR lease.
Before validation or review mutation on a selected PR:
1. Call `gitea_acquire_reviewer_pr_lease` with worktree path, candidate head SHA,
and target branch SHA.
2. Post heartbeats via `gitea_heartbeat_reviewer_pr_lease` before validation,
after validation, before review mutation, and before merge.
3. Do not approve, request changes, or merge unless the in-session lease
matches the selected PR.
If PR head or target branch advances during the lease, stop and refresh
inventory before continuing.
Final reports must include lease session id, acquisition proof, heartbeat
status, and release/blocked status.
## 27. Merge rules
Before merge, rerun fresh live checks:
@@ -297,6 +297,36 @@ Report:
Do not create another branch/PR for the same issue unless the project explicitly allows taking over or updating existing work and exact capability is proven.
### 10A. Duplicate-work gate phases (#400)
Before any file edits, prove duplicate-work clearance with
`gitea_assess_work_issue_duplicate` or `gitea_lock_issue` (which runs the same
gate). The gate checks live:
* open PRs linked to the issue (head branch or Closes/Fixes reference),
* remote branches matching `issue-<number>`,
* active claim leases from structured heartbeats.
Re-check immediately before:
* `gitea_commit_files` (commit),
* branch push,
* `gitea_create_pr` (PR creation).
If a concurrent open PR appears after work begins:
* before commit/push → stop and preserve local work without pushing,
* after commit but before push → stop without pushing,
* after push but before PR creation → stop and produce a reconciliation
handoff instead of opening a PR.
Final reports must state exactly one duplicate-work outcome:
* `duplicate PR prevented`
* `duplicate branch prevented`
* `duplicate commit prevented`
* `duplicate work not prevented`
## 11. Claim or lock the issue before implementation
Claim/lock the issue before implementation if the project provides a claim/lock mechanism.
@@ -309,15 +339,6 @@ Do not implement unclaimed work.
If the claim/lock gates are broken, produce a recovery handoff.
### Lost lock recovery after push (non-destructive)
If the MCP server restarts after the issue branch was pushed but before PR creation:
* **Do not** delete the remote branch as the normal recovery path.
* Call `gitea_lock_issue` again with the same issue number and exact branch name. Own-branch adoption rebinds the session when open-PR, competing-lock, and worktree safety checks pass.
* Call `gitea_create_pr` afterward. Durable keyed locks resolve from the session pointer or by matching the PR `head` branch without manual lock seeding.
* If adoption is blocked by an open PR, a competing live lease, or a different same-issue branch, stop and produce a recovery handoff.
Create a tooling issue only if this run is explicitly authorized to switch to issue-creation mode and exact `create_issue` capability is proven.
Report:
@@ -726,6 +747,12 @@ Use only precise categories:
* External-state mutations:
* Read-only diagnostics:
Issue-lock file (`/tmp/gitea_issue_lock.json`) read/write/delete is always an
external-state mutation. Never claim `External-state mutations: none` after
seeding, restoring, or removing that file. Manual lock seeding is not a normal
recovery path (#447); use `gitea_lock_issue` or the #442 adoption recovery path
instead. Link broader redesign: #438.
`git fetch`, `git remote update`, and any command that updates refs must be listed under `Git ref mutations`, not read-only diagnostics.
If `git reset --hard`, checkout, clean, worktree add/remove, merge simulation, merge abort, or similar commands occurred, report them under `Worktree/index mutations`.
+4
View File
@@ -83,6 +83,10 @@ class TestIssueLockArtifactWarning(unittest.TestCase):
self._env_patcher.stop()
self._lock_dir.cleanup()
@patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
@patch("mcp_server.api_get_all", return_value=[])
@patch("mcp_server._auth", return_value="token x")
@patch("mcp_server._resolve", return_value=("h", "o", "r"))
+29 -3
View File
@@ -286,6 +286,21 @@ class TestSimpleToolAudit(_AuditWiringBase):
class TestGatedToolAudit(_AuditWiringBase):
def setUp(self):
super().setUp()
from tests.test_mcp_server import _install_owned_reviewer_lease
import reviewer_pr_lease
self._lease_patch = _install_owned_reviewer_lease(8)
self._lease_patch.start()
self._auth_identity_patch = patch(
"mcp_server._authenticated_username", return_value="reviewer-bot"
)
self._auth_identity_patch.start()
self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def _pr(self, author, state="open", sha="abc123", mergeable=True):
return {"user": {"login": author}, "state": state,
"head": {"sha": sha}, "mergeable": mergeable}
@@ -344,11 +359,22 @@ class TestGatedToolAudit(_AuditWiringBase):
GITEA_ALLOWED_OPERATIONS="read,review,approve")
with patch.dict(os.environ, env, clear=True):
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
from tests.test_mcp_server import _install_owned_reviewer_lease
import reviewer_pr_lease
init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(8, "approve", remote="prgs")
r = gitea_submit_pr_review(pr_number=8, action="approve",
body="LGTM", remote="prgs",
final_review_decision_ready=True)
lease_patch = _install_owned_reviewer_lease(8)
lease_patch.start()
try:
r = gitea_submit_pr_review(
pr_number=8, action="approve",
body="LGTM", remote="prgs",
final_review_decision_ready=True,
)
finally:
lease_patch.stop()
reviewer_pr_lease.clear_session_lease()
self.assertTrue(r["performed"])
recs = self._records()
self.assertEqual(len(recs), 1)
+7
View File
@@ -76,7 +76,14 @@ class CommitFilesCapabilityBase(unittest.TestCase):
with open(self.config_path, "w", encoding="utf-8") as fh:
fh.write(json.dumps(CONFIG))
self._dup_fetcher_patcher = patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
self._dup_fetcher_patcher.start()
def tearDown(self):
self._dup_fetcher_patcher.stop()
self._remotes.stop()
mcp_server._IDENTITY_CACHE.clear()
mcp_server._preflight_whoami_called, mcp_server._preflight_capability_called = (
+21 -4
View File
@@ -67,9 +67,18 @@ class TestCommitPayloads(unittest.TestCase):
self.locked_worktree_path = os.path.realpath(self.locked_worktree_dir.name)
import issue_lock_store
import issue_lock_provenance
self._lock_dir = tempfile.TemporaryDirectory()
os.environ["GITEA_ISSUE_LOCK_DIR"] = self._lock_dir.name
work_lease = {
"operation_type": "author_issue_work",
"issue_number": 263,
"branch": "feat/issue-263-native-commit-payloads",
"claimant": {"username": "test-user", "profile": "test-author"},
"expires_at": "2999-01-01T00:00:00Z",
}
self.lock_data = {
"issue_number": 263,
"branch_name": "feat/issue-263-native-commit-payloads",
@@ -77,10 +86,11 @@ class TestCommitPayloads(unittest.TestCase):
"org": "Example-Org",
"repo": "Example-Repo",
"worktree_path": self.locked_worktree_path,
"work_lease": {
"operation_type": "author_issue_work",
"expires_at": "2999-01-01T00:00:00Z",
},
"work_lease": work_lease,
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
tool="gitea_lock_issue",
claimant=work_lease.get("claimant"),
),
}
self.lock_file_path = issue_lock_store.bind_session_lock(self.lock_data)
@@ -90,7 +100,14 @@ class TestCommitPayloads(unittest.TestCase):
mcp_server._preflight_whoami_called = True
mcp_server._preflight_capability_called = True
self._dup_fetcher_patcher = patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
self._dup_fetcher_patcher.start()
def tearDown(self):
self._dup_fetcher_patcher.stop()
self._remotes.stop()
mcp_server._IDENTITY_CACHE.clear()
+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("os.path.exists", return_value=True)
@patch("os.path.isdir", return_value=True)
@patch("author_mutation_worktree.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):
# 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
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):
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 self.assertRaises(RuntimeError) as ctx:
srv.gitea_create_issue(
title="Test issue", body="body", worktree_path=wrong_repo_path
)
with patch("gitea_mcp_server._get_workspace_porcelain", return_value=""):
with self.assertRaises(RuntimeError) as ctx:
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))
@patch("gitea_mcp_server._auth", return_value=FAKE_AUTH)
-35
View File
@@ -1,35 +0,0 @@
"""Unit tests for structured issue/branch ownership parsing (#440)."""
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from issue_branch_ownership import ( # noqa: E402
branch_tracks_issue,
parse_tracked_issue_number,
)
class TestIssueBranchOwnership(unittest.TestCase):
def test_parses_canonical_author_branch(self):
self.assertEqual(
parse_tracked_issue_number("feat/issue-420-server-code-parity"),
420,
)
def test_issue_4200_does_not_track_issue_420(self):
self.assertEqual(parse_tracked_issue_number("feat/issue-4200-unrelated"), 4200)
self.assertFalse(branch_tracks_issue("feat/issue-4200-unrelated", 420))
def test_branch_tracks_issue_exact_match(self):
self.assertTrue(
branch_tracks_issue("fix/issue-440-branch-recovery", 440)
)
def test_unrelated_branch_does_not_track(self):
self.assertFalse(branch_tracks_issue("feat/issue-999-other", 440))
if __name__ == "__main__":
unittest.main()
+67 -5
View File
@@ -25,6 +25,16 @@ class TestAssessOwnBranchAdoption(unittest.TestCase):
)
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(
@@ -34,6 +44,20 @@ class TestAssessOwnBranchAdoption(unittest.TestCase):
)
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(
@@ -42,18 +66,44 @@ class TestAssessOwnBranchAdoption(unittest.TestCase):
existing_branches=[{"name": "feat/issue-999-unrelated"}],
)
self.assertEqual(result["outcome"], NO_MATCH)
self.assertFalse(result["block"])
self.assertFalse(result["adopt"])
def test_issue_number_substring_collision_is_ignored(self):
def test_empty_branch_list_is_normal_path(self):
result = assess_own_branch_adoption(
issue_number=420,
requested_branch=REQ,
existing_branches=[{"name": "feat/issue-4200-unrelated"}],
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_required_fields(self):
def test_proof_has_all_required_fields(self):
assessment = assess_own_branch_adoption(
issue_number=420,
requested_branch=REQ,
@@ -68,8 +118,20 @@ class TestBuildAdoptionProof(unittest.TestCase):
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__":
+130
View File
@@ -0,0 +1,130 @@
"""Tests for issue-lock provenance and external-state disclosure (#447)."""
from __future__ import annotations
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import issue_lock_provenance as ilp # noqa: E402
from final_report_validator import assess_final_report_validator # noqa: E402
def _sanctioned_lock(**overrides):
work_lease = {
"operation_type": "author_issue_work",
"issue_number": 447,
"branch": "feat/issue-447-lock-provenance",
"claimant": {"username": "jcwalker3", "profile": "prgs-author"},
"expires_at": "2999-01-01T00:00:00Z",
}
data = {
"issue_number": 447,
"branch_name": "feat/issue-447-lock-provenance",
"work_lease": work_lease,
"lock_provenance": ilp.build_sanctioned_lock_provenance(
tool="gitea_lock_issue",
claimant=work_lease["claimant"],
),
}
data.update(overrides)
return data
class TestLockProvenanceForCreatePr(unittest.TestCase):
def test_sanctioned_lock_passes(self):
result = ilp.assess_lock_file_for_create_pr(_sanctioned_lock())
self.assertTrue(result["proven"])
self.assertFalse(result["block"])
def test_manual_seed_without_provenance_blocked(self):
result = ilp.assess_lock_file_for_create_pr(
{"issue_number": 420, "branch_name": "feat/x", "work_lease": {}}
)
self.assertTrue(result["block"])
self.assertIn("lock_provenance", result["reasons"][0])
def test_operator_override_requires_reason(self):
result = ilp.assess_lock_file_for_create_pr(
_sanctioned_lock(
lock_provenance=ilp.build_sanctioned_lock_provenance(
tool="operator_override",
source=ilp.SOURCE_OPERATOR_OVERRIDE,
)
)
)
self.assertTrue(result["block"])
class TestExternalStateReportRules(unittest.TestCase):
def test_seed_with_external_none_blocked(self):
report = (
"Restored /tmp/gitea_issue_lock.json to unblock PR creation.\n"
"- External-state mutations: none\n"
)
result = ilp.assess_issue_lock_external_state_report(report)
self.assertTrue(result["block"])
def test_seed_with_disclosure_passes(self):
report = (
"Restored /tmp/gitea_issue_lock.json after MCP restart.\n"
"- External-state mutations: wrote /tmp/gitea_issue_lock.json\n"
)
result = ilp.assess_issue_lock_external_state_report(report)
self.assertTrue(result["proven"])
def test_remove_claimed_as_cleanup_only_blocked(self):
report = (
"rm /tmp/gitea_issue_lock.json after PR creation.\n"
"- Cleanup mutations: lock removed\n"
"- External-state mutations: none\n"
)
result = ilp.assess_issue_lock_external_state_report(report)
self.assertTrue(result["block"])
def test_manual_lock_pr_without_override_blocked(self):
report = (
"Programmatically seeded gitea_issue_lock.json then gitea_create_pr.\n"
"PR #444 created.\n"
)
result = ilp.assess_manual_lock_pr_without_override(report)
self.assertTrue(result["block"])
def test_author_reviewer_same_run_blocked(self):
report = (
"gitea_create_pr opened PR #444.\n"
"Submitted approve review on PR #444.\n"
)
result = ilp.assess_author_reviewer_same_run_report(report)
self.assertTrue(result["block"])
class TestFinalReportValidatorIntegration(unittest.TestCase):
def test_work_issue_blocks_hidden_lock_mutation(self):
report = (
"## Controller Handoff\n"
"- Task: work issue #420\n"
"- External-state mutations: none\n"
"Restored /tmp/gitea_issue_lock.json before PR creation.\n"
)
result = assess_final_report_validator(report, "work_issue")
rule_ids = {f["rule_id"] for f in result["findings"]}
self.assertIn("shared.issue_lock_external_state", rule_ids)
self.assertTrue(result["blocked"])
def test_review_pr_blocks_create_and_approve(self):
report = (
"## Controller Handoff\n"
"- Task: review PR #444\n"
"- Review decision: approve\n"
"Created PR #444 via gitea_create_pr earlier in this run.\n"
)
result = assess_final_report_validator(report, "review_pr")
rule_ids = {f["rule_id"] for f in result["findings"]}
self.assertIn("shared.author_reviewer_same_run", rule_ids)
if __name__ == "__main__":
unittest.main()
-152
View File
@@ -1,152 +0,0 @@
"""Integration tests for non-destructive lock/PR recovery (#440)."""
import os
import sys
import tempfile
import unittest
from pathlib import Path
from unittest import mock
from unittest.mock import patch
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import issue_lock_adoption # noqa: E402
import issue_lock_store as ils # noqa: E402
import mcp_server # noqa: E402
from mcp_server import gitea_create_pr, gitea_lock_issue # noqa: E402
from tests.test_mcp_server import CREATE_PR_ENV, ISSUE_WRITE_ENV # noqa: E402
FAKE_AUTH = "token fake"
BRANCH = "feat/issue-420-server-code-parity"
def _lock_record(**overrides):
record = {
"issue_number": 420,
"branch_name": BRANCH,
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": mcp_server.REMOTES["prgs"]["repo"],
"worktree_path": "/tmp/wt-420",
"work_lease": {
"operation_type": "author_issue_work",
"expires_at": "2999-01-01T00:00:00Z",
},
}
record.update(overrides)
return record
def _clean_git_state():
return {
"current_branch": BRANCH,
"porcelain_status": "",
"base_equivalent": True,
"inspected_git_root": os.getcwd(),
"base_branch": "master",
}
class TestIssueLockRecovery(unittest.TestCase):
def setUp(self):
self._tmpdir = tempfile.TemporaryDirectory()
self._lock_dir = self._tmpdir.name
self._env = {
**ISSUE_WRITE_ENV,
"GITEA_ISSUE_LOCK_DIR": self._lock_dir,
}
self._create_pr_env = {
**CREATE_PR_ENV,
"GITEA_ISSUE_LOCK_DIR": self._lock_dir,
}
def tearDown(self):
self._tmpdir.cleanup()
def test_adoption_after_restart_without_session_pointer(self):
with patch.dict(os.environ, self._env, clear=True):
with mock.patch("os.getpid", return_value=9999):
self.assertIsNone(ils.read_session_issue_lock())
with mock.patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_git_state(),
), mock.patch("mcp_server.api_get_all") as mock_api, mock.patch(
"mcp_server.get_auth_header", return_value=FAKE_AUTH
):
mock_api.side_effect = [
[],
[{"name": BRANCH, "commit": {"id": "934688a"}}],
]
res = gitea_lock_issue(
issue_number=420,
branch_name=BRANCH,
remote="prgs",
worktree_path=os.path.realpath(os.getcwd()),
)
self.assertTrue(res["success"])
self.assertIn("adoption", res)
self.assertEqual(res["adoption"]["branch_head_commit"], "934688a")
@mock.patch(
"mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []),
)
@mock.patch("mcp_server.api_request")
@mock.patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_create_pr_resolves_keyed_lock_after_restart(self, _auth, mock_api, _role):
mock_api.return_value = {"number": 501, "html_url": "https://example/pr/501"}
worktree = os.path.realpath(os.getcwd())
with patch.dict(os.environ, self._create_pr_env, clear=True):
path = ils.lock_file_path(
remote="prgs",
org=mcp_server.REMOTES["prgs"]["org"],
repo=mcp_server.REMOTES["prgs"]["repo"],
issue_number=420,
lock_dir=self._lock_dir,
)
ils.save_lock_file(path, _lock_record(worktree_path=worktree))
with mock.patch("os.getpid", return_value=4242):
self.assertIsNone(ils.read_session_issue_lock())
res = gitea_create_pr(
title="feat: recovery Closes #420",
head=BRANCH,
base="master",
remote="prgs",
worktree_path=worktree,
)
self.assertEqual(res["number"], 501)
def test_open_pr_blocks_adoption(self):
with patch.dict(os.environ, self._env, clear=True):
with mock.patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_git_state(),
), mock.patch("mcp_server.api_get_all") as mock_api, mock.patch(
"mcp_server.get_auth_header", return_value=FAKE_AUTH
):
mock_api.side_effect = [
[{"number": 99, "head": {"ref": BRANCH}, "title": "", "body": ""}],
[{"name": BRANCH, "commit": {"id": "934688a"}}],
]
with self.assertRaises(ValueError) as ctx:
gitea_lock_issue(
issue_number=420,
branch_name=BRANCH,
remote="prgs",
worktree_path=os.path.realpath(os.getcwd()),
)
self.assertIn("already tied to an open PR", str(ctx.exception))
def test_structured_ownership_ignores_issue_4200_collision(self):
result = issue_lock_adoption.assess_own_branch_adoption(
issue_number=420,
requested_branch=BRANCH,
existing_branches=[{"name": "feat/issue-4200-unrelated"}],
)
self.assertEqual(result["outcome"], issue_lock_adoption.NO_MATCH)
if __name__ == "__main__":
unittest.main()
+272
View File
@@ -0,0 +1,272 @@
"""Tests for early duplicate-work detection (#400)."""
import os
import sys
import tempfile
import unittest
from pathlib import Path
from unittest.mock import patch
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import issue_lock_provenance
import issue_lock_store
import issue_work_duplicate_gate as dup_gate
import mcp_server
from issue_work_duplicate_gate import (
OUTCOME_DUPLICATE_BRANCH_PREVENTED,
OUTCOME_DUPLICATE_COMMIT_PREVENTED,
OUTCOME_DUPLICATE_PR_PREVENTED,
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED,
PHASE_COMMIT,
PHASE_CREATE_PR,
PHASE_LOCK,
assess_work_issue_duplicate_gate,
assess_work_issue_duplicate_report,
)
class TestDuplicateGateAssessment(unittest.TestCase):
def test_clear_issue_passes(self):
result = assess_work_issue_duplicate_gate(
400,
open_prs=[],
branch_names=["feat/other-issue-99"],
claim_entry={"status": "not_claimed"},
locked_branch="feat/issue-400-duplicate-work-preflight",
phase=PHASE_LOCK,
)
self.assertFalse(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_WORK_NOT_PREVENTED)
def test_open_pr_blocks(self):
prs = [{
"number": 397,
"title": "feat: handoff",
"body": "Closes #395",
"head": {"ref": "feat/issue-395-proof-backed-review-handoff"},
}]
result = assess_work_issue_duplicate_gate(
395,
open_prs=prs,
branch_names=[],
phase=PHASE_LOCK,
)
self.assertTrue(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_PR_PREVENTED)
def test_conflicting_remote_branch_blocks(self):
result = assess_work_issue_duplicate_gate(
395,
open_prs=[],
branch_names=["feat/issue-395-proof-backed-handoff-claims"],
locked_branch="feat/issue-395-new-attempt",
phase=PHASE_LOCK,
)
self.assertTrue(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_BRANCH_PREVENTED)
def test_active_claim_on_other_branch_blocks(self):
result = assess_work_issue_duplicate_gate(
398,
open_prs=[],
branch_names=[],
claim_entry={
"status": "active",
"latest_heartbeat": {
"branch": "feat/issue-398-validation-cwd-proof",
},
},
locked_branch="feat/issue-398-other-branch",
phase=PHASE_LOCK,
)
self.assertTrue(result["block"])
def test_commit_phase_maps_to_commit_outcome(self):
prs = [{
"number": 411,
"title": "x",
"body": "Closes #398",
"head": {"ref": "feat/issue-398-validation-cwd-proof"},
}]
result = assess_work_issue_duplicate_gate(
398,
open_prs=prs,
branch_names=[],
locked_branch="feat/issue-398-alt",
phase=PHASE_COMMIT,
)
self.assertTrue(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_COMMIT_PREVENTED)
def test_stale_claim_does_not_block_by_status_alone(self):
result = assess_work_issue_duplicate_gate(
400,
open_prs=[],
branch_names=[],
claim_entry={"status": "reclaimable", "reasons": ["stale"]},
locked_branch="feat/issue-400-duplicate-work-preflight",
phase=PHASE_LOCK,
)
self.assertFalse(result["block"])
class TestDuplicateReportOutcome(unittest.TestCase):
def test_requires_exactly_one_outcome(self):
bad = assess_work_issue_duplicate_report("work finished")
self.assertFalse(bad["complete"])
good = assess_work_issue_duplicate_report(
"Duplicate work not prevented for issue #400."
)
self.assertTrue(good["complete"])
self.assertEqual(good["outcome"], OUTCOME_DUPLICATE_WORK_NOT_PREVENTED)
class TestInjectableDuplicateFetcher(unittest.TestCase):
@patch("mcp_server.api_get_all", return_value=[])
@patch("mcp_server.get_auth_header", return_value="token x")
def test_lock_issue_uses_injected_fetcher(self, _auth, _api):
seen = {}
def fetcher(h, o, r, auth, issue_number):
seen["issue_number"] = issue_number
return [], [], {"status": "not_claimed"}
with patch(
"mcp_server.issue_duplicate_context_fetcher",
side_effect=fetcher,
), patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value={
"current_branch": "master",
"porcelain_status": "",
"base_equivalent": True,
},
):
with tempfile.TemporaryDirectory() as lock_dir:
with patch.dict(os.environ, {
"GITEA_ALLOWED_OPERATIONS": "gitea.issue.comment",
"GITEA_ISSUE_LOCK_DIR": lock_dir,
}, clear=True):
mcp_server.gitea_lock_issue(
issue_number=400,
branch_name="feat/issue-400-duplicate-work-preflight",
remote="prgs",
)
self.assertEqual(seen["issue_number"], 400)
class TestMcpDuplicateRecheck(unittest.TestCase):
def setUp(self):
self._dir = tempfile.TemporaryDirectory()
self._env_patch = patch.dict(
os.environ,
{"GITEA_ISSUE_LOCK_DIR": self._dir.name},
clear=False,
)
self._env_patch.start()
self._remotes = patch.dict(mcp_server.REMOTES, {
"prgs": {"host": "gitea.example.com", "org": "Example-Org",
"repo": "Example-Repo"},
})
self._remotes.start()
mcp_server._IDENTITY_CACHE.clear()
def tearDown(self):
patch.stopall()
self._dir.cleanup()
def _write_lock(self, issue_number=400, branch="feat/issue-400-x"):
worktree_path = os.path.realpath(os.getcwd())
work_lease = {
"operation_type": "author_issue_work",
"issue_number": issue_number,
"branch": branch,
"claimant": {"username": "test-user", "profile": "test-author"},
"expires_at": "2999-01-01T00:00:00Z",
}
issue_lock_store.bind_session_lock({
"issue_number": issue_number,
"branch_name": branch,
"remote": "prgs",
"org": "Example-Org",
"repo": "Example-Repo",
"worktree_path": worktree_path,
"work_lease": work_lease,
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
tool="gitea_lock_issue",
claimant=work_lease.get("claimant"),
),
})
@patch("mcp_server._assess_issue_duplicate_gate")
@patch("mcp_server.get_profile", return_value={
"profile_name": "test-author",
"allowed_operations": ["gitea.read", "gitea.repo.commit"],
"forbidden_operations": [],
"audit_label": "test-author",
})
@patch("mcp_server.get_auth_header", return_value="token x")
def test_commit_files_blocked_on_recheck(self, _auth, _profile, mock_gate):
self._write_lock()
mock_gate.return_value = {
"block": True,
"reasons": ["open PR #412 already covers issue #400"],
"outcome": OUTCOME_DUPLICATE_COMMIT_PREVENTED,
"safe_next_action": "stop",
}
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", resolved_role="author")
with patch(
"mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []),
):
result = mcp_server.gitea_commit_files(
files=[{
"operation": "create",
"path": "a.txt",
"content_plain": "hi",
}],
message="test",
remote="prgs",
)
self.assertFalse(result["success"])
self.assertIn("duplicate_gate", result)
@patch("mcp_server._assess_issue_duplicate_gate")
@patch("mcp_server.get_profile", return_value={
"profile_name": "test-author",
"allowed_operations": ["gitea.read", "gitea.pr.create"],
"forbidden_operations": [],
"audit_label": "test-author",
})
@patch("mcp_server.get_auth_header", return_value="token x")
def test_create_pr_returns_handoff_on_duplicate(self, _auth, _profile, mock_gate):
self._write_lock()
mock_gate.return_value = {
"block": True,
"reasons": ["open PR #412 already covers issue #400"],
"outcome": OUTCOME_DUPLICATE_PR_PREVENTED,
"safe_next_action": "reconciliation handoff",
}
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", resolved_role="author")
with patch(
"mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []),
):
result = mcp_server.gitea_create_pr(
title="feat: x (Closes #400)",
head="feat/issue-400-x",
base="master",
body="Closes #400",
remote="prgs",
worktree_path=os.path.realpath(os.getcwd()),
)
self.assertFalse(result["success"])
self.assertIsNone(result.get("number"))
self.assertIn("duplicate_gate", result)
if __name__ == "__main__":
unittest.main()
+220 -43
View File
@@ -81,6 +81,64 @@ def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"):
return [_formal_review(reviewer, "APPROVED", sha=sha)]
_DEFAULT_LEASE_SESSION = "mcp-test-reviewer-lease"
def _reviewer_lease_comment(
pr_number,
*,
session_id=_DEFAULT_LEASE_SESSION,
head_sha="abc123",
reviewer="reviewer-bot",
):
from datetime import datetime, timezone
import reviewer_pr_lease
body = reviewer_pr_lease.format_lease_body(
repo="Scaled-Tech-Consulting/Gitea-Tools",
pr_number=pr_number,
issue_number=407,
reviewer_identity=reviewer,
profile="gitea-reviewer",
session_id=session_id,
worktree="branches/review-test",
phase="claimed",
candidate_head=head_sha,
target_branch="master",
target_branch_sha="b" * 40,
last_activity=datetime.now(timezone.utc),
)
return {"id": 9001, "body": body, "user": {"login": reviewer}}
def _install_owned_reviewer_lease(
pr_number,
*,
session_id=_DEFAULT_LEASE_SESSION,
head_sha="abc123",
):
import reviewer_pr_lease
reviewer_pr_lease.clear_session_lease()
reviewer_pr_lease.record_session_lease({
"pr_number": pr_number,
"session_id": session_id,
"candidate_head": head_sha,
"target_branch": "master",
})
return patch(
"mcp_server._fetch_pr_comments",
return_value=[
_reviewer_lease_comment(
pr_number,
session_id=session_id,
head_sha=head_sha,
)
],
)
# Issue-write tools are profile-gated (#69).
ISSUE_WRITE_ENV = {
"GITEA_ALLOWED_OPERATIONS": (
@@ -99,7 +157,19 @@ CREATE_PR_ENV = {
),
}
ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json"
def _sample_issue_lock(issue_number=123, branch_name="feat/x", **overrides):
import issue_lock_provenance
work_lease = {
"operation_type": "author_issue_work",
"issue_number": issue_number,
"branch": branch_name,
"claimant": {"username": "test-user", "profile": "test-author"},
"expires_at": "2999-01-01T00:00:00Z",
}
record = {
"issue_number": issue_number,
"branch_name": branch_name,
@@ -107,15 +177,21 @@ def _sample_issue_lock(issue_number=123, branch_name="feat/x", **overrides):
"org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools",
"worktree_path": "/tmp/test-worktree",
"work_lease": {
"operation_type": "author_issue_work",
"expires_at": "2999-01-01T00:00:00Z",
},
"work_lease": work_lease,
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
tool="gitea_lock_issue",
claimant=work_lease.get("claimant"),
),
}
record.update(overrides)
return record
def _clear_duplicate_context_fetcher(*_args, **_kwargs):
"""Default injectable duplicate-work context for lock/create_pr tests."""
return [], [], {"status": "not_claimed"}
def _bind_test_lock(**overrides) -> str:
remote = overrides.get("remote", "dadeschools")
record = _sample_issue_lock(**overrides)
@@ -181,11 +257,15 @@ class TestCreateIssue(unittest.TestCase):
# ---------------------------------------------------------------------------
class TestCreatePR(unittest.TestCase):
@patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []))
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_creates_pr(self, _auth, mock_api, _role):
def test_creates_pr(self, _auth, mock_api, _role, _dup_fetcher):
worktree = os.path.realpath(os.getcwd())
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
with tempfile.TemporaryDirectory() as lock_dir:
@@ -205,11 +285,15 @@ class TestCreatePR(unittest.TestCase):
self.assertEqual(payload["base"], "main")
self.assertIn("Closes #123", payload["title"])
@patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []))
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_create_pr_reveal_opt_in_includes_url(self, _auth, mock_api, _role):
def test_create_pr_reveal_opt_in_includes_url(self, _auth, mock_api, _role, _dup_fetcher):
worktree = os.path.realpath(os.getcwd())
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
with tempfile.TemporaryDirectory() as lock_dir:
@@ -542,6 +626,19 @@ class TestViewPR(unittest.TestCase):
class TestMergePR(unittest.TestCase):
"""Gated merge workflow (#16). gitea_merge_pr is the only merge path."""
def setUp(self):
import reviewer_pr_lease
self._lease_patch = _install_owned_reviewer_lease(8)
self._lease_patch.start()
self._auth_identity_patch = patch(
"mcp_server._authenticated_username", return_value="reviewer-bot"
)
self._auth_identity_patch.start()
self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def _pr(self, author, state="open", sha="abc123", mergeable=True):
return {
"user": {"login": author},
@@ -802,9 +899,11 @@ class TestMergePR(unittest.TestCase):
pr_number=8, confirmation=self._confirm(8),
expected_head_sha="deadbeef", remote="prgs")
self.assertFalse(r["performed"])
self.assertIn(
"expected head SHA does not match current PR head (fail closed)",
r["reasons"])
self.assertTrue(any(
"expected head SHA does not match current PR head (fail closed)" in reason
or "PR head changed during lease" in reason
for reason in r["reasons"]
))
self._assert_no_merge_call(mock_api)
@patch("mcp_server.api_request")
@@ -1683,7 +1782,20 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
}
def setUp(self):
import reviewer_pr_lease
init_review_decision_lock("prgs", "review_pr")
self._lease_patch = _install_owned_reviewer_lease(
self.PR, head_sha=self.SHA,
)
self._lease_patch.start()
self._auth_identity_patch = patch(
"mcp_server._authenticated_username", return_value="reviewer-bot"
)
self._auth_identity_patch.start()
self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def _env(self):
return patch.dict(os.environ, {
@@ -1778,8 +1890,19 @@ class TestSubmitPrReview(unittest.TestCase):
"""Gated review-mutation tool (#15)."""
def setUp(self):
import reviewer_pr_lease
init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(8, "approve", remote="prgs")
self._lease_patch = _install_owned_reviewer_lease(8)
self._lease_patch.start()
self._auth_identity_patch = patch(
"mcp_server._authenticated_username", return_value="reviewer-bot"
)
self._auth_identity_patch.start()
self.addCleanup(self._auth_identity_patch.stop)
self.addCleanup(self._lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def _pr(self, author, state="open", sha="abc123", mergeable=True):
return {
@@ -2017,9 +2140,11 @@ class TestSubmitPrReview(unittest.TestCase):
final_review_decision_ready=True,
)
self.assertFalse(r["performed"])
self.assertIn(
"expected head SHA does not match current PR head (fail closed)",
r["reasons"])
self.assertTrue(any(
"expected head SHA does not match current PR head (fail closed)" in reason
or "PR head changed during lease" in reason
for reason in r["reasons"]
))
self._assert_no_mutation(mock_api)
def test_head_sha_match_allows(self):
@@ -2082,9 +2207,9 @@ class TestSubmitPrReview(unittest.TestCase):
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
with patch.dict(os.environ, env, clear=True):
gitea_mark_final_review_decision(5, "approve", remote="prgs")
gitea_mark_final_review_decision(8, "approve", remote="prgs")
r = gitea_submit_pr_review(
pr_number=5, action="approve", remote="prgs",
pr_number=8, action="approve", remote="prgs",
final_review_decision_ready=True,
)
self.assertFalse(r["performed"])
@@ -2303,6 +2428,13 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
self.assertEqual(res["cleanup_status"].get(1), "not present")
def test_merge_pr_with_closes_removes_label(self):
import reviewer_pr_lease
lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123")
lease_patch.start()
self.addCleanup(lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def api_side_effect(method, url, auth, payload=None):
if method == "GET" and "/user" in url:
return {"login": "merger"}
@@ -2337,6 +2469,13 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
self.assertEqual(res["cleanup_status"].get(123), "released")
def test_merge_pr_with_branch_name_removes_label(self):
import reviewer_pr_lease
lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123")
lease_patch.start()
self.addCleanup(lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
def api_side_effect(method, url, auth, payload=None):
if method == "GET" and "/user" in url:
return {"login": "merger"}
@@ -3018,10 +3157,12 @@ class TestVerifyMutationAuthority(unittest.TestCase):
# profile; the active profile resolves as reviewer — side-channel
# override rejected even with a matching in-process authority.
self._authority()
with patch.dict(os.environ,
{"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}):
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_mutation_authority("prgs")
with patch("mcp_server.gitea_config.is_runtime_switching_enabled",
return_value=False):
with patch.dict(os.environ,
{"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}):
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_mutation_authority("prgs")
self.assertIn("side-channel override rejected", str(ctx.exception))
def test_foreign_pid_authority_is_not_trusted(self):
@@ -3079,8 +3220,14 @@ class TestIssueLocking(unittest.TestCase):
}
self._env_patcher = patch.dict(os.environ, env, clear=True)
self._env_patcher.start()
self._dup_fetcher_patcher = patch(
"mcp_server.issue_duplicate_context_fetcher",
return_value=([], [], {"status": "not_claimed"}),
)
self.mock_dup_fetcher = self._dup_fetcher_patcher.start()
def tearDown(self):
self._dup_fetcher_patcher.stop()
self._env_patcher.stop()
self._lock_dir.cleanup()
@@ -3094,10 +3241,9 @@ class TestIssueLocking(unittest.TestCase):
"mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value=_clean_master_git_state_for_lock(),
)
@patch("mcp_server.api_get_all")
@patch("mcp_server.api_get_all", return_value=[])
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_lock_issue_success(self, _auth, mock_api, _git_state):
mock_api.return_value = [] # no open PRs
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")
self.assertTrue(res["success"])
self.assertEqual(res["work_lease"]["operation_type"], "author_issue_work")
@@ -3121,50 +3267,48 @@ class TestIssueLocking(unittest.TestCase):
"mcp_server.issue_lock_worktree.read_worktree_git_state",
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_reused_by_open_pr_branch(self, _auth, mock_api, _git_state):
mock_api.return_value = [{
def test_lock_issue_reused_by_open_pr_branch(self, _auth, _git_state):
self.mock_dup_fetcher.return_value = ([{
"number": 200,
"head": {"ref": "feat/issue-196-boundary"},
"title": "Some PR",
"body": "No closes ref"
}]
"body": "No closes ref",
}], [], {"status": "not_claimed"})
with self.assertRaises(ValueError) as ctx:
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
self.assertIn("already tied to an open PR", str(ctx.exception))
self.assertIn("open PR #200 already covers issue", str(ctx.exception))
@patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
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_reused_by_open_pr_closes_ref(self, _auth, mock_api, _git_state):
mock_api.return_value = [{
def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, _git_state):
self.mock_dup_fetcher.return_value = ([{
"number": 200,
"head": {"ref": "feat/other-branch"},
"title": "Some PR",
"body": "fixes #196"
}]
"body": "fixes #196",
}], [], {"status": "not_claimed"})
with self.assertRaises(ValueError) as ctx:
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
self.assertIn("already tied to an open PR", str(ctx.exception))
self.assertIn("open PR #200 already covers issue", str(ctx.exception))
@patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
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_reused_by_remote_branch(self, _auth, mock_api, _git_state):
mock_api.side_effect = [
def test_lock_issue_reused_by_remote_branch(self, _auth, _git_state):
self.mock_dup_fetcher.return_value = (
[],
[{"name": "feat/issue-196-existing-work"}],
]
["feat/issue-196-existing-work"],
{"status": "not_claimed"},
)
with self.assertRaises(ValueError) as ctx:
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
self.assertIn("not the requested branch", str(ctx.exception))
self.assertIn("remote branch(es) already match issue pattern", str(ctx.exception))
@patch(
"mcp_server.issue_lock_worktree.read_worktree_git_state",
@@ -3174,10 +3318,8 @@ class TestIssueLocking(unittest.TestCase):
@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"
mock_api.side_effect = [
[],
[{"name": branch, "commit": {"id": "abc123"}}],
]
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)
@@ -3399,6 +3541,41 @@ class TestIssueLocking(unittest.TestCase):
)
self.assertIn("does not match locked worktree", str(ctx.exception))
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []))
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_create_pr_manual_lock_seed_blocked(self, _auth, _role):
worktree = os.path.realpath(os.getcwd())
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(
issue_number=447,
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,
),
)
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())
@patch("mcp_server.api_request")
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []))
+16 -3
View File
@@ -156,9 +156,22 @@ class TestPRQueueInventory(unittest.TestCase):
]
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(1, "approve", remote="prgs")
result = gitea_review_pr(pr_number=1, event="APPROVE", remote="prgs", final_review_decision_ready=True)
from tests.test_mcp_server import _install_owned_reviewer_lease
import reviewer_pr_lease
with patch("mcp_server._authenticated_username", return_value="reviewer1"):
init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(1, "approve", remote="prgs")
lease_patch = _install_owned_reviewer_lease(
1, head_sha="abc1", session_id="inventory-review-lease",
)
lease_patch.start()
self.addCleanup(lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
result = gitea_review_pr(
pr_number=1, event="APPROVE", remote="prgs",
final_review_decision_ready=True,
)
self.assertTrue(result["success"])
self.assertIn("=== PR Queue Inventory ===", result["message"])
self.assertIn("Repository:", result["message"])
+1
View File
@@ -2346,6 +2346,7 @@ class TestWorkIssueFinalReport(unittest.TestCase):
"- Safe next action: open PR",
"- Next: open PR",
"- Safety statement: no review/merge",
"- Duplicate work outcome: duplicate work not prevented",
])
def test_complete_work_issue_report_earns_a(self):
+193
View File
@@ -0,0 +1,193 @@
"""Tests for per-PR reviewer leases (#407)."""
import sys
import unittest
from datetime import datetime, timedelta, timezone
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import reviewer_pr_lease as leases
def _lease_comment(
pr_number: int,
session_id: str,
*,
phase: str = "claimed",
minutes_ago: int = 0,
candidate_head: str = "a" * 40,
) -> dict:
now = datetime.now(timezone.utc) - timedelta(minutes=minutes_ago)
body = leases.format_lease_body(
repo="Scaled-Tech-Consulting/Gitea-Tools",
pr_number=pr_number,
issue_number=295,
reviewer_identity="rev1",
profile="prgs-reviewer",
session_id=session_id,
worktree="branches/review-pr382",
phase=phase,
candidate_head=candidate_head,
target_branch="master",
target_branch_sha="b" * 40,
last_activity=now,
)
return {"id": 1, "body": body, "user": {"login": "rev1"}}
class TestReviewerLeaseAcquire(unittest.TestCase):
def setUp(self):
leases.clear_session_lease()
def test_two_reviewers_cannot_lease_same_pr(self):
comments = [_lease_comment(382, "session-a")]
result = leases.assess_acquire_lease(
comments,
pr_number=382,
reviewer_identity="rev2",
profile="prgs-reviewer",
session_id="session-b",
repo="Scaled-Tech-Consulting/Gitea-Tools",
issue_number=295,
worktree="branches/review-pr382-b",
candidate_head="c" * 40,
target_branch="master",
target_branch_sha="d" * 40,
)
self.assertFalse(result["acquire_allowed"])
self.assertTrue(any("already has active" in r for r in result["reasons"]))
def test_two_reviewers_can_lease_different_prs(self):
comments = [_lease_comment(382, "session-a")]
result = leases.assess_acquire_lease(
comments,
pr_number=383,
reviewer_identity="rev2",
profile="prgs-reviewer",
session_id="session-b",
repo="Scaled-Tech-Consulting/Gitea-Tools",
issue_number=296,
worktree="branches/review-pr383",
candidate_head="c" * 40,
target_branch="master",
target_branch_sha="d" * 40,
)
self.assertTrue(result["acquire_allowed"])
self.assertIsNotNone(result["lease_body"])
class TestReviewerLeaseFreshness(unittest.TestCase):
def test_stale_warning_after_30_minutes(self):
lease = leases.parse_lease_comment(
_lease_comment(382, "session-a", minutes_ago=35)["body"]
)
self.assertEqual(
leases.classify_lease_freshness(lease),
"stale_warning",
)
def test_reclaimable_after_60_minutes(self):
lease = leases.parse_lease_comment(
_lease_comment(382, "session-a", minutes_ago=65)["body"]
)
self.assertEqual(
leases.classify_lease_freshness(lease),
"reclaimable",
)
class TestReviewerLeaseMutationGate(unittest.TestCase):
def setUp(self):
leases.clear_session_lease()
def test_reviewer_without_lease_cannot_mutate(self):
head = "f" * 40
comments = [_lease_comment(382, "other-session", candidate_head=head)]
result = leases.assess_mutation_lease_gate(
pr_number=382,
comments=comments,
reviewer_identity="rev1",
session_id="my-session",
mutation="approve",
live_head_sha=head,
pinned_head_sha=head,
)
self.assertTrue(result["block"])
def test_owned_lease_allows_mutation(self):
head = "f" * 40
comments = [_lease_comment(382, "my-session", candidate_head=head)]
leases.record_session_lease({
"pr_number": 382,
"session_id": "my-session",
"candidate_head": head,
"target_branch": "master",
})
result = leases.assess_mutation_lease_gate(
pr_number=382,
comments=comments,
reviewer_identity="rev1",
session_id="my-session",
mutation="approve",
live_head_sha=head,
pinned_head_sha=head,
)
self.assertFalse(result["block"])
def test_head_change_invalidates_lease(self):
reviewed = "f" * 40
live = "e" * 40
comments = [_lease_comment(382, "my-session", candidate_head=reviewed)]
leases.record_session_lease({
"pr_number": 382,
"session_id": "my-session",
"candidate_head": reviewed,
})
result = leases.assess_mutation_lease_gate(
pr_number=382,
comments=comments,
reviewer_identity="rev1",
session_id="my-session",
mutation="merge",
live_head_sha=live,
pinned_head_sha=reviewed,
)
self.assertTrue(result["block"])
self.assertTrue(any("head" in r.lower() for r in result["reasons"]))
class TestReviewerLeaseMcpGate(unittest.TestCase):
def setUp(self):
leases.clear_session_lease()
patch("mcp_server.verify_preflight_purity").start()
patch("gitea_audit.audit_enabled", return_value=False).start()
mcp_server = __import__("mcp_server")
mcp_server._IDENTITY_CACHE.clear()
mcp_server.init_review_decision_lock("prgs", "review_pr")
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", "reviewer")
def tearDown(self):
patch.stopall()
leases.clear_session_lease()
def test_reviewer_pr_lease_gate_helper_blocks_without_session(self):
import mcp_server
head = "a" * 40
with patch("mcp_server._fetch_pr_comments", return_value=[]):
reasons = mcp_server._reviewer_pr_lease_gate(
pr_number=382,
remote="prgs",
host=None,
org=None,
repo=None,
mutation="approve",
live_head_sha=head,
pinned_head_sha=head,
)
self.assertTrue(any("lease" in r.lower() for r in reasons))
if __name__ == "__main__":
unittest.main()
+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()