Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
1158594a20 |
+13
-32
@@ -53,28 +53,6 @@ def enter_from_capability_result(capability: dict) -> dict | None:
|
||||
return dict(record)
|
||||
|
||||
|
||||
def _is_reviewer_denial(capability: dict) -> bool:
|
||||
task = (capability or {}).get("requested_task", "")
|
||||
required_role = (capability or {}).get("required_role_kind")
|
||||
return (
|
||||
required_role == "reviewer"
|
||||
or task in REVIEWER_CAPABILITY_TASKS
|
||||
)
|
||||
|
||||
|
||||
def sync_from_capability_result(capability: dict) -> dict | None:
|
||||
"""Enter or clear terminal mode from a capability resolution (#238).
|
||||
|
||||
Reviewer denials activate terminal mode for the denied operation only.
|
||||
A later allowed task route clears stale denial state so author read-only
|
||||
tools (e.g. ``list_prs``) are not permanently blocked.
|
||||
"""
|
||||
if (capability or {}).get("stop_required") and _is_reviewer_denial(capability):
|
||||
return enter_from_capability_result(capability)
|
||||
clear()
|
||||
return None
|
||||
|
||||
|
||||
def enter_from_route_result(route: dict) -> dict | None:
|
||||
"""Enter terminal mode from a role router wrong_role_stop (#206 compat)."""
|
||||
if (route or {}).get("route_result") != "wrong_role_stop":
|
||||
@@ -112,13 +90,11 @@ def check_reviewer_queue_tool(tool_name: str) -> tuple[bool, list[str]]:
|
||||
return True, []
|
||||
name = (tool_name or "").strip().lower().removeprefix("gitea_")
|
||||
if name in BLOCKED_QUEUE_TOOLS:
|
||||
denied_task = (_session_terminal or {}).get("requested_task") or "unknown"
|
||||
return False, [
|
||||
TERMINAL_REPORT_HEADING,
|
||||
f"Reviewer queue tool '{tool_name}' is blocked by the current "
|
||||
f"capability denial for task '{denied_task}' (fail closed).",
|
||||
"Resolve or route an allowed author task to clear stale denial "
|
||||
"state, or relaunch a reviewer MCP namespace for reviewer work.",
|
||||
f"Reviewer queue tool '{tool_name}' is blocked after "
|
||||
"capability denial (fail closed).",
|
||||
"Relaunch a reviewer MCP namespace to perform reviewer work.",
|
||||
]
|
||||
return True, []
|
||||
|
||||
@@ -144,6 +120,11 @@ def assess_capability_stop_report(
|
||||
capability_denied: bool = True,
|
||||
) -> dict:
|
||||
"""Validate final report purity after reviewer capability denial."""
|
||||
from review_proofs import (
|
||||
assess_empty_queue_report,
|
||||
parse_trust_gate_status_from_report,
|
||||
)
|
||||
|
||||
text = report_text or ""
|
||||
lower = text.lower()
|
||||
violations = []
|
||||
@@ -172,11 +153,7 @@ def assess_capability_stop_report(
|
||||
r"inventory empty",
|
||||
re.I,
|
||||
)
|
||||
parsed_status = None
|
||||
for line in text.splitlines():
|
||||
if "pr_inventory_trust_gate.status:" in line.lower():
|
||||
parsed_status = line.split(":", 1)[1].strip()
|
||||
break
|
||||
parsed_status = parse_trust_gate_status_from_report(text)
|
||||
effective_status = trust_gate_status or parsed_status
|
||||
if empty_queue_patterns.search(text):
|
||||
if effective_status != "trusted_empty":
|
||||
@@ -185,6 +162,10 @@ def assess_capability_stop_report(
|
||||
"pr_inventory_trust_gate.status == trusted_empty"
|
||||
)
|
||||
|
||||
empty_queue = assess_empty_queue_report(text)
|
||||
if empty_queue.get("claimed") and not empty_queue.get("proven"):
|
||||
violations.extend(empty_queue.get("reasons") or [])
|
||||
|
||||
ok, elig_violations = validate_eligibility_wording(text)
|
||||
violations.extend(elig_violations)
|
||||
|
||||
|
||||
+11
-234
@@ -104,7 +104,7 @@ def verify_mutation_authority(remote: str | None, host: str | None = None,
|
||||
)
|
||||
|
||||
session_lock = (os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip()
|
||||
if session_lock and session_lock != active_profile and not gitea_config.is_runtime_switching_enabled():
|
||||
if session_lock and session_lock != active_profile:
|
||||
raise RuntimeError(
|
||||
f"Active profile '{active_profile}' does not match the session "
|
||||
f"profile lock '{session_lock}' — profile side-channel override "
|
||||
@@ -260,7 +260,6 @@ import role_session_router # noqa: E402
|
||||
import role_namespace_gate # noqa: E402
|
||||
import task_capability_map # noqa: E402
|
||||
import review_proofs # noqa: E402
|
||||
import issue_lock_worktree # noqa: E402
|
||||
|
||||
|
||||
# Fail-closed exact-issue-lock file (#204): written by gitea_lock_issue,
|
||||
@@ -416,67 +415,6 @@ def _authenticated_username(host: str):
|
||||
return user
|
||||
|
||||
|
||||
def _ensure_matching_profile(required_permission: str, required_role: str, remote: str | None, host: str | None = None) -> str | None:
|
||||
"""Check if the active profile is allowed to perform *required_permission*.
|
||||
If not, automatically switch to the first matching usable configured profile.
|
||||
"""
|
||||
try:
|
||||
profile = get_profile()
|
||||
except Exception:
|
||||
return None
|
||||
active_profile = profile.get("profile_name")
|
||||
active_allowed = profile.get("allowed_operations") or []
|
||||
active_forbidden = profile.get("forbidden_operations") or []
|
||||
allowed, _ = gitea_config.check_operation(required_permission, active_allowed, active_forbidden)
|
||||
if allowed:
|
||||
return active_profile
|
||||
|
||||
# Try to find a matching usable profile in config
|
||||
if gitea_config.is_runtime_switching_enabled():
|
||||
config = gitea_config.load_config()
|
||||
if config and "profiles" in config:
|
||||
for p_name, p_data in config["profiles"].items():
|
||||
p_allowed = p_data.get("allowed_operations") or []
|
||||
p_forbidden = p_data.get("forbidden_operations") or []
|
||||
p_role = p_data.get("role") or _role_kind(p_allowed, p_forbidden)
|
||||
if required_role and p_role != required_role:
|
||||
continue
|
||||
p_allowed_n = []
|
||||
for op in p_allowed:
|
||||
try:
|
||||
p_allowed_n.append(gitea_config.normalize_operation(op))
|
||||
except Exception:
|
||||
pass
|
||||
p_forbidden_n = []
|
||||
for op in p_forbidden:
|
||||
try:
|
||||
p_forbidden_n.append(gitea_config.normalize_operation(op))
|
||||
except Exception:
|
||||
pass
|
||||
ok, _ = gitea_config.check_operation(required_permission, p_allowed_n, p_forbidden_n)
|
||||
if ok:
|
||||
# Verify credentials/token are available
|
||||
try:
|
||||
tok = gitea_config.resolve_token(p_data)
|
||||
if tok:
|
||||
# Perform automatic switch
|
||||
gitea_config._active_profile_override = p_name
|
||||
h = host or (REMOTES.get(remote, {}).get("host") if remote in REMOTES else None)
|
||||
if h:
|
||||
_IDENTITY_CACHE.pop(h, None)
|
||||
username = _authenticated_username(h) if h else None
|
||||
# Update mutation authority
|
||||
global _MUTATION_AUTHORITY
|
||||
if _MUTATION_AUTHORITY is not None:
|
||||
_MUTATION_AUTHORITY["current_profile"] = p_name
|
||||
_MUTATION_AUTHORITY["current_identity"] = username
|
||||
_MUTATION_AUTHORITY["role_pivot_authorized"] = True
|
||||
return p_name
|
||||
except Exception:
|
||||
pass
|
||||
return None
|
||||
|
||||
|
||||
def _audit(action: str, *, host, remote, result, org=None, repo=None,
|
||||
reason=None, request_metadata=None, issue_number=None,
|
||||
pr_number=None, target_branch=None, head_sha=None, username=_UNSET,
|
||||
@@ -687,7 +625,6 @@ def gitea_lock_issue(
|
||||
host: str | None = None,
|
||||
org: str | None = None,
|
||||
repo: str | None = None,
|
||||
worktree_path: str | None = None,
|
||||
) -> dict:
|
||||
"""Lock exactly one Gitea issue and its branch name to ensure durable tracking.
|
||||
|
||||
@@ -698,8 +635,6 @@ def gitea_lock_issue(
|
||||
host: Override Gitea host.
|
||||
org: Override Org.
|
||||
repo: Override Repo.
|
||||
worktree_path: Author scratch-clone path to validate (defaults to
|
||||
GITEA_AUTHOR_WORKTREE or the MCP server project root).
|
||||
"""
|
||||
# 1. Enforce branch name includes issue number
|
||||
expected_pattern = f"issue-{issue_number}"
|
||||
@@ -708,20 +643,6 @@ def gitea_lock_issue(
|
||||
f"Branch name '{branch_name}' must contain locked issue pattern '{expected_pattern}' (fail closed)"
|
||||
)
|
||||
|
||||
resolved_worktree = issue_lock_worktree.resolve_author_worktree_path(
|
||||
worktree_path, PROJECT_ROOT
|
||||
)
|
||||
git_state = issue_lock_worktree.read_worktree_git_state(resolved_worktree)
|
||||
lock_assessment = issue_lock_worktree.assess_issue_lock_worktree(
|
||||
worktree_path=resolved_worktree,
|
||||
current_branch=git_state.get("current_branch"),
|
||||
porcelain_status=git_state.get("porcelain_status") or "",
|
||||
)
|
||||
if lock_assessment["block"]:
|
||||
raise RuntimeError(
|
||||
issue_lock_worktree.format_issue_lock_worktree_error(lock_assessment)
|
||||
)
|
||||
|
||||
# 2. Check if the issue already has an open PR (reuse protection)
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
@@ -758,7 +679,6 @@ def gitea_lock_issue(
|
||||
"remote": remote,
|
||||
"org": o,
|
||||
"repo": r,
|
||||
"worktree_path": resolved_worktree,
|
||||
}
|
||||
|
||||
try:
|
||||
@@ -769,13 +689,9 @@ def gitea_lock_issue(
|
||||
|
||||
return {
|
||||
"success": True,
|
||||
"message": (
|
||||
f"Successfully locked issue #{issue_number} to branch '{branch_name}' "
|
||||
f"from worktree '{resolved_worktree}' (fail-closed check complete)."
|
||||
),
|
||||
"message": f"Successfully locked issue #{issue_number} to branch '{branch_name}' (fail-closed check complete).",
|
||||
"issue_number": issue_number,
|
||||
"branch_name": branch_name,
|
||||
"worktree_path": resolved_worktree,
|
||||
}
|
||||
|
||||
|
||||
@@ -789,7 +705,6 @@ def gitea_create_pr(
|
||||
host: str | None = None,
|
||||
org: str | None = None,
|
||||
repo: str | None = None,
|
||||
worktree_path: str | None = None,
|
||||
) -> dict:
|
||||
"""Create a pull request on a Gitea repository.
|
||||
|
||||
@@ -802,7 +717,6 @@ def gitea_create_pr(
|
||||
host: Override the Gitea host.
|
||||
org: Override the owner/organization.
|
||||
repo: Override the repository name.
|
||||
worktree_path: Author worktree path; must match the path stored at lock time.
|
||||
|
||||
Returns:
|
||||
dict with 'number' of the created PR ('url' only with the reveal opt-in).
|
||||
@@ -841,13 +755,6 @@ def gitea_create_pr(
|
||||
|
||||
locked_issue = lock_data.get("issue_number")
|
||||
locked_branch = lock_data.get("branch_name")
|
||||
locked_worktree = lock_data.get("worktree_path")
|
||||
|
||||
worktree_check = issue_lock_worktree.verify_pr_worktree_matches_lock(
|
||||
locked_worktree, worktree_path, PROJECT_ROOT
|
||||
)
|
||||
if worktree_check["block"]:
|
||||
raise ValueError(worktree_check["reasons"][0])
|
||||
|
||||
if head != locked_branch:
|
||||
raise ValueError(
|
||||
@@ -1273,8 +1180,7 @@ _REVIEW_ACTIONS = {
|
||||
# 'comment' posts review findings without an approval/rejection state.
|
||||
# #14 names this eligibility category 'review'.
|
||||
"comment": ("review", "COMMENT"),
|
||||
# Gitea ReviewStateType uses APPROVED, not APPROVE — wrong event leaves PENDING (#244).
|
||||
"approve": ("approve", "APPROVED"),
|
||||
"approve": ("approve", "APPROVE"),
|
||||
"request_changes": ("request_changes", "REQUEST_CHANGES"),
|
||||
}
|
||||
|
||||
@@ -1484,42 +1390,6 @@ def _redact(text: str) -> str:
|
||||
# neither may drive the blocking/approval summaries.
|
||||
_VERDICT_STATES = ("APPROVED", "REQUEST_CHANGES", "COMMENT")
|
||||
|
||||
# Terminal review events that must be visible after live submission (#244).
|
||||
_SUBMIT_VISIBLE_EVENTS = frozenset({"APPROVED", "REQUEST_CHANGES"})
|
||||
|
||||
|
||||
def _latest_review_state_for_reviewer(raw_reviews: list, reviewer: str) -> str | None:
|
||||
"""Return the latest non-COMMENT verdict for *reviewer*, or None."""
|
||||
ordered = sorted(
|
||||
raw_reviews or [],
|
||||
key=lambda rv: ((rv.get("submitted_at") or ""), rv.get("id") or 0),
|
||||
)
|
||||
latest = None
|
||||
for rv in ordered:
|
||||
state = (rv.get("state") or "").upper()
|
||||
login = (rv.get("user") or {}).get("login", "")
|
||||
if login != reviewer or state not in _VERDICT_STATES or state == "COMMENT":
|
||||
continue
|
||||
latest = state
|
||||
return latest
|
||||
|
||||
|
||||
def _submit_pending_pull_review(
|
||||
h: str,
|
||||
o: str,
|
||||
r: str,
|
||||
pr_number: int,
|
||||
review_id: int,
|
||||
event: str,
|
||||
body: str,
|
||||
auth,
|
||||
) -> dict | None:
|
||||
"""Submit a PENDING draft review via Gitea's pending-review endpoint."""
|
||||
submit_url = (
|
||||
f"{repo_api_url(h, o, r)}/pulls/{pr_number}/reviews/{review_id}"
|
||||
)
|
||||
return api_request("POST", submit_url, auth, {"body": body, "event": event})
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_get_pr_review_feedback(
|
||||
@@ -1760,7 +1630,6 @@ def _evaluate_pr_review_submission(
|
||||
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
review_id = None
|
||||
submitted_state = None
|
||||
try:
|
||||
auth = _auth(h)
|
||||
review_url = f"{repo_api_url(h, o, r)}/pulls/{pr_number}/reviews"
|
||||
@@ -1768,43 +1637,10 @@ def _evaluate_pr_review_submission(
|
||||
resp = api_request("POST", review_url, auth, payload)
|
||||
if isinstance(resp, dict):
|
||||
review_id = resp.get("id")
|
||||
submitted_state = (resp.get("state") or "").upper() or None
|
||||
if (
|
||||
submitted_state == "PENDING"
|
||||
and review_id
|
||||
and event in _SUBMIT_VISIBLE_EVENTS
|
||||
):
|
||||
resp = _submit_pending_pull_review(
|
||||
h, o, r, pr_number, review_id, event, body, auth,
|
||||
)
|
||||
if isinstance(resp, dict):
|
||||
submitted_state = (resp.get("state") or "").upper() or None
|
||||
except Exception as exc: # noqa: BLE001 — redact before surfacing
|
||||
reasons.append(f"review submission failed: {_redact(str(exc))}")
|
||||
return result
|
||||
|
||||
if action in _TERMINAL_REVIEW_ACTIONS:
|
||||
try:
|
||||
auth = _auth(h)
|
||||
raw_reviews = (
|
||||
api_request("GET", f"{review_url}", auth) or []
|
||||
)
|
||||
visible = _latest_review_state_for_reviewer(raw_reviews, auth_user)
|
||||
except Exception as exc: # noqa: BLE001 — redact before surfacing
|
||||
reasons.append(
|
||||
f"could not verify submitted review verdict (fail closed): "
|
||||
f"{_redact(str(exc))}"
|
||||
)
|
||||
return result
|
||||
result["submitted_verdict"] = visible
|
||||
result["review_verdict_visible"] = visible == event
|
||||
if visible != event:
|
||||
reasons.append(
|
||||
f"review submission left verdict '{submitted_state or visible or 'PENDING'}'; "
|
||||
f"expected visible '{event}' for reviewer '{auth_user}' (fail closed)"
|
||||
)
|
||||
return result
|
||||
|
||||
record_live_review_mutation(pr_number, action, review_id)
|
||||
result["performed"] = True
|
||||
reasons.append(f"all gates passed; submitted '{event}' review on PR #{pr_number}")
|
||||
@@ -2259,9 +2095,6 @@ def gitea_merge_pr(
|
||||
5. If ``expected_changed_files`` is given and the PR's changed file set
|
||||
differs → refuse.
|
||||
6. Redundant self-merge block (authenticated user == PR author).
|
||||
7. Re-read formal review feedback (#167): refuse when
|
||||
``approval_visible`` is false or undismissed REQUEST_CHANGES block
|
||||
merge — PENDING draft approvals do not count (#244).
|
||||
|
||||
No force / ignore-checks option is exposed. Gitea's own ``mergeable`` signal
|
||||
(which reflects branch-protection required reviews and status checks) must
|
||||
@@ -2386,33 +2219,7 @@ def gitea_merge_pr(
|
||||
reasons.append("self-merge blocked (authenticated user is PR author)")
|
||||
return result
|
||||
|
||||
# Gate 7 — visible formal approval required (#244). PENDING drafts and
|
||||
# absent verdicts do not satisfy this gate even when Gitea mergeable is true.
|
||||
feedback = gitea_get_pr_review_feedback(
|
||||
pr_number=pr_number, remote=remote, host=host, org=org, repo=repo,
|
||||
)
|
||||
if not feedback.get("success"):
|
||||
reasons.append("PR review feedback unavailable before merge (fail closed)")
|
||||
reasons.extend(feedback.get("reasons", []))
|
||||
if feedback.get("permission_report"):
|
||||
result["permission_report"] = feedback["permission_report"]
|
||||
return result
|
||||
result["approval_visible"] = feedback.get("approval_visible")
|
||||
result["has_blocking_change_requests"] = feedback.get(
|
||||
"has_blocking_change_requests")
|
||||
if feedback.get("has_blocking_change_requests"):
|
||||
reasons.append(
|
||||
"undismissed REQUEST_CHANGES review blocks merge (fail closed)"
|
||||
)
|
||||
return result
|
||||
if not feedback.get("approval_visible"):
|
||||
reasons.append(
|
||||
"no visible APPROVED review on PR; verify review submission "
|
||||
"completed before merge (fail closed)"
|
||||
)
|
||||
return result
|
||||
|
||||
# Gate 8 — in-process mutation authority (#199): the last check before
|
||||
# Gate 7 — in-process mutation authority (#199): the last check before
|
||||
# the merge mutation, using the identity the eligibility gate proved.
|
||||
# A profile/identity flip or side-channel override between preflight
|
||||
# and merge fails closed here.
|
||||
@@ -2983,11 +2790,6 @@ def _profile_permission_block(required_operation: str, **extra_fields) -> dict |
|
||||
Returns a block dict when the active profile forbids *required_operation*,
|
||||
or ``None`` when the gate passes. Never performs network I/O.
|
||||
"""
|
||||
req_role = "reviewer" if any(required_operation.startswith(p) for p in (
|
||||
"gitea.pr.approve", "gitea.pr.merge", "gitea.pr.request_changes", "gitea.pr.review"
|
||||
)) else "author"
|
||||
_ensure_matching_profile(required_operation, req_role, extra_fields.get("remote"))
|
||||
|
||||
reasons = _profile_operation_gate(required_operation)
|
||||
if not reasons:
|
||||
return None
|
||||
@@ -3003,10 +2805,6 @@ def _profile_permission_block(required_operation: str, **extra_fields) -> dict |
|
||||
|
||||
def _namespace_mutation_block(mutation_task: str, **extra_fields) -> dict | None:
|
||||
"""Reviewer/author namespace alignment gate (#209)."""
|
||||
required_permission = task_capability_map.required_permission(mutation_task)
|
||||
required_role = task_capability_map.required_role(mutation_task)
|
||||
_ensure_matching_profile(required_permission, required_role, extra_fields.get("remote"))
|
||||
|
||||
try:
|
||||
profile = get_profile()
|
||||
except Exception as exc:
|
||||
@@ -4613,9 +4411,6 @@ def gitea_resolve_task_capability(
|
||||
|
||||
record_preflight_check("capability", required_role)
|
||||
|
||||
# Try automatic dispatch switching
|
||||
_ensure_matching_profile(required_permission, required_role, remote, host)
|
||||
|
||||
profile = get_profile()
|
||||
config = gitea_config.load_config()
|
||||
|
||||
@@ -4653,18 +4448,6 @@ def gitea_resolve_task_capability(
|
||||
if ok:
|
||||
matching_profiles.append(p_name)
|
||||
|
||||
configured = len(matching_profiles) > 0
|
||||
available_in_session = allowed_in_current_session
|
||||
restart_required = False
|
||||
reason = ""
|
||||
|
||||
if not allowed_in_current_session:
|
||||
if configured:
|
||||
restart_required = True
|
||||
reason = f"Reviewer profile exists but MCP server was added after session startup and is not attached."
|
||||
else:
|
||||
reason = f"No profile configured with permission '{required_permission}'."
|
||||
|
||||
switching = gitea_config.is_runtime_switching_enabled()
|
||||
different_namespace_required = False
|
||||
next_safe_action = "None; ready for operations."
|
||||
@@ -4749,20 +4532,14 @@ def gitea_resolve_task_capability(
|
||||
"runtime_switching_supported": switching,
|
||||
"different_mcp_namespace_required": different_namespace_required,
|
||||
"exact_safe_next_action": next_safe_action,
|
||||
"available_in_session": available_in_session,
|
||||
"configured": configured,
|
||||
"restart_required": restart_required,
|
||||
"reason": reason,
|
||||
}
|
||||
was_terminal = capability_stop_terminal.is_active()
|
||||
terminal = capability_stop_terminal.sync_from_capability_result(result)
|
||||
if terminal:
|
||||
result["terminal_mode"] = True
|
||||
result["terminal_report"] = (
|
||||
capability_stop_terminal.build_terminal_report(result)
|
||||
)
|
||||
elif was_terminal and not stop_required:
|
||||
result["cleared_stale_denial"] = True
|
||||
if stop_required:
|
||||
terminal = capability_stop_terminal.enter_from_capability_result(result)
|
||||
if terminal:
|
||||
result["terminal_mode"] = True
|
||||
result["terminal_report"] = (
|
||||
capability_stop_terminal.build_terminal_report(result)
|
||||
)
|
||||
return result
|
||||
|
||||
|
||||
|
||||
@@ -1,156 +0,0 @@
|
||||
"""Issue-lock worktree validation (#249).
|
||||
|
||||
Author issue locks must validate the caller's own scratch clone (or declared
|
||||
worktree path), not the shared MCP server working directory. A clean scratch at
|
||||
``master``/``main`` must remain lockable while an unrelated session leaves the
|
||||
shared dev worktree dirty or on a feature branch.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
|
||||
from reviewer_worktree import parse_dirty_tracked_files
|
||||
|
||||
AUTHOR_WORKTREE_ENV = "GITEA_AUTHOR_WORKTREE"
|
||||
BASE_BRANCHES = frozenset({"master", "main"})
|
||||
|
||||
|
||||
def resolve_author_worktree_path(
|
||||
explicit: str | None,
|
||||
project_root: str,
|
||||
) -> str:
|
||||
"""Resolve the author worktree path for lock/PR gates."""
|
||||
path = (explicit 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))
|
||||
|
||||
|
||||
def read_worktree_git_state(worktree_path: str) -> dict:
|
||||
"""Read branch name and porcelain status from a git worktree."""
|
||||
path = (worktree_path or "").strip()
|
||||
if not path:
|
||||
return {"current_branch": None, "porcelain_status": ""}
|
||||
|
||||
branch_res = subprocess.run(
|
||||
["git", "-C", path, "branch", "--show-current"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
current_branch = (branch_res.stdout or "").strip() or None
|
||||
|
||||
status_res = subprocess.run(
|
||||
["git", "-C", path, "status", "--porcelain"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
return {
|
||||
"current_branch": current_branch,
|
||||
"porcelain_status": status_res.stdout or "",
|
||||
}
|
||||
|
||||
|
||||
def assess_issue_lock_worktree(
|
||||
*,
|
||||
worktree_path: str,
|
||||
current_branch: str | None,
|
||||
porcelain_status: str,
|
||||
base_branches: frozenset[str] | None = None,
|
||||
) -> dict:
|
||||
"""Fail closed when lock preconditions are not met on the declared worktree."""
|
||||
bases = base_branches or BASE_BRANCHES
|
||||
reasons: list[str] = []
|
||||
path = (worktree_path or "").strip()
|
||||
if not path:
|
||||
reasons.append("worktree path not declared for issue lock; fail closed")
|
||||
return _assessment(False, reasons, path, None, [])
|
||||
|
||||
branch = (current_branch or "").strip()
|
||||
dirty_files = parse_dirty_tracked_files(porcelain_status)
|
||||
|
||||
if dirty_files:
|
||||
reasons.append(
|
||||
"tracked file edits exist before issue lock; "
|
||||
"lock must precede implementation work"
|
||||
)
|
||||
if not branch:
|
||||
reasons.append(
|
||||
"current branch unknown (detached HEAD?); issue lock must be taken "
|
||||
f"from base branch ({_base_list(bases)})"
|
||||
)
|
||||
elif branch not in bases:
|
||||
reasons.append(
|
||||
f"issue lock must be taken from base branch ({_base_list(bases)}), "
|
||||
f"not '{branch}'"
|
||||
)
|
||||
|
||||
proven = not reasons
|
||||
return _assessment(proven, reasons, path, branch or None, dirty_files)
|
||||
|
||||
|
||||
def format_issue_lock_worktree_error(assessment: dict) -> str:
|
||||
"""Format a single fail-closed error for ``gitea_lock_issue``."""
|
||||
reasons = list(assessment.get("reasons") or [])
|
||||
if not reasons:
|
||||
reasons = ["issue lock worktree validation failed"]
|
||||
return "; ".join(reasons) + " (fail closed)"
|
||||
|
||||
|
||||
def verify_pr_worktree_matches_lock(
|
||||
locked_worktree_path: str | None,
|
||||
declared_worktree_path: str | None,
|
||||
project_root: str,
|
||||
) -> dict:
|
||||
"""PR creation must use the same worktree the lock was validated against."""
|
||||
locked = (locked_worktree_path or "").strip()
|
||||
if not locked:
|
||||
return {"proven": True, "block": False, "reasons": []}
|
||||
|
||||
declared = resolve_author_worktree_path(declared_worktree_path, project_root)
|
||||
locked_real = os.path.realpath(locked)
|
||||
declared_real = os.path.realpath(declared)
|
||||
if locked_real != declared_real:
|
||||
return {
|
||||
"proven": False,
|
||||
"block": True,
|
||||
"reasons": [
|
||||
f"PR worktree '{declared_real}' does not match locked worktree "
|
||||
f"'{locked_real}' (fail closed)"
|
||||
],
|
||||
"locked_worktree_path": locked_real,
|
||||
"declared_worktree_path": declared_real,
|
||||
}
|
||||
return {
|
||||
"proven": True,
|
||||
"block": False,
|
||||
"reasons": [],
|
||||
"locked_worktree_path": locked_real,
|
||||
"declared_worktree_path": declared_real,
|
||||
}
|
||||
|
||||
|
||||
def _base_list(bases: frozenset[str]) -> str:
|
||||
return "/".join(sorted(bases))
|
||||
|
||||
|
||||
def _assessment(
|
||||
proven: bool,
|
||||
reasons: list[str],
|
||||
worktree_path: str,
|
||||
current_branch: str | None,
|
||||
dirty_files: list[str],
|
||||
) -> dict:
|
||||
return {
|
||||
"proven": proven,
|
||||
"block": not proven,
|
||||
"reasons": reasons,
|
||||
"worktree_path": worktree_path or None,
|
||||
"current_branch": current_branch,
|
||||
"dirty_files": dirty_files,
|
||||
}
|
||||
+119
-292
@@ -69,251 +69,6 @@ def resolve_repos_from_user_reference(
|
||||
return list(configured)
|
||||
|
||||
|
||||
_PR_NUMBER_RE = re.compile(r"(?:\bPR\s*#?|#)(\d+)\b", re.IGNORECASE)
|
||||
|
||||
|
||||
def _parse_pr_numbers(text):
|
||||
"""Extract PR numbers from operator context or backlog prose."""
|
||||
if not text:
|
||||
return []
|
||||
seen = set()
|
||||
ordered = []
|
||||
for match in _PR_NUMBER_RE.finditer(text):
|
||||
num = int(match.group(1))
|
||||
if num not in seen:
|
||||
seen.add(num)
|
||||
ordered.append(num)
|
||||
return ordered
|
||||
|
||||
|
||||
def _repo_hint_from_text(text, configured):
|
||||
"""Return a single configured repo named explicitly in *text*, if any."""
|
||||
if not text:
|
||||
return None
|
||||
lower = text.lower()
|
||||
for repo in configured:
|
||||
if repo.lower() in lower:
|
||||
return repo
|
||||
resolved = resolve_repos_from_user_reference(text, configured)
|
||||
if len(resolved) == 1:
|
||||
return resolved[0]
|
||||
return None
|
||||
|
||||
|
||||
def reconcile_queue_target(
|
||||
*,
|
||||
operator_context: str | None = None,
|
||||
supplied_pr_backlog: list[dict] | None = None,
|
||||
inventoried_repo: str | None = None,
|
||||
project_context: str | None = None,
|
||||
configured_repos: list[str] | None = None,
|
||||
) -> dict:
|
||||
"""Reconcile the inventory target repo before listing open PRs (#200).
|
||||
|
||||
Compares operator-supplied PR numbers/titles/backlog against the repo the
|
||||
workflow is about to inventory. Returns a ``queue_target_lock`` dict whose
|
||||
``status`` must be ``resolved`` before an empty queue may stop cleanly.
|
||||
"""
|
||||
if configured_repos is None:
|
||||
configured_repos = [
|
||||
"Scaled-Tech-Consulting/Gitea-Tools",
|
||||
"Scaled-Tech-Consulting/mcp-control-plane",
|
||||
]
|
||||
|
||||
backlog_items = []
|
||||
for item in supplied_pr_backlog or []:
|
||||
number = item.get("number")
|
||||
if number is None:
|
||||
continue
|
||||
repo = (item.get("repo") or "").strip() or None
|
||||
backlog_items.append({
|
||||
"number": int(number),
|
||||
"repo": repo,
|
||||
"title": (item.get("title") or "").strip() or None,
|
||||
})
|
||||
|
||||
context_numbers = _parse_pr_numbers(operator_context or "")
|
||||
backlog_numbers = [item["number"] for item in backlog_items]
|
||||
supplied_pr_numbers = list(dict.fromkeys(backlog_numbers + context_numbers))
|
||||
|
||||
context_repo = _repo_hint_from_text(operator_context, configured_repos)
|
||||
project_repo = _repo_hint_from_text(project_context, configured_repos)
|
||||
|
||||
resolution_source = None
|
||||
resolved_repo = None
|
||||
reasons = []
|
||||
|
||||
explicit_repos = {
|
||||
item["repo"] for item in backlog_items if item.get("repo")
|
||||
}
|
||||
if len(explicit_repos) == 1:
|
||||
resolved_repo = next(iter(explicit_repos))
|
||||
resolution_source = "supplied_pr_backlog"
|
||||
elif context_repo:
|
||||
resolved_repo = context_repo
|
||||
resolution_source = "operator_context"
|
||||
elif project_repo and supplied_pr_numbers:
|
||||
resolved_repo = project_repo
|
||||
resolution_source = "project_context"
|
||||
|
||||
if (
|
||||
context_repo
|
||||
and explicit_repos
|
||||
and context_repo not in explicit_repos
|
||||
):
|
||||
return {
|
||||
"status": "unresolved",
|
||||
"resolved_repo": None,
|
||||
"resolution_source": None,
|
||||
"supplied_pr_numbers": supplied_pr_numbers,
|
||||
"reconciliation": [],
|
||||
"inventoried_repo": (inventoried_repo or "").strip() or None,
|
||||
"reasons": [
|
||||
"operator context repo conflicts with supplied PR backlog "
|
||||
f"repos ({context_repo} vs {sorted(explicit_repos)})"
|
||||
],
|
||||
"allow_clean_stop": False,
|
||||
"allow_trusted_empty": False,
|
||||
}
|
||||
|
||||
reconciliation = []
|
||||
for number in supplied_pr_numbers:
|
||||
expected_repo = None
|
||||
for item in backlog_items:
|
||||
if item["number"] == number and item.get("repo"):
|
||||
expected_repo = item["repo"]
|
||||
break
|
||||
if expected_repo is None:
|
||||
expected_repo = resolved_repo
|
||||
reconciliation.append({
|
||||
"pr_number": number,
|
||||
"expected_repo": expected_repo,
|
||||
"inventoried_repo": inventoried_repo,
|
||||
"matches_inventoried_repo": (
|
||||
expected_repo is not None
|
||||
and inventoried_repo is not None
|
||||
and expected_repo == inventoried_repo
|
||||
),
|
||||
})
|
||||
|
||||
inventoried = (inventoried_repo or "").strip() or None
|
||||
|
||||
if supplied_pr_numbers and resolved_repo is None:
|
||||
status = "unresolved"
|
||||
reasons.append(
|
||||
"operator supplied PR numbers but target repository could not "
|
||||
"be resolved"
|
||||
)
|
||||
elif (
|
||||
supplied_pr_numbers
|
||||
and resolved_repo
|
||||
and inventoried
|
||||
and inventoried != resolved_repo
|
||||
):
|
||||
status = "target_repo_mismatch"
|
||||
reasons.append(
|
||||
f"inventoried repository '{inventoried}' does not own the "
|
||||
f"operator-supplied PR backlog (expected '{resolved_repo}')"
|
||||
)
|
||||
elif supplied_pr_numbers and resolved_repo and inventoried == resolved_repo:
|
||||
status = "resolved"
|
||||
elif not supplied_pr_numbers and inventoried:
|
||||
status = "resolved"
|
||||
resolution_source = resolution_source or "inventoried_repo_only"
|
||||
resolved_repo = inventoried
|
||||
elif not supplied_pr_numbers:
|
||||
status = "unresolved"
|
||||
reasons.append("no operator-supplied PR backlog to reconcile")
|
||||
else:
|
||||
status = "resolved"
|
||||
|
||||
allow_clean_stop = status == "resolved"
|
||||
allow_trusted_empty = status == "resolved"
|
||||
|
||||
return {
|
||||
"status": status,
|
||||
"resolved_repo": resolved_repo,
|
||||
"resolution_source": resolution_source,
|
||||
"supplied_pr_numbers": supplied_pr_numbers,
|
||||
"reconciliation": reconciliation,
|
||||
"inventoried_repo": inventoried,
|
||||
"reasons": reasons,
|
||||
"allow_clean_stop": allow_clean_stop,
|
||||
"allow_trusted_empty": allow_trusted_empty,
|
||||
}
|
||||
|
||||
|
||||
resolve_pr_queue_target = reconcile_queue_target
|
||||
|
||||
|
||||
def assess_queue_target_final_report(report_text, queue_target_lock):
|
||||
"""Require final reports to document queue-target reconciliation."""
|
||||
lock = queue_target_lock or {}
|
||||
text = report_text or ""
|
||||
lower = text.lower()
|
||||
missing = []
|
||||
|
||||
if not lock.get("resolved_repo"):
|
||||
missing.append("resolved repo")
|
||||
elif lock["resolved_repo"].lower() not in lower:
|
||||
missing.append("resolved repo")
|
||||
|
||||
source = lock.get("resolution_source")
|
||||
if not source:
|
||||
missing.append("resolution source")
|
||||
else:
|
||||
source_lower = str(source).lower()
|
||||
if (
|
||||
source_lower not in lower
|
||||
and source_lower.replace("_", " ") not in lower
|
||||
):
|
||||
missing.append("resolution source")
|
||||
|
||||
for number in lock.get("supplied_pr_numbers") or []:
|
||||
if f"#{number}" not in lower and f"pr {number}" not in lower:
|
||||
missing.append(f"supplied PR #{number} reconciliation")
|
||||
break
|
||||
|
||||
if lock.get("status") and str(lock["status"]).lower() not in lower:
|
||||
missing.append("queue_target_lock.status")
|
||||
|
||||
if missing:
|
||||
return {
|
||||
"complete": False,
|
||||
"downgraded": True,
|
||||
"missing_fields": missing,
|
||||
"reasons": [
|
||||
f"final report missing queue-target field: {field}"
|
||||
for field in missing
|
||||
],
|
||||
}
|
||||
return {
|
||||
"complete": True,
|
||||
"downgraded": False,
|
||||
"missing_fields": [],
|
||||
"reasons": [],
|
||||
}
|
||||
|
||||
|
||||
def format_queue_target_lock_report(lock: dict) -> list[str]:
|
||||
"""Render queue-target lock lines for inventory output."""
|
||||
lines = [
|
||||
f"queue_target_lock.status: {lock.get('status', 'unknown')}",
|
||||
]
|
||||
if lock.get("resolved_repo"):
|
||||
lines.append(f"queue_target_lock.resolved_repo: {lock['resolved_repo']}")
|
||||
if lock.get("resolution_source"):
|
||||
lines.append(
|
||||
f"queue_target_lock.resolution_source: {lock['resolution_source']}"
|
||||
)
|
||||
if lock.get("supplied_pr_numbers"):
|
||||
nums = ", ".join(f"#{n}" for n in lock["supplied_pr_numbers"])
|
||||
lines.append(f"queue_target_lock.supplied_pr_numbers: {nums}")
|
||||
for reason in lock.get("reasons") or []:
|
||||
lines.append(f"queue_target_lock.reason: {reason}")
|
||||
return lines
|
||||
|
||||
|
||||
SAFE_NEXT_ACTION_UNKNOWN_CONTAMINATION = (
|
||||
"evidence missing: report contamination as unknown and "
|
||||
"choose another PR or stop"
|
||||
@@ -1291,6 +1046,8 @@ HANDOFF_ROLE_FIELDS = {
|
||||
("Continuation mode", ("continuation mode", "continuation")),
|
||||
("Existing PR", ("existing pr", "pr number")),
|
||||
("PR author", ("pr author", "existing pr author")),
|
||||
("Issue claim status", ("issue claim", "claim status",
|
||||
"status:in-progress")),
|
||||
("Branch", ("branch", "existing branch")),
|
||||
("Old PR head", ("old pr head", "old head")),
|
||||
("New PR head", ("new pr head", "new head")),
|
||||
@@ -1299,6 +1056,13 @@ HANDOFF_ROLE_FIELDS = {
|
||||
),
|
||||
}
|
||||
|
||||
# Canonical secret/provenance sweep for comparable continuation runs (#189).
|
||||
CANONICAL_SECRET_SWEEP_COMMAND = (
|
||||
"git diff prgs/master...HEAD | rg -i "
|
||||
"'(token|password|secret|api[_-]?key|authorization:)'"
|
||||
)
|
||||
CANONICAL_SECRET_SWEEP_SCOPE = "full feature-branch diff against prgs/master"
|
||||
|
||||
|
||||
def _handoff_section_lines(report_text):
|
||||
"""Return the lines of the Controller Handoff section, or None."""
|
||||
@@ -1511,31 +1275,11 @@ def pr_inventory_trust_gate(
|
||||
user_context: str | None = None,
|
||||
corroboration_open_pr_counter: int | None = None,
|
||||
has_finality_metadata: bool = False,
|
||||
queue_target_lock: dict | None = None,
|
||||
) -> dict:
|
||||
"""Evaluate whether an empty PR list is trusted or untrusted.
|
||||
|
||||
Returns a dict with 'status', 'reasons', and 'corroborated'.
|
||||
"""
|
||||
lock = queue_target_lock or {}
|
||||
lock_status = lock.get("status")
|
||||
if lock_status == "target_repo_mismatch":
|
||||
return {
|
||||
"status": "target_repo_mismatch",
|
||||
"reasons": list(lock.get("reasons") or []),
|
||||
"corroborated": False,
|
||||
"queue_target_lock": lock_status,
|
||||
}
|
||||
if lock and lock_status != "resolved":
|
||||
return {
|
||||
"status": "untrusted_empty",
|
||||
"reasons": [
|
||||
f"queue_target_lock.status is '{lock_status}', not 'resolved'"
|
||||
] + list(lock.get("reasons") or []),
|
||||
"corroborated": False,
|
||||
"queue_target_lock": lock_status,
|
||||
}
|
||||
|
||||
if list_prs_response is None or not isinstance(list_prs_response, list):
|
||||
return {
|
||||
"status": "inventory_error",
|
||||
@@ -1605,7 +1349,6 @@ def pr_inventory_trust_gate(
|
||||
"status": "trusted_empty",
|
||||
"reasons": [],
|
||||
"corroborated": corroborated,
|
||||
"queue_target_lock": lock_status or "resolved",
|
||||
}
|
||||
|
||||
|
||||
@@ -1621,9 +1364,6 @@ def assess_reviewer_queue_inventory(
|
||||
required_repos: list[str] | None = None,
|
||||
*,
|
||||
user_context: str | None = None,
|
||||
operator_context: str | None = None,
|
||||
supplied_pr_backlog: list[dict] | None = None,
|
||||
project_context: str | None = None,
|
||||
) -> dict:
|
||||
"""Canonical reviewer queue path: completeness plus per-repo trust gates (#196).
|
||||
|
||||
@@ -1638,12 +1378,9 @@ def assess_reviewer_queue_inventory(
|
||||
completeness = assess_inventory_completeness(repo_reports, required)
|
||||
|
||||
trust_gates: dict[str, dict] = {}
|
||||
queue_target_locks: dict[str, dict] = {}
|
||||
blockers: list[str] = []
|
||||
can_claim_empty = bool(completeness.get("complete"))
|
||||
|
||||
context = operator_context or user_context
|
||||
|
||||
for report in repo_reports or []:
|
||||
repo = (report.get("repo") or "").strip()
|
||||
count = report.get("open_pr_count")
|
||||
@@ -1655,15 +1392,6 @@ def assess_reviewer_queue_inventory(
|
||||
if list_response is None:
|
||||
list_response = []
|
||||
|
||||
queue_target_lock = reconcile_queue_target(
|
||||
operator_context=context,
|
||||
supplied_pr_backlog=supplied_pr_backlog,
|
||||
inventoried_repo=repo,
|
||||
project_context=project_context,
|
||||
configured_repos=required,
|
||||
)
|
||||
queue_target_locks[repo] = queue_target_lock
|
||||
|
||||
gate = pr_inventory_trust_gate(
|
||||
list_response,
|
||||
remote=report.get("remote"),
|
||||
@@ -1677,7 +1405,6 @@ def assess_reviewer_queue_inventory(
|
||||
"corroboration_open_pr_counter"
|
||||
),
|
||||
has_finality_metadata=report.get("pagination_complete") is True,
|
||||
queue_target_lock=queue_target_lock,
|
||||
)
|
||||
trust_gates[repo] = gate
|
||||
status = gate.get("status")
|
||||
@@ -1697,21 +1424,14 @@ def assess_reviewer_queue_inventory(
|
||||
),
|
||||
"inventory_reasons": list(completeness.get("reasons") or []),
|
||||
"trust_gates": trust_gates,
|
||||
"queue_target_locks": queue_target_locks,
|
||||
"blockers": blockers,
|
||||
"reasons": list(completeness.get("reasons") or []) + blockers,
|
||||
}
|
||||
|
||||
|
||||
def format_pr_inventory_trust_gate_report(
|
||||
gate: dict,
|
||||
queue_target_lock: dict | None = None,
|
||||
) -> list[str]:
|
||||
def format_pr_inventory_trust_gate_report(gate: dict) -> list[str]:
|
||||
"""Render trust-gate lines for MCP inventory output."""
|
||||
lines = []
|
||||
if queue_target_lock:
|
||||
lines.extend(format_queue_target_lock_report(queue_target_lock))
|
||||
lines.append(f"pr_inventory_trust_gate.status: {gate.get('status', 'unknown')}")
|
||||
lines = [f"pr_inventory_trust_gate.status: {gate.get('status', 'unknown')}"]
|
||||
if gate.get("corroborated"):
|
||||
lines.append("pr_inventory_trust_gate.corroborated: true")
|
||||
for reason in gate.get("reasons") or []:
|
||||
@@ -1946,18 +1666,89 @@ def assess_fresh_issue_selection(classifications: list[dict] | None) -> dict:
|
||||
}
|
||||
|
||||
|
||||
def canonical_secret_sweep_report(*, clean: bool) -> dict:
|
||||
"""Return a sweep report using the canonical command/scope (#189)."""
|
||||
return {
|
||||
"method": CANONICAL_SECRET_SWEEP_COMMAND,
|
||||
"scope": CANONICAL_SECRET_SWEEP_SCOPE,
|
||||
"clean": clean,
|
||||
}
|
||||
|
||||
|
||||
def assess_force_with_lease_push_report(
|
||||
report_text: str,
|
||||
*,
|
||||
old_remote_head: str | None = None,
|
||||
expected_lease_head: str | None = None,
|
||||
new_pushed_head: str | None = None,
|
||||
branch_pushed: str | None = None,
|
||||
used_force_with_lease: bool | None = None,
|
||||
) -> dict:
|
||||
"""Issue #189: force-with-lease pushes must disclose lease evidence."""
|
||||
if not used_force_with_lease:
|
||||
return {"complete": True, "downgraded": False, "reasons": []}
|
||||
|
||||
text = report_text or ""
|
||||
lower = text.lower()
|
||||
reasons = []
|
||||
|
||||
if "force-with-lease" not in lower and "force with lease" not in lower:
|
||||
reasons.append(
|
||||
"continuation push used force-with-lease but report does not say so"
|
||||
)
|
||||
|
||||
for label, sha in (
|
||||
("old remote", old_remote_head),
|
||||
("lease", expected_lease_head),
|
||||
("pushed", new_pushed_head),
|
||||
):
|
||||
if not sha:
|
||||
reasons.append(f"force-with-lease proof missing {label} head SHA")
|
||||
elif not _FULL_SHA.match(sha.lower()):
|
||||
reasons.append(
|
||||
f"force-with-lease {label} head SHA is not a full 40-hex SHA"
|
||||
)
|
||||
elif sha.lower() not in lower:
|
||||
reasons.append(
|
||||
f"continuation report missing {label} head SHA in evidence"
|
||||
)
|
||||
|
||||
if branch_pushed:
|
||||
if branch_pushed.lower() not in lower:
|
||||
reasons.append("continuation report missing pushed branch name")
|
||||
only_branch_tokens = (
|
||||
"only feature branch",
|
||||
"push branch only",
|
||||
"only the feature branch",
|
||||
"only pushed feature branch",
|
||||
)
|
||||
if not any(t in lower for t in only_branch_tokens):
|
||||
reasons.append(
|
||||
"continuation report missing confirmation that only the "
|
||||
"feature branch was pushed"
|
||||
)
|
||||
|
||||
return {
|
||||
"complete": not reasons,
|
||||
"downgraded": bool(reasons),
|
||||
"reasons": reasons,
|
||||
}
|
||||
|
||||
|
||||
def assess_continuation_mode_report(
|
||||
report_text: str,
|
||||
*,
|
||||
pr_number: int | None = None,
|
||||
pr_author: str | None = None,
|
||||
issue_number: int | None = None,
|
||||
issue_claim_status: str | None = None,
|
||||
branch: str | None = None,
|
||||
old_head_sha: str | None = None,
|
||||
new_head_sha: str | None = None,
|
||||
session_authored_pr: bool | None = None,
|
||||
continuation_allowed_reason: str | None = None,
|
||||
) -> dict:
|
||||
"""Issue #188: continuation mode must disclose full PR evidence."""
|
||||
"""Issue #188/#189: continuation mode must disclose full PR evidence."""
|
||||
text = report_text or ""
|
||||
lower = text.lower()
|
||||
reasons = []
|
||||
@@ -1970,6 +1761,23 @@ def assess_continuation_mode_report(
|
||||
reasons.append(f"continuation report missing PR #{pr_number}")
|
||||
if pr_author and pr_author.lower() not in lower:
|
||||
reasons.append("continuation report missing PR author")
|
||||
if issue_number is not None:
|
||||
if (
|
||||
f"#{issue_number}" not in lower
|
||||
and f"issue #{issue_number}" not in lower
|
||||
and f"issue {issue_number}" not in lower
|
||||
):
|
||||
reasons.append(f"continuation report missing issue #{issue_number}")
|
||||
if issue_claim_status:
|
||||
claim_lower = issue_claim_status.lower()
|
||||
claim_tokens = (
|
||||
claim_lower,
|
||||
"claim status",
|
||||
"issue claim",
|
||||
"status:in-progress",
|
||||
)
|
||||
if not any(t in lower for t in claim_tokens):
|
||||
reasons.append("continuation report missing issue claim status")
|
||||
if branch and branch.lower() not in lower:
|
||||
reasons.append("continuation report missing branch name")
|
||||
|
||||
@@ -2106,12 +1914,26 @@ def assess_issue_selection_final_report(
|
||||
report_text,
|
||||
pr_number=proof.get("pr_number"),
|
||||
pr_author=proof.get("pr_author"),
|
||||
issue_number=proof.get("issue_number"),
|
||||
issue_claim_status=proof.get("issue_claim_status"),
|
||||
branch=proof.get("branch"),
|
||||
old_head_sha=proof.get("old_head_sha"),
|
||||
new_head_sha=proof.get("new_head_sha"),
|
||||
session_authored_pr=proof.get("session_authored_pr"),
|
||||
continuation_allowed_reason=proof.get("continuation_allowed_reason"),
|
||||
)
|
||||
push_proof = proof.get("push_proof") or {}
|
||||
checks["force_with_lease_push"] = assess_force_with_lease_push_report(
|
||||
report_text,
|
||||
old_remote_head=push_proof.get("old_remote_head"),
|
||||
expected_lease_head=push_proof.get("expected_lease_head"),
|
||||
new_pushed_head=push_proof.get("new_pushed_head"),
|
||||
branch_pushed=push_proof.get("branch_pushed"),
|
||||
used_force_with_lease=push_proof.get("used_force_with_lease"),
|
||||
)
|
||||
sweep = proof.get("secret_sweep")
|
||||
if sweep is not None:
|
||||
checks["secret_sweep"] = assess_secret_sweep(sweep)
|
||||
|
||||
reasons = []
|
||||
downgraded = False
|
||||
@@ -2120,6 +1942,11 @@ def assess_issue_selection_final_report(
|
||||
if verdict in ("missing", "incomplete"):
|
||||
downgraded = True
|
||||
reasons.extend(result.get("reasons") or [])
|
||||
elif result.get("proven") is False:
|
||||
downgraded = True
|
||||
reasons.extend(
|
||||
f"{name}: {r}" for r in (result.get("reasons") or [])
|
||||
)
|
||||
elif result.get("downgraded") or not result.get("complete", True):
|
||||
downgraded = True
|
||||
reasons.extend(
|
||||
|
||||
+28
-19
@@ -4,11 +4,6 @@ Reviewer sessions must never stash, reset, or otherwise manipulate unrelated
|
||||
local changes from another session. When the active worktree has dirty tracked
|
||||
files outside the PR scope, the workflow must stop or switch to a disposable
|
||||
scratch worktree (``scripts/worktree-review``).
|
||||
|
||||
Git command policy (#243): reviewers use an allowlist, not a blocklist.
|
||||
Any ``git`` invocation that does not match ``_READONLY_REVIEWER_GIT`` is
|
||||
forbidden — including ``checkout HEAD --``, ``checkout .``, ``switch``,
|
||||
and uncommon ``stash`` subcommands that older blocklists missed.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -16,7 +11,25 @@ from __future__ import annotations
|
||||
import re
|
||||
import shlex
|
||||
|
||||
# Read-only git operations reviewers may use for validation (#243 allowlist).
|
||||
# Subcommands that mutate unrelated local state — forbidden for reviewers.
|
||||
_FORBIDDEN_REVIEWER_GIT = re.compile(
|
||||
r"\bgit\b(?:\s+(?:-C\s+\S+\s+)?)?"
|
||||
r"(?:stash(?:\s+(?:push|pop|drop|apply|clear|list))?|"
|
||||
r"checkout\s+--|"
|
||||
r"restore\s+|"
|
||||
r"reset(?:\s+(?:--hard|--soft|--mixed|--merge))?|"
|
||||
r"clean(?:\s+(?:-f|-fd|-fdx|-x|-d|-n))*|"
|
||||
r"cherry-pick|"
|
||||
r"rebase|"
|
||||
r"merge|"
|
||||
r"commit(?:\s+(?:--amend|-a|-am))?|"
|
||||
r"push(?:\s+(?:--force|--force-with-lease))?|"
|
||||
r"branch\s+-[dD]|"
|
||||
r"worktree\s+remove)",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
# Read-only git operations reviewers may use for validation.
|
||||
_READONLY_REVIEWER_GIT = re.compile(
|
||||
r"\bgit\b(?:\s+(?:-C\s+\S+\s+)?)?"
|
||||
r"(?:fetch|status|diff|log|show|rev-parse|branch(?:\s+--show-current)?|"
|
||||
@@ -24,8 +37,6 @@ _READONLY_REVIEWER_GIT = re.compile(
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
_GIT_INVOCATION = re.compile(r"\bgit\b", re.IGNORECASE)
|
||||
|
||||
|
||||
def parse_dirty_tracked_files(porcelain: str) -> list[str]:
|
||||
"""Return tracked paths with local modifications from ``git status --porcelain``.
|
||||
@@ -62,26 +73,24 @@ def files_outside_pr_scope(
|
||||
return [path for path in dirty if path not in scope]
|
||||
|
||||
|
||||
def _is_git_command(command: str) -> bool:
|
||||
return bool(_GIT_INVOCATION.search((command or "").strip()))
|
||||
def is_forbidden_reviewer_git_command(command: str) -> bool:
|
||||
"""True when a shell command would mutate unrelated local/remote git state."""
|
||||
text = (command or "").strip()
|
||||
if not text:
|
||||
return False
|
||||
return bool(_FORBIDDEN_REVIEWER_GIT.search(text))
|
||||
|
||||
|
||||
def is_readonly_reviewer_git_command(command: str) -> bool:
|
||||
"""True when the command is an explicitly allowed read-only git operation."""
|
||||
text = (command or "").strip()
|
||||
if not text or not _is_git_command(text):
|
||||
if not text:
|
||||
return False
|
||||
if is_forbidden_reviewer_git_command(text):
|
||||
return False
|
||||
return bool(_READONLY_REVIEWER_GIT.search(text))
|
||||
|
||||
|
||||
def is_forbidden_reviewer_git_command(command: str) -> bool:
|
||||
"""True when a git command is not on the reviewer readonly allowlist."""
|
||||
text = (command or "").strip()
|
||||
if not text or not _is_git_command(text):
|
||||
return False
|
||||
return not is_readonly_reviewer_git_command(text)
|
||||
|
||||
|
||||
def assess_reviewer_git_command_log(commands: list[str] | None) -> dict:
|
||||
"""Fail closed when reviewer shell history includes forbidden git mutations."""
|
||||
forbidden = [
|
||||
|
||||
@@ -146,15 +146,6 @@ Worktree folder = branch with `/` replaced by `-`
|
||||
2. Fetch/prune: `git fetch <remote> --prune`.
|
||||
3. Confirm local `master` equals remote `master` (`git rev-list --left-right --count <remote>/master...master` → `0 0`).
|
||||
4. Create/claim the issue (§A).
|
||||
4b. **Issue lock from your scratch clone (#249):** when using
|
||||
`gitea_lock_issue`, pass `worktree_path` pointing at your own clean
|
||||
scratch clone (or set `GITEA_AUTHOR_WORKTREE`). The lock gate validates
|
||||
*that* path — clean tree on `master`/`main`, no tracked edits yet —
|
||||
not the shared MCP/orchestration checkout. Another session's dirty
|
||||
feature branch in the shared dev worktree must not block your lock.
|
||||
Never stash, reset, or checkout files in the shared worktree to satisfy
|
||||
the gate. Pass the same `worktree_path` to `gitea_create_pr` so the PR
|
||||
gate matches the lock record.
|
||||
5. Create the isolated worktree (§B) from latest remote `master`.
|
||||
6. Implement the narrow scope only — no unrelated refactors or formatting churn.
|
||||
7. Add/update focused tests when behavior changes.
|
||||
@@ -225,10 +216,7 @@ Worktree folder = branch with `/` replaced by `-`
|
||||
the other.
|
||||
Both configured repos must be reported with state filter, pagination proof,
|
||||
and open-PR count (`review_proofs.assess_inventory_completeness` and
|
||||
`resolve_repos_from_user_reference`). Before inventory, reconcile the
|
||||
operator-supplied PR backlog against the target repo
|
||||
(`review_proofs.reconcile_queue_target`); never report `trusted_empty`
|
||||
for one repo while ignoring contradictory supplied PR numbers in another.
|
||||
`resolve_repos_from_user_reference`).
|
||||
7. **Role-boundary proof (#175):** a reviewer queue task must not silently
|
||||
become author implementation. If no eligible PR exists, stop with the
|
||||
queue report. Do not claim issues, create branches, commit, push, or open
|
||||
@@ -249,9 +237,6 @@ Worktree folder = branch with `/` replaced by `-`
|
||||
validation completes, call `gitea_mark_final_review_decision`, then submit
|
||||
exactly one live review via
|
||||
`gitea_submit_pr_review(..., final_review_decision_ready=True)`.
|
||||
After submitting, re-read `gitea_get_pr_review_feedback` and confirm the
|
||||
verdict is visible (`approval_visible` true for APPROVE; PENDING drafts do
|
||||
not count — #244). Do not merge until a visible APPROVED review exists.
|
||||
Final reports must list exactly one review mutation
|
||||
(`review_proofs.assess_review_mutation_final_report`) unless an
|
||||
operator-approved correction flow was invoked and explained.
|
||||
@@ -323,9 +308,7 @@ When in doubt, stop and surface the discrepancy; do not guess or work around a g
|
||||
## I. Recovery patterns
|
||||
|
||||
- **Dirty worktree from another issue:** do not touch it. Start your issue in its
|
||||
own new worktree; unrelated dirty work must not block you. For Gitea-Tools
|
||||
author flows, lock the issue from your scratch clone (`worktree_path` on
|
||||
`gitea_lock_issue`) — do not manipulate the shared dev checkout.
|
||||
own new worktree; unrelated dirty work must not block you.
|
||||
- **Local `master` ahead of remote unexpectedly:** do not push `master`. Confirm
|
||||
the commits are preserved on a feature branch (local + remote) first, then
|
||||
`git reset --hard <remote>/master` to realign. Never discard commits that are
|
||||
@@ -396,11 +379,16 @@ Role-specific fields (append to the compact block):
|
||||
`Linked issue status:`, `Cleanup status:`
|
||||
- author tasks: `Selected issue:`, `Claim/comment status:`,
|
||||
`PR number opened:`, `No review/merge:` (explicit confirmation)
|
||||
- continuation tasks (#188): `Continuation mode:`, `Existing PR:`,
|
||||
`PR author:`, `Branch:`, `Old PR head:`, `New PR head:`,
|
||||
`Session authored PR:`, `Why continuation allowed:` — issues with open
|
||||
PRs are excluded from fresh selection unless operator explicitly requests
|
||||
continuation (`review_proofs.classify_issue_for_selection`,
|
||||
- continuation tasks (#188/#189): `Continuation mode:`, `Existing PR:`,
|
||||
`PR author:`, `Issue claim status:`, `Branch:`, `Old PR head:`,
|
||||
`New PR head:`, `Session authored PR:`, `Why continuation allowed:` —
|
||||
when rebasing with `git push --force-with-lease`, also record
|
||||
`Old remote head:`, `Lease head:`, `Pushed head:`, and confirm
|
||||
`Push branch only:` (feature branch only). Use the canonical secret sweep
|
||||
from `review_proofs.CANONICAL_SECRET_SWEEP_COMMAND` so runs are
|
||||
comparable. Issues with open PRs are excluded from fresh selection unless
|
||||
the operator explicitly requests continuation
|
||||
(`review_proofs.classify_issue_for_selection`,
|
||||
`assess_issue_selection_final_report`)
|
||||
- queue/inventory tasks: `Repositories checked:`, `Open PR counts:`,
|
||||
`Selected PR or reason none selected:`, `Inventory completeness:`
|
||||
|
||||
@@ -36,8 +36,6 @@ Rules (llm-project-workflow):
|
||||
report the starting worktree path and whether it was dirty. If unrelated
|
||||
tracked files exist outside the PR scope, STOP or run
|
||||
`scripts/worktree-review <pr-head-branch>` and validate in the scratch path.
|
||||
Scratch-clone validation is the norm; tests must not assume the shared
|
||||
development worktree or a repo-local ``venv/`` (#245).
|
||||
NEVER run `git stash`, `git stash pop/drop`, `git checkout --`, `git reset`,
|
||||
or `git clean` to manage another session's dirty files.
|
||||
- Final report must state: Worktree path, Worktree dirty (yes/no),
|
||||
|
||||
+2
-8
@@ -293,12 +293,9 @@ class TestGatedToolAudit(_AuditWiringBase):
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_merge_success_audited(self, _auth, mock_api):
|
||||
# user, pr, feedback pr+reviews, merge POST, readback.
|
||||
# user, pr, merge POST, readback — no extra identity call (uses result).
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
self._pr("author-bot"),
|
||||
[{"id": 1, "user": {"login": "reviewer-bot"}, "state": "APPROVED",
|
||||
"submitted_at": "2026-07-06T10:00:00Z", "dismissed": False}],
|
||||
{}, {"merged_commit_sha": "c1"},
|
||||
]
|
||||
env = self._env(GITEA_PROFILE_NAME="gitea-merger",
|
||||
@@ -335,10 +332,7 @@ class TestGatedToolAudit(_AuditWiringBase):
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_submit_review_success_audited(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 7, "state": "APPROVED"},
|
||||
[{"id": 7, "user": {"login": "reviewer-bot"}, "state": "APPROVED",
|
||||
"submitted_at": "2026-07-06T10:00:00Z", "dismissed": False}],
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 7},
|
||||
]
|
||||
env = self._env(GITEA_PROFILE_NAME="gitea-reviewer",
|
||||
GITEA_ALLOWED_OPERATIONS="read,review,approve")
|
||||
|
||||
@@ -29,8 +29,8 @@ CONFIG = {
|
||||
"username": "jcwalker3",
|
||||
"auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"},
|
||||
"allowed_operations": [
|
||||
"gitea.read", "gitea.issue.create", "gitea.issue.comment",
|
||||
"gitea.pr.create", "gitea.branch.push",
|
||||
"gitea.read", "gitea.issue.create", "gitea.pr.create",
|
||||
"gitea.branch.push",
|
||||
],
|
||||
"forbidden_operations": [
|
||||
"gitea.pr.approve", "gitea.pr.merge", "gitea.pr.review",
|
||||
@@ -97,30 +97,6 @@ class TestCapabilityStopTerminal(unittest.TestCase):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
mcp_server.gitea_list_prs(remote="prgs")
|
||||
self.assertIn("Cannot perform reviewer task", str(ctx.exception))
|
||||
self.assertIn("review_pr", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.api_request", return_value={"login": "jcwalker3"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_list_prs_allowed_after_author_task_clears_stale_denial(
|
||||
self, _auth, _api, _get_all,
|
||||
):
|
||||
with patch.dict(os.environ, self._env()):
|
||||
denied = mcp_server.gitea_resolve_task_capability(
|
||||
task="review_pr", remote="prgs"
|
||||
)
|
||||
self.assertTrue(denied["stop_required"])
|
||||
self.assertTrue(capability_stop_terminal.is_active())
|
||||
|
||||
cleared = mcp_server.gitea_resolve_task_capability(
|
||||
task="claim_issue", remote="prgs"
|
||||
)
|
||||
self.assertTrue(cleared["allowed_in_current_session"])
|
||||
self.assertTrue(cleared.get("cleared_stale_denial"))
|
||||
self.assertFalse(capability_stop_terminal.is_active())
|
||||
|
||||
prs = mcp_server.gitea_list_prs(remote="prgs")
|
||||
self.assertEqual(prs, [])
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "jcwalker3"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
|
||||
+15
-89
@@ -3,60 +3,9 @@ import subprocess
|
||||
import sys
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
import role_session_router
|
||||
from mcp_server import gitea_route_task_session, gitea_resolve_task_capability
|
||||
|
||||
_HEALTH_SUBPROCESS_TIMEOUT_SEC = 30
|
||||
|
||||
|
||||
def _health_test_python():
|
||||
"""Portable interpreter for subprocess health checks (#245).
|
||||
|
||||
Prefer the active pytest interpreter, then ``GITEA_TOOLS_TEST_PYTHON``,
|
||||
else skip — never hard-code ``<repo>/venv/bin/python``.
|
||||
"""
|
||||
if (
|
||||
sys.executable
|
||||
and os.path.isfile(sys.executable)
|
||||
and os.access(sys.executable, os.X_OK)
|
||||
):
|
||||
return sys.executable
|
||||
override = (os.environ.get("GITEA_TOOLS_TEST_PYTHON") or "").strip()
|
||||
if override and os.path.isfile(override) and os.access(override, os.X_OK):
|
||||
return override
|
||||
raise unittest.SkipTest(
|
||||
"repo venv not available; run under a python interpreter or set "
|
||||
"GITEA_TOOLS_TEST_PYTHON"
|
||||
)
|
||||
|
||||
|
||||
def _run_python_script(cwd, script_path, *, timeout=_HEALTH_SUBPROCESS_TIMEOUT_SEC):
|
||||
"""Run a script in a child process with timeout and guaranteed teardown."""
|
||||
python = _health_test_python()
|
||||
proc = subprocess.Popen(
|
||||
[python, script_path],
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
cwd=cwd,
|
||||
)
|
||||
try:
|
||||
stdout, stderr = proc.communicate(timeout=timeout)
|
||||
except subprocess.TimeoutExpired:
|
||||
proc.kill()
|
||||
stdout, stderr = proc.communicate()
|
||||
raise
|
||||
finally:
|
||||
if proc.poll() is None:
|
||||
proc.terminate()
|
||||
try:
|
||||
proc.wait(timeout=5)
|
||||
except subprocess.TimeoutExpired:
|
||||
proc.kill()
|
||||
proc.wait()
|
||||
return proc.returncode, stdout, stderr, proc
|
||||
|
||||
|
||||
class TestMCPHealth(unittest.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
@@ -69,17 +18,6 @@ class TestMCPHealth(unittest.TestCase):
|
||||
if os.path.exists(self.temp_file):
|
||||
os.remove(self.temp_file)
|
||||
|
||||
def test_health_test_python_prefers_sys_executable(self):
|
||||
resolved = _health_test_python()
|
||||
self.assertEqual(resolved, sys.executable)
|
||||
|
||||
def test_health_test_python_skips_when_no_interpreter(self):
|
||||
with patch.object(sys, "executable", ""), patch.dict(
|
||||
os.environ, {}, clear=True
|
||||
):
|
||||
with self.assertRaises(unittest.SkipTest):
|
||||
_health_test_python()
|
||||
|
||||
def test_startup_conflict_detection(self):
|
||||
# Create a Python file with conflict markers constructed dynamically
|
||||
with open(self.temp_file, "w") as f:
|
||||
@@ -89,44 +27,32 @@ class TestMCPHealth(unittest.TestCase):
|
||||
f.write("print('world')\n")
|
||||
f.write(">" * 7 + " main\n")
|
||||
|
||||
# Run mcp_server.py
|
||||
venv_python = os.path.join(self.project_root, "venv", "bin", "python")
|
||||
script_path = os.path.join(self.project_root, "mcp_server.py")
|
||||
returncode, _stdout, stderr, _proc = _run_python_script(
|
||||
self.project_root, script_path
|
||||
res = subprocess.run(
|
||||
[venv_python, script_path],
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
cwd=self.project_root
|
||||
)
|
||||
|
||||
self.assertEqual(returncode, 1)
|
||||
stderr_text = stderr.decode()
|
||||
self.assertIn("infra_stop", stderr_text)
|
||||
self.assertIn(
|
||||
"Unresolved merge conflict detected in test_temp_conflict.py",
|
||||
stderr_text,
|
||||
)
|
||||
self.assertEqual(res.returncode, 1)
|
||||
stderr = res.stderr.decode()
|
||||
self.assertIn("infra_stop", stderr)
|
||||
self.assertIn("Unresolved merge conflict detected in test_temp_conflict.py", stderr)
|
||||
|
||||
@patch("role_session_router.check_mid_merge", return_value=True)
|
||||
@patch(
|
||||
"mcp_server.get_profile",
|
||||
return_value={
|
||||
"profile_name": "prgs-reviewer",
|
||||
"allowed_operations": ["gitea.pr.review"],
|
||||
},
|
||||
)
|
||||
@patch("mcp_server.get_profile", return_value={"profile_name": "prgs-reviewer", "allowed_operations": ["gitea.pr.review"]})
|
||||
def test_route_task_session_blocks_during_merge(self, mock_profile, mock_check):
|
||||
res = gitea_route_task_session("review_pr")
|
||||
self.assertEqual(res["route_result"], "infra_stop")
|
||||
self.assertIn("infra_stop", res["message"])
|
||||
|
||||
@patch("role_session_router.check_mid_merge", return_value=True)
|
||||
@patch(
|
||||
"mcp_server.get_profile",
|
||||
return_value={
|
||||
"profile_name": "prgs-reviewer",
|
||||
"allowed_operations": ["gitea.pr.review"],
|
||||
},
|
||||
)
|
||||
def test_resolve_task_capability_blocks_during_merge(
|
||||
self, mock_profile, mock_check
|
||||
):
|
||||
@patch("mcp_server.get_profile", return_value={"profile_name": "prgs-reviewer", "allowed_operations": ["gitea.pr.review"]})
|
||||
def test_resolve_task_capability_blocks_during_merge(self, mock_profile, mock_check):
|
||||
res = gitea_resolve_task_capability("review_pr")
|
||||
self.assertTrue(res["infra_stop"])
|
||||
self.assertFalse(res["allowed_in_current_session"])
|
||||
self.assertIn("infra_stop", res["exact_safe_next_action"])
|
||||
self.assertIn("infra_stop", res["exact_safe_next_action"])
|
||||
|
||||
@@ -1,99 +0,0 @@
|
||||
"""Tests for issue-lock worktree validation (#249)."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
import issue_lock_worktree # noqa: E402
|
||||
|
||||
|
||||
class TestIssueLockWorktreeAssessment(unittest.TestCase):
|
||||
def test_clean_base_branch_passes(self):
|
||||
result = issue_lock_worktree.assess_issue_lock_worktree(
|
||||
worktree_path="/scratch/wt",
|
||||
current_branch="master",
|
||||
porcelain_status="",
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
self.assertFalse(result["block"])
|
||||
|
||||
def test_dirty_tracked_files_fail(self):
|
||||
result = issue_lock_worktree.assess_issue_lock_worktree(
|
||||
worktree_path="/scratch/wt",
|
||||
current_branch="master",
|
||||
porcelain_status=" M gitea_mcp_server.py\n",
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertIn("tracked file edits exist before issue lock", result["reasons"][0])
|
||||
|
||||
def test_feature_branch_fails(self):
|
||||
result = issue_lock_worktree.assess_issue_lock_worktree(
|
||||
worktree_path="/scratch/wt",
|
||||
current_branch="feat/issue-243-forbidden-git-gaps",
|
||||
porcelain_status="",
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertIn("issue lock must be taken from base branch", result["reasons"][0])
|
||||
|
||||
def test_untracked_files_do_not_block(self):
|
||||
result = issue_lock_worktree.assess_issue_lock_worktree(
|
||||
worktree_path="/scratch/wt",
|
||||
current_branch="main",
|
||||
porcelain_status="?? notes.txt\n",
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
|
||||
class TestIssueLockWorktreeResolution(unittest.TestCase):
|
||||
def test_explicit_path_wins(self):
|
||||
resolved = issue_lock_worktree.resolve_author_worktree_path(
|
||||
"/tmp/scratch/wt", "/shared/dev"
|
||||
)
|
||||
self.assertEqual(resolved, os.path.realpath("/tmp/scratch/wt"))
|
||||
|
||||
def test_env_var_when_explicit_missing(self):
|
||||
with patch.dict(os.environ, {"GITEA_AUTHOR_WORKTREE": "/tmp/from-env"}):
|
||||
resolved = issue_lock_worktree.resolve_author_worktree_path(
|
||||
None, "/shared/dev"
|
||||
)
|
||||
self.assertEqual(resolved, os.path.realpath("/tmp/from-env"))
|
||||
|
||||
def test_project_root_fallback(self):
|
||||
resolved = issue_lock_worktree.resolve_author_worktree_path(
|
||||
None, "/shared/dev"
|
||||
)
|
||||
self.assertEqual(resolved, os.path.realpath("/shared/dev"))
|
||||
|
||||
|
||||
class TestPrWorktreeMatch(unittest.TestCase):
|
||||
def test_matching_paths_pass(self):
|
||||
result = issue_lock_worktree.verify_pr_worktree_matches_lock(
|
||||
"/tmp/scratch/wt",
|
||||
"/tmp/scratch/wt",
|
||||
"/shared/dev",
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_mismatch_fails(self):
|
||||
result = issue_lock_worktree.verify_pr_worktree_matches_lock(
|
||||
"/tmp/scratch/wt",
|
||||
"/shared/dev",
|
||||
"/shared/dev",
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertIn("does not match locked worktree", result["reasons"][0])
|
||||
|
||||
def test_missing_locked_path_skips_check(self):
|
||||
result = issue_lock_worktree.verify_pr_worktree_matches_lock(
|
||||
None,
|
||||
"/any/path",
|
||||
"/shared/dev",
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
+20
-270
@@ -47,22 +47,6 @@ import mcp_server
|
||||
|
||||
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
|
||||
|
||||
|
||||
def _formal_review(reviewer, verdict, sha="abc123", review_id=1):
|
||||
return {
|
||||
"id": review_id,
|
||||
"user": {"login": reviewer},
|
||||
"state": verdict,
|
||||
"commit_id": sha,
|
||||
"submitted_at": "2026-07-06T10:00:00Z",
|
||||
"dismissed": False,
|
||||
}
|
||||
|
||||
|
||||
def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"):
|
||||
return [_formal_review(reviewer, "APPROVED", sha=sha)]
|
||||
|
||||
|
||||
# Issue-write tools are profile-gated (#69).
|
||||
ISSUE_WRITE_ENV = {
|
||||
"GITEA_ALLOWED_OPERATIONS": (
|
||||
@@ -504,10 +488,6 @@ class TestMergePR(unittest.TestCase):
|
||||
f"unexpected merge mutation: {method} {url}",
|
||||
)
|
||||
|
||||
def _feedback_reads(self, author="author-bot", sha="abc123"):
|
||||
"""PR + reviews GETs for gitea_get_pr_review_feedback during merge."""
|
||||
return [self._pr(author, sha=sha), _visible_approval_reviews(sha=sha)]
|
||||
|
||||
# -- success --------------------------------------------------------------
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@@ -515,7 +495,6 @@ class TestMergePR(unittest.TestCase):
|
||||
def test_merge_succeeds_when_all_gates_pass(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
*self._feedback_reads(),
|
||||
{}, # merge POST
|
||||
{"merged_commit_sha": "mergecommit99"}, # read-back
|
||||
]
|
||||
@@ -532,8 +511,8 @@ class TestMergePR(unittest.TestCase):
|
||||
self.assertEqual(r["head_sha"], "abc123")
|
||||
self.assertEqual(r["merge_method"], "squash")
|
||||
self.assertEqual(r["merge_commit"], "mergecommit99")
|
||||
# 5th call is the merge POST with the requested method/title/message.
|
||||
merge_call = mock_api.call_args_list[4]
|
||||
# 3rd call is the merge POST with the requested method/title/message.
|
||||
merge_call = mock_api.call_args_list[2]
|
||||
self.assertEqual(merge_call.args[0], "POST")
|
||||
self.assertTrue(merge_call.args[1].endswith("/pulls/8/merge"))
|
||||
payload = merge_call.args[3]
|
||||
@@ -548,7 +527,6 @@ class TestMergePR(unittest.TestCase):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
[{"filename": "a.py"}, {"filename": "b.py"}], # files
|
||||
*self._feedback_reads(),
|
||||
{}, # merge POST
|
||||
{"merged_commit_sha": "c1"}, # read-back
|
||||
]
|
||||
@@ -568,7 +546,6 @@ class TestMergePR(unittest.TestCase):
|
||||
"""Merge OK + read-back GET failure => explicit cleanup skip, not silence."""
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
*self._feedback_reads(),
|
||||
{}, # merge POST
|
||||
RuntimeError("HTTP 502: Gitea upstream unavailable"), # read-back fails
|
||||
]
|
||||
@@ -584,8 +561,8 @@ class TestMergePR(unittest.TestCase):
|
||||
# The skip is explicit, not silent.
|
||||
self.assertEqual(r["cleanup_status"], "skipped (merge read-back failed)")
|
||||
# No tracker-cleanup API traffic after the failed read-back:
|
||||
# user, PR (eligibility), feedback PR+reviews, merge POST, read-back.
|
||||
self.assertEqual(mock_api.call_count, 6)
|
||||
# user, PR (eligibility), merge POST, read-back — and nothing more.
|
||||
self.assertEqual(mock_api.call_count, 4)
|
||||
for c in mock_api.call_args_list:
|
||||
self.assertNotEqual(c.args[0], "DELETE")
|
||||
|
||||
@@ -597,7 +574,6 @@ class TestMergePR(unittest.TestCase):
|
||||
"""Unexpected cleanup exception => merge still succeeds; error surfaced redacted."""
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
*self._feedback_reads(),
|
||||
{}, # merge POST
|
||||
{"merged_commit_sha": "c9"}, # read-back OK
|
||||
]
|
||||
@@ -785,7 +761,6 @@ class TestMergePR(unittest.TestCase):
|
||||
def test_output_redacts_secrets(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
*self._feedback_reads(),
|
||||
{}, {"merged_commit_sha": "c1"},
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
@@ -803,7 +778,6 @@ class TestMergePR(unittest.TestCase):
|
||||
def test_merge_error_message_redacts_credential(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
*self._feedback_reads(),
|
||||
RuntimeError("HTTP 500: token abc-secret-xyz rejected"),
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
@@ -816,45 +790,6 @@ class TestMergePR(unittest.TestCase):
|
||||
self.assertIn("[REDACTED]", blob)
|
||||
self.assertNotIn("abc-secret-xyz", blob)
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_merge_blocked_without_visible_approval(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
self._pr("author-bot"),
|
||||
[_formal_review("sysadmin", "PENDING")],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8), remote="prgs")
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertFalse(r.get("approval_visible"))
|
||||
self.assertTrue(any("no visible APPROVED review" in x for x in r["reasons"]))
|
||||
self._assert_no_merge_call(mock_api)
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_merge_blocked_on_request_changes(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
self._pr("author-bot"),
|
||||
[
|
||||
_formal_review("reviewer-bot", "APPROVED"),
|
||||
_formal_review("reviewer-bot", "REQUEST_CHANGES", review_id=2),
|
||||
],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8), remote="prgs")
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertTrue(r.get("has_blocking_change_requests"))
|
||||
self.assertTrue(any("REQUEST_CHANGES" in x for x in r["reasons"]))
|
||||
self._assert_no_merge_call(mock_api)
|
||||
|
||||
|
||||
class TestNoUngatedMergePath(unittest.TestCase):
|
||||
"""Prove no other exposed tool can merge (#16 surface audit)."""
|
||||
@@ -1610,9 +1545,7 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_duplicate_terminal_decision_blocked(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 1, "state": "APPROVED"},
|
||||
[_formal_review("reviewer-bot", "APPROVED", review_id=1)],
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 1},
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
]
|
||||
gitea_mark_final_review_decision(
|
||||
@@ -1695,9 +1628,7 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_approve_succeeds_when_eligible(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 7, "state": "APPROVED"},
|
||||
[_formal_review("reviewer-bot", "APPROVED", review_id=7)],
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 7},
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||
@@ -1707,63 +1638,16 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertTrue(r["performed"])
|
||||
self.assertTrue(r.get("review_verdict_visible"))
|
||||
self.assertEqual(r["authenticated_user"], "reviewer-bot")
|
||||
self.assertEqual(r["pr_author"], "author-bot")
|
||||
self.assertEqual(r["head_sha"], "abc123")
|
||||
post_calls = [
|
||||
c for c in mock_api.call_args_list
|
||||
if c.args[0] == "POST" and c.args[1].endswith("/pulls/8/reviews")
|
||||
]
|
||||
self.assertEqual(len(post_calls), 1)
|
||||
payload = post_calls[0].args[3]
|
||||
self.assertEqual(payload["event"], "APPROVED")
|
||||
method, url = mock_api.call_args.args[0], mock_api.call_args.args[1]
|
||||
self.assertEqual(method, "POST")
|
||||
self.assertTrue(url.endswith("/pulls/8/reviews"))
|
||||
payload = mock_api.call_args.args[3]
|
||||
self.assertEqual(payload["event"], "APPROVE")
|
||||
self.assertEqual(payload["commit_id"], "abc123")
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_approve_fails_when_verdict_stays_pending(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 7, "state": "PENDING"},
|
||||
{"id": 7, "state": "PENDING"},
|
||||
[_formal_review("reviewer-bot", "PENDING", review_id=7)],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_submit_pr_review(
|
||||
pr_number=8, action="approve", body="LGTM", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertFalse(r.get("review_verdict_visible"))
|
||||
self.assertTrue(any("expected visible 'APPROVED'" in x for x in r["reasons"]))
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_approve_submits_pending_draft_when_api_returns_pending(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 7, "state": "PENDING"},
|
||||
{"id": 7, "state": "APPROVED"},
|
||||
[_formal_review("reviewer-bot", "APPROVED", review_id=7)],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_submit_pr_review(
|
||||
pr_number=8, action="approve", body="LGTM", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertTrue(r["performed"])
|
||||
submit_calls = [
|
||||
c for c in mock_api.call_args_list
|
||||
if c.args[0] == "POST" and "/reviews/7" in c.args[1]
|
||||
]
|
||||
self.assertEqual(len(submit_calls), 1)
|
||||
self.assertEqual(submit_calls[0].args[3]["event"], "APPROVED")
|
||||
|
||||
# -- request_changes ------------------------------------------------------
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@@ -1771,9 +1655,7 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
def test_request_changes_succeeds_when_eligible(self, _auth, mock_api):
|
||||
gitea_mark_final_review_decision(8, "request_changes", remote="prgs")
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 9, "state": "REQUEST_CHANGES"},
|
||||
[_formal_review("reviewer-bot", "REQUEST_CHANGES", review_id=9)],
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 9},
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,request_changes"}
|
||||
@@ -1784,12 +1666,7 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertTrue(r["performed"])
|
||||
post_calls = [
|
||||
c for c in mock_api.call_args_list
|
||||
if c.args[0] == "POST" and c.args[1].endswith("/pulls/8/reviews")
|
||||
]
|
||||
self.assertEqual(len(post_calls), 1)
|
||||
self.assertEqual(post_calls[0].args[3]["event"], "REQUEST_CHANGES")
|
||||
self.assertEqual(mock_api.call_args.args[3]["event"], "REQUEST_CHANGES")
|
||||
|
||||
def test_request_changes_blocked_without_eligibility(self):
|
||||
gitea_mark_final_review_decision(8, "request_changes", remote="prgs")
|
||||
@@ -1900,8 +1777,7 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
patch("mcp_server.api_request") as mock_api:
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot", sha="abc123"),
|
||||
{"id": 5, "state": "APPROVED"},
|
||||
[_formal_review("reviewer-bot", "APPROVED", review_id=5)],
|
||||
{"id": 5},
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||
@@ -2053,12 +1929,8 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_correction_flow_allows_second_terminal_review(self, _auth, mock_api):
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 42, "state": "APPROVED"},
|
||||
[_formal_review("reviewer-bot", "APPROVED", review_id=42)],
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 43, "state": "REQUEST_CHANGES"},
|
||||
[_formal_review("reviewer-bot", "REQUEST_CHANGES", review_id=43)],
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 42},
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 43},
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve,request_changes"}
|
||||
@@ -2131,7 +2003,7 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
patch("gitea_audit.audit_enabled", return_value=True).start()
|
||||
self.mock_audit = patch("gitea_audit.write_event").start()
|
||||
# gitea.pr.close: closing a PR via gitea_edit_pr is capability-gated (#216).
|
||||
patch("mcp_server.get_profile", return_value={"profile_name": "test", "allowed_operations": ["read", "merge", "edit", "close", "gitea.pr.close", "gitea.issue.close"], "audit_label": "test", "forbidden_operations": []}).start()
|
||||
patch("mcp_server.get_profile", return_value={"profile_name": "test", "allowed_operations": ["merge", "edit", "close", "gitea.pr.close", "gitea.issue.close"], "audit_label": "test", "forbidden_operations": []}).start()
|
||||
|
||||
def tearDown(self):
|
||||
patch.stopall()
|
||||
@@ -2179,8 +2051,6 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "GET" and "/user" in url:
|
||||
return {"login": "merger"}
|
||||
if method == "GET" and url.endswith("/reviews"):
|
||||
return [_formal_review("reviewer", "APPROVED", sha="sha123")]
|
||||
if method == "GET" and "pulls/1" in url and "/files" not in url:
|
||||
return {
|
||||
"user": {"login": "author"},
|
||||
@@ -2213,8 +2083,6 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "GET" and "/user" in url:
|
||||
return {"login": "merger"}
|
||||
if method == "GET" and url.endswith("/reviews"):
|
||||
return [_formal_review("reviewer", "APPROVED", sha="sha123")]
|
||||
if method == "GET" and "pulls/1" in url and "/files" not in url:
|
||||
return {
|
||||
"user": {"login": "author"},
|
||||
@@ -2934,41 +2802,26 @@ class TestVerifyMutationAuthority(unittest.TestCase):
|
||||
class TestIssueLocking(unittest.TestCase):
|
||||
"""Test issue locking and PR gating constraints."""
|
||||
|
||||
@staticmethod
|
||||
def _clean_master_git_state():
|
||||
return {"current_branch": "master", "porcelain_status": ""}
|
||||
|
||||
def tearDown(self):
|
||||
if os.path.exists(ISSUE_LOCK_FILE):
|
||||
os.remove(ISSUE_LOCK_FILE)
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={"current_branch": "master", "porcelain_status": ""},
|
||||
)
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_issue_success(self, _auth, mock_api, _git_state):
|
||||
def test_lock_issue_success(self, _auth, mock_api):
|
||||
mock_api.return_value = [] # no open PRs
|
||||
res = gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertTrue(res["success"])
|
||||
self.assertTrue(os.path.exists(ISSUE_LOCK_FILE))
|
||||
with open(ISSUE_LOCK_FILE, encoding="utf-8") as f:
|
||||
lock = json.load(f)
|
||||
self.assertIn("worktree_path", lock)
|
||||
|
||||
def test_lock_issue_mismatch_branch_fails(self):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-195-mutations", remote="prgs")
|
||||
self.assertIn("must contain locked issue pattern", str(ctx.exception))
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={"current_branch": "master", "porcelain_status": ""},
|
||||
)
|
||||
@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):
|
||||
def test_lock_issue_reused_by_open_pr_branch(self, _auth, mock_api):
|
||||
mock_api.return_value = [{
|
||||
"number": 200,
|
||||
"head": {"ref": "feat/issue-196-boundary"},
|
||||
@@ -2979,13 +2832,9 @@ class TestIssueLocking(unittest.TestCase):
|
||||
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))
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={"current_branch": "master", "porcelain_status": ""},
|
||||
)
|
||||
@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):
|
||||
def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, mock_api):
|
||||
mock_api.return_value = [{
|
||||
"number": 200,
|
||||
"head": {"ref": "feat/other-branch"},
|
||||
@@ -2996,62 +2845,6 @@ class TestIssueLocking(unittest.TestCase):
|
||||
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))
|
||||
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_from_clean_scratch_worktree(self, _auth, _api):
|
||||
scratch = "/tmp/gitea-tools-author-scratch/issue-249-clean"
|
||||
with patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={"current_branch": "master", "porcelain_status": ""},
|
||||
) as mock_git:
|
||||
res = gitea_lock_issue(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
remote="prgs",
|
||||
worktree_path=scratch,
|
||||
)
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["worktree_path"], os.path.realpath(scratch))
|
||||
mock_git.assert_called_once_with(os.path.realpath(scratch))
|
||||
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_fails_when_declared_worktree_dirty(self, _auth, _api):
|
||||
with patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={
|
||||
"current_branch": "master",
|
||||
"porcelain_status": " M gitea_mcp_server.py\n",
|
||||
},
|
||||
):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_lock_issue(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
remote="prgs",
|
||||
worktree_path="/tmp/scratch/wt",
|
||||
)
|
||||
self.assertIn("tracked file edits exist before issue lock", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_fails_when_declared_worktree_not_on_base(self, _auth, _api):
|
||||
with patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={
|
||||
"current_branch": "feat/issue-243-forbidden-git-gaps",
|
||||
"porcelain_status": "",
|
||||
},
|
||||
):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_lock_issue(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
remote="prgs",
|
||||
worktree_path="/tmp/scratch/wt",
|
||||
)
|
||||
self.assertIn("issue lock must be taken from base branch", 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)
|
||||
@@ -3100,49 +2893,6 @@ class TestIssueLocking(unittest.TestCase):
|
||||
gitea_create_pr(title="feat: X refs #196", head="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("must contain 'Closes #196' or 'Fixes #196' exactly", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_create_pr_worktree_mismatch_fails(self, _auth, _role):
|
||||
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-pr")
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(_sample_issue_lock(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
worktree_path=scratch,
|
||||
), f)
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_create_pr(
|
||||
title="feat: lock scratch worktree Closes #249",
|
||||
head="feat/issue-249-issue-lock-scratch-worktree",
|
||||
remote="prgs",
|
||||
worktree_path="/tmp/other-scratch",
|
||||
)
|
||||
self.assertIn("does not match locked worktree", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@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_honors_scratch_worktree_lock(self, _auth, _role, mock_api):
|
||||
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-e2e")
|
||||
mock_api.return_value = {"number": 250, "html_url": "https://example/pr/250"}
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(_sample_issue_lock(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
worktree_path=scratch,
|
||||
), f)
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
res = gitea_create_pr(
|
||||
title="feat: issue-lock scratch worktree Closes #249",
|
||||
head="feat/issue-249-issue-lock-scratch-worktree",
|
||||
remote="prgs",
|
||||
worktree_path=scratch,
|
||||
)
|
||||
self.assertEqual(res["number"], 250)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Pre-flight ordering and workspace edit block (#210)
|
||||
|
||||
@@ -119,28 +119,12 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
mock_get_all.return_value = [
|
||||
{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": "abc1"}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "other_user"}}
|
||||
]
|
||||
# mock_api: inventory whoami, eligibility whoami, eligibility PR,
|
||||
# POST review (#244: state + visible-verdict GET reviews).
|
||||
# mock_api: 1) /user (inventory), 2) /user (eligibility), 3) /pulls/1 (eligibility), 4) /pulls/1/reviews (POST review)
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer1"},
|
||||
{"login": "reviewer1"},
|
||||
{
|
||||
"user": {"login": "other_user"},
|
||||
"state": "open",
|
||||
"head": {"sha": "abc1"},
|
||||
"mergeable": True,
|
||||
},
|
||||
{"id": 100, "state": "APPROVED"},
|
||||
[
|
||||
{
|
||||
"id": 100,
|
||||
"user": {"login": "reviewer1"},
|
||||
"state": "APPROVED",
|
||||
"commit_id": "abc1",
|
||||
"submitted_at": "2026-07-06T10:00:00Z",
|
||||
"dismissed": False,
|
||||
}
|
||||
],
|
||||
{"login": "reviewer1"}, # inventory whoami
|
||||
{"login": "reviewer1"}, # submit eligibility whoami
|
||||
{"user": {"login": "other_user"}, "state": "open", "head": {"sha": "abc1"}, "mergeable": True}, # submit eligibility PR
|
||||
{"id": 100}, # POST review
|
||||
]
|
||||
|
||||
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
|
||||
|
||||
+137
-140
@@ -28,13 +28,15 @@ from review_proofs import ( # noqa: E402
|
||||
assess_capability_proof,
|
||||
assess_contradictory_no_pr_claim,
|
||||
assess_continuation_mode_report,
|
||||
assess_force_with_lease_push_report,
|
||||
canonical_secret_sweep_report,
|
||||
CANONICAL_SECRET_SWEEP_COMMAND,
|
||||
assess_controller_handoff,
|
||||
assess_edited_pr_inventory_coverage,
|
||||
assess_empty_queue_report,
|
||||
assess_fresh_issue_selection,
|
||||
assess_inventory_completeness,
|
||||
assess_issue_selection_final_report,
|
||||
assess_queue_target_final_report,
|
||||
assess_reviewer_queue_inventory,
|
||||
assess_live_state_recheck,
|
||||
assess_review_mutation_final_report,
|
||||
@@ -46,7 +48,6 @@ from review_proofs import ( # noqa: E402
|
||||
build_final_report,
|
||||
classify_issue_for_selection,
|
||||
pr_inventory_trust_gate,
|
||||
reconcile_queue_target,
|
||||
resolve_repos_from_user_reference,
|
||||
verify_pinned_head_checkout,
|
||||
)
|
||||
@@ -1145,144 +1146,6 @@ class TestReviewMutationFinalReport(unittest.TestCase):
|
||||
self.assertTrue(final["review_mutation_complete"])
|
||||
|
||||
|
||||
class TestQueueTargetReconciliation(unittest.TestCase):
|
||||
"""Queue target lock: reconcile operator-supplied backlog before inventory (#200)."""
|
||||
|
||||
CONFIGURED = [
|
||||
"Scaled-Tech-Consulting/Gitea-Tools",
|
||||
"Scaled-Tech-Consulting/mcp-control-plane",
|
||||
]
|
||||
GITEA_TOOLS = "Scaled-Tech-Consulting/Gitea-Tools"
|
||||
MCP = "Scaled-Tech-Consulting/mcp-control-plane"
|
||||
OPERATOR_CONTEXT = (
|
||||
"six open PRs in Scaled-Tech-Consulting/Gitea-Tools including "
|
||||
"#195, #193, #192, #190, #187, and #181"
|
||||
)
|
||||
PROFILE = {
|
||||
"profile_name": "prgs-reviewer",
|
||||
"allowed_operations": ["read", "gitea.read"],
|
||||
}
|
||||
|
||||
def test_supplied_gitea_tools_prs_but_inventoried_mcp_is_mismatch(self):
|
||||
lock = reconcile_queue_target(
|
||||
operator_context=self.OPERATOR_CONTEXT,
|
||||
inventoried_repo=self.MCP,
|
||||
configured_repos=self.CONFIGURED,
|
||||
)
|
||||
self.assertEqual(lock["status"], "target_repo_mismatch")
|
||||
self.assertEqual(lock["resolved_repo"], self.GITEA_TOOLS)
|
||||
self.assertEqual(lock["resolution_source"], "operator_context")
|
||||
self.assertIn(195, lock["supplied_pr_numbers"])
|
||||
self.assertFalse(lock["allow_clean_stop"])
|
||||
self.assertFalse(lock["allow_trusted_empty"])
|
||||
|
||||
def test_wrong_repo_zero_open_cannot_stop_cleanly(self):
|
||||
lock = reconcile_queue_target(
|
||||
operator_context=self.OPERATOR_CONTEXT,
|
||||
inventoried_repo=self.MCP,
|
||||
configured_repos=self.CONFIGURED,
|
||||
)
|
||||
gate = pr_inventory_trust_gate(
|
||||
[],
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="mcp-control-plane",
|
||||
state="open",
|
||||
authenticated_profile=self.PROFILE,
|
||||
local_remote_url=(
|
||||
"https://gitea.prgs.cc/Scaled-Tech-Consulting/"
|
||||
"mcp-control-plane.git"
|
||||
),
|
||||
corroboration_open_pr_counter=0,
|
||||
queue_target_lock=lock,
|
||||
)
|
||||
self.assertEqual(gate["status"], "target_repo_mismatch")
|
||||
self.assertNotEqual(gate["status"], "trusted_empty")
|
||||
|
||||
def test_supplied_pr_numbers_reconciled_before_empty_stop(self):
|
||||
lock = reconcile_queue_target(
|
||||
operator_context=(
|
||||
"currently we have 6 open PRs: #195, #193, #192, "
|
||||
"#190, #187, #181 in Gitea-Tools"
|
||||
),
|
||||
inventoried_repo=self.GITEA_TOOLS,
|
||||
configured_repos=self.CONFIGURED,
|
||||
)
|
||||
self.assertEqual(lock["status"], "resolved")
|
||||
self.assertEqual(len(lock["supplied_pr_numbers"]), 6)
|
||||
self.assertTrue(all(item["matches_inventoried_repo"]
|
||||
for item in lock["reconciliation"]))
|
||||
|
||||
def test_ambiguous_repo_context_fails_closed(self):
|
||||
lock = reconcile_queue_target(
|
||||
operator_context=(
|
||||
"open PRs in mcp-control-plane and gitea-tools including PR #195"
|
||||
),
|
||||
inventoried_repo=self.MCP,
|
||||
configured_repos=self.CONFIGURED,
|
||||
)
|
||||
self.assertEqual(lock["status"], "unresolved")
|
||||
self.assertFalse(lock["allow_trusted_empty"])
|
||||
self.assertTrue(
|
||||
any("could not be resolved" in r for r in lock["reasons"])
|
||||
)
|
||||
|
||||
def test_conflicting_directive_vs_backlog_fails_closed(self):
|
||||
lock = reconcile_queue_target(
|
||||
operator_context="Repository: Scaled-Tech-Consulting/mcp-control-plane",
|
||||
supplied_pr_backlog=[
|
||||
{"number": 195, "repo": self.GITEA_TOOLS},
|
||||
],
|
||||
inventoried_repo=self.MCP,
|
||||
configured_repos=self.CONFIGURED,
|
||||
)
|
||||
self.assertEqual(lock["status"], "unresolved")
|
||||
self.assertIn("conflicts", " ".join(lock["reasons"]).lower())
|
||||
|
||||
def test_final_report_must_document_reconciliation(self):
|
||||
lock = reconcile_queue_target(
|
||||
operator_context=self.OPERATOR_CONTEXT,
|
||||
inventoried_repo=self.GITEA_TOOLS,
|
||||
configured_repos=self.CONFIGURED,
|
||||
)
|
||||
incomplete = assess_queue_target_final_report(
|
||||
"Open PRs: 0. Stopping.", lock
|
||||
)
|
||||
self.assertFalse(incomplete["complete"])
|
||||
self.assertTrue(incomplete["downgraded"])
|
||||
|
||||
complete_report = "\n".join([
|
||||
"Queue inventory complete.",
|
||||
f"Resolved repo: {self.GITEA_TOOLS}",
|
||||
"Resolution source: operator_context",
|
||||
"queue_target_lock.status: resolved",
|
||||
"Supplied PR reconciliation: #195, #193, #192, #190, #187, #181",
|
||||
])
|
||||
complete = assess_queue_target_final_report(complete_report, lock)
|
||||
self.assertTrue(complete["complete"])
|
||||
self.assertFalse(complete["downgraded"])
|
||||
|
||||
def test_assess_reviewer_queue_inventory_blocks_mismatch(self):
|
||||
result = assess_reviewer_queue_inventory(
|
||||
[{
|
||||
"repo": self.MCP,
|
||||
"state_filter": "open",
|
||||
"pagination_complete": True,
|
||||
"open_pr_count": 0,
|
||||
"list_prs_response": [],
|
||||
"remote": "prgs",
|
||||
"authenticated_profile": self.PROFILE,
|
||||
"local_remote_url": (
|
||||
"https://gitea.prgs.cc/Scaled-Tech-Consulting/"
|
||||
"mcp-control-plane.git"
|
||||
),
|
||||
}],
|
||||
operator_context=self.OPERATOR_CONTEXT,
|
||||
)
|
||||
self.assertFalse(result["can_claim_empty_queue"])
|
||||
self.assertIn("target_repo_mismatch", str(result["trust_gates"]))
|
||||
|
||||
|
||||
class TestPRInventoryTrustGate(unittest.TestCase):
|
||||
"""Issue #194: unit tests for the PR inventory trust gate."""
|
||||
|
||||
@@ -1901,6 +1764,7 @@ class TestIssueSelectionContinuation(unittest.TestCase):
|
||||
"- Continuation mode: issue #182 continuation",
|
||||
"- Existing PR: #186",
|
||||
"- PR author: jcwalker3",
|
||||
"- Issue claim status: status:in-progress",
|
||||
"- Branch: feat/issue-182-controller-handoff",
|
||||
f"- Old PR head: {self.OLD_SHA}",
|
||||
f"- New PR head: {self.NEW_SHA}",
|
||||
@@ -1913,6 +1777,8 @@ class TestIssueSelectionContinuation(unittest.TestCase):
|
||||
continuation_proof={
|
||||
"pr_number": 186,
|
||||
"pr_author": "jcwalker3",
|
||||
"issue_number": 182,
|
||||
"issue_claim_status": "status:in-progress",
|
||||
"branch": "feat/issue-182-controller-handoff",
|
||||
"old_head_sha": self.OLD_SHA,
|
||||
"new_head_sha": self.NEW_SHA,
|
||||
@@ -1925,5 +1791,136 @@ class TestIssueSelectionContinuation(unittest.TestCase):
|
||||
self.assertEqual(result["grade"], "A")
|
||||
|
||||
|
||||
class TestContinuationModeProofs(unittest.TestCase):
|
||||
"""Issue #189: formal continuation proofs beyond #188 selection wall."""
|
||||
|
||||
OLD_SHA = PINNED
|
||||
NEW_SHA = OTHER
|
||||
REMOTE_OLD = "601c608c00000000000000000000000000000000"
|
||||
LEASE_SHA = "45c5cac2bc49dd112766ea218b18a71e9c5f8e99"
|
||||
PUSHED_SHA = "45c5cac2bc49dd112766ea218b18a71e9c5f8e99"
|
||||
|
||||
def test_force_with_lease_requires_lease_evidence(self):
|
||||
report = (
|
||||
"Issue #182 continuation. force-with-lease push to "
|
||||
"feat/issue-182-controller-handoff-enforcement."
|
||||
)
|
||||
result = assess_force_with_lease_push_report(
|
||||
report,
|
||||
old_remote_head=self.REMOTE_OLD,
|
||||
expected_lease_head=self.LEASE_SHA,
|
||||
new_pushed_head=self.PUSHED_SHA,
|
||||
branch_pushed="feat/issue-182-controller-handoff-enforcement",
|
||||
used_force_with_lease=True,
|
||||
)
|
||||
self.assertTrue(result["downgraded"])
|
||||
|
||||
def test_force_with_lease_complete_with_full_evidence(self):
|
||||
report = (
|
||||
"Issue #182 continuation with git push --force-with-lease. "
|
||||
f"Old remote head {self.REMOTE_OLD}. "
|
||||
f"Lease head {self.LEASE_SHA}. "
|
||||
f"Pushed head {self.PUSHED_SHA}. "
|
||||
"Branch feat/issue-182-controller-handoff-enforcement. "
|
||||
"Push branch only: only the feature branch was pushed."
|
||||
)
|
||||
result = assess_force_with_lease_push_report(
|
||||
report,
|
||||
old_remote_head=self.REMOTE_OLD,
|
||||
expected_lease_head=self.LEASE_SHA,
|
||||
new_pushed_head=self.PUSHED_SHA,
|
||||
branch_pushed="feat/issue-182-controller-handoff-enforcement",
|
||||
used_force_with_lease=True,
|
||||
)
|
||||
self.assertTrue(result["complete"])
|
||||
|
||||
def test_continuation_requires_issue_claim_status(self):
|
||||
report = (
|
||||
"Issue #182 continuation mode. PR #186. "
|
||||
f"old head {self.OLD_SHA} new head {self.NEW_SHA}. "
|
||||
"PR author jcwalker3. Branch feat/issue-182-test."
|
||||
)
|
||||
result = assess_continuation_mode_report(
|
||||
report,
|
||||
pr_number=186,
|
||||
pr_author="jcwalker3",
|
||||
issue_number=182,
|
||||
issue_claim_status="status:in-progress",
|
||||
branch="feat/issue-182-test",
|
||||
old_head_sha=self.OLD_SHA,
|
||||
new_head_sha=self.NEW_SHA,
|
||||
)
|
||||
self.assertTrue(result["downgraded"])
|
||||
|
||||
def test_canonical_secret_sweep_helper(self):
|
||||
sweep = canonical_secret_sweep_report(clean=True)
|
||||
self.assertEqual(sweep["method"], CANONICAL_SECRET_SWEEP_COMMAND)
|
||||
self.assertTrue(sweep["clean"])
|
||||
|
||||
def test_continuation_final_report_with_push_and_sweep_earns_a(self):
|
||||
branch = "feat/issue-182-controller-handoff-enforcement"
|
||||
report = "\n".join([
|
||||
"Issue #182 continuation mode — status:in-progress already set.",
|
||||
f"PR #186 updated via git push --force-with-lease.",
|
||||
f"Old PR head {self.OLD_SHA}; new PR head {self.NEW_SHA}.",
|
||||
f"Old remote head {self.REMOTE_OLD}.",
|
||||
f"Lease head {self.LEASE_SHA}. Pushed head {self.PUSHED_SHA}.",
|
||||
f"Branch {branch}. Push branch only: only the feature branch.",
|
||||
"PR author: jcwalker3. Session authored PR: yes.",
|
||||
"Continuation allowed: operator requested rebase.",
|
||||
"Open PR inventory included PR #186.",
|
||||
"## Controller Handoff",
|
||||
"- Task: continuation",
|
||||
"- Repo: Scaled-Tech-Consulting/Gitea-Tools",
|
||||
"- Role: author",
|
||||
"- Identity: prgs-author",
|
||||
"- Issue/PR: #182 / PR #186",
|
||||
f"- Branch/SHA: {branch}",
|
||||
"- Files changed: review_proofs.py",
|
||||
"- Validation: tests passed",
|
||||
"- Mutations: push_branch",
|
||||
"- Workspace mutations: none",
|
||||
"- Current status: PR mergeable",
|
||||
"- Blockers: none",
|
||||
"- Next: review",
|
||||
"- Safety: no review/merge",
|
||||
"- Continuation mode: issue #182 continuation",
|
||||
"- Existing PR: #186",
|
||||
"- PR author: jcwalker3",
|
||||
"- Issue claim status: status:in-progress",
|
||||
f"- Branch: {branch}",
|
||||
f"- Old PR head: {self.OLD_SHA}",
|
||||
f"- New PR head: {self.NEW_SHA}",
|
||||
"- Session authored PR: yes",
|
||||
"- Why continuation allowed: operator requested rebase",
|
||||
])
|
||||
result = assess_issue_selection_final_report(
|
||||
report,
|
||||
mode="continuation",
|
||||
continuation_proof={
|
||||
"pr_number": 186,
|
||||
"pr_author": "jcwalker3",
|
||||
"issue_number": 182,
|
||||
"issue_claim_status": "status:in-progress",
|
||||
"branch": branch,
|
||||
"old_head_sha": self.OLD_SHA,
|
||||
"new_head_sha": self.NEW_SHA,
|
||||
"session_authored_pr": True,
|
||||
"continuation_allowed_reason": "operator requested rebase",
|
||||
"push_proof": {
|
||||
"old_remote_head": self.REMOTE_OLD,
|
||||
"expected_lease_head": self.LEASE_SHA,
|
||||
"new_pushed_head": self.PUSHED_SHA,
|
||||
"branch_pushed": branch,
|
||||
"used_force_with_lease": True,
|
||||
},
|
||||
"secret_sweep": canonical_secret_sweep_report(clean=True),
|
||||
},
|
||||
edited_pr_numbers=[186],
|
||||
inventoried_pr_numbers=[186],
|
||||
)
|
||||
self.assertEqual(result["grade"], "A")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -65,23 +65,6 @@ class TestForbiddenGitCommands(unittest.TestCase):
|
||||
self.assertTrue(is_forbidden_reviewer_git_command("git reset --hard"))
|
||||
self.assertTrue(is_forbidden_reviewer_git_command("git clean -fd"))
|
||||
|
||||
def test_blocks_checkout_restore_and_switch_bypasses(self):
|
||||
"""Issue #243: blocklist gaps closed via readonly allowlist model."""
|
||||
blocked = (
|
||||
"git checkout HEAD -- review_proofs.py",
|
||||
"git checkout prgs/master -- review_proofs.py",
|
||||
"git checkout .",
|
||||
"git switch -",
|
||||
"git switch -C feat/other-branch",
|
||||
"git switch master",
|
||||
"git stash store",
|
||||
"git stash branch wip-stash",
|
||||
)
|
||||
for cmd in blocked:
|
||||
with self.subTest(cmd=cmd):
|
||||
self.assertTrue(is_forbidden_reviewer_git_command(cmd))
|
||||
self.assertFalse(is_readonly_reviewer_git_command(cmd))
|
||||
|
||||
def test_allows_readonly_commands(self):
|
||||
for cmd in (
|
||||
"git fetch prgs master",
|
||||
|
||||
@@ -168,74 +168,6 @@ class TestRoleSessionRouter(unittest.TestCase):
|
||||
self.assertFalse(result["success"])
|
||||
self.assertIn("switching is disabled", result["message"])
|
||||
|
||||
def _switching_env(self, profile):
|
||||
switching_config = dict(CONFIG)
|
||||
switching_config["rules"] = {"allow_runtime_switching": True}
|
||||
switching_config_path = os.path.join(self._dir.name, "profiles_switching.json")
|
||||
with open(switching_config_path, "w", encoding="utf-8") as fh:
|
||||
fh.write(json.dumps(switching_config))
|
||||
return {
|
||||
"GITEA_MCP_CONFIG": switching_config_path,
|
||||
"GITEA_MCP_PROFILE": profile,
|
||||
"GITEA_TOKEN_AUTHOR": "author-pass",
|
||||
"GITEA_TOKEN_REVIEWER": "reviewer-pass",
|
||||
}
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
|
||||
def test_dynamic_switch_to_reviewer_profile(self, _auth, mock_api):
|
||||
mock_api.side_effect = lambda method, url, header: (
|
||||
{"login": "sysadmin"} if "reviewer-pass" in str(header) else {"login": "jcwalker3"}
|
||||
)
|
||||
with patch.dict(os.environ, self._switching_env("prgs-author")):
|
||||
self.assertEqual(gitea_config.selected_profile_name(), "prgs-author")
|
||||
route = mcp_server.gitea_route_task_session(
|
||||
task_type="review_pr", remote="prgs"
|
||||
)
|
||||
self.assertEqual(route["route_result"], role_session_router.ROUTE_ALLOWED)
|
||||
self.assertTrue(route["downstream_allowed"])
|
||||
self.assertEqual(gitea_config.selected_profile_name(), "prgs-reviewer")
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_dynamic_switch_to_author_profile(self, _auth, mock_api):
|
||||
mock_api.side_effect = lambda method, url, header: (
|
||||
{"login": "jcwalker3"} if "author-pass" in str(header) else {"login": "sysadmin"}
|
||||
)
|
||||
with patch.dict(os.environ, self._switching_env("prgs-reviewer")):
|
||||
self.assertEqual(gitea_config.selected_profile_name(), "prgs-reviewer")
|
||||
route = mcp_server.gitea_route_task_session(
|
||||
task_type="create_issue", remote="prgs"
|
||||
)
|
||||
self.assertEqual(route["route_result"], role_session_router.ROUTE_ALLOWED)
|
||||
self.assertTrue(route["downstream_allowed"])
|
||||
self.assertEqual(gitea_config.selected_profile_name(), "prgs-author")
|
||||
|
||||
def test_dynamic_switch_blocked_for_wrong_role(self):
|
||||
with patch.dict(os.environ, self._switching_env("prgs-author")):
|
||||
config_data = dict(CONFIG)
|
||||
config_data["rules"] = {"allow_runtime_switching": True}
|
||||
config_data["profiles"]["prgs-author"]["allowed_operations"] = [
|
||||
"gitea.read", "gitea.issue.create", "gitea.pr.create", "gitea.branch.push"
|
||||
]
|
||||
custom_path = os.path.join(self._dir.name, "profiles_wrong_role.json")
|
||||
with open(custom_path, "w", encoding="utf-8") as fh:
|
||||
fh.write(json.dumps(config_data))
|
||||
|
||||
env = {
|
||||
"GITEA_MCP_CONFIG": custom_path,
|
||||
"GITEA_MCP_PROFILE": "prgs-author",
|
||||
"GITEA_TOKEN_AUTHOR": "author-pass",
|
||||
"GITEA_TOKEN_REVIEWER": "reviewer-pass",
|
||||
}
|
||||
with patch.dict(os.environ, env):
|
||||
route = mcp_server.gitea_route_task_session(
|
||||
task_type="comment_issue", remote="prgs"
|
||||
)
|
||||
self.assertEqual(route["route_result"], role_session_router.ROUTE_TO_AUTHOR)
|
||||
self.assertFalse(route["downstream_allowed"])
|
||||
self.assertEqual(gitea_config.selected_profile_name(), "prgs-author")
|
||||
|
||||
def test_handoff_requires_route_fields(self):
|
||||
incomplete = assess_role_route_handoff("Task done.")
|
||||
self.assertFalse(incomplete["complete"])
|
||||
|
||||
Reference in New Issue
Block a user