Merge remote-tracking branch 'prgs/master' into feat/issue-337-create-issue-verifier
This commit is contained in:
@@ -312,6 +312,39 @@ Required behavior (fail closed, issue #258):
|
||||
|
||||
Doc-contract tests: `tests/test_shell_spawn_hard_stop_docs.py`.
|
||||
|
||||
## Subagent Tool-Budget Guardrails
|
||||
|
||||
General-purpose subagents tasked with **deterministic MCP work** (for example a
|
||||
single `gitea_commit_files` call) have expanded to 100–122 tool calls,
|
||||
WebFetch/Playwright fallbacks, and throwaway helper-script generation instead of
|
||||
calling the native MCP tool once (observed during #152 closure, issue #259).
|
||||
|
||||
**Default budgets** (fail closed when exceeded):
|
||||
|
||||
| Task class | Max tool calls | Max wall time |
|
||||
|------------|----------------|---------------|
|
||||
| Single-step MCP mutation (`commit_files`, `create_pr`, `lock_issue`) | 15 | 5 minutes |
|
||||
| Review / merge queue inspection | 40 | 15 minutes |
|
||||
| Exploration / codebase search (non-mutating) | 60 | 20 minutes |
|
||||
|
||||
**Required behavior:**
|
||||
|
||||
1. **Main session first.** When the active author profile allows
|
||||
`gitea.repo.commit` and `gitea_commit_files` is visible, the main session
|
||||
must call it directly — do not delegate commit authority to a subagent
|
||||
(see #260).
|
||||
2. **Native MCP before fallback.** After a shell spawn failure (#258), attempt
|
||||
the native MCP tool once before any alternate path. Shell unavailability
|
||||
never authorizes WebFetch, Playwright, or manual base64 encoding.
|
||||
3. **No retry spirals.** Never resume a failed subagent into a larger retry
|
||||
loop or spawn a second subagent for the same deterministic step. Stop and
|
||||
emit a recovery report instead.
|
||||
4. **Forbidden detours** when `gitea_commit_files` is available: WebFetch,
|
||||
Playwright/browser automation, manual LLM-generated base64, and ad-hoc
|
||||
`_encode_*` / `_emit_*` helper scripts left in the repo.
|
||||
|
||||
Doc-contract tests: `tests/test_subagent_tool_budget_docs.py`.
|
||||
|
||||
## Branch worktree isolation
|
||||
|
||||
All LLM implementation and review work happens in an isolated branch worktree
|
||||
|
||||
+180
-24
@@ -475,6 +475,7 @@ import task_capability_map # noqa: E402
|
||||
import review_proofs # noqa: E402
|
||||
import agent_temp_artifacts
|
||||
import issue_lock_worktree # noqa: E402
|
||||
import merged_cleanup_reconcile # noqa: E402
|
||||
|
||||
|
||||
# Fail-closed exact-issue-lock file (#204): written by gitea_lock_issue,
|
||||
@@ -3104,6 +3105,141 @@ def gitea_delete_branch(
|
||||
return {"success": True, "message": f"Remote branch '{branch}' deleted."}
|
||||
|
||||
|
||||
def _remote_branch_exists(h: str, o: str, r: str, auth: str, branch: str) -> bool:
|
||||
import urllib.parse
|
||||
|
||||
encoded = urllib.parse.quote(branch, safe="")
|
||||
url = f"{repo_api_url(h, o, r)}/branches/{encoded}"
|
||||
try:
|
||||
api_request("GET", url, auth)
|
||||
return True
|
||||
except Exception as exc:
|
||||
message = str(exc).lower()
|
||||
if "404" in message or "not found" in message:
|
||||
return False
|
||||
raise
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_reconcile_merged_cleanups(
|
||||
dry_run: bool = True,
|
||||
execute_confirmed: bool = False,
|
||||
limit: int = 50,
|
||||
remote: str = "dadeschools",
|
||||
host: str | None = None,
|
||||
org: str | None = None,
|
||||
repo: str | None = None,
|
||||
) -> dict:
|
||||
"""Reconcile merged PRs by reporting and optionally cleaning remote branches and local worktrees.
|
||||
|
||||
Args:
|
||||
dry_run: Defaults to True. When True, only builds the reconciliation report.
|
||||
execute_confirmed: Must be True when dry_run=False.
|
||||
limit: Max number of closed PRs to inspect.
|
||||
remote: Known Gitea instance ('dadeschools' or 'prgs').
|
||||
host: Override the Gitea host.
|
||||
org: Override the owner/organization.
|
||||
repo: Override the repository name.
|
||||
|
||||
Returns:
|
||||
dict with reconciliation report and any executed cleanup actions.
|
||||
"""
|
||||
read_block = _profile_operation_gate("gitea.read")
|
||||
if read_block:
|
||||
return {
|
||||
"success": False,
|
||||
"performed": False,
|
||||
"reasons": read_block,
|
||||
"permission_report": _permission_block_report("gitea.read"),
|
||||
}
|
||||
|
||||
if not dry_run and not execute_confirmed:
|
||||
raise ValueError(
|
||||
"execute_confirmed must be True when dry_run=False (fail closed)"
|
||||
)
|
||||
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
base = repo_api_url(h, o, r)
|
||||
closed_prs = api_get_all(f"{base}/pulls?state=closed", auth, limit=limit)
|
||||
open_prs = api_get_all(f"{base}/pulls?state=open", auth)
|
||||
|
||||
merged_closed: list[dict] = []
|
||||
remote_branch_exists: dict[str, bool] = {}
|
||||
head_on_master: dict[int, bool | None] = {}
|
||||
master_ref = f"{remote}/master" if remote in REMOTES else "origin/master"
|
||||
|
||||
for pr in closed_prs:
|
||||
if not (pr.get("merged") or pr.get("merged_at")):
|
||||
continue
|
||||
merged_closed.append(pr)
|
||||
head_ref = (pr.get("head") or {}).get("ref") or ""
|
||||
head_sha = (pr.get("head") or {}).get("sha")
|
||||
if head_ref and head_ref not in remote_branch_exists:
|
||||
remote_branch_exists[head_ref] = _remote_branch_exists(
|
||||
h, o, r, auth, head_ref
|
||||
)
|
||||
head_on_master[int(pr["number"])] = merged_cleanup_reconcile.is_head_ancestor_of_ref(
|
||||
PROJECT_ROOT, head_sha, master_ref
|
||||
)
|
||||
|
||||
delete_capability_allowed = not _profile_operation_gate("gitea.branch.delete")
|
||||
report = merged_cleanup_reconcile.build_reconciliation_report(
|
||||
project_root=PROJECT_ROOT,
|
||||
closed_prs=merged_closed,
|
||||
open_prs=open_prs,
|
||||
remote_branch_exists=remote_branch_exists,
|
||||
head_on_master=head_on_master,
|
||||
delete_capability_allowed=delete_capability_allowed,
|
||||
)
|
||||
|
||||
if dry_run:
|
||||
report["dry_run"] = True
|
||||
report["executed"] = False
|
||||
return {"success": True, "performed": False, **report}
|
||||
|
||||
verify_preflight_purity(remote)
|
||||
actions: list[dict] = []
|
||||
for entry in report.get("entries") or []:
|
||||
head_branch = entry.get("head_branch") or ""
|
||||
remote_assessment = entry.get("remote_branch") or {}
|
||||
local_assessment = entry.get("local_worktree") or {}
|
||||
|
||||
if remote_assessment.get("safe_to_delete_remote"):
|
||||
import urllib.parse
|
||||
|
||||
encoded = urllib.parse.quote(head_branch, safe="")
|
||||
url = f"{base}/branches/{encoded}"
|
||||
with _audited(
|
||||
"delete_branch",
|
||||
host=h,
|
||||
remote=remote,
|
||||
org=o,
|
||||
repo=r,
|
||||
target_branch=head_branch,
|
||||
request_metadata={"branch": head_branch, "source": "reconcile_merged_cleanups"},
|
||||
):
|
||||
api_request("DELETE", url, auth)
|
||||
actions.append(
|
||||
{
|
||||
"action": "delete_remote_branch",
|
||||
"branch": head_branch,
|
||||
"success": True,
|
||||
}
|
||||
)
|
||||
|
||||
if local_assessment.get("safe_to_remove_worktree"):
|
||||
result = merged_cleanup_reconcile.remove_local_worktree(
|
||||
PROJECT_ROOT, head_branch
|
||||
)
|
||||
actions.append({"action": "remove_local_worktree", **result})
|
||||
|
||||
report["dry_run"] = False
|
||||
report["executed"] = True
|
||||
report["actions"] = actions
|
||||
return {"success": True, "performed": True, **report}
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_close_issue(
|
||||
issue_number: int,
|
||||
@@ -3738,6 +3874,16 @@ _GUIDE_RULES = {
|
||||
"paths (e.g. gitea_commit_files) for remaining mutations. Shell "
|
||||
"unavailability never authorizes WebFetch/browser/manual-encoding "
|
||||
"fallbacks (#258)."),
|
||||
"subagent_tool_budget": (
|
||||
"General-purpose subagents on deterministic MCP tasks must stay within "
|
||||
"budget: single-step mutations (commit_files, create_pr, lock_issue) "
|
||||
"max 15 tool calls / 5 min; review/merge queue inspection max 40 / "
|
||||
"15 min. The main session with gitea.repo.commit must call "
|
||||
"gitea_commit_files directly — no subagent delegation (#260). After "
|
||||
"shell spawn failure (#258), attempt native MCP once; never authorize "
|
||||
"WebFetch/Playwright/manual base64 when gitea_commit_files is "
|
||||
"available. Never resume a failed subagent into a larger retry loop "
|
||||
"(#259)."),
|
||||
"subagent_delegation": (
|
||||
"Deterministic write workflows (issue claim, branch creation, code "
|
||||
"edits, commits, PR creation, review, merge, cleanup) run inline in "
|
||||
@@ -5177,30 +5323,40 @@ def gitea_resolve_task_capability(
|
||||
required_permission = task_capability_map.required_permission(task)
|
||||
required_role = task_capability_map.required_role(task)
|
||||
|
||||
if required_role == "reviewer" and role_session_router.check_mid_merge():
|
||||
profile = get_profile()
|
||||
h = host or (REMOTES.get(remote, {}).get("host") if remote in REMOTES else None)
|
||||
username = _authenticated_username(h) if h else None
|
||||
next_safe_action = (
|
||||
"infra_stop: Unresolved merge conflict or mid-merge state detected in MCP runtime source. "
|
||||
"Please resolve all conflicts manually, finish/abort the merge, restart the MCP server, and retry."
|
||||
)
|
||||
return {
|
||||
"requested_task": task,
|
||||
"required_operation_permission": required_permission,
|
||||
"required_role_kind": required_role,
|
||||
"active_profile": profile["profile_name"],
|
||||
"active_identity": username,
|
||||
"active_profile_allowed_operations": profile.get("allowed_operations") or [],
|
||||
"allowed_in_current_session": False,
|
||||
"stop_required": True,
|
||||
"infra_stop": True,
|
||||
"task_role_guidance": [next_safe_action],
|
||||
"matching_configured_profile": [],
|
||||
"runtime_switching_supported": gitea_config.is_runtime_switching_enabled(),
|
||||
"different_mcp_namespace_required": False,
|
||||
"exact_safe_next_action": next_safe_action,
|
||||
}
|
||||
infra_assessment = role_session_router.assess_infra_stop(PROJECT_ROOT)
|
||||
if required_role == "reviewer":
|
||||
if not infra_assessment["infra_stop"]:
|
||||
capability_stop_terminal.clear()
|
||||
last_route = role_session_router.last_route()
|
||||
if last_route and last_route.get("route_result") == role_session_router.ROUTE_INFRA_STOP:
|
||||
role_session_router.clear_route_state()
|
||||
elif infra_assessment["infra_stop"]:
|
||||
profile = get_profile()
|
||||
h = host or (REMOTES.get(remote, {}).get("host") if remote in REMOTES else None)
|
||||
username = _authenticated_username(h) if h else None
|
||||
detail = "; ".join(infra_assessment.get("infra_stop_reasons") or [])
|
||||
next_safe_action = (
|
||||
"infra_stop: Unresolved merge conflict or mid-merge state detected in "
|
||||
f"MCP runtime source ({detail}). Please resolve all conflicts manually, "
|
||||
"finish/abort the merge, and retry."
|
||||
)
|
||||
return {
|
||||
"requested_task": task,
|
||||
"required_operation_permission": required_permission,
|
||||
"required_role_kind": required_role,
|
||||
"active_profile": profile["profile_name"],
|
||||
"active_identity": username,
|
||||
"active_profile_allowed_operations": profile.get("allowed_operations") or [],
|
||||
"allowed_in_current_session": False,
|
||||
"stop_required": True,
|
||||
"infra_stop": True,
|
||||
"infra_stop_assessment": infra_assessment,
|
||||
"task_role_guidance": [next_safe_action],
|
||||
"matching_configured_profile": [],
|
||||
"runtime_switching_supported": gitea_config.is_runtime_switching_enabled(),
|
||||
"different_mcp_namespace_required": False,
|
||||
"exact_safe_next_action": next_safe_action,
|
||||
}
|
||||
|
||||
record_preflight_check("capability", required_role)
|
||||
|
||||
|
||||
@@ -0,0 +1,333 @@
|
||||
"""Merged PR branch and worktree cleanup reconciliation (#269).
|
||||
|
||||
Builds dry-run reports for merged pull requests and optionally executes
|
||||
remote branch deletion and local worktree removal when every safety gate passes.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
from typing import Any
|
||||
|
||||
from reviewer_worktree import parse_dirty_tracked_files
|
||||
|
||||
PROTECTED_BRANCHES = frozenset({"master", "main", "dev"})
|
||||
ISSUE_LOCK_FILE = os.environ.get("GITEA_ISSUE_LOCK_FILE", "/tmp/gitea_issue_lock.json")
|
||||
CLOSES_FIXES_RE = re.compile(r"\b(?:closes|fixes)\s+#(\d+)\b", re.IGNORECASE)
|
||||
|
||||
|
||||
def extract_linked_issue(title: str, body: str) -> int | None:
|
||||
"""Return the first Closes/Fixes issue number from PR metadata."""
|
||||
for text in (title or "", body or ""):
|
||||
match = CLOSES_FIXES_RE.search(text)
|
||||
if match:
|
||||
return int(match.group(1))
|
||||
return None
|
||||
|
||||
|
||||
def branch_worktree_folder(branch: str) -> str:
|
||||
return (branch or "").replace("/", "-")
|
||||
|
||||
|
||||
def resolve_worktree_path(project_root: str, branch: str) -> str:
|
||||
return os.path.join(project_root, "branches", branch_worktree_folder(branch))
|
||||
|
||||
|
||||
def read_issue_lock(path: str | None = None) -> dict[str, Any] | None:
|
||||
lock_path = (path or ISSUE_LOCK_FILE).strip()
|
||||
if not lock_path or not os.path.exists(lock_path):
|
||||
return None
|
||||
try:
|
||||
with open(lock_path, encoding="utf-8") as handle:
|
||||
data = json.load(handle)
|
||||
except (OSError, json.JSONDecodeError):
|
||||
return None
|
||||
return data if isinstance(data, dict) else None
|
||||
|
||||
|
||||
def has_active_issue_lock(branch: str, lock_path: str | None = None) -> bool:
|
||||
lock = read_issue_lock(lock_path)
|
||||
if not lock:
|
||||
return False
|
||||
return (lock.get("branch_name") or "").strip() == (branch or "").strip()
|
||||
|
||||
|
||||
def collect_open_pr_heads(open_prs: list[dict[str, Any]]) -> set[str]:
|
||||
heads: set[str] = set()
|
||||
for pr in open_prs or []:
|
||||
head = pr.get("head")
|
||||
if isinstance(head, dict):
|
||||
ref = head.get("ref")
|
||||
else:
|
||||
ref = head
|
||||
if ref:
|
||||
heads.add(str(ref))
|
||||
return heads
|
||||
|
||||
|
||||
def read_local_worktree_state(worktree_path: str) -> dict[str, Any]:
|
||||
path = (worktree_path or "").strip()
|
||||
if not path or not os.path.isdir(path):
|
||||
return {
|
||||
"exists": False,
|
||||
"clean": None,
|
||||
"current_branch": None,
|
||||
"head_sha": None,
|
||||
"dirty_files": [],
|
||||
}
|
||||
|
||||
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,
|
||||
)
|
||||
dirty_files = parse_dirty_tracked_files(status_res.stdout or "")
|
||||
|
||||
head_res = subprocess.run(
|
||||
["git", "-C", path, "rev-parse", "HEAD"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
head_sha = (head_res.stdout or "").strip() if head_res.returncode == 0 else None
|
||||
|
||||
return {
|
||||
"exists": True,
|
||||
"clean": not dirty_files,
|
||||
"current_branch": current_branch,
|
||||
"head_sha": head_sha,
|
||||
"dirty_files": dirty_files,
|
||||
}
|
||||
|
||||
|
||||
def is_head_ancestor_of_ref(project_root: str, head_sha: str | None, base_ref: str) -> bool | None:
|
||||
if not head_sha:
|
||||
return None
|
||||
res = subprocess.run(
|
||||
["git", "-C", project_root, "merge-base", "--is-ancestor", head_sha, base_ref],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
if res.returncode == 0:
|
||||
return True
|
||||
if res.returncode == 1:
|
||||
return False
|
||||
return None
|
||||
|
||||
|
||||
def assess_remote_branch_cleanup(
|
||||
*,
|
||||
pr_number: int,
|
||||
head_branch: str,
|
||||
merged: bool,
|
||||
remote_branch_exists: bool,
|
||||
open_pr_heads: set[str],
|
||||
protected_branches: frozenset[str] | None = None,
|
||||
head_on_master: bool | None,
|
||||
delete_capability_allowed: bool,
|
||||
active_lock: bool,
|
||||
) -> dict[str, Any]:
|
||||
protected = protected_branches or PROTECTED_BRANCHES
|
||||
reasons: list[str] = []
|
||||
if not merged:
|
||||
reasons.append("PR is not merged")
|
||||
if not remote_branch_exists:
|
||||
reasons.append("remote branch already absent")
|
||||
if head_branch in protected:
|
||||
reasons.append(f"branch '{head_branch}' is protected")
|
||||
if head_branch in open_pr_heads:
|
||||
reasons.append("an open PR still references this head branch")
|
||||
if active_lock:
|
||||
reasons.append("active issue lock references this branch")
|
||||
if head_on_master is False:
|
||||
reasons.append("PR head is not an ancestor of master")
|
||||
if not delete_capability_allowed:
|
||||
reasons.append("delete_branch capability is not allowed in the active profile")
|
||||
|
||||
safe = merged and remote_branch_exists and not reasons
|
||||
return {
|
||||
"pr_number": pr_number,
|
||||
"head_branch": head_branch,
|
||||
"remote_branch_exists": remote_branch_exists,
|
||||
"safe_to_delete_remote": safe,
|
||||
"block_reasons": reasons,
|
||||
"recommended_action": "delete_remote_branch" if safe else "keep_remote_branch",
|
||||
}
|
||||
|
||||
|
||||
def assess_local_worktree_cleanup(
|
||||
*,
|
||||
pr_number: int,
|
||||
head_branch: str,
|
||||
merged: bool,
|
||||
worktree_state: dict[str, Any],
|
||||
active_lock: bool,
|
||||
) -> dict[str, Any]:
|
||||
reasons: list[str] = []
|
||||
exists = bool(worktree_state.get("exists"))
|
||||
if not merged:
|
||||
reasons.append("PR is not merged")
|
||||
if not exists:
|
||||
reasons.append("local worktree not present")
|
||||
if exists and worktree_state.get("clean") is False:
|
||||
dirty = worktree_state.get("dirty_files") or []
|
||||
reasons.append(
|
||||
"local worktree has tracked edits"
|
||||
+ (f" ({', '.join(dirty)})" if dirty else "")
|
||||
)
|
||||
if exists:
|
||||
current_branch = worktree_state.get("current_branch")
|
||||
if current_branch and current_branch != head_branch:
|
||||
reasons.append(
|
||||
f"worktree branch '{current_branch}' does not match PR head '{head_branch}'"
|
||||
)
|
||||
if active_lock:
|
||||
reasons.append("active issue lock references this branch")
|
||||
|
||||
safe = merged and exists and not reasons
|
||||
return {
|
||||
"pr_number": pr_number,
|
||||
"head_branch": head_branch,
|
||||
"worktree_path": worktree_state.get("worktree_path"),
|
||||
"worktree_exists": exists,
|
||||
"worktree_clean": worktree_state.get("clean"),
|
||||
"safe_to_remove_worktree": safe,
|
||||
"block_reasons": reasons,
|
||||
"recommended_action": "remove_local_worktree" if safe else "keep_local_worktree",
|
||||
}
|
||||
|
||||
|
||||
def build_pr_cleanup_entry(
|
||||
*,
|
||||
pr: dict[str, Any],
|
||||
project_root: str,
|
||||
open_pr_heads: set[str],
|
||||
remote_branch_exists: bool,
|
||||
head_on_master: bool | None,
|
||||
delete_capability_allowed: bool,
|
||||
issue_lock_path: str | None = None,
|
||||
protected_branches: frozenset[str] | None = None,
|
||||
) -> dict[str, Any]:
|
||||
pr_number = int(pr["number"])
|
||||
head_branch = pr.get("head") or ""
|
||||
if isinstance(head_branch, dict):
|
||||
head_branch = head_branch.get("ref") or ""
|
||||
title = pr.get("title") or ""
|
||||
body = pr.get("body") or ""
|
||||
merged = bool(pr.get("merged_at"))
|
||||
worktree_path = resolve_worktree_path(project_root, head_branch)
|
||||
worktree_state = read_local_worktree_state(worktree_path)
|
||||
worktree_state["worktree_path"] = worktree_path
|
||||
active_lock = has_active_issue_lock(head_branch, issue_lock_path)
|
||||
|
||||
remote = assess_remote_branch_cleanup(
|
||||
pr_number=pr_number,
|
||||
head_branch=head_branch,
|
||||
merged=merged,
|
||||
remote_branch_exists=remote_branch_exists,
|
||||
open_pr_heads=open_pr_heads,
|
||||
protected_branches=protected_branches,
|
||||
head_on_master=head_on_master,
|
||||
delete_capability_allowed=delete_capability_allowed,
|
||||
active_lock=active_lock,
|
||||
)
|
||||
local = assess_local_worktree_cleanup(
|
||||
pr_number=pr_number,
|
||||
head_branch=head_branch,
|
||||
merged=merged,
|
||||
worktree_state=worktree_state,
|
||||
active_lock=active_lock,
|
||||
)
|
||||
return {
|
||||
"pr_number": pr_number,
|
||||
"issue_number": extract_linked_issue(title, body),
|
||||
"title": title,
|
||||
"head_branch": head_branch,
|
||||
"merge_commit_sha": pr.get("merge_commit_sha"),
|
||||
"merged_at": pr.get("merged_at"),
|
||||
"merged": merged,
|
||||
"remote_branch": remote,
|
||||
"local_worktree": local,
|
||||
}
|
||||
|
||||
|
||||
def build_reconciliation_report(
|
||||
*,
|
||||
project_root: str,
|
||||
closed_prs: list[dict[str, Any]],
|
||||
open_prs: list[dict[str, Any]],
|
||||
remote_branch_exists: dict[str, bool],
|
||||
head_on_master: dict[int, bool | None],
|
||||
delete_capability_allowed: bool,
|
||||
issue_lock_path: str | None = None,
|
||||
protected_branches: frozenset[str] | None = None,
|
||||
) -> dict[str, Any]:
|
||||
open_heads = collect_open_pr_heads(open_prs)
|
||||
entries: list[dict[str, Any]] = []
|
||||
for pr in closed_prs or []:
|
||||
if not pr.get("merged_at"):
|
||||
continue
|
||||
pr_number = int(pr["number"])
|
||||
head = pr.get("head") or ""
|
||||
if isinstance(head, dict):
|
||||
head = head.get("ref") or ""
|
||||
entries.append(
|
||||
build_pr_cleanup_entry(
|
||||
pr=pr,
|
||||
project_root=project_root,
|
||||
open_pr_heads=open_heads,
|
||||
remote_branch_exists=bool(remote_branch_exists.get(head)),
|
||||
head_on_master=head_on_master.get(pr_number),
|
||||
delete_capability_allowed=delete_capability_allowed,
|
||||
issue_lock_path=issue_lock_path,
|
||||
protected_branches=protected_branches,
|
||||
)
|
||||
)
|
||||
return {
|
||||
"project_root": os.path.realpath(project_root),
|
||||
"merged_pr_count": len(entries),
|
||||
"entries": entries,
|
||||
"dry_run": True,
|
||||
"executed": False,
|
||||
}
|
||||
|
||||
|
||||
def remove_local_worktree(project_root: str, branch: str) -> dict[str, Any]:
|
||||
worktree_path = resolve_worktree_path(project_root, branch)
|
||||
if not os.path.isdir(worktree_path):
|
||||
return {
|
||||
"success": False,
|
||||
"performed": False,
|
||||
"message": f"worktree not found: {worktree_path}",
|
||||
}
|
||||
res = subprocess.run(
|
||||
["git", "-C", project_root, "worktree", "remove", worktree_path],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
if res.returncode != 0:
|
||||
return {
|
||||
"success": False,
|
||||
"performed": False,
|
||||
"message": (res.stderr or res.stdout or "worktree remove failed").strip(),
|
||||
}
|
||||
return {
|
||||
"success": True,
|
||||
"performed": True,
|
||||
"message": f"removed worktree {worktree_path}",
|
||||
"worktree_path": worktree_path,
|
||||
}
|
||||
+58
-26
@@ -134,22 +134,28 @@ def route_task_session(
|
||||
_record_route(result)
|
||||
return result
|
||||
|
||||
if required_role == "reviewer" and check_mid_merge():
|
||||
result = {
|
||||
"task_type": task_type,
|
||||
"required_role": required_role,
|
||||
"active_role": active_role_kind,
|
||||
"active_profile": active_profile,
|
||||
"route_result": ROUTE_INFRA_STOP,
|
||||
"downstream_allowed": False,
|
||||
"reasons": [
|
||||
"infra_stop: Unresolved merge conflict or mid-merge state detected in MCP runtime source.",
|
||||
"Please resolve all conflicts manually, finish/abort the merge, restart the MCP server, and retry."
|
||||
],
|
||||
"message": "infra_stop: Unresolved merge conflict or mid-merge state detected in MCP runtime source.",
|
||||
}
|
||||
_record_route(result)
|
||||
return result
|
||||
if required_role == "reviewer":
|
||||
infra = assess_infra_stop()
|
||||
if infra["infra_stop"]:
|
||||
detail = "; ".join(infra.get("infra_stop_reasons") or [])
|
||||
message = (
|
||||
"infra_stop: Unresolved merge conflict or mid-merge state detected "
|
||||
f"in MCP runtime source ({detail}). Please resolve all conflicts "
|
||||
"manually, finish/abort the merge, and retry."
|
||||
)
|
||||
result = {
|
||||
"task_type": task_type,
|
||||
"required_role": required_role,
|
||||
"active_role": active_role_kind,
|
||||
"active_profile": active_profile,
|
||||
"route_result": ROUTE_INFRA_STOP,
|
||||
"downstream_allowed": False,
|
||||
"reasons": [message],
|
||||
"message": message,
|
||||
"infra_stop_assessment": infra,
|
||||
}
|
||||
_record_route(result)
|
||||
return result
|
||||
|
||||
if allowed_in_current_session:
|
||||
result = {
|
||||
@@ -317,14 +323,40 @@ def first_conflict_marker_path(project_root: str | None = None) -> str | None:
|
||||
return None
|
||||
|
||||
|
||||
def check_mid_merge() -> bool:
|
||||
"""Return True if the repository is mid-merge, mid-rebase, or has conflict markers."""
|
||||
project_root = os.path.dirname(os.path.abspath(__file__))
|
||||
git_dir = os.path.join(project_root, ".git")
|
||||
if os.path.exists(git_dir):
|
||||
if (os.path.exists(os.path.join(git_dir, "MERGE_HEAD"))
|
||||
or os.path.exists(os.path.join(git_dir, "rebase-merge"))
|
||||
or os.path.exists(os.path.join(git_dir, "rebase-apply"))):
|
||||
return True
|
||||
def _default_project_root() -> str:
|
||||
override = (os.environ.get("GITEA_MCP_PROJECT_ROOT") or "").strip()
|
||||
if override:
|
||||
return os.path.realpath(override)
|
||||
return os.path.dirname(os.path.abspath(__file__))
|
||||
|
||||
return first_conflict_marker_path(project_root) is not None
|
||||
|
||||
def assess_infra_stop(project_root: str | None = None) -> dict:
|
||||
"""Recompute infra_stop from live git and source scan state (#285)."""
|
||||
root_dir = os.path.realpath(project_root or _default_project_root())
|
||||
reasons: list[str] = []
|
||||
mid_merge = False
|
||||
git_dir = os.path.join(root_dir, ".git")
|
||||
if os.path.isdir(git_dir):
|
||||
for marker in ("MERGE_HEAD", "rebase-merge", "rebase-apply"):
|
||||
marker_path = os.path.join(git_dir, marker)
|
||||
if os.path.exists(marker_path):
|
||||
mid_merge = True
|
||||
reasons.append(f"git state: {marker} present under {git_dir}")
|
||||
conflict_file = first_conflict_marker_path(root_dir)
|
||||
if conflict_file:
|
||||
reasons.append(
|
||||
f"conflict markers detected in {conflict_file} (project_root={root_dir})"
|
||||
)
|
||||
infra_stop = mid_merge or bool(conflict_file)
|
||||
return {
|
||||
"infra_stop": infra_stop,
|
||||
"project_root": root_dir,
|
||||
"conflict_file": conflict_file,
|
||||
"mid_merge": mid_merge,
|
||||
"infra_stop_reasons": reasons,
|
||||
}
|
||||
|
||||
|
||||
def check_mid_merge(project_root: str | None = None) -> bool:
|
||||
"""Return True if the repository is mid-merge, mid-rebase, or has conflict markers."""
|
||||
return assess_infra_stop(project_root)["infra_stop"]
|
||||
@@ -106,6 +106,31 @@ Implementation: `(fix|feat|docs|chore)/issue-<number>-<short-description>`
|
||||
|
||||
Review: `review/pr-<number>-<short-description>`
|
||||
|
||||
## Subagent Tool-Budget Guardrails
|
||||
|
||||
General-purpose subagents on **single-step MCP tasks** (for example
|
||||
`gitea_commit_files`) must not expand into 100+ tool-call retry spirals with
|
||||
WebFetch/Playwright/manual-encoding fallbacks (issue #259).
|
||||
|
||||
Default budgets (stop when exceeded):
|
||||
|
||||
- **Single-step MCP mutation** (`commit_files`, `create_pr`, `lock_issue`):
|
||||
15 tool calls, 5 minutes wall time.
|
||||
- **Review / merge queue inspection**: 40 tool calls, 15 minutes.
|
||||
- **Non-mutating exploration**: 60 tool calls, 20 minutes.
|
||||
|
||||
Rules:
|
||||
|
||||
1. When the main session has `gitea.repo.commit`, call `gitea_commit_files`
|
||||
directly — do not delegate commit to a subagent (#260).
|
||||
2. After shell spawn failure (#258), attempt the native MCP tool once before
|
||||
any fallback; shell unavailability never authorizes WebFetch/Playwright/
|
||||
manual base64.
|
||||
3. Never resume a failed subagent into a larger retry loop or spawn a second
|
||||
subagent for the same deterministic step — stop and report.
|
||||
4. When `gitea_commit_files` is available, forbid WebFetch, Playwright,
|
||||
manual encoding, and ad-hoc `_encode_*` / `_emit_*` helpers in the repo.
|
||||
|
||||
Worktree folder: branch with `/` replaced by `-` under `branches/`.
|
||||
|
||||
Helpers: `scripts/worktree-start`, `scripts/worktree-review`,
|
||||
|
||||
@@ -92,6 +92,10 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
|
||||
"permission": "gitea.repo.commit",
|
||||
"role": "author",
|
||||
},
|
||||
"reconcile_merged_cleanups": {
|
||||
"permission": "gitea.read",
|
||||
"role": "author",
|
||||
},
|
||||
}
|
||||
|
||||
# Issue-mutating MCP tools and their resolver task keys.
|
||||
|
||||
+93
-5
@@ -1,9 +1,12 @@
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
import capability_stop_terminal
|
||||
import role_session_router
|
||||
from role_session_router import python_bytes_have_conflict_markers
|
||||
from mcp_server import gitea_route_task_session, gitea_resolve_task_capability
|
||||
@@ -117,7 +120,7 @@ class TestMCPHealth(unittest.TestCase):
|
||||
stderr_text,
|
||||
)
|
||||
|
||||
@patch("role_session_router.check_mid_merge", return_value=True)
|
||||
@patch("role_session_router.assess_infra_stop")
|
||||
@patch(
|
||||
"mcp_server.get_profile",
|
||||
return_value={
|
||||
@@ -125,12 +128,23 @@ class TestMCPHealth(unittest.TestCase):
|
||||
"allowed_operations": ["gitea.pr.review"],
|
||||
},
|
||||
)
|
||||
def test_route_task_session_blocks_during_merge(self, mock_profile, mock_check):
|
||||
def test_route_task_session_blocks_during_merge(self, mock_profile, mock_assess):
|
||||
mock_assess.return_value = {
|
||||
"infra_stop": True,
|
||||
"project_root": "/repo",
|
||||
"conflict_file": None,
|
||||
"mid_merge": True,
|
||||
"infra_stop_reasons": [
|
||||
"mid-merge markers detected in /repo/.git/MERGE_HEAD (project_root=/repo)"
|
||||
],
|
||||
}
|
||||
res = gitea_route_task_session("review_pr")
|
||||
self.assertEqual(res["route_result"], "infra_stop")
|
||||
self.assertIn("infra_stop", res["message"])
|
||||
self.assertTrue(res.get("infra_stop_assessment", {}).get("infra_stop"))
|
||||
mock_assess.assert_called()
|
||||
|
||||
@patch("role_session_router.check_mid_merge", return_value=True)
|
||||
@patch("role_session_router.assess_infra_stop")
|
||||
@patch(
|
||||
"mcp_server.get_profile",
|
||||
return_value={
|
||||
@@ -139,9 +153,83 @@ class TestMCPHealth(unittest.TestCase):
|
||||
},
|
||||
)
|
||||
def test_resolve_task_capability_blocks_during_merge(
|
||||
self, mock_profile, mock_check
|
||||
self, mock_profile, mock_assess
|
||||
):
|
||||
mock_assess.return_value = {
|
||||
"infra_stop": True,
|
||||
"project_root": "/repo",
|
||||
"conflict_file": "/repo/gitea_mcp_server.py",
|
||||
"mid_merge": False,
|
||||
"infra_stop_reasons": [
|
||||
"conflict markers detected in /repo/gitea_mcp_server.py (project_root=/repo)"
|
||||
],
|
||||
}
|
||||
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"])
|
||||
self.assertEqual(
|
||||
res["infra_stop_assessment"]["conflict_file"],
|
||||
"/repo/gitea_mcp_server.py",
|
||||
)
|
||||
|
||||
@patch("role_session_router.assess_infra_stop")
|
||||
@patch("mcp_server._authenticated_username", return_value="reviewer-user")
|
||||
@patch(
|
||||
"mcp_server.get_profile",
|
||||
return_value={
|
||||
"profile_name": "prgs-reviewer",
|
||||
"allowed_operations": ["gitea.pr.review", "gitea.pr.approve"],
|
||||
},
|
||||
)
|
||||
def test_resolve_task_capability_clears_stale_terminal_when_infra_clean(
|
||||
self, mock_profile, _username, mock_assess
|
||||
):
|
||||
mock_assess.return_value = {
|
||||
"infra_stop": False,
|
||||
"project_root": "/repo",
|
||||
"conflict_file": None,
|
||||
"mid_merge": False,
|
||||
"infra_stop_reasons": [],
|
||||
}
|
||||
capability_stop_terminal.enter_from_capability_result({
|
||||
"requested_task": "review_pr",
|
||||
"required_role_kind": "reviewer",
|
||||
"stop_required": True,
|
||||
"active_profile": "prgs-reviewer",
|
||||
"active_identity": "reviewer-user",
|
||||
"exact_safe_next_action": "blocked",
|
||||
})
|
||||
self.assertTrue(capability_stop_terminal.is_active())
|
||||
res = gitea_resolve_task_capability("review_pr", remote="prgs")
|
||||
self.assertFalse(capability_stop_terminal.is_active())
|
||||
self.assertTrue(res["allowed_in_current_session"])
|
||||
|
||||
|
||||
class TestInfraStopLiveAssessment(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.tempdir = tempfile.mkdtemp()
|
||||
|
||||
def tearDown(self):
|
||||
shutil.rmtree(self.tempdir, ignore_errors=True)
|
||||
|
||||
def test_clean_project_root_is_not_infra_stop(self):
|
||||
infra = role_session_router.assess_infra_stop(self.tempdir)
|
||||
self.assertFalse(infra["infra_stop"])
|
||||
self.assertIsNone(infra["conflict_file"])
|
||||
|
||||
def test_conflict_present_then_resolved_recomputes_allowed(self):
|
||||
conflict_path = os.path.join(self.tempdir, "conflicted.py")
|
||||
with open(conflict_path, "wb") as handle:
|
||||
handle.write(b"<<<<<<< HEAD\n")
|
||||
blocked = role_session_router.assess_infra_stop(self.tempdir)
|
||||
self.assertTrue(blocked["infra_stop"])
|
||||
self.assertEqual(
|
||||
os.path.realpath(blocked["conflict_file"]),
|
||||
os.path.realpath(conflict_path),
|
||||
)
|
||||
|
||||
os.remove(conflict_path)
|
||||
cleared = role_session_router.assess_infra_stop(self.tempdir)
|
||||
self.assertFalse(cleared["infra_stop"])
|
||||
self.assertIsNone(cleared["conflict_file"])
|
||||
@@ -24,6 +24,7 @@ from mcp_server import ( # noqa: E402
|
||||
gitea_merge_pr,
|
||||
gitea_review_pr,
|
||||
gitea_delete_branch,
|
||||
gitea_reconcile_merged_cleanups,
|
||||
gitea_edit_pr,
|
||||
gitea_get_file,
|
||||
gitea_commit_files,
|
||||
@@ -990,6 +991,60 @@ class TestDeleteBranch(unittest.TestCase):
|
||||
self.assertIn("feat%2Fbranch", url)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Reconcile merged cleanups (#269)
|
||||
# ---------------------------------------------------------------------------
|
||||
READ_ENV = {
|
||||
"GITEA_ALLOWED_OPERATIONS": "gitea.read",
|
||||
}
|
||||
|
||||
|
||||
class TestReconcileMergedCleanups(unittest.TestCase):
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_dry_run_builds_report_without_mutations(self, _auth, mock_api_get_all):
|
||||
mock_api_get_all.side_effect = [
|
||||
[
|
||||
{
|
||||
"number": 50,
|
||||
"title": "merged feature",
|
||||
"body": "Closes #50",
|
||||
"merged": True,
|
||||
"merged_at": "2026-07-06T12:00:00Z",
|
||||
"merge_commit_sha": "deadbeef",
|
||||
"head": {"ref": "feat/issue-50-example", "sha": "cafebabe"},
|
||||
}
|
||||
],
|
||||
[],
|
||||
]
|
||||
with patch.dict(os.environ, READ_ENV, clear=True):
|
||||
with patch(
|
||||
"mcp_server._remote_branch_exists",
|
||||
return_value=True,
|
||||
):
|
||||
with patch(
|
||||
"mcp_server.merged_cleanup_reconcile.is_head_ancestor_of_ref",
|
||||
return_value=True,
|
||||
):
|
||||
result = gitea_reconcile_merged_cleanups(
|
||||
dry_run=True,
|
||||
remote="prgs",
|
||||
limit=10,
|
||||
)
|
||||
self.assertTrue(result["success"])
|
||||
self.assertTrue(result["dry_run"])
|
||||
self.assertFalse(result["executed"])
|
||||
self.assertEqual(result["merged_pr_count"], 1)
|
||||
self.assertEqual(result["entries"][0]["issue_number"], 50)
|
||||
|
||||
def test_execute_requires_confirmation(self):
|
||||
with patch.dict(os.environ, READ_ENV, clear=True):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_reconcile_merged_cleanups(dry_run=False, execute_confirmed=False)
|
||||
self.assertIn("execute_confirmed", str(ctx.exception))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Edit PR
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -0,0 +1,159 @@
|
||||
"""Tests for merged PR cleanup reconciliation (#269)."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
import merged_cleanup_reconcile as mcr # noqa: E402
|
||||
|
||||
|
||||
class TestMergedCleanupAssessment(unittest.TestCase):
|
||||
def test_extract_linked_issue_from_closes(self):
|
||||
issue = mcr.extract_linked_issue(
|
||||
"feat: cleanup (Closes #269)",
|
||||
"No other refs",
|
||||
)
|
||||
self.assertEqual(issue, 269)
|
||||
|
||||
def test_merged_remote_branch_safe_when_gates_pass(self):
|
||||
result = mcr.assess_remote_branch_cleanup(
|
||||
pr_number=42,
|
||||
head_branch="feat/issue-269-merged-pr-cleanup-reconcile",
|
||||
merged=True,
|
||||
remote_branch_exists=True,
|
||||
open_pr_heads=set(),
|
||||
head_on_master=True,
|
||||
delete_capability_allowed=True,
|
||||
active_lock=False,
|
||||
)
|
||||
self.assertTrue(result["safe_to_delete_remote"])
|
||||
self.assertEqual(result["block_reasons"], [])
|
||||
|
||||
def test_open_pr_blocks_remote_delete(self):
|
||||
result = mcr.assess_remote_branch_cleanup(
|
||||
pr_number=42,
|
||||
head_branch="feat/issue-9-example",
|
||||
merged=True,
|
||||
remote_branch_exists=True,
|
||||
open_pr_heads={"feat/issue-9-example"},
|
||||
head_on_master=True,
|
||||
delete_capability_allowed=True,
|
||||
active_lock=False,
|
||||
)
|
||||
self.assertFalse(result["safe_to_delete_remote"])
|
||||
self.assertIn("open PR", result["block_reasons"][0])
|
||||
|
||||
def test_protected_branch_blocks_remote_delete(self):
|
||||
result = mcr.assess_remote_branch_cleanup(
|
||||
pr_number=1,
|
||||
head_branch="master",
|
||||
merged=True,
|
||||
remote_branch_exists=True,
|
||||
open_pr_heads=set(),
|
||||
head_on_master=True,
|
||||
delete_capability_allowed=True,
|
||||
active_lock=False,
|
||||
)
|
||||
self.assertFalse(result["safe_to_delete_remote"])
|
||||
self.assertIn("protected", result["block_reasons"][0])
|
||||
|
||||
def test_active_lock_blocks_remote_and_worktree_cleanup(self):
|
||||
remote = mcr.assess_remote_branch_cleanup(
|
||||
pr_number=42,
|
||||
head_branch="feat/issue-9-example",
|
||||
merged=True,
|
||||
remote_branch_exists=True,
|
||||
open_pr_heads=set(),
|
||||
head_on_master=True,
|
||||
delete_capability_allowed=True,
|
||||
active_lock=True,
|
||||
)
|
||||
local = mcr.assess_local_worktree_cleanup(
|
||||
pr_number=42,
|
||||
head_branch="feat/issue-9-example",
|
||||
merged=True,
|
||||
worktree_state={
|
||||
"exists": True,
|
||||
"clean": True,
|
||||
"current_branch": "feat/issue-9-example",
|
||||
"worktree_path": "/repo/branches/feat-issue-9-example",
|
||||
},
|
||||
active_lock=True,
|
||||
)
|
||||
self.assertFalse(remote["safe_to_delete_remote"])
|
||||
self.assertFalse(local["safe_to_remove_worktree"])
|
||||
self.assertIn("active issue lock", remote["block_reasons"][0])
|
||||
|
||||
def test_dirty_worktree_blocks_local_cleanup(self):
|
||||
result = mcr.assess_local_worktree_cleanup(
|
||||
pr_number=42,
|
||||
head_branch="feat/issue-9-example",
|
||||
merged=True,
|
||||
worktree_state={
|
||||
"exists": True,
|
||||
"clean": False,
|
||||
"dirty_files": ["gitea_mcp_server.py"],
|
||||
"current_branch": "feat/issue-9-example",
|
||||
"worktree_path": "/repo/branches/feat-issue-9-example",
|
||||
},
|
||||
active_lock=False,
|
||||
)
|
||||
self.assertFalse(result["safe_to_remove_worktree"])
|
||||
self.assertIn("tracked edits", result["block_reasons"][0])
|
||||
|
||||
|
||||
class TestMergedCleanupReport(unittest.TestCase):
|
||||
def test_build_report_skips_unmerged_closed_prs(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
report = mcr.build_reconciliation_report(
|
||||
project_root=tmp,
|
||||
closed_prs=[
|
||||
{
|
||||
"number": 10,
|
||||
"title": "closed not merged",
|
||||
"body": "Closes #10",
|
||||
"head": {"ref": "feat/issue-10-x"},
|
||||
"merged_at": None,
|
||||
},
|
||||
{
|
||||
"number": 11,
|
||||
"title": "merged",
|
||||
"body": "Closes #11",
|
||||
"head": {"ref": "feat/issue-11-x"},
|
||||
"merged_at": "2026-07-06T12:00:00Z",
|
||||
"merge_commit_sha": "abc123",
|
||||
},
|
||||
],
|
||||
open_prs=[],
|
||||
remote_branch_exists={"feat/issue-11-x": True},
|
||||
head_on_master={11: True},
|
||||
delete_capability_allowed=False,
|
||||
)
|
||||
self.assertEqual(report["merged_pr_count"], 1)
|
||||
entry = report["entries"][0]
|
||||
self.assertEqual(entry["pr_number"], 11)
|
||||
self.assertEqual(entry["issue_number"], 11)
|
||||
self.assertFalse(entry["remote_branch"]["safe_to_delete_remote"])
|
||||
self.assertIn("delete_branch capability", entry["remote_branch"]["block_reasons"][0])
|
||||
|
||||
def test_has_active_issue_lock_reads_lock_file(self):
|
||||
with tempfile.NamedTemporaryFile("w", encoding="utf-8", delete=False) as handle:
|
||||
handle.write('{"branch_name": "feat/issue-9-example"}')
|
||||
lock_path = handle.name
|
||||
try:
|
||||
self.assertTrue(
|
||||
mcr.has_active_issue_lock("feat/issue-9-example", lock_path=lock_path)
|
||||
)
|
||||
self.assertFalse(
|
||||
mcr.has_active_issue_lock("feat/other-branch", lock_path=lock_path)
|
||||
)
|
||||
finally:
|
||||
os.remove(lock_path)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -134,7 +134,8 @@ class TestControlPlaneGuide(GuideTestBase):
|
||||
"merge_confirmation", "redaction", "separation",
|
||||
"profile_switching", "identity_verification",
|
||||
"work_selection", "global_worktree",
|
||||
"shell_spawn_hard_stop"):
|
||||
"shell_spawn_hard_stop", "subagent_tool_budget",
|
||||
"subagent_delegation"):
|
||||
self.assertIn(key, rules)
|
||||
self.assertIn("MERGE PR", json.dumps(rules["merge_confirmation"]))
|
||||
self.assertTrue(rules["hard_stops"])
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
"""Doc-contract checks for subagent tool-budget guardrails (#259).
|
||||
|
||||
Single-step MCP tasks must not expand into 100+ tool-call retry spirals with
|
||||
WebFetch/Playwright/manual-encoding fallbacks. These tests pin budget defaults,
|
||||
native-MCP-first rules, and forbidden detours in the runbooks, portable skill,
|
||||
and operator guide.
|
||||
"""
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
|
||||
RUNBOOK = REPO_ROOT / "docs" / "llm-workflow-runbooks.md"
|
||||
SKILL = REPO_ROOT / "skills" / "llm-project-workflow" / "SKILL.md"
|
||||
|
||||
|
||||
def _normalize(text: str) -> str:
|
||||
"""Collapse whitespace so phrase checks survive markdown line wrapping."""
|
||||
return " ".join(text.split())
|
||||
|
||||
|
||||
def _runbook_text() -> str:
|
||||
return _normalize(RUNBOOK.read_text(encoding="utf-8"))
|
||||
|
||||
|
||||
def _skill_text() -> str:
|
||||
return _normalize(SKILL.read_text(encoding="utf-8"))
|
||||
|
||||
|
||||
def test_runbook_has_subagent_tool_budget_section():
|
||||
text = _runbook_text()
|
||||
assert "## Subagent Tool-Budget Guardrails" in text
|
||||
for phrase in (
|
||||
"15",
|
||||
"5 minutes",
|
||||
"40",
|
||||
"15 minutes",
|
||||
"gitea_commit_files",
|
||||
"WebFetch",
|
||||
"Playwright",
|
||||
"no retry spirals",
|
||||
):
|
||||
assert phrase.lower() in text.lower(), f"runbook missing phrase: {phrase!r}"
|
||||
|
||||
|
||||
def test_runbook_forbids_subagent_commit_delegation():
|
||||
text = _runbook_text()
|
||||
for phrase in (
|
||||
"do not delegate commit authority to a subagent",
|
||||
"main session first",
|
||||
"native MCP before fallback",
|
||||
):
|
||||
assert phrase.lower() in text.lower(), f"runbook missing: {phrase!r}"
|
||||
|
||||
|
||||
def test_runbook_rejects_fallback_detours_when_mcp_commit_available():
|
||||
lower = _runbook_text().lower()
|
||||
for forbidden in ("webfetch", "playwright", "manual llm-generated base64"):
|
||||
assert forbidden in lower, f"runbook must name forbidden detour: {forbidden!r}"
|
||||
assert "_encode_" in lower
|
||||
assert "gitea_commit_files" in lower
|
||||
|
||||
|
||||
def test_skill_doc_declares_subagent_tool_budget_guardrails():
|
||||
text = _skill_text()
|
||||
assert "## Subagent Tool-Budget Guardrails" in text
|
||||
for phrase in (
|
||||
"15 tool calls",
|
||||
"5 minutes",
|
||||
"gitea_commit_files",
|
||||
"WebFetch",
|
||||
"never resume a failed subagent",
|
||||
):
|
||||
assert phrase.lower() in text.lower(), f"SKILL.md missing phrase: {phrase!r}"
|
||||
|
||||
|
||||
def test_operator_guide_rules_include_subagent_tool_budget():
|
||||
import sys
|
||||
|
||||
sys.path.insert(0, str(REPO_ROOT))
|
||||
import gitea_mcp_server
|
||||
|
||||
rule = gitea_mcp_server._GUIDE_RULES["subagent_tool_budget"]
|
||||
for phrase in (
|
||||
"15 tool calls",
|
||||
"gitea_commit_files",
|
||||
"WebFetch",
|
||||
"retry loop",
|
||||
):
|
||||
assert phrase in rule, f"operator guide rule missing: {phrase!r}"
|
||||
Reference in New Issue
Block a user