Merge commit 'b5ee6567aade4fdce4c57d826184bfd01ebb522e' into HEAD
# Conflicts: # tests/test_llm_workflow_split.py
This commit is contained in:
@@ -13,6 +13,8 @@ REVIEWER_CAPABILITY_TASKS = frozenset({
|
|||||||
"review_pr",
|
"review_pr",
|
||||||
"merge_pr",
|
"merge_pr",
|
||||||
"blind_pr_queue_review",
|
"blind_pr_queue_review",
|
||||||
|
"pr_queue_cleanup",
|
||||||
|
"pr-queue-cleanup",
|
||||||
"request_changes_pr",
|
"request_changes_pr",
|
||||||
"approve_pr",
|
"approve_pr",
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -749,3 +749,22 @@ scripts/release-tag v0.4.0 --notes-file /tmp/release-notes.md --push
|
|||||||
- [`credential-isolation.md`](credential-isolation.md) — credential handling.
|
- [`credential-isolation.md`](credential-isolation.md) — credential handling.
|
||||||
- [`release-workflows.md`](release-workflows.md) — release/merge workflow.
|
- [`release-workflows.md`](release-workflows.md) — release/merge workflow.
|
||||||
- [`../README.md`](../README.md) — canonical config, thin launchers, the menu.
|
- [`../README.md`](../README.md) — canonical config, thin launchers, the menu.
|
||||||
|
|
||||||
|
## PR-only queue cleanup mode (#390)
|
||||||
|
|
||||||
|
Use `pr-queue-cleanup` mode when the queue holds many open PRs and the goal
|
||||||
|
is to drain reviews deterministically. One run = one PR = one canonical
|
||||||
|
review (`skills/llm-project-workflow/workflows/pr-queue-cleanup.md`).
|
||||||
|
|
||||||
|
* Cleanup runs are reviewer-role only (`pr_queue_cleanup` resolver task);
|
||||||
|
author sessions get `wrong_role_stop`.
|
||||||
|
* Forbidden in cleanup mode: issue claiming, branch creation, implementation
|
||||||
|
edits, new issue filing, and any second-PR review after a terminal
|
||||||
|
mutation.
|
||||||
|
* Terminal chain: stop after `REQUEST_CHANGES`; after `APPROVED` continue
|
||||||
|
only to same-PR merge with explicit per-PR operator authorization and
|
||||||
|
passing gates; stop after merge or merge blocker.
|
||||||
|
* Every run reports the next suggested PR without continuing to it; the next
|
||||||
|
PR requires a fresh run with fresh identity/capability/inventory proof.
|
||||||
|
* Use full `review-merge-pr` mode instead when the operator wants a single
|
||||||
|
targeted review, and `work-issue` mode for any authoring.
|
||||||
|
|||||||
+415
-1
@@ -476,7 +476,10 @@ import task_capability_map # noqa: E402
|
|||||||
import review_proofs # noqa: E402
|
import review_proofs # noqa: E402
|
||||||
import agent_temp_artifacts
|
import agent_temp_artifacts
|
||||||
import issue_lock_worktree # noqa: E402
|
import issue_lock_worktree # noqa: E402
|
||||||
|
import issue_claim_heartbeat # noqa: E402
|
||||||
import merged_cleanup_reconcile # noqa: E402
|
import merged_cleanup_reconcile # noqa: E402
|
||||||
|
import review_merge_state_machine # noqa: E402
|
||||||
|
import native_mcp_preference # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
# Fail-closed exact-issue-lock file (#204): written by gitea_lock_issue,
|
# Fail-closed exact-issue-lock file (#204): written by gitea_lock_issue,
|
||||||
@@ -4082,6 +4085,22 @@ _GUIDE_RULES = {
|
|||||||
"inventory, summarize) needs no explicit authorization. Enforced "
|
"inventory, summarize) needs no explicit authorization. Enforced "
|
||||||
"fail closed via subagent_gate.assess_subagent_delegation and "
|
"fail closed via subagent_gate.assess_subagent_delegation and "
|
||||||
"validate_subagent_report (#266)."),
|
"validate_subagent_report (#266)."),
|
||||||
|
"review_merge_state_machine": (
|
||||||
|
"PR review/merge must follow the enforced state machine: PRECHECK → "
|
||||||
|
"INVENTORY → SELECT_PR → PIN_HEAD_SHA → CREATE_BRANCHES_WORKTREE → "
|
||||||
|
"VALIDATE → REVIEW_DECISION → APPROVE_OR_REQUEST_CHANGES → "
|
||||||
|
"PRE_MERGE_RECHECK → MERGE → POST_MERGE_REPORT. Failed upstream "
|
||||||
|
"states forbid downstream approve/merge. infra_stop and capability "
|
||||||
|
"blocks stop immediately. Blocked recovery handoffs must restart the "
|
||||||
|
"full workflow — never replay approve/merge commands (#290)."),
|
||||||
|
"native_mcp_preference": (
|
||||||
|
"Gitea mutations must use native MCP tools first. When MCP is "
|
||||||
|
"available, block shell scripts, direct API calls, WebFetch, "
|
||||||
|
"Playwright, helper encoders, and profile-secret reads unless "
|
||||||
|
"explicit recovery-mode proof is complete. If MCP transport is "
|
||||||
|
"broken or the shell circuit breaker trips, emit a terminal recovery "
|
||||||
|
"report instead of improvising unsafe fallback. Reviewers must not "
|
||||||
|
"touch or restart MCP server files/processes (#270)."),
|
||||||
}
|
}
|
||||||
|
|
||||||
_COMMON_WORKFLOWS = [
|
_COMMON_WORKFLOWS = [
|
||||||
@@ -4786,6 +4805,128 @@ def _build_runtime_task_capabilities(
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@mcp.tool()
|
||||||
|
def gitea_assess_review_merge_state_machine(
|
||||||
|
target_state: str | None = None,
|
||||||
|
state_completion: dict[str, bool] | None = None,
|
||||||
|
pre_merge_gates: dict[str, bool] | None = None,
|
||||||
|
infra_stop: bool = False,
|
||||||
|
capability_blocked: bool = False,
|
||||||
|
recovery_handoff_text: str | None = None,
|
||||||
|
final_report_text: str | None = None,
|
||||||
|
) -> dict:
|
||||||
|
"""Read-only: assess enforced PR review/merge workflow state (#290)."""
|
||||||
|
completion = state_completion or {}
|
||||||
|
blockers = review_merge_state_machine.assess_workflow_blockers(
|
||||||
|
infra_stop=infra_stop,
|
||||||
|
capability_blocked=capability_blocked,
|
||||||
|
)
|
||||||
|
result = {
|
||||||
|
"workflow": review_merge_state_machine.workflow_status(
|
||||||
|
completion,
|
||||||
|
infra_stop=infra_stop,
|
||||||
|
capability_blocked=capability_blocked,
|
||||||
|
),
|
||||||
|
"blockers": blockers,
|
||||||
|
"approve": review_merge_state_machine.can_approve(
|
||||||
|
completion,
|
||||||
|
infra_stop=infra_stop,
|
||||||
|
capability_blocked=capability_blocked,
|
||||||
|
),
|
||||||
|
"merge": review_merge_state_machine.can_merge(
|
||||||
|
completion,
|
||||||
|
pre_merge_gates=pre_merge_gates,
|
||||||
|
infra_stop=infra_stop,
|
||||||
|
capability_blocked=capability_blocked,
|
||||||
|
),
|
||||||
|
}
|
||||||
|
if target_state:
|
||||||
|
result["advancement"] = review_merge_state_machine.assess_state_advancement(
|
||||||
|
completion,
|
||||||
|
target_state=target_state,
|
||||||
|
infra_stop=infra_stop,
|
||||||
|
capability_blocked=capability_blocked,
|
||||||
|
)
|
||||||
|
if recovery_handoff_text is not None:
|
||||||
|
result["recovery_handoff"] = (
|
||||||
|
review_merge_state_machine.assess_blocked_recovery_handoff(
|
||||||
|
recovery_handoff_text
|
||||||
|
)
|
||||||
|
)
|
||||||
|
if final_report_text is not None:
|
||||||
|
result["final_report_claims"] = (
|
||||||
|
review_merge_state_machine.assess_final_report_state_claims(
|
||||||
|
final_report_text,
|
||||||
|
state_completion=completion,
|
||||||
|
approve_completed=bool(completion.get("APPROVE_OR_REQUEST_CHANGES")),
|
||||||
|
merge_completed=bool(completion.get("MERGE")),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
return result
|
||||||
|
|
||||||
|
|
||||||
|
@mcp.tool()
|
||||||
|
def gitea_assess_gitea_operation_path(
|
||||||
|
task: str,
|
||||||
|
path_kind: str | None = None,
|
||||||
|
command_or_detail: str | None = None,
|
||||||
|
mcp_available: bool = True,
|
||||||
|
mcp_tool_visible: bool = True,
|
||||||
|
recovery_mode: bool = False,
|
||||||
|
recovery_proof_complete: bool = False,
|
||||||
|
remote: str = "dadeschools",
|
||||||
|
host: str | None = None,
|
||||||
|
) -> dict:
|
||||||
|
"""Read-only: assess whether a proposed Gitea mutation path is allowed (#270).
|
||||||
|
|
||||||
|
Native MCP must be preferred when available. Shell scripts, direct API
|
||||||
|
calls, WebFetch, Playwright, and helper encoders are blocked unless
|
||||||
|
explicit recovery-mode proof is supplied.
|
||||||
|
"""
|
||||||
|
profile = get_profile()
|
||||||
|
role = _role_kind(
|
||||||
|
profile.get("allowed_operations") or [],
|
||||||
|
profile.get("forbidden_operations") or [],
|
||||||
|
)
|
||||||
|
return native_mcp_preference.assess_gitea_operation_path(
|
||||||
|
task=task,
|
||||||
|
path_kind=path_kind,
|
||||||
|
command_or_detail=command_or_detail,
|
||||||
|
mcp_available=mcp_available,
|
||||||
|
mcp_tool_visible=mcp_tool_visible,
|
||||||
|
recovery_mode=recovery_mode,
|
||||||
|
recovery_proof_complete=recovery_proof_complete,
|
||||||
|
role=role,
|
||||||
|
active_profile=profile.get("profile_name"),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@mcp.tool()
|
||||||
|
def gitea_record_shell_spawn_outcome(
|
||||||
|
exit_code: int | None = None,
|
||||||
|
stdout: str = "",
|
||||||
|
stderr: str = "",
|
||||||
|
spawn_failure: bool | None = None,
|
||||||
|
probe_attempted: bool = False,
|
||||||
|
probe_succeeded: bool | None = None,
|
||||||
|
) -> dict:
|
||||||
|
"""Record shell spawn outcome and update the session circuit breaker (#270)."""
|
||||||
|
return native_mcp_preference.record_shell_spawn_outcome(
|
||||||
|
exit_code=exit_code,
|
||||||
|
stdout=stdout,
|
||||||
|
stderr=stderr,
|
||||||
|
spawn_failure=spawn_failure,
|
||||||
|
probe_attempted=probe_attempted,
|
||||||
|
probe_succeeded=probe_succeeded,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@mcp.tool()
|
||||||
|
def gitea_get_shell_health() -> dict:
|
||||||
|
"""Read-only: return shell spawn circuit-breaker state for this MCP session (#270)."""
|
||||||
|
return native_mcp_preference.shell_health_status()
|
||||||
|
|
||||||
|
|
||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
def gitea_get_runtime_context(
|
def gitea_get_runtime_context(
|
||||||
remote: str = "dadeschools",
|
remote: str = "dadeschools",
|
||||||
@@ -4904,6 +5045,7 @@ def gitea_get_runtime_context(
|
|||||||
"preflight_block_reasons": preflight["preflight_block_reasons"],
|
"preflight_block_reasons": preflight["preflight_block_reasons"],
|
||||||
"preflight_workspace": preflight.get("preflight_workspace"),
|
"preflight_workspace": preflight.get("preflight_workspace"),
|
||||||
"session_capabilities": session_capabilities,
|
"session_capabilities": session_capabilities,
|
||||||
|
"shell_health": native_mcp_preference.shell_health_status(),
|
||||||
}
|
}
|
||||||
|
|
||||||
if reveal and h:
|
if reveal and h:
|
||||||
@@ -5146,6 +5288,41 @@ def gitea_audit_config() -> dict:
|
|||||||
return report
|
return report
|
||||||
|
|
||||||
|
|
||||||
|
def _post_structured_issue_comment(
|
||||||
|
*,
|
||||||
|
issue_number: int,
|
||||||
|
body: str,
|
||||||
|
remote: str,
|
||||||
|
host: str | None,
|
||||||
|
org: str | None,
|
||||||
|
repo: str | None,
|
||||||
|
audit_op: str = "create_issue_comment",
|
||||||
|
) -> dict:
|
||||||
|
"""Post an issue-thread comment after permission gates already passed."""
|
||||||
|
h, o, r = _resolve(remote, host, org, repo)
|
||||||
|
auth = _auth(h)
|
||||||
|
api = f"{repo_api_url(h, o, r)}/issues/{issue_number}/comments"
|
||||||
|
with _audited(
|
||||||
|
audit_op,
|
||||||
|
host=h,
|
||||||
|
remote=remote,
|
||||||
|
org=o,
|
||||||
|
repo=r,
|
||||||
|
issue_number=issue_number,
|
||||||
|
request_metadata={"body_chars": len(body)},
|
||||||
|
):
|
||||||
|
data = api_request("POST", api, auth, {"body": body})
|
||||||
|
result = {
|
||||||
|
"success": True,
|
||||||
|
"performed": True,
|
||||||
|
"comment_id": data["id"],
|
||||||
|
"issue_number": issue_number,
|
||||||
|
}
|
||||||
|
if _reveal_endpoints():
|
||||||
|
result["url"] = data.get("html_url")
|
||||||
|
return result
|
||||||
|
|
||||||
|
|
||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
def gitea_mark_issue(
|
def gitea_mark_issue(
|
||||||
issue_number: int,
|
issue_number: int,
|
||||||
@@ -5155,6 +5332,8 @@ def gitea_mark_issue(
|
|||||||
org: str | None = None,
|
org: str | None = None,
|
||||||
repo: str | None = None,
|
repo: str | None = None,
|
||||||
worktree_path: str | None = None,
|
worktree_path: str | None = None,
|
||||||
|
branch_name: str | None = None,
|
||||||
|
profile: str | None = None,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Claim or release an issue via the status:in-progress label.
|
"""Claim or release an issue via the status:in-progress label.
|
||||||
|
|
||||||
@@ -5204,7 +5383,31 @@ def gitea_mark_issue(
|
|||||||
request_metadata={"op": "add", "label": "status:in-progress"}):
|
request_metadata={"op": "add", "label": "status:in-progress"}):
|
||||||
api_request("POST", f"{base}/issues/{issue_number}/labels", auth,
|
api_request("POST", f"{base}/issues/{issue_number}/labels", auth,
|
||||||
{"labels": [label_id]})
|
{"labels": [label_id]})
|
||||||
return {"success": True, "message": f"Issue #{issue_number} claimed."}
|
active_profile = (profile or get_profile().get("profile_name") or "unknown")
|
||||||
|
branch = (branch_name or "pending").strip() or "pending"
|
||||||
|
heartbeat_body = issue_claim_heartbeat.format_heartbeat_body(
|
||||||
|
kind="claim",
|
||||||
|
issue_number=issue_number,
|
||||||
|
branch=branch,
|
||||||
|
phase="claimed",
|
||||||
|
profile=active_profile,
|
||||||
|
next_action="create worktree and begin implementation",
|
||||||
|
)
|
||||||
|
heartbeat = _post_structured_issue_comment(
|
||||||
|
issue_number=issue_number,
|
||||||
|
body=heartbeat_body,
|
||||||
|
remote=remote,
|
||||||
|
host=host,
|
||||||
|
org=org,
|
||||||
|
repo=repo,
|
||||||
|
audit_op="claim_heartbeat",
|
||||||
|
)
|
||||||
|
return {
|
||||||
|
"success": True,
|
||||||
|
"message": f"Issue #{issue_number} claimed.",
|
||||||
|
"heartbeat_posted": heartbeat.get("success", False),
|
||||||
|
"heartbeat_comment_id": heartbeat.get("comment_id"),
|
||||||
|
}
|
||||||
else:
|
else:
|
||||||
with _audited("unlabel_issue", host=h, remote=remote, org=o, repo=r,
|
with _audited("unlabel_issue", host=h, remote=remote, org=o, repo=r,
|
||||||
issue_number=issue_number,
|
issue_number=issue_number,
|
||||||
@@ -5214,6 +5417,217 @@ def gitea_mark_issue(
|
|||||||
return {"success": True, "message": f"Issue #{issue_number} released."}
|
return {"success": True, "message": f"Issue #{issue_number} released."}
|
||||||
|
|
||||||
|
|
||||||
|
@mcp.tool()
|
||||||
|
def gitea_post_heartbeat(
|
||||||
|
issue_number: int,
|
||||||
|
branch: str,
|
||||||
|
phase: str,
|
||||||
|
pr: str = "none",
|
||||||
|
next_action: str = "none",
|
||||||
|
blocker: str = "none",
|
||||||
|
profile: str | None = None,
|
||||||
|
remote: str = "dadeschools",
|
||||||
|
host: str | None = None,
|
||||||
|
org: str | None = None,
|
||||||
|
repo: str | None = None,
|
||||||
|
) -> dict:
|
||||||
|
"""Post a structured progress heartbeat on a claimed issue (#268)."""
|
||||||
|
blocked = _profile_permission_block(
|
||||||
|
task_capability_map.required_permission("post_heartbeat"))
|
||||||
|
if blocked:
|
||||||
|
return blocked
|
||||||
|
verify_preflight_purity(remote)
|
||||||
|
active_profile = profile or get_profile().get("profile_name")
|
||||||
|
body = issue_claim_heartbeat.format_heartbeat_body(
|
||||||
|
kind="progress",
|
||||||
|
issue_number=issue_number,
|
||||||
|
branch=branch,
|
||||||
|
phase=phase,
|
||||||
|
profile=active_profile,
|
||||||
|
pr=pr,
|
||||||
|
next_action=next_action,
|
||||||
|
blocker=blocker,
|
||||||
|
)
|
||||||
|
return _post_structured_issue_comment(
|
||||||
|
issue_number=issue_number,
|
||||||
|
body=body,
|
||||||
|
remote=remote,
|
||||||
|
host=host,
|
||||||
|
org=org,
|
||||||
|
repo=repo,
|
||||||
|
audit_op="progress_heartbeat",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@mcp.tool()
|
||||||
|
def gitea_reconcile_issue_claims(
|
||||||
|
state: str = "open",
|
||||||
|
stale_after_hours: int = 24,
|
||||||
|
heartbeat_lease_minutes: int = 30,
|
||||||
|
limit: int = 100,
|
||||||
|
remote: str = "dadeschools",
|
||||||
|
host: str | None = None,
|
||||||
|
org: str | None = None,
|
||||||
|
repo: str | None = None,
|
||||||
|
) -> dict:
|
||||||
|
"""Read-only inventory of issue claims and heartbeat lease status (#268)."""
|
||||||
|
read_block = _profile_operation_gate("gitea.read")
|
||||||
|
if read_block:
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"reasons": read_block,
|
||||||
|
"permission_report": _permission_block_report("gitea.read"),
|
||||||
|
}
|
||||||
|
|
||||||
|
h, o, r = _resolve(remote, host, org, repo)
|
||||||
|
auth = _auth(h)
|
||||||
|
base = repo_api_url(h, o, r)
|
||||||
|
issues = api_get_all(f"{base}/issues?state={state}&type=issues", auth, limit=limit)
|
||||||
|
open_prs = api_get_all(f"{base}/pulls?state=open", auth)
|
||||||
|
branches = api_get_all(f"{base}/branches", auth, limit=limit)
|
||||||
|
branch_names = [b.get("name") for b in branches if b.get("name")]
|
||||||
|
|
||||||
|
comments_by_issue: dict[int, list[dict]] = {}
|
||||||
|
for issue in issues:
|
||||||
|
if not issue_claim_heartbeat.issue_has_in_progress_label(issue):
|
||||||
|
continue
|
||||||
|
number = int(issue["number"])
|
||||||
|
api = f"{base}/issues/{number}/comments"
|
||||||
|
comments_by_issue[number] = api_request("GET", api, auth) or []
|
||||||
|
|
||||||
|
reclaim_after_minutes = max(heartbeat_lease_minutes * 2, 60)
|
||||||
|
inventory = issue_claim_heartbeat.build_claim_inventory(
|
||||||
|
issues=issues,
|
||||||
|
comments_by_issue=comments_by_issue,
|
||||||
|
open_prs=open_prs,
|
||||||
|
branch_names=branch_names,
|
||||||
|
heartbeat_lease_minutes=heartbeat_lease_minutes,
|
||||||
|
reclaim_after_minutes=reclaim_after_minutes,
|
||||||
|
)
|
||||||
|
inventory["cleanup_plan"] = issue_claim_heartbeat.build_cleanup_plan(inventory)
|
||||||
|
inventory["success"] = True
|
||||||
|
inventory["performed"] = False
|
||||||
|
return inventory
|
||||||
|
|
||||||
|
|
||||||
|
@mcp.tool()
|
||||||
|
def gitea_cleanup_stale_claims(
|
||||||
|
dry_run: bool = True,
|
||||||
|
execute_confirmed: bool = False,
|
||||||
|
heartbeat_lease_minutes: int = 30,
|
||||||
|
limit: int = 100,
|
||||||
|
remote: str = "dadeschools",
|
||||||
|
host: str | None = None,
|
||||||
|
org: str | None = None,
|
||||||
|
repo: str | None = None,
|
||||||
|
) -> dict:
|
||||||
|
"""Propose or execute stale-claim cleanup for phantom/reclaimable issues (#268)."""
|
||||||
|
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"),
|
||||||
|
}
|
||||||
|
|
||||||
|
inventory = gitea_reconcile_issue_claims(
|
||||||
|
heartbeat_lease_minutes=heartbeat_lease_minutes,
|
||||||
|
limit=limit,
|
||||||
|
remote=remote,
|
||||||
|
host=host,
|
||||||
|
org=org,
|
||||||
|
repo=repo,
|
||||||
|
)
|
||||||
|
if not inventory.get("success"):
|
||||||
|
return inventory
|
||||||
|
|
||||||
|
plan = [
|
||||||
|
entry
|
||||||
|
for entry in inventory.get("cleanup_plan") or []
|
||||||
|
if entry.get("action") == "remove_status_in_progress_and_comment"
|
||||||
|
]
|
||||||
|
report = {
|
||||||
|
"success": True,
|
||||||
|
"dry_run": dry_run,
|
||||||
|
"performed": False,
|
||||||
|
"planned_actions": plan,
|
||||||
|
"inventory_counts": inventory.get("counts"),
|
||||||
|
}
|
||||||
|
if dry_run:
|
||||||
|
return report
|
||||||
|
|
||||||
|
if not execute_confirmed:
|
||||||
|
raise ValueError(
|
||||||
|
"execute_confirmed must be True when dry_run=False (fail closed)"
|
||||||
|
)
|
||||||
|
|
||||||
|
blocked = _profile_permission_block(
|
||||||
|
task_capability_map.required_permission("cleanup_stale_claims"))
|
||||||
|
if blocked:
|
||||||
|
return blocked
|
||||||
|
verify_preflight_purity(remote)
|
||||||
|
|
||||||
|
h, o, r = _resolve(remote, host, org, repo)
|
||||||
|
auth = _auth(h)
|
||||||
|
base = repo_api_url(h, o, r)
|
||||||
|
labels = api_request("GET", f"{base}/labels?limit=100", auth)
|
||||||
|
label_id = next(
|
||||||
|
(lb["id"] for lb in labels if lb.get("name") == "status:in-progress"),
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
if label_id is None:
|
||||||
|
raise RuntimeError("Label 'status:in-progress' not found")
|
||||||
|
|
||||||
|
actions: list[dict] = []
|
||||||
|
active_profile = get_profile().get("profile_name")
|
||||||
|
for entry in plan:
|
||||||
|
issue_number = int(entry["issue_number"])
|
||||||
|
with _audited(
|
||||||
|
"unlabel_issue",
|
||||||
|
host=h,
|
||||||
|
remote=remote,
|
||||||
|
org=o,
|
||||||
|
repo=r,
|
||||||
|
issue_number=issue_number,
|
||||||
|
request_metadata={"op": "cleanup_stale_claim", "label": "status:in-progress"},
|
||||||
|
):
|
||||||
|
api_request(
|
||||||
|
"DELETE",
|
||||||
|
f"{base}/issues/{issue_number}/labels/{label_id}",
|
||||||
|
auth,
|
||||||
|
)
|
||||||
|
cleanup_body = issue_claim_heartbeat.format_heartbeat_body(
|
||||||
|
kind="cleanup",
|
||||||
|
issue_number=issue_number,
|
||||||
|
branch="none",
|
||||||
|
phase="stale-claim-cleanup",
|
||||||
|
profile=active_profile,
|
||||||
|
next_action="issue reclaimable by queue",
|
||||||
|
blocker=entry.get("status") or "stale",
|
||||||
|
)
|
||||||
|
comment = _post_structured_issue_comment(
|
||||||
|
issue_number=issue_number,
|
||||||
|
body=cleanup_body,
|
||||||
|
remote=remote,
|
||||||
|
host=host,
|
||||||
|
org=org,
|
||||||
|
repo=repo,
|
||||||
|
audit_op="cleanup_heartbeat",
|
||||||
|
)
|
||||||
|
actions.append(
|
||||||
|
{
|
||||||
|
"issue_number": issue_number,
|
||||||
|
"label_removed": True,
|
||||||
|
"cleanup_comment_id": comment.get("comment_id"),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
report["performed"] = True
|
||||||
|
report["actions"] = actions
|
||||||
|
return report
|
||||||
|
|
||||||
|
|
||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
def gitea_list_labels(
|
def gitea_list_labels(
|
||||||
remote: str = "dadeschools",
|
remote: str = "dadeschools",
|
||||||
|
|||||||
@@ -0,0 +1,295 @@
|
|||||||
|
"""Issue claim heartbeat leases and stale-claim reconciliation (#268).
|
||||||
|
|
||||||
|
Structured issue-thread comments prove live ownership beyond the
|
||||||
|
``status:in-progress`` label alone. Queue inventory can classify claims as
|
||||||
|
active, stale, reclaimable, PR-backed, or phantom.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from datetime import datetime, timedelta, timezone
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
MARKER = "<!-- gitea-issue-claim-heartbeat:v1 -->"
|
||||||
|
IN_PROGRESS_LABEL = "status:in-progress"
|
||||||
|
|
||||||
|
_KIND_CLAIM = "claim"
|
||||||
|
_KIND_PROGRESS = "progress"
|
||||||
|
_KIND_CLEANUP = "cleanup"
|
||||||
|
|
||||||
|
_FIELD_RE = re.compile(
|
||||||
|
r"^\s*-\s*([a-z_]+)\s*:\s*(.+?)\s*$",
|
||||||
|
re.IGNORECASE | re.MULTILINE,
|
||||||
|
)
|
||||||
|
_ISSUE_REF_RE = re.compile(r"issue-(\d+)", re.IGNORECASE)
|
||||||
|
|
||||||
|
|
||||||
|
def _parse_timestamp(value: str | None) -> datetime | None:
|
||||||
|
if not value:
|
||||||
|
return None
|
||||||
|
text = value.strip()
|
||||||
|
if text.endswith("Z"):
|
||||||
|
text = text[:-1] + "+00:00"
|
||||||
|
try:
|
||||||
|
parsed = datetime.fromisoformat(text)
|
||||||
|
except ValueError:
|
||||||
|
return None
|
||||||
|
if parsed.tzinfo is None:
|
||||||
|
return parsed.replace(tzinfo=timezone.utc)
|
||||||
|
return parsed
|
||||||
|
|
||||||
|
|
||||||
|
def format_heartbeat_body(
|
||||||
|
*,
|
||||||
|
kind: str,
|
||||||
|
issue_number: int,
|
||||||
|
branch: str,
|
||||||
|
phase: str,
|
||||||
|
profile: str | None = None,
|
||||||
|
pr: str = "none",
|
||||||
|
next_action: str = "none",
|
||||||
|
blocker: str = "none",
|
||||||
|
) -> str:
|
||||||
|
"""Return a structured, machine-parseable issue comment body."""
|
||||||
|
profile_value = (profile or "unknown").strip() or "unknown"
|
||||||
|
lines = [
|
||||||
|
MARKER,
|
||||||
|
"**Issue claim heartbeat**",
|
||||||
|
f"- kind: {kind}",
|
||||||
|
f"- issue: #{issue_number}",
|
||||||
|
f"- branch: {branch}",
|
||||||
|
f"- phase: {phase}",
|
||||||
|
f"- profile: {profile_value}",
|
||||||
|
f"- pr: {pr}",
|
||||||
|
f"- blocker: {blocker}",
|
||||||
|
f"- next_action: {next_action}",
|
||||||
|
]
|
||||||
|
return "\n".join(lines)
|
||||||
|
|
||||||
|
|
||||||
|
def parse_heartbeat_comment(body: str) -> dict[str, Any] | None:
|
||||||
|
"""Parse one structured heartbeat comment, or None when not a heartbeat."""
|
||||||
|
text = body or ""
|
||||||
|
if MARKER not in text:
|
||||||
|
return None
|
||||||
|
fields: dict[str, str] = {}
|
||||||
|
for match in _FIELD_RE.finditer(text):
|
||||||
|
fields[match.group(1).strip().lower()] = match.group(2).strip()
|
||||||
|
if not fields:
|
||||||
|
return None
|
||||||
|
issue_raw = fields.get("issue", "")
|
||||||
|
issue_digits = re.sub(r"[^\d]", "", issue_raw)
|
||||||
|
issue_number = int(issue_digits) if issue_digits.isdigit() else None
|
||||||
|
return {
|
||||||
|
"kind": fields.get("kind"),
|
||||||
|
"issue_number": issue_number,
|
||||||
|
"branch": fields.get("branch"),
|
||||||
|
"phase": fields.get("phase"),
|
||||||
|
"profile": fields.get("profile"),
|
||||||
|
"pr": fields.get("pr"),
|
||||||
|
"blocker": fields.get("blocker"),
|
||||||
|
"next_action": fields.get("next_action"),
|
||||||
|
"raw_fields": fields,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def extract_issue_heartbeats(
|
||||||
|
comments: list[dict],
|
||||||
|
*,
|
||||||
|
issue_number: int | None = None,
|
||||||
|
) -> list[dict]:
|
||||||
|
"""Return parsed heartbeats newest-last, optionally filtered to *issue_number*."""
|
||||||
|
heartbeats: list[dict] = []
|
||||||
|
for comment in comments or []:
|
||||||
|
parsed = parse_heartbeat_comment(comment.get("body") or "")
|
||||||
|
if not parsed:
|
||||||
|
continue
|
||||||
|
if issue_number is not None and parsed.get("issue_number") != issue_number:
|
||||||
|
continue
|
||||||
|
heartbeats.append(
|
||||||
|
{
|
||||||
|
**parsed,
|
||||||
|
"comment_id": comment.get("id"),
|
||||||
|
"author": (comment.get("user") or {}).get("login")
|
||||||
|
or comment.get("author"),
|
||||||
|
"created_at": comment.get("created_at"),
|
||||||
|
"updated_at": comment.get("updated_at"),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
return heartbeats
|
||||||
|
|
||||||
|
|
||||||
|
def issue_has_in_progress_label(issue: dict) -> bool:
|
||||||
|
labels = issue.get("labels") or []
|
||||||
|
for label in labels:
|
||||||
|
name = label if isinstance(label, str) else label.get("name")
|
||||||
|
if name == IN_PROGRESS_LABEL:
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def _linked_open_pr(issue_number: int, open_prs: list[dict]) -> dict | None:
|
||||||
|
pattern = f"issue-{issue_number}"
|
||||||
|
closes = f"closes #{issue_number}"
|
||||||
|
fixes = f"fixes #{issue_number}"
|
||||||
|
for pr in open_prs or []:
|
||||||
|
head = (pr.get("head") or {}).get("ref") or ""
|
||||||
|
text = f"{pr.get('title', '')} {pr.get('body', '')}".lower()
|
||||||
|
if pattern in head.lower():
|
||||||
|
return pr
|
||||||
|
if closes in text or fixes in text:
|
||||||
|
return pr
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def _matching_branch_names(issue_number: int, branch_names: list[str]) -> list[str]:
|
||||||
|
pattern = f"issue-{issue_number}"
|
||||||
|
return [name for name in branch_names if pattern in (name or "").lower()]
|
||||||
|
|
||||||
|
|
||||||
|
def classify_issue_claim(
|
||||||
|
*,
|
||||||
|
issue: dict,
|
||||||
|
comments: list[dict],
|
||||||
|
open_prs: list[dict] | None = None,
|
||||||
|
branch_names: list[str] | None = None,
|
||||||
|
now: datetime | None = None,
|
||||||
|
heartbeat_lease_minutes: int = 30,
|
||||||
|
reclaim_after_minutes: int = 60,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Classify one issue's claim state from labels, heartbeats, PRs, and branches."""
|
||||||
|
issue_number = int(issue.get("number") or 0)
|
||||||
|
open_prs = open_prs or []
|
||||||
|
branch_names = branch_names or []
|
||||||
|
current = now or datetime.now(timezone.utc)
|
||||||
|
|
||||||
|
has_label = issue_has_in_progress_label(issue)
|
||||||
|
heartbeats = extract_issue_heartbeats(comments, issue_number=issue_number)
|
||||||
|
latest = heartbeats[-1] if heartbeats else None
|
||||||
|
latest_at = _parse_timestamp(
|
||||||
|
(latest or {}).get("updated_at") or (latest or {}).get("created_at")
|
||||||
|
)
|
||||||
|
age_minutes = None
|
||||||
|
if latest_at:
|
||||||
|
age_minutes = int((current - latest_at).total_seconds() // 60)
|
||||||
|
|
||||||
|
linked_pr = _linked_open_pr(issue_number, open_prs)
|
||||||
|
matching_branches = _matching_branch_names(issue_number, branch_names)
|
||||||
|
|
||||||
|
if not has_label:
|
||||||
|
status = "not_claimed"
|
||||||
|
reasons = ["issue lacks status:in-progress label"]
|
||||||
|
elif linked_pr:
|
||||||
|
status = "awaiting_review"
|
||||||
|
reasons = [f"open PR #{linked_pr.get('number')} covers this issue"]
|
||||||
|
elif not heartbeats:
|
||||||
|
status = "phantom"
|
||||||
|
reasons = [
|
||||||
|
"status:in-progress label present without structured claim heartbeat"
|
||||||
|
]
|
||||||
|
elif latest_at is None:
|
||||||
|
status = "phantom"
|
||||||
|
reasons = ["heartbeat comments present but timestamps could not be parsed"]
|
||||||
|
elif age_minutes is not None and age_minutes <= heartbeat_lease_minutes:
|
||||||
|
status = "active"
|
||||||
|
reasons = [f"heartbeat age {age_minutes}m within {heartbeat_lease_minutes}m lease"]
|
||||||
|
elif matching_branches and age_minutes is not None and age_minutes <= reclaim_after_minutes:
|
||||||
|
status = "active"
|
||||||
|
reasons = [
|
||||||
|
f"matching branch(es) {', '.join(matching_branches)} with heartbeat "
|
||||||
|
f"age {age_minutes}m"
|
||||||
|
]
|
||||||
|
elif age_minutes is not None and age_minutes > reclaim_after_minutes and not matching_branches:
|
||||||
|
status = "reclaimable"
|
||||||
|
reasons = [
|
||||||
|
f"no heartbeat within {reclaim_after_minutes}m and no matching branch"
|
||||||
|
]
|
||||||
|
elif age_minutes is not None and age_minutes > heartbeat_lease_minutes:
|
||||||
|
status = "stale"
|
||||||
|
reasons = [
|
||||||
|
f"heartbeat age {age_minutes}m exceeds {heartbeat_lease_minutes}m lease"
|
||||||
|
]
|
||||||
|
else:
|
||||||
|
status = "active"
|
||||||
|
reasons = ["claim has structured heartbeat proof"]
|
||||||
|
|
||||||
|
return {
|
||||||
|
"issue_number": issue_number,
|
||||||
|
"title": issue.get("title"),
|
||||||
|
"status": status,
|
||||||
|
"has_in_progress_label": has_label,
|
||||||
|
"heartbeat_count": len(heartbeats),
|
||||||
|
"latest_heartbeat": latest,
|
||||||
|
"heartbeat_age_minutes": age_minutes,
|
||||||
|
"linked_open_pr": linked_pr.get("number") if linked_pr else None,
|
||||||
|
"matching_branches": matching_branches,
|
||||||
|
"reasons": reasons,
|
||||||
|
"reclaimable": status == "reclaimable",
|
||||||
|
"stale": status in {"stale", "phantom", "reclaimable"},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def build_claim_inventory(
|
||||||
|
*,
|
||||||
|
issues: list[dict],
|
||||||
|
comments_by_issue: dict[int, list[dict]],
|
||||||
|
open_prs: list[dict],
|
||||||
|
branch_names: list[str],
|
||||||
|
now: datetime | None = None,
|
||||||
|
heartbeat_lease_minutes: int = 30,
|
||||||
|
reclaim_after_minutes: int = 60,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Build a queue inventory report for in-progress issue claims."""
|
||||||
|
entries: list[dict] = []
|
||||||
|
for issue in issues or []:
|
||||||
|
if not issue_has_in_progress_label(issue):
|
||||||
|
continue
|
||||||
|
number = int(issue["number"])
|
||||||
|
entry = classify_issue_claim(
|
||||||
|
issue=issue,
|
||||||
|
comments=comments_by_issue.get(number, []),
|
||||||
|
open_prs=open_prs,
|
||||||
|
branch_names=branch_names,
|
||||||
|
now=now,
|
||||||
|
heartbeat_lease_minutes=heartbeat_lease_minutes,
|
||||||
|
reclaim_after_minutes=reclaim_after_minutes,
|
||||||
|
)
|
||||||
|
entries.append(entry)
|
||||||
|
|
||||||
|
counts: dict[str, int] = {}
|
||||||
|
for entry in entries:
|
||||||
|
counts[entry["status"]] = counts.get(entry["status"], 0) + 1
|
||||||
|
|
||||||
|
return {
|
||||||
|
"entries": entries,
|
||||||
|
"counts": counts,
|
||||||
|
"heartbeat_lease_minutes": heartbeat_lease_minutes,
|
||||||
|
"reclaim_after_minutes": reclaim_after_minutes,
|
||||||
|
"in_progress_total": len(entries),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def build_cleanup_plan(inventory: dict[str, Any]) -> list[dict]:
|
||||||
|
"""Return stale/reclaimable/phantom claims that may be cleaned up."""
|
||||||
|
plan: list[dict] = []
|
||||||
|
for entry in inventory.get("entries") or []:
|
||||||
|
if entry.get("status") in {"reclaimable", "phantom"}:
|
||||||
|
plan.append(
|
||||||
|
{
|
||||||
|
"issue_number": entry["issue_number"],
|
||||||
|
"status": entry["status"],
|
||||||
|
"action": "remove_status_in_progress_and_comment",
|
||||||
|
"reasons": list(entry.get("reasons") or []),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
elif entry.get("status") == "stale" and not entry.get("linked_open_pr"):
|
||||||
|
plan.append(
|
||||||
|
{
|
||||||
|
"issue_number": entry["issue_number"],
|
||||||
|
"status": entry["status"],
|
||||||
|
"action": "report_only",
|
||||||
|
"reasons": list(entry.get("reasons") or []),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
return plan
|
||||||
@@ -0,0 +1,369 @@
|
|||||||
|
"""Native MCP preference gate and shell health circuit breaker (#270).
|
||||||
|
|
||||||
|
Gitea mutations must use native MCP tools first. Shell scripts, direct API
|
||||||
|
calls, browser helpers, and improvised encoders are blocked when MCP is
|
||||||
|
available unless explicit recovery-mode proof is supplied.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
from reviewer_fallback import LOCAL_GITEA_SCRIPT_NAMES
|
||||||
|
|
||||||
|
# Tasks that must prefer native MCP over shell/API/helper fallbacks.
|
||||||
|
GITEA_MUTATION_TASKS = frozenset({
|
||||||
|
"comment_issue",
|
||||||
|
"mark_issue",
|
||||||
|
"lock_issue",
|
||||||
|
"set_issue_labels",
|
||||||
|
"create_issue",
|
||||||
|
"close_issue",
|
||||||
|
"create_branch",
|
||||||
|
"push_branch",
|
||||||
|
"create_pr",
|
||||||
|
"close_pr",
|
||||||
|
"comment_pr",
|
||||||
|
"review_pr",
|
||||||
|
"merge_pr",
|
||||||
|
"approve_pr",
|
||||||
|
"request_changes_pr",
|
||||||
|
"commit_files",
|
||||||
|
"gitea_commit_files",
|
||||||
|
"delete_branch",
|
||||||
|
"address_pr_change_requests",
|
||||||
|
})
|
||||||
|
|
||||||
|
ALLOWED_PATH_KINDS = frozenset({
|
||||||
|
"mcp_native",
|
||||||
|
"recovery_fallback",
|
||||||
|
})
|
||||||
|
|
||||||
|
BLOCKED_PATH_KINDS = frozenset({
|
||||||
|
"shell_script",
|
||||||
|
"direct_api",
|
||||||
|
"webfetch",
|
||||||
|
"playwright",
|
||||||
|
"helper_script",
|
||||||
|
"unsafe_helper",
|
||||||
|
"mcp_server_touch",
|
||||||
|
})
|
||||||
|
|
||||||
|
SHELL_SPAWN_FAILURE_THRESHOLD = 2
|
||||||
|
|
||||||
|
TERMINAL_REPORT_HEADING = (
|
||||||
|
"MCP transport unavailable or shell circuit breaker tripped. "
|
||||||
|
"No unsafe Gitea fallback performed."
|
||||||
|
)
|
||||||
|
|
||||||
|
_RECOVERY_MODE_RE = re.compile(
|
||||||
|
r"\b(?:recovery mode|explicit recovery|mcp unavailable|mcp not available|"
|
||||||
|
r"mcp tools unavailable|no mcp path)\b",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_LOCAL_SCRIPT_RE = re.compile(
|
||||||
|
r"(?:^|[\s\"'`/])(?:python3?|bash|sh)?\s*(?:"
|
||||||
|
+ "|".join(re.escape(name) for name in LOCAL_GITEA_SCRIPT_NAMES)
|
||||||
|
+ r")",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_WEBFETCH_RE = re.compile(r"\b(?:webfetch|mcp_web_fetch|fetch\s+url)\b", re.IGNORECASE)
|
||||||
|
_PLAYWRIGHT_RE = re.compile(r"\b(?:playwright|browser_navigate|browser_click)\b", re.IGNORECASE)
|
||||||
|
_HELPER_SCRIPT_RE = re.compile(
|
||||||
|
r"(?:^|[\s\"'`/])(?:_encode_|_emit_|_inline_)[\w-]+\.py",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_MANUAL_BASE64_RE = re.compile(
|
||||||
|
r"\b(?:manual\s+base64|llm-generated\s+base64|base64-encode\s+in\s+chat)\b",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_DIRECT_API_RE = re.compile(
|
||||||
|
r"\b(?:api_request|urllib\.request|requests\.(?:post|patch|put)|curl\s+-X\s+POST)\b",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_MCP_SERVER_TOUCH_RE = re.compile(
|
||||||
|
r"\b(?:kill|pkill|restart|reload|touch|edit|modify|write)\b.{0,40}\b(?:mcp_server|mcp-server|gitea_mcp_server)\b",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_CLI_AUTH_DIVERGENCE_RE = re.compile(
|
||||||
|
r"GITEA_MCP_PROFILE\s*=\s*['\"]?([^\s'\";]+)",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
|
||||||
|
_session_shell: dict[str, Any] = {
|
||||||
|
"consecutive_spawn_failures": 0,
|
||||||
|
"shell_unavailable": False,
|
||||||
|
"hard_stopped": False,
|
||||||
|
"last_exit_code": None,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _clean(value: str | None) -> str:
|
||||||
|
return (value or "").strip()
|
||||||
|
|
||||||
|
|
||||||
|
def is_spawn_failure(
|
||||||
|
*,
|
||||||
|
exit_code: int | None = None,
|
||||||
|
stdout: str | None = None,
|
||||||
|
stderr: str | None = None,
|
||||||
|
spawn_failure: bool | None = None,
|
||||||
|
) -> bool:
|
||||||
|
"""True for executor spawn failures (exit_code -1, empty output)."""
|
||||||
|
if spawn_failure is True:
|
||||||
|
return True
|
||||||
|
if spawn_failure is False:
|
||||||
|
return False
|
||||||
|
if exit_code != -1:
|
||||||
|
return False
|
||||||
|
return not (_clean(stdout) or _clean(stderr))
|
||||||
|
|
||||||
|
|
||||||
|
def record_shell_spawn_outcome(
|
||||||
|
*,
|
||||||
|
exit_code: int | None = None,
|
||||||
|
stdout: str | None = None,
|
||||||
|
stderr: str | None = None,
|
||||||
|
spawn_failure: bool | None = None,
|
||||||
|
probe_attempted: bool = False,
|
||||||
|
probe_succeeded: bool | None = None,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Track shell spawn outcomes and trip the session circuit breaker (#270 AC4)."""
|
||||||
|
global _session_shell
|
||||||
|
failed = is_spawn_failure(
|
||||||
|
exit_code=exit_code,
|
||||||
|
stdout=stdout,
|
||||||
|
stderr=stderr,
|
||||||
|
spawn_failure=spawn_failure,
|
||||||
|
)
|
||||||
|
|
||||||
|
if failed:
|
||||||
|
_session_shell["consecutive_spawn_failures"] = (
|
||||||
|
int(_session_shell.get("consecutive_spawn_failures") or 0) + 1
|
||||||
|
)
|
||||||
|
_session_shell["last_exit_code"] = exit_code
|
||||||
|
if probe_attempted and probe_succeeded is False:
|
||||||
|
_session_shell["shell_unavailable"] = True
|
||||||
|
else:
|
||||||
|
_session_shell["consecutive_spawn_failures"] = 0
|
||||||
|
_session_shell["shell_unavailable"] = False
|
||||||
|
_session_shell["hard_stopped"] = False
|
||||||
|
_session_shell["last_exit_code"] = exit_code
|
||||||
|
|
||||||
|
failures = int(_session_shell["consecutive_spawn_failures"])
|
||||||
|
if failures >= SHELL_SPAWN_FAILURE_THRESHOLD:
|
||||||
|
_session_shell["shell_unavailable"] = True
|
||||||
|
_session_shell["hard_stopped"] = True
|
||||||
|
|
||||||
|
return shell_health_status()
|
||||||
|
|
||||||
|
|
||||||
|
def shell_health_status() -> dict[str, Any]:
|
||||||
|
"""Return the current shell health / circuit-breaker state."""
|
||||||
|
failures = int(_session_shell.get("consecutive_spawn_failures") or 0)
|
||||||
|
hard_stopped = bool(_session_shell.get("hard_stopped"))
|
||||||
|
shell_unavailable = bool(_session_shell.get("shell_unavailable"))
|
||||||
|
return {
|
||||||
|
"consecutive_spawn_failures": failures,
|
||||||
|
"shell_unavailable": shell_unavailable,
|
||||||
|
"hard_stopped": hard_stopped,
|
||||||
|
"threshold": SHELL_SPAWN_FAILURE_THRESHOLD,
|
||||||
|
"shell_use_allowed": not hard_stopped,
|
||||||
|
"last_exit_code": _session_shell.get("last_exit_code"),
|
||||||
|
"safe_next_action": (
|
||||||
|
"emit terminal recovery report; prefer native MCP for remaining Gitea mutations"
|
||||||
|
if hard_stopped
|
||||||
|
else (
|
||||||
|
"probe shell once with echo/pwd; if probe fails mark shell unavailable"
|
||||||
|
if failures == 1
|
||||||
|
else "shell healthy"
|
||||||
|
)
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def clear_shell_health_for_tests() -> None:
|
||||||
|
"""Reset session shell state (tests only)."""
|
||||||
|
global _session_shell
|
||||||
|
_session_shell = {
|
||||||
|
"consecutive_spawn_failures": 0,
|
||||||
|
"shell_unavailable": False,
|
||||||
|
"hard_stopped": False,
|
||||||
|
"last_exit_code": None,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def classify_command_path(command_or_detail: str | None) -> str:
|
||||||
|
"""Classify a proposed command/detail into a path kind."""
|
||||||
|
text = command_or_detail or ""
|
||||||
|
if _MCP_SERVER_TOUCH_RE.search(text):
|
||||||
|
return "mcp_server_touch"
|
||||||
|
if _WEBFETCH_RE.search(text):
|
||||||
|
return "webfetch"
|
||||||
|
if _PLAYWRIGHT_RE.search(text):
|
||||||
|
return "playwright"
|
||||||
|
if _HELPER_SCRIPT_RE.search(text) or _MANUAL_BASE64_RE.search(text):
|
||||||
|
return "unsafe_helper"
|
||||||
|
if _LOCAL_SCRIPT_RE.search(text):
|
||||||
|
return "shell_script"
|
||||||
|
if _DIRECT_API_RE.search(text):
|
||||||
|
return "direct_api"
|
||||||
|
return "mcp_native"
|
||||||
|
|
||||||
|
|
||||||
|
def detect_cli_auth_divergence(
|
||||||
|
command_or_detail: str | None,
|
||||||
|
*,
|
||||||
|
active_profile: str | None,
|
||||||
|
) -> list[str]:
|
||||||
|
"""Flag shell commands that override GITEA_MCP_PROFILE away from the session."""
|
||||||
|
reasons: list[str] = []
|
||||||
|
text = command_or_detail or ""
|
||||||
|
active = _clean(active_profile)
|
||||||
|
match = _CLI_AUTH_DIVERGENCE_RE.search(text)
|
||||||
|
if not match or not active:
|
||||||
|
return reasons
|
||||||
|
requested = _clean(match.group(1))
|
||||||
|
if requested and requested != active:
|
||||||
|
reasons.append(
|
||||||
|
f"CLI auth divergence: command sets GITEA_MCP_PROFILE='{requested}' "
|
||||||
|
f"but active session profile is '{active}' (fail closed)"
|
||||||
|
)
|
||||||
|
return reasons
|
||||||
|
|
||||||
|
|
||||||
|
def format_mcp_unavailable_terminal_report(
|
||||||
|
*,
|
||||||
|
task: str | None = None,
|
||||||
|
reasons: list[str] | None = None,
|
||||||
|
) -> str:
|
||||||
|
"""Terminal report when MCP is broken and unsafe recovery is forbidden (#270 AC3)."""
|
||||||
|
lines = [TERMINAL_REPORT_HEADING, ""]
|
||||||
|
if task:
|
||||||
|
lines.append(f"Blocked task: {task}")
|
||||||
|
if reasons:
|
||||||
|
lines.extend(["", "Reasons:"])
|
||||||
|
lines.extend(f"- {reason}" for reason in reasons)
|
||||||
|
lines.extend([
|
||||||
|
"",
|
||||||
|
"Required recovery (no improvised fallback):",
|
||||||
|
"- Restart the MCP session",
|
||||||
|
"- Kill hung background terminals holding the shell executor",
|
||||||
|
"- Retry the native MCP tool once after reconnect",
|
||||||
|
"- If MCP remains unavailable, stop and hand off to the operator",
|
||||||
|
"- Do not run local Gitea scripts, WebFetch, Playwright, or manual base64 encoders",
|
||||||
|
])
|
||||||
|
return "\n".join(lines)
|
||||||
|
|
||||||
|
|
||||||
|
def assess_gitea_operation_path(
|
||||||
|
*,
|
||||||
|
task: str,
|
||||||
|
path_kind: str | None = None,
|
||||||
|
command_or_detail: str | None = None,
|
||||||
|
mcp_available: bool = True,
|
||||||
|
mcp_tool_visible: bool = True,
|
||||||
|
recovery_mode: bool = False,
|
||||||
|
recovery_proof_complete: bool = False,
|
||||||
|
role: str | None = None,
|
||||||
|
active_profile: str | None = None,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Fail closed when a Gitea mutation would bypass native MCP (#270)."""
|
||||||
|
task_name = _clean(task)
|
||||||
|
resolved_kind = _clean(path_kind) or classify_command_path(command_or_detail)
|
||||||
|
reasons: list[str] = []
|
||||||
|
|
||||||
|
if task_name and task_name not in GITEA_MUTATION_TASKS:
|
||||||
|
reasons.append(
|
||||||
|
f"unknown or non-mutation Gitea task '{task_name}'; "
|
||||||
|
"native MCP preference gate applies only to Gitea mutations"
|
||||||
|
)
|
||||||
|
|
||||||
|
shell = shell_health_status()
|
||||||
|
if shell["hard_stopped"] and resolved_kind != "mcp_native":
|
||||||
|
reasons.append(
|
||||||
|
"shell circuit breaker is hard-stopped after consecutive spawn failures; "
|
||||||
|
"use native MCP or emit terminal recovery report"
|
||||||
|
)
|
||||||
|
|
||||||
|
recovery_declared = recovery_mode or bool(
|
||||||
|
_RECOVERY_MODE_RE.search(command_or_detail or "")
|
||||||
|
)
|
||||||
|
recovery_allowed = (
|
||||||
|
recovery_declared
|
||||||
|
and recovery_proof_complete
|
||||||
|
and not mcp_available
|
||||||
|
and resolved_kind in BLOCKED_PATH_KINDS - {"mcp_server_touch"}
|
||||||
|
)
|
||||||
|
|
||||||
|
if resolved_kind in BLOCKED_PATH_KINDS and not recovery_allowed:
|
||||||
|
reasons.append(f"path kind '{resolved_kind}' is not an approved Gitea mutation path")
|
||||||
|
|
||||||
|
if resolved_kind == "mcp_server_touch":
|
||||||
|
if _clean(role) == "reviewer" or (
|
||||||
|
active_profile and "reviewer" in active_profile.lower()
|
||||||
|
):
|
||||||
|
reasons.append(
|
||||||
|
"reviewer workflows must not touch, restart, or kill MCP server files/processes"
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
reasons.append(
|
||||||
|
"MCP server files/processes must not be modified during normal Gitea workflows"
|
||||||
|
)
|
||||||
|
|
||||||
|
reasons.extend(
|
||||||
|
detect_cli_auth_divergence(command_or_detail, active_profile=active_profile)
|
||||||
|
)
|
||||||
|
|
||||||
|
if (
|
||||||
|
mcp_available
|
||||||
|
and mcp_tool_visible
|
||||||
|
and resolved_kind != "mcp_native"
|
||||||
|
and not recovery_allowed
|
||||||
|
):
|
||||||
|
if not recovery_declared:
|
||||||
|
reasons.append(
|
||||||
|
"native MCP tools are available; shell/API/helper fallback is forbidden"
|
||||||
|
)
|
||||||
|
elif not recovery_proof_complete:
|
||||||
|
reasons.append(
|
||||||
|
"recovery-mode fallback requires complete recovery proof before proceeding"
|
||||||
|
)
|
||||||
|
|
||||||
|
if not mcp_available and resolved_kind != "mcp_native" and not recovery_allowed:
|
||||||
|
if not recovery_declared:
|
||||||
|
reasons.append(
|
||||||
|
"MCP transport unavailable; produce terminal recovery report instead of "
|
||||||
|
"improvising unsafe fallback"
|
||||||
|
)
|
||||||
|
|
||||||
|
block = bool(reasons)
|
||||||
|
terminal_report = None
|
||||||
|
if block and (not mcp_available or shell["hard_stopped"]):
|
||||||
|
terminal_report = format_mcp_unavailable_terminal_report(
|
||||||
|
task=task_name or None,
|
||||||
|
reasons=reasons,
|
||||||
|
)
|
||||||
|
|
||||||
|
return {
|
||||||
|
"task": task_name or None,
|
||||||
|
"path_kind": resolved_kind,
|
||||||
|
"mcp_available": mcp_available,
|
||||||
|
"mcp_tool_visible": mcp_tool_visible,
|
||||||
|
"recovery_mode": recovery_declared,
|
||||||
|
"shell_health": shell,
|
||||||
|
"block": block,
|
||||||
|
"allowed": not block,
|
||||||
|
"reasons": reasons,
|
||||||
|
"terminal_report": terminal_report,
|
||||||
|
"safe_next_action": (
|
||||||
|
"use the native MCP tool for this task"
|
||||||
|
if mcp_available and mcp_tool_visible and block
|
||||||
|
else (
|
||||||
|
terminal_report or "proceed with native MCP"
|
||||||
|
if block
|
||||||
|
else "proceed with native MCP"
|
||||||
|
)
|
||||||
|
),
|
||||||
|
}
|
||||||
@@ -0,0 +1,225 @@
|
|||||||
|
"""PR-only queue cleanup mode gates and report verifier (#390).
|
||||||
|
|
||||||
|
Cleanup mode dispatches exactly one canonical review run per PR. It forbids
|
||||||
|
author-side mutations (issue claiming, branch creation, implementation edits,
|
||||||
|
issue filing) and enforces the terminal-mutation chain: stop after
|
||||||
|
``REQUEST_CHANGES``; after ``APPROVED`` continue only to same-PR merge when
|
||||||
|
merge is explicitly authorized for that PR and merge gates pass; stop after
|
||||||
|
merge or a merge blocker. The next PR always requires a fresh run.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
|
||||||
|
CLEANUP_WORKFLOW_PATH = "workflows/pr-queue-cleanup.md"
|
||||||
|
|
||||||
|
# Author-side resolver tasks that must never run inside cleanup mode.
|
||||||
|
CLEANUP_FORBIDDEN_TASKS = frozenset({
|
||||||
|
"claim_issue",
|
||||||
|
"mark_issue",
|
||||||
|
"lock_issue",
|
||||||
|
"create_issue",
|
||||||
|
"create_branch",
|
||||||
|
"push_branch",
|
||||||
|
"create_pr",
|
||||||
|
"commit_files",
|
||||||
|
"gitea_commit_files",
|
||||||
|
"address_pr_change_requests",
|
||||||
|
})
|
||||||
|
|
||||||
|
# Run-state outcomes for one cleanup dispatch.
|
||||||
|
STOP_AFTER_REQUEST_CHANGES = "STOP_AFTER_REQUEST_CHANGES"
|
||||||
|
STOP_AFTER_DECISION = "STOP_AFTER_DECISION"
|
||||||
|
CONTINUE_TO_SAME_PR_MERGE = "CONTINUE_TO_SAME_PR_MERGE"
|
||||||
|
STOP_APPROVED_NO_MERGE_AUTH = "STOP_APPROVED_NO_MERGE_AUTH"
|
||||||
|
STOP_APPROVED_MERGE_GATES_FAILED = "STOP_APPROVED_MERGE_GATES_FAILED"
|
||||||
|
STOP_AFTER_MERGE = "STOP_AFTER_MERGE"
|
||||||
|
STOP_AFTER_MERGE_BLOCKER = "STOP_AFTER_MERGE_BLOCKER"
|
||||||
|
STOP_GATE_NOT_PROVEN = "STOP_GATE_NOT_PROVEN"
|
||||||
|
|
||||||
|
_TERMINAL_DECISIONS = frozenset({"approved", "request_changes", "comment", "skip"})
|
||||||
|
|
||||||
|
|
||||||
|
def resolve_cleanup_run_state(
|
||||||
|
decision: str | None,
|
||||||
|
*,
|
||||||
|
merge_authorized_for_pr: bool = False,
|
||||||
|
merge_gates_passed: bool | None = None,
|
||||||
|
merge_completed: bool = False,
|
||||||
|
merge_blocker: bool = False,
|
||||||
|
) -> dict:
|
||||||
|
"""Resolve what one cleanup run may do after its terminal review decision.
|
||||||
|
|
||||||
|
Fail closed: unknown decisions stop the run with no further mutation.
|
||||||
|
"""
|
||||||
|
normalized = (decision or "").strip().lower()
|
||||||
|
|
||||||
|
if normalized not in _TERMINAL_DECISIONS:
|
||||||
|
return {
|
||||||
|
"outcome": STOP_GATE_NOT_PROVEN,
|
||||||
|
"further_mutation_allowed": False,
|
||||||
|
"reasons": [
|
||||||
|
f"unknown terminal decision {decision!r}; cleanup run stops "
|
||||||
|
"(fail closed)"
|
||||||
|
],
|
||||||
|
}
|
||||||
|
|
||||||
|
if normalized == "request_changes":
|
||||||
|
return {
|
||||||
|
"outcome": STOP_AFTER_REQUEST_CHANGES,
|
||||||
|
"further_mutation_allowed": False,
|
||||||
|
"reasons": ["REQUEST_CHANGES is terminal in cleanup mode"],
|
||||||
|
}
|
||||||
|
|
||||||
|
if normalized in {"comment", "skip"}:
|
||||||
|
return {
|
||||||
|
"outcome": STOP_AFTER_DECISION,
|
||||||
|
"further_mutation_allowed": False,
|
||||||
|
"reasons": [f"{normalized} decision ends the cleanup run"],
|
||||||
|
}
|
||||||
|
|
||||||
|
# normalized == "approved"
|
||||||
|
if merge_completed:
|
||||||
|
return {
|
||||||
|
"outcome": STOP_AFTER_MERGE,
|
||||||
|
"further_mutation_allowed": False,
|
||||||
|
"reasons": ["merge completed; cleanup run stops"],
|
||||||
|
}
|
||||||
|
if merge_blocker:
|
||||||
|
return {
|
||||||
|
"outcome": STOP_AFTER_MERGE_BLOCKER,
|
||||||
|
"further_mutation_allowed": False,
|
||||||
|
"reasons": ["merge blocker recorded; cleanup run stops"],
|
||||||
|
}
|
||||||
|
if not merge_authorized_for_pr:
|
||||||
|
return {
|
||||||
|
"outcome": STOP_APPROVED_NO_MERGE_AUTH,
|
||||||
|
"further_mutation_allowed": False,
|
||||||
|
"reasons": [
|
||||||
|
"APPROVED without explicit per-PR merge authorization; "
|
||||||
|
"cleanup run stops"
|
||||||
|
],
|
||||||
|
}
|
||||||
|
if merge_gates_passed is not True:
|
||||||
|
return {
|
||||||
|
"outcome": STOP_APPROVED_MERGE_GATES_FAILED,
|
||||||
|
"further_mutation_allowed": False,
|
||||||
|
"reasons": [
|
||||||
|
"merge gates not proven passed; cleanup run stops before merge"
|
||||||
|
],
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
"outcome": CONTINUE_TO_SAME_PR_MERGE,
|
||||||
|
"further_mutation_allowed": True,
|
||||||
|
"reasons": [],
|
||||||
|
"allowed_mutation": "merge same PR only",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def check_cleanup_task_allowed(task: str) -> tuple[bool, list[str]]:
|
||||||
|
"""Fail closed on any author-side mutation task inside cleanup mode."""
|
||||||
|
normalized = (task or "").strip().lower()
|
||||||
|
if normalized in CLEANUP_FORBIDDEN_TASKS:
|
||||||
|
return False, [
|
||||||
|
f"task '{normalized}' is forbidden in PR-only cleanup mode: "
|
||||||
|
"no issue claiming, branch creation, implementation edits, or "
|
||||||
|
"issue filing"
|
||||||
|
]
|
||||||
|
return True, []
|
||||||
|
|
||||||
|
|
||||||
|
_SELECTED_PR_RE = re.compile(
|
||||||
|
r"^\s*[-*]?\s*selected pr\s*:\s*#?(\d+)", re.IGNORECASE | re.MULTILINE
|
||||||
|
)
|
||||||
|
_NEXT_SUGGESTED_RE = re.compile(
|
||||||
|
r"^\s*[-*]?\s*next suggested pr\s*:", re.IGNORECASE | re.MULTILINE
|
||||||
|
)
|
||||||
|
_TERMINAL_MUTATION_RE = re.compile(
|
||||||
|
r"^\s*[-*]?\s*(?:review (?:decision|verdict)|terminal (?:review )?"
|
||||||
|
r"(?:decision|mutation))\s*:\s*"
|
||||||
|
r"(approved|request_changes|request changes|merged|comment)",
|
||||||
|
re.IGNORECASE | re.MULTILINE,
|
||||||
|
)
|
||||||
|
_PAGINATION_HINT_RE = re.compile(
|
||||||
|
r"inventory_complete|final page|has_more\s*[=:]\s*false|total[_ ]count",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_MERGE_RESULT_RE = re.compile(
|
||||||
|
r"^\s*[-*]?\s*merge result\s*:\s*(?!none\b|not attempted\b)\S",
|
||||||
|
re.IGNORECASE | re.MULTILINE,
|
||||||
|
)
|
||||||
|
_MERGE_AUTHORIZED_RE = re.compile(
|
||||||
|
r"^\s*[-*]?\s*merge authorized(?: for pr)?\s*:\s*true",
|
||||||
|
re.IGNORECASE | re.MULTILINE,
|
||||||
|
)
|
||||||
|
_ISSUE_MUTATION_CLAIM_RE = re.compile(
|
||||||
|
r"^\s*[-*]?\s*issue mutations\s*:\s*(?!none\b)\S",
|
||||||
|
re.IGNORECASE | re.MULTILINE,
|
||||||
|
)
|
||||||
|
_BRANCH_MUTATION_CLAIM_RE = re.compile(
|
||||||
|
r"^\s*[-*]?\s*branch mutations\s*:\s*(?!none\b)\S",
|
||||||
|
re.IGNORECASE | re.MULTILINE,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def assess_pr_queue_cleanup_report(report_text: str) -> dict:
|
||||||
|
"""Validate a PR-only cleanup run report (single dispatch, fail closed)."""
|
||||||
|
reasons: list[str] = []
|
||||||
|
text = report_text or ""
|
||||||
|
|
||||||
|
if CLEANUP_WORKFLOW_PATH not in text:
|
||||||
|
reasons.append(
|
||||||
|
f"report must cite the canonical cleanup workflow "
|
||||||
|
f"({CLEANUP_WORKFLOW_PATH})"
|
||||||
|
)
|
||||||
|
|
||||||
|
selected = _SELECTED_PR_RE.findall(text)
|
||||||
|
if len(selected) == 0:
|
||||||
|
reasons.append("report must name exactly one Selected PR")
|
||||||
|
elif len(set(selected)) > 1:
|
||||||
|
reasons.append(
|
||||||
|
"cleanup run selected multiple PRs "
|
||||||
|
f"({', '.join(sorted(set(selected)))}); one canonical review per "
|
||||||
|
"PR per run"
|
||||||
|
)
|
||||||
|
|
||||||
|
terminal = _TERMINAL_MUTATION_RE.findall(text)
|
||||||
|
if len(terminal) > 1:
|
||||||
|
reasons.append(
|
||||||
|
"multiple terminal review mutations reported in one cleanup run"
|
||||||
|
)
|
||||||
|
|
||||||
|
if not _PAGINATION_HINT_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"report missing PR inventory pagination proof "
|
||||||
|
"(inventory_complete / final page / total_count)"
|
||||||
|
)
|
||||||
|
|
||||||
|
if not _NEXT_SUGGESTED_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"report must include 'Next suggested PR' (without continuing to it)"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _MERGE_RESULT_RE.search(text) and not _MERGE_AUTHORIZED_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"merge reported without explicit per-PR merge authorization "
|
||||||
|
"('Merge authorized: true')"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _ISSUE_MUTATION_CLAIM_RE.search(text):
|
||||||
|
reasons.append("issue mutations are forbidden in PR-only cleanup mode")
|
||||||
|
if _BRANCH_MUTATION_CLAIM_RE.search(text):
|
||||||
|
reasons.append("branch mutations are forbidden in PR-only cleanup mode")
|
||||||
|
|
||||||
|
proven = not reasons
|
||||||
|
return {
|
||||||
|
"proven": proven,
|
||||||
|
"block": not proven,
|
||||||
|
"reasons": reasons,
|
||||||
|
"safe_next_action": (
|
||||||
|
"proceed" if proven else
|
||||||
|
"fix the cleanup report: one PR, one terminal mutation, pagination "
|
||||||
|
"proof, next-suggested-PR field, and no issue/branch mutations"
|
||||||
|
),
|
||||||
|
}
|
||||||
@@ -0,0 +1,332 @@
|
|||||||
|
"""Enforced PR review/merge workflow state machine (#290).
|
||||||
|
|
||||||
|
Review and merge must advance through explicit states. Any failed upstream
|
||||||
|
gate forbids downstream approve/merge mutations until the full workflow is
|
||||||
|
restarted after blockers clear.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
REVIEW_MERGE_STATES: tuple[str, ...] = (
|
||||||
|
"PRECHECK",
|
||||||
|
"INVENTORY",
|
||||||
|
"SELECT_PR",
|
||||||
|
"PIN_HEAD_SHA",
|
||||||
|
"CREATE_BRANCHES_WORKTREE",
|
||||||
|
"VALIDATE",
|
||||||
|
"REVIEW_DECISION",
|
||||||
|
"APPROVE_OR_REQUEST_CHANGES",
|
||||||
|
"PRE_MERGE_RECHECK",
|
||||||
|
"MERGE",
|
||||||
|
"POST_MERGE_REPORT",
|
||||||
|
)
|
||||||
|
|
||||||
|
TERMINAL_BLOCKED_HEADING = (
|
||||||
|
"PR review/merge workflow blocked. Restart the full workflow after blockers clear."
|
||||||
|
)
|
||||||
|
|
||||||
|
_FORBIDDEN_RECOVERY_REPLAY_RE = re.compile(
|
||||||
|
r"\b(?:approve(?:\s+pr)?\s*#?\d+|merge(?:\s+pr)?\s*#?\d+|gitea_merge_pr|"
|
||||||
|
r"gitea_submit_pr_review|submit\s+approve|run\s+merge)\b",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_RESTART_WORKFLOW_RE = re.compile(
|
||||||
|
r"restart(?:\s+the)?\s+full\s+workflow|rerun\s+the\s+full\s+workflow",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_READY_TO_MERGE_RE = re.compile(
|
||||||
|
r"\bready\s+to\s+merge\b",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_REVIEWED_VALIDATED_RE = re.compile(
|
||||||
|
r"\b(?:reviewed|validated|approval\s+submitted)\b",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
|
||||||
|
_PRE_MERGE_REQUIRED_GATES = (
|
||||||
|
"whoami_verified",
|
||||||
|
"profile_runtime_verified",
|
||||||
|
"merge_capability_verified",
|
||||||
|
"pr_refetched",
|
||||||
|
"reviewed_head_sha_unchanged",
|
||||||
|
"pr_mergeable",
|
||||||
|
"checks_passed",
|
||||||
|
"reviewer_not_author",
|
||||||
|
"worktree_clean",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _clean(value: str | None) -> str:
|
||||||
|
return (value or "").strip()
|
||||||
|
|
||||||
|
|
||||||
|
def state_index(state: str) -> int:
|
||||||
|
name = _clean(state).upper()
|
||||||
|
try:
|
||||||
|
return REVIEW_MERGE_STATES.index(name)
|
||||||
|
except ValueError as exc:
|
||||||
|
raise ValueError(f"unknown review/merge state '{state}' (fail closed)") from exc
|
||||||
|
|
||||||
|
|
||||||
|
def downstream_states(from_state: str) -> list[str]:
|
||||||
|
idx = state_index(from_state)
|
||||||
|
return list(REVIEW_MERGE_STATES[idx + 1 :])
|
||||||
|
|
||||||
|
|
||||||
|
def _completed_through(state_completion: dict[str, bool], state: str) -> bool:
|
||||||
|
return bool(state_completion.get(state))
|
||||||
|
|
||||||
|
|
||||||
|
def _first_incomplete_state(state_completion: dict[str, bool]) -> str | None:
|
||||||
|
for state in REVIEW_MERGE_STATES:
|
||||||
|
if not _completed_through(state_completion, state):
|
||||||
|
return state
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def assess_workflow_blockers(
|
||||||
|
*,
|
||||||
|
infra_stop: bool = False,
|
||||||
|
capability_blocked: bool = False,
|
||||||
|
mcp_reconnect_failed: bool = False,
|
||||||
|
stale_capability_state: bool = False,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Return hard blockers that forbid all PR queue work (#290 AC3)."""
|
||||||
|
reasons: list[str] = []
|
||||||
|
if infra_stop:
|
||||||
|
reasons.append("infra_stop is active; PR selection/review/merge is forbidden")
|
||||||
|
if capability_blocked:
|
||||||
|
reasons.append("gitea_resolve_task_capability returned blocked/stop_required")
|
||||||
|
if mcp_reconnect_failed:
|
||||||
|
reasons.append("MCP reconnect failed; stale session state cannot be reused")
|
||||||
|
if stale_capability_state:
|
||||||
|
reasons.append("stale MCP capability state detected after reconnect failure")
|
||||||
|
return {
|
||||||
|
"block": bool(reasons),
|
||||||
|
"reasons": reasons,
|
||||||
|
"forbidden_states": list(REVIEW_MERGE_STATES) if reasons else [],
|
||||||
|
"safe_next_action": (
|
||||||
|
TERMINAL_BLOCKED_HEADING if reasons else "proceed with PRECHECK"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def assess_state_advancement(
|
||||||
|
state_completion: dict[str, bool] | None,
|
||||||
|
*,
|
||||||
|
target_state: str,
|
||||||
|
infra_stop: bool = False,
|
||||||
|
capability_blocked: bool = False,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Fail closed when *target_state* is requested before upstream gates pass."""
|
||||||
|
completion = dict(state_completion or {})
|
||||||
|
target = _clean(target_state).upper()
|
||||||
|
blockers = assess_workflow_blockers(
|
||||||
|
infra_stop=infra_stop,
|
||||||
|
capability_blocked=capability_blocked,
|
||||||
|
)
|
||||||
|
reasons = list(blockers["reasons"])
|
||||||
|
|
||||||
|
try:
|
||||||
|
target_idx = state_index(target)
|
||||||
|
except ValueError as exc:
|
||||||
|
return {
|
||||||
|
"target_state": target,
|
||||||
|
"allowed": False,
|
||||||
|
"block": True,
|
||||||
|
"reasons": [str(exc)],
|
||||||
|
"next_allowed_state": None,
|
||||||
|
"safe_next_action": TERMINAL_BLOCKED_HEADING,
|
||||||
|
}
|
||||||
|
|
||||||
|
if blockers["block"]:
|
||||||
|
return {
|
||||||
|
"target_state": target,
|
||||||
|
"allowed": False,
|
||||||
|
"block": True,
|
||||||
|
"reasons": reasons,
|
||||||
|
"next_allowed_state": None,
|
||||||
|
"safe_next_action": blockers["safe_next_action"],
|
||||||
|
}
|
||||||
|
|
||||||
|
next_allowed = _first_incomplete_state(completion)
|
||||||
|
if next_allowed is None:
|
||||||
|
allowed = target_idx == len(REVIEW_MERGE_STATES) - 1
|
||||||
|
if not allowed:
|
||||||
|
reasons.append("workflow already completed through POST_MERGE_REPORT")
|
||||||
|
else:
|
||||||
|
allowed = state_index(next_allowed) >= target_idx
|
||||||
|
if not allowed:
|
||||||
|
reasons.append(
|
||||||
|
f"state '{target}' is forbidden until '{next_allowed}' completes"
|
||||||
|
)
|
||||||
|
|
||||||
|
return {
|
||||||
|
"target_state": target,
|
||||||
|
"allowed": allowed and not reasons,
|
||||||
|
"block": bool(reasons),
|
||||||
|
"reasons": reasons,
|
||||||
|
"next_allowed_state": next_allowed,
|
||||||
|
"completed_states": [s for s in REVIEW_MERGE_STATES if completion.get(s)],
|
||||||
|
"safe_next_action": (
|
||||||
|
f"complete state '{next_allowed}' before advancing"
|
||||||
|
if next_allowed and not allowed
|
||||||
|
else (
|
||||||
|
TERMINAL_BLOCKED_HEADING
|
||||||
|
if reasons
|
||||||
|
else f"advance to {target}"
|
||||||
|
)
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def can_approve(state_completion: dict[str, bool] | None, **blocker_kwargs) -> dict[str, Any]:
|
||||||
|
"""Approval requires all states through REVIEW_DECISION (#290 AC)."""
|
||||||
|
required = REVIEW_MERGE_STATES[: REVIEW_MERGE_STATES.index("REVIEW_DECISION") + 1]
|
||||||
|
completion = dict(state_completion or {})
|
||||||
|
advance = assess_state_advancement(
|
||||||
|
completion,
|
||||||
|
target_state="APPROVE_OR_REQUEST_CHANGES",
|
||||||
|
**blocker_kwargs,
|
||||||
|
)
|
||||||
|
missing = [s for s in required if not completion.get(s)]
|
||||||
|
reasons = list(advance["reasons"])
|
||||||
|
if missing:
|
||||||
|
reasons.append(
|
||||||
|
"approval blocked: incomplete states: " + ", ".join(missing)
|
||||||
|
)
|
||||||
|
block = bool(reasons) or advance["block"]
|
||||||
|
return {
|
||||||
|
"allowed": not block,
|
||||||
|
"block": block,
|
||||||
|
"reasons": reasons,
|
||||||
|
"missing_states": missing,
|
||||||
|
"safe_next_action": advance["safe_next_action"],
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def can_merge(
|
||||||
|
state_completion: dict[str, bool] | None,
|
||||||
|
*,
|
||||||
|
pre_merge_gates: dict[str, bool] | None = None,
|
||||||
|
**blocker_kwargs,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Merge requires approve path plus fresh PRE_MERGE_RECHECK gates (#290 AC6)."""
|
||||||
|
completion = dict(state_completion or {})
|
||||||
|
approve = can_approve(completion, **blocker_kwargs)
|
||||||
|
reasons = list(approve["reasons"])
|
||||||
|
|
||||||
|
if not completion.get("APPROVE_OR_REQUEST_CHANGES"):
|
||||||
|
reasons.append("merge blocked: APPROVE_OR_REQUEST_CHANGES not completed")
|
||||||
|
|
||||||
|
gate_map = dict(pre_merge_gates or {})
|
||||||
|
missing_gates = [g for g in _PRE_MERGE_REQUIRED_GATES if not gate_map.get(g)]
|
||||||
|
if missing_gates:
|
||||||
|
reasons.append(
|
||||||
|
"merge blocked: pre-merge gates incomplete: " + ", ".join(missing_gates)
|
||||||
|
)
|
||||||
|
|
||||||
|
advance = assess_state_advancement(
|
||||||
|
completion,
|
||||||
|
target_state="MERGE",
|
||||||
|
**blocker_kwargs,
|
||||||
|
)
|
||||||
|
reasons.extend(advance["reasons"])
|
||||||
|
|
||||||
|
block = bool(reasons)
|
||||||
|
return {
|
||||||
|
"allowed": not block,
|
||||||
|
"block": block,
|
||||||
|
"reasons": reasons,
|
||||||
|
"missing_pre_merge_gates": missing_gates,
|
||||||
|
"safe_next_action": (
|
||||||
|
"complete PRE_MERGE_RECHECK with fresh whoami/capability/PR re-fetch "
|
||||||
|
"before merge"
|
||||||
|
if block
|
||||||
|
else "merge allowed"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def assess_blocked_recovery_handoff(report_text: str) -> dict[str, Any]:
|
||||||
|
"""Blocked handoffs must not replay approve/merge commands (#290 AC4)."""
|
||||||
|
text = report_text or ""
|
||||||
|
reasons: list[str] = []
|
||||||
|
if _FORBIDDEN_RECOVERY_REPLAY_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"blocked recovery handoff contains direct approve/merge replay command"
|
||||||
|
)
|
||||||
|
if reasons and not _RESTART_WORKFLOW_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"blocked recovery handoff must direct operator to restart full workflow"
|
||||||
|
)
|
||||||
|
return {
|
||||||
|
"block": bool(reasons),
|
||||||
|
"allowed": not reasons,
|
||||||
|
"reasons": reasons,
|
||||||
|
"safe_next_action": (
|
||||||
|
"remove approve/merge replay commands and say to restart full workflow "
|
||||||
|
"after blockers clear"
|
||||||
|
if reasons
|
||||||
|
else "recovery handoff wording acceptable"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def assess_final_report_state_claims(
|
||||||
|
report_text: str,
|
||||||
|
*,
|
||||||
|
state_completion: dict[str, bool] | None = None,
|
||||||
|
approve_completed: bool = False,
|
||||||
|
merge_completed: bool = False,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Reports must not claim reviewed/ready-to-merge without gate proof (#290 AC10)."""
|
||||||
|
text = report_text or ""
|
||||||
|
completion = dict(state_completion or {})
|
||||||
|
reasons: list[str] = []
|
||||||
|
|
||||||
|
if _READY_TO_MERGE_RE.search(text) and not (
|
||||||
|
merge_completed or completion.get("PRE_MERGE_RECHECK")
|
||||||
|
):
|
||||||
|
reasons.append(
|
||||||
|
"report claims ready-to-merge without PRE_MERGE_RECHECK completion"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _REVIEWED_VALIDATED_RE.search(text):
|
||||||
|
if not (approve_completed or completion.get("VALIDATE")):
|
||||||
|
reasons.append(
|
||||||
|
"report claims reviewed/validated without VALIDATE/APPROVE proof"
|
||||||
|
)
|
||||||
|
|
||||||
|
return {
|
||||||
|
"block": bool(reasons),
|
||||||
|
"allowed": not reasons,
|
||||||
|
"reasons": reasons,
|
||||||
|
"safe_next_action": (
|
||||||
|
"remove stale reviewed/ready-to-merge claims unless gates passed"
|
||||||
|
if reasons
|
||||||
|
else "final report state claims consistent with workflow"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def workflow_status(
|
||||||
|
state_completion: dict[str, bool] | None,
|
||||||
|
**blocker_kwargs,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Summarize current state-machine position for MCP/runtime reporting."""
|
||||||
|
completion = dict(state_completion or {})
|
||||||
|
blockers = assess_workflow_blockers(**blocker_kwargs)
|
||||||
|
next_state = _first_incomplete_state(completion)
|
||||||
|
return {
|
||||||
|
"states": list(REVIEW_MERGE_STATES),
|
||||||
|
"completed_states": [s for s in REVIEW_MERGE_STATES if completion.get(s)],
|
||||||
|
"next_required_state": next_state,
|
||||||
|
"workflow_complete": next_state is None,
|
||||||
|
"approve_allowed": can_approve(completion, **blocker_kwargs)["allowed"],
|
||||||
|
"merge_allowed": can_merge(completion, **blocker_kwargs)["allowed"],
|
||||||
|
"blockers": blockers,
|
||||||
|
}
|
||||||
@@ -14,6 +14,7 @@ is usable from prompts, harness assertions, and tests. The MCP-level gates
|
|||||||
here weakens or replaces them.
|
here weakens or replaces them.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import hashlib
|
||||||
import re
|
import re
|
||||||
|
|
||||||
import issue_duplicate_gate
|
import issue_duplicate_gate
|
||||||
@@ -1242,6 +1243,121 @@ def assess_workspace_mutation_consistency(report_text, observed_commands=None):
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
# ── Reconciliation controller-handoff schema (Issue #303) ────────────────────
|
||||||
|
#
|
||||||
|
# Already-landed PR reconciliation is neither author issue work nor
|
||||||
|
# reviewer merge work; its handoff needs its own schema. Stale
|
||||||
|
# author/reviewer fields fail validation, and observed git ref
|
||||||
|
# mutations (fetch) must be reported under the precise category.
|
||||||
|
|
||||||
|
RECONCILIATION_ELIGIBILITY_CLASS = "ALREADY_LANDED_RECONCILE_REQUIRED"
|
||||||
|
|
||||||
|
RECONCILIATION_HANDOFF_FIELDS = (
|
||||||
|
("Task", ("task",)),
|
||||||
|
("Repo", ("repo",)),
|
||||||
|
("Role/profile", ("role/profile", "role", "profile")),
|
||||||
|
("Identity", ("identity",)),
|
||||||
|
("Selected PR", ("selected pr",)),
|
||||||
|
("PR live state", ("pr live state",)),
|
||||||
|
("Candidate head SHA", ("candidate head sha",)),
|
||||||
|
("Target branch", ("target branch",)),
|
||||||
|
("Target branch SHA", ("target branch sha",)),
|
||||||
|
("Ancestor proof", ("ancestor proof",)),
|
||||||
|
("Linked issue", ("linked issue",)),
|
||||||
|
("Linked issue live status", ("linked issue live status",)),
|
||||||
|
("Eligibility class", ("eligibility class",)),
|
||||||
|
("Capabilities proven", ("capabilities proven",)),
|
||||||
|
("Missing capabilities", ("missing capabilities",)),
|
||||||
|
("PR comments posted", ("pr comments posted",)),
|
||||||
|
("Issue comments posted", ("issue comments posted",)),
|
||||||
|
("PRs closed", ("prs closed",)),
|
||||||
|
("Issues closed", ("issues closed",)),
|
||||||
|
("Git ref mutations", ("git ref mutations",)),
|
||||||
|
("Worktree mutations", ("worktree mutations",)),
|
||||||
|
("MCP/Gitea mutations", ("mcp/gitea mutations",)),
|
||||||
|
("External-state mutations", ("external-state mutations",)),
|
||||||
|
("Read-only diagnostics", ("read-only diagnostics",)),
|
||||||
|
("Blocker", ("blocker",)),
|
||||||
|
("Safe next action", ("safe next action",)),
|
||||||
|
("No review/merge confirmation", ("no review/merge",)),
|
||||||
|
)
|
||||||
|
|
||||||
|
RECONCILIATION_FORBIDDEN_FIELDS = (
|
||||||
|
"pr number opened",
|
||||||
|
"pinned reviewed head",
|
||||||
|
"scratch worktree used",
|
||||||
|
"workspace mutations",
|
||||||
|
"issue lock proof",
|
||||||
|
"claim/comment status",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def assess_reconciliation_handoff(report_text, observed_commands=None):
|
||||||
|
"""#303: validate the dedicated reconciliation handoff schema.
|
||||||
|
|
||||||
|
Requires a Controller Handoff section carrying every reconciliation
|
||||||
|
field, ``Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED``, and
|
||||||
|
no stale author/reviewer fields. *observed_commands* is the git
|
||||||
|
command log; an observed fetch/pull must be reported under ``Git ref
|
||||||
|
mutations`` (never claimed none).
|
||||||
|
|
||||||
|
Returns {'complete', 'downgraded', 'reasons'}; fails closed.
|
||||||
|
"""
|
||||||
|
section = _handoff_section_lines(report_text)
|
||||||
|
if section is None:
|
||||||
|
return {
|
||||||
|
"complete": False,
|
||||||
|
"downgraded": True,
|
||||||
|
"reasons": [
|
||||||
|
"reconciliation report has no section titled exactly "
|
||||||
|
f"'{HANDOFF_HEADING}'"
|
||||||
|
],
|
||||||
|
}
|
||||||
|
|
||||||
|
fields = {}
|
||||||
|
for line in section:
|
||||||
|
stripped = line.strip().lstrip("-*").strip()
|
||||||
|
if ":" in stripped:
|
||||||
|
k, v = stripped.split(":", 1)
|
||||||
|
fields[k.strip().lower()] = v.strip()
|
||||||
|
|
||||||
|
reasons = []
|
||||||
|
|
||||||
|
for name, aliases in RECONCILIATION_HANDOFF_FIELDS:
|
||||||
|
if not any(label.startswith(alias)
|
||||||
|
for label in fields for alias in aliases):
|
||||||
|
reasons.append(
|
||||||
|
f"reconciliation handoff missing required field: {name}"
|
||||||
|
)
|
||||||
|
|
||||||
|
for stale in RECONCILIATION_FORBIDDEN_FIELDS:
|
||||||
|
if any(label.startswith(stale) for label in fields):
|
||||||
|
reasons.append(
|
||||||
|
f"reconciliation handoff must not include stale field "
|
||||||
|
f"'{stale}'"
|
||||||
|
)
|
||||||
|
|
||||||
|
eligibility = fields.get("eligibility class", "")
|
||||||
|
if eligibility and RECONCILIATION_ELIGIBILITY_CLASS not in eligibility.upper():
|
||||||
|
reasons.append(
|
||||||
|
"reconciliation handoff eligibility class must be "
|
||||||
|
f"'{RECONCILIATION_ELIGIBILITY_CLASS}' (got '{eligibility}')"
|
||||||
|
)
|
||||||
|
|
||||||
|
_, ref_cmds = _classify_git_commands(observed_commands)
|
||||||
|
if ref_cmds and _field_claims_none(fields, "git ref mutations"):
|
||||||
|
reasons.append(
|
||||||
|
"observed git ref mutation not reported under 'Git ref "
|
||||||
|
f"mutations': {ref_cmds[0]}"
|
||||||
|
)
|
||||||
|
|
||||||
|
return {
|
||||||
|
"complete": not reasons,
|
||||||
|
"downgraded": bool(reasons),
|
||||||
|
"reasons": reasons,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
# ── Already-landed report wording (Issue #298) ───────────────────────────────
|
# ── Already-landed report wording (Issue #298) ───────────────────────────────
|
||||||
#
|
#
|
||||||
# An already-landed PR is reconciliation-only: it must never be called
|
# An already-landed PR is reconciliation-only: it must never be called
|
||||||
@@ -1584,11 +1700,26 @@ def build_final_report(checkout_proof, inventory, validation, contamination,
|
|||||||
if report_text and _QUEUE_STATUS_REPORT_HINT.search(report_text)
|
if report_text and _QUEUE_STATUS_REPORT_HINT.search(report_text)
|
||||||
else {"proven": True, "block": False, "reasons": [], "violations": []}
|
else {"proven": True, "block": False, "reasons": [], "violations": []}
|
||||||
)
|
)
|
||||||
|
pr_queue_cleanup_report = (
|
||||||
|
assess_pr_queue_cleanup_report(report_text)
|
||||||
|
if report_text and _PR_QUEUE_CLEANUP_REPORT_HINT.search(report_text)
|
||||||
|
else {"proven": True, "block": False, "reasons": [], "violations": []}
|
||||||
|
)
|
||||||
proof_wording = (
|
proof_wording = (
|
||||||
assess_proof_wording(report_text)
|
assess_proof_wording(report_text)
|
||||||
if report_text
|
if report_text
|
||||||
else {"proven": True, "block": False, "reasons": [], "violations": []}
|
else {"proven": True, "block": False, "reasons": [], "violations": []}
|
||||||
)
|
)
|
||||||
|
already_landed_handoff = (
|
||||||
|
assess_already_landed_handoff_report(report_text)
|
||||||
|
if report_text
|
||||||
|
else {"proven": True, "block": False, "reasons": []}
|
||||||
|
)
|
||||||
|
inventory_worktree = (
|
||||||
|
assess_inventory_worktree_report(report_text)
|
||||||
|
if report_text
|
||||||
|
else {"proven": True, "block": False, "reasons": []}
|
||||||
|
)
|
||||||
if baseline_validation is None and report_text:
|
if baseline_validation is None and report_text:
|
||||||
baseline_validation = assess_reviewer_baseline_validation_proof(
|
baseline_validation = assess_reviewer_baseline_validation_proof(
|
||||||
report_text=report_text,
|
report_text=report_text,
|
||||||
@@ -1761,6 +1892,21 @@ def build_final_report(checkout_proof, inventory, validation, contamination,
|
|||||||
"queue-status report violates loaded workflow proof gates (#339)"
|
"queue-status report violates loaded workflow proof gates (#339)"
|
||||||
)
|
)
|
||||||
downgrade_reasons.extend(queue_status_report.get("reasons", []))
|
downgrade_reasons.extend(queue_status_report.get("reasons", []))
|
||||||
|
if not pr_queue_cleanup_report.get("proven"):
|
||||||
|
downgrade_reasons.append(
|
||||||
|
"PR-only cleanup report violates cleanup-mode proof gates (#390)"
|
||||||
|
)
|
||||||
|
downgrade_reasons.extend(pr_queue_cleanup_report.get("reasons", []))
|
||||||
|
if not already_landed_handoff.get("proven"):
|
||||||
|
downgrade_reasons.append(
|
||||||
|
"already-landed controller handoff has stale or inconsistent fields (#299)"
|
||||||
|
)
|
||||||
|
downgrade_reasons.extend(already_landed_handoff.get("reasons", []))
|
||||||
|
if not inventory_worktree.get("proven"):
|
||||||
|
downgrade_reasons.append(
|
||||||
|
"inventory pagination or worktree fields inconsistent (#293)"
|
||||||
|
)
|
||||||
|
downgrade_reasons.extend(inventory_worktree.get("reasons", []))
|
||||||
if not baseline_validation.get("proven"):
|
if not baseline_validation.get("proven"):
|
||||||
downgrade_reasons.append(
|
downgrade_reasons.append(
|
||||||
"reviewer baseline validation proof missing or failed (#325)"
|
"reviewer baseline validation proof missing or failed (#325)"
|
||||||
@@ -1859,6 +2005,20 @@ def build_final_report(checkout_proof, inventory, validation, contamination,
|
|||||||
"queue_status_violations": list(
|
"queue_status_violations": list(
|
||||||
queue_status_report.get("violations") or []
|
queue_status_report.get("violations") or []
|
||||||
),
|
),
|
||||||
|
"pr_queue_cleanup_report_proven": bool(
|
||||||
|
pr_queue_cleanup_report.get("proven")
|
||||||
|
),
|
||||||
|
"pr_queue_cleanup_violations": list(
|
||||||
|
pr_queue_cleanup_report.get("reasons") or []
|
||||||
|
),
|
||||||
|
"already_landed_handoff_proven": bool(already_landed_handoff.get("proven")),
|
||||||
|
"already_landed_handoff_violations": list(
|
||||||
|
already_landed_handoff.get("reasons") or []
|
||||||
|
),
|
||||||
|
"inventory_worktree_proven": bool(inventory_worktree.get("proven")),
|
||||||
|
"inventory_worktree_violations": list(
|
||||||
|
inventory_worktree.get("reasons") or []
|
||||||
|
),
|
||||||
"baseline_validation_proven": bool(baseline_validation.get("proven")),
|
"baseline_validation_proven": bool(baseline_validation.get("proven")),
|
||||||
"baseline_validation_violations": list(
|
"baseline_validation_violations": list(
|
||||||
baseline_validation.get("violations") or []
|
baseline_validation.get("violations") or []
|
||||||
@@ -3245,6 +3405,67 @@ def assess_issue_filing_duplicate_summary(
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
# ── Canonical review-workflow citation and version proof (Issue #296) ────────
|
||||||
|
#
|
||||||
|
# The canonical PR review/merge workflow lives in
|
||||||
|
# skills/llm-project-workflow/workflows/review-merge-pr.md and is exposed
|
||||||
|
# via mcp_get_skill_guide. Review reports must prove they loaded it and
|
||||||
|
# cite the version hash used, so stale or shortened prompt copies are
|
||||||
|
# rejected.
|
||||||
|
|
||||||
|
REVIEW_WORKFLOW_MARKERS = (
|
||||||
|
"workflows/review-merge-pr.md",
|
||||||
|
"review-merge-pr.md",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def compute_workflow_hash(workflow_text):
|
||||||
|
"""Return the short (12-hex) sha256 version hash of a workflow text."""
|
||||||
|
digest = hashlib.sha256((workflow_text or "").encode("utf-8"))
|
||||||
|
return digest.hexdigest()[:12]
|
||||||
|
|
||||||
|
|
||||||
|
def assess_review_workflow_source(report_text, *, canonical_hash=None):
|
||||||
|
"""#296: review reports must cite the canonical workflow and version.
|
||||||
|
|
||||||
|
The report must reference the canonical workflow file and carry a
|
||||||
|
populated ``Workflow version`` (or ``Workflow hash``) field. When
|
||||||
|
*canonical_hash* — ``compute_workflow_hash`` of the current
|
||||||
|
workflows/review-merge-pr.md content — is supplied, the cited hash
|
||||||
|
must match it, rejecting stale copies.
|
||||||
|
|
||||||
|
Returns {'complete', 'downgraded', 'reasons'}; fails closed.
|
||||||
|
"""
|
||||||
|
lower = (report_text or "").lower()
|
||||||
|
reasons = []
|
||||||
|
|
||||||
|
if not any(marker in lower for marker in REVIEW_WORKFLOW_MARKERS):
|
||||||
|
reasons.append(
|
||||||
|
"review report missing canonical workflow source citation "
|
||||||
|
"(workflows/review-merge-pr.md)"
|
||||||
|
)
|
||||||
|
|
||||||
|
fields = _report_labeled_fields(report_text)
|
||||||
|
version = fields.get("workflow version") or fields.get("workflow hash")
|
||||||
|
if not version or version.strip().lower().startswith(("none", "unknown")):
|
||||||
|
reasons.append(
|
||||||
|
"review report missing populated 'Workflow version' field "
|
||||||
|
"(version/hash of the canonical workflow used)"
|
||||||
|
)
|
||||||
|
elif canonical_hash and canonical_hash.lower() not in version.lower():
|
||||||
|
reasons.append(
|
||||||
|
f"review report cites stale workflow version '{version}'; "
|
||||||
|
f"current canonical hash is '{canonical_hash}' — reload the "
|
||||||
|
"workflow via mcp_get_skill_guide"
|
||||||
|
)
|
||||||
|
|
||||||
|
return {
|
||||||
|
"complete": not reasons,
|
||||||
|
"downgraded": bool(reasons),
|
||||||
|
"reasons": reasons,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def assess_create_issue_workflow_source(report_text: str) -> dict:
|
def assess_create_issue_workflow_source(report_text: str) -> dict:
|
||||||
"""#337: create-issue reports must cite the canonical workflow file."""
|
"""#337: create-issue reports must cite the canonical workflow file."""
|
||||||
lower = (report_text or "").lower()
|
lower = (report_text or "").lower()
|
||||||
@@ -3987,6 +4208,168 @@ def assess_reviewer_baseline_validation_proof(
|
|||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
# Partial reconciliation policy (#302)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
# Durable policy choice for already-landed PR reconciliation when
|
||||||
|
# ``gitea.pr.close`` is unavailable: Option B, comment-then-stop.
|
||||||
|
PARTIAL_RECONCILIATION_POLICY = "comment_then_stop"
|
||||||
|
|
||||||
|
PARTIAL_RECONCILIATION_COMMENT_FIELDS = (
|
||||||
|
"PR head SHA",
|
||||||
|
"target branch SHA",
|
||||||
|
"ancestor proof",
|
||||||
|
"linked issue status",
|
||||||
|
"required missing capability",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def resolve_partial_reconciliation_plan(
|
||||||
|
*,
|
||||||
|
ancestor_proven: bool,
|
||||||
|
capabilities: dict | None,
|
||||||
|
) -> dict:
|
||||||
|
"""#302: decide reconciliation mutations from proven capabilities.
|
||||||
|
|
||||||
|
Implements the comment-then-stop policy (Option B): with ancestor
|
||||||
|
proof but no ``close_pr`` capability, a reconciliation comment with
|
||||||
|
the full proof is posted when ``comment_pr`` is proven, then the
|
||||||
|
workflow stops for an authorized close. Missing comment capability
|
||||||
|
degrades to a recovery handoff with no Gitea mutation. Unproven
|
||||||
|
ancestry blocks every mutation regardless of capability.
|
||||||
|
|
||||||
|
*capabilities* maps ``close_pr``/``comment_pr``/``close_issue``/
|
||||||
|
``comment_issue`` to proven booleans; ``None`` or missing keys fail
|
||||||
|
closed.
|
||||||
|
"""
|
||||||
|
caps = capabilities or {}
|
||||||
|
if not ancestor_proven:
|
||||||
|
return {
|
||||||
|
"policy": PARTIAL_RECONCILIATION_POLICY,
|
||||||
|
"outcome": "GATE_NOT_PROVEN",
|
||||||
|
"allowed_mutations": (),
|
||||||
|
"required_comment_fields": (),
|
||||||
|
"report_requirements": (
|
||||||
|
"state that ancestry was not proven and no Gitea mutation "
|
||||||
|
"was performed",
|
||||||
|
),
|
||||||
|
"reasons": [
|
||||||
|
"ancestor proof missing; no reconciliation mutation may "
|
||||||
|
"run (#302)"
|
||||||
|
],
|
||||||
|
"safe_next_action": (
|
||||||
|
"run the already-landed ancestry check before any "
|
||||||
|
"reconciliation mutation"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
if caps.get("close_pr") is True:
|
||||||
|
return {
|
||||||
|
"policy": PARTIAL_RECONCILIATION_POLICY,
|
||||||
|
"outcome": "FULL_RECONCILE_CLOSE_ALLOWED",
|
||||||
|
"allowed_mutations": ("close_pr", "comment_pr"),
|
||||||
|
"required_comment_fields": (),
|
||||||
|
"report_requirements": (
|
||||||
|
"report the PR close result",
|
||||||
|
),
|
||||||
|
"reasons": [],
|
||||||
|
"safe_next_action": (
|
||||||
|
"close the already-landed PR with the proven close_pr "
|
||||||
|
"capability and report the close result"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
if caps.get("comment_pr") is True:
|
||||||
|
return {
|
||||||
|
"policy": PARTIAL_RECONCILIATION_POLICY,
|
||||||
|
"outcome": "PARTIAL_RECONCILE_COMMENT_THEN_STOP",
|
||||||
|
"allowed_mutations": ("comment_pr",),
|
||||||
|
"required_comment_fields": PARTIAL_RECONCILIATION_COMMENT_FIELDS,
|
||||||
|
"report_requirements": (
|
||||||
|
"report that the reconciliation comment was posted",
|
||||||
|
"report the missing close capability that prevented the "
|
||||||
|
"PR close",
|
||||||
|
),
|
||||||
|
"reasons": [
|
||||||
|
"close_pr capability missing; policy is comment-then-stop "
|
||||||
|
"(#302)"
|
||||||
|
],
|
||||||
|
"safe_next_action": (
|
||||||
|
"post one reconciliation comment carrying the ancestor "
|
||||||
|
"proof, then stop for a human or authorized close"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
"policy": PARTIAL_RECONCILIATION_POLICY,
|
||||||
|
"outcome": "RECOVERY_HANDOFF_ONLY",
|
||||||
|
"allowed_mutations": (),
|
||||||
|
"required_comment_fields": (),
|
||||||
|
"report_requirements": (
|
||||||
|
"report that no reconciliation comment was posted and name "
|
||||||
|
"the missing capability",
|
||||||
|
"report that no Gitea mutation was performed",
|
||||||
|
),
|
||||||
|
"reasons": [
|
||||||
|
"close_pr and comment_pr capabilities missing; recovery "
|
||||||
|
"handoff only (#302)"
|
||||||
|
],
|
||||||
|
"safe_next_action": (
|
||||||
|
"produce a recovery handoff recording the intended comment "
|
||||||
|
"and required capabilities; perform no Gitea mutation"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def assess_partial_reconciliation_report(
|
||||||
|
report_text: str | None,
|
||||||
|
*,
|
||||||
|
plan: dict,
|
||||||
|
) -> dict:
|
||||||
|
"""#302: final reports must explain why comments were or were not posted."""
|
||||||
|
text = (report_text or "").lower()
|
||||||
|
outcome = (plan or {}).get("outcome", "")
|
||||||
|
reasons: list[str] = []
|
||||||
|
|
||||||
|
if outcome == "FULL_RECONCILE_CLOSE_ALLOWED":
|
||||||
|
if "close" not in text or "result" not in text:
|
||||||
|
reasons.append(
|
||||||
|
"full reconciliation report must state the PR close "
|
||||||
|
"result (#302)"
|
||||||
|
)
|
||||||
|
elif outcome == "PARTIAL_RECONCILE_COMMENT_THEN_STOP":
|
||||||
|
if "comment" not in text:
|
||||||
|
reasons.append(
|
||||||
|
"partial reconciliation report must state that the "
|
||||||
|
"reconciliation comment was posted (#302)"
|
||||||
|
)
|
||||||
|
if "capability" not in text and "close_pr" not in text:
|
||||||
|
reasons.append(
|
||||||
|
"partial reconciliation report must name the missing "
|
||||||
|
"close capability (#302)"
|
||||||
|
)
|
||||||
|
elif outcome == "RECOVERY_HANDOFF_ONLY":
|
||||||
|
if "comment" not in text or "capability" not in text:
|
||||||
|
reasons.append(
|
||||||
|
"recovery handoff report must explain that no comment was "
|
||||||
|
"posted and name the missing capability (#302)"
|
||||||
|
)
|
||||||
|
if "no gitea mutation" not in text and "no mutation" not in text:
|
||||||
|
reasons.append(
|
||||||
|
"recovery handoff report must state that no Gitea "
|
||||||
|
"mutation was performed (#302)"
|
||||||
|
)
|
||||||
|
|
||||||
|
return {
|
||||||
|
"complete": not reasons,
|
||||||
|
"block": bool(reasons),
|
||||||
|
"reasons": reasons,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
# Already-landed review gate (#292)
|
# Already-landed review gate (#292)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
@@ -4152,6 +4535,9 @@ def assess_already_landed_report_state(
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Full-suite failure approval gate (#323)
|
# Full-suite failure approval gate (#323)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -4259,6 +4645,169 @@ def assess_full_suite_failure_approval_gate(
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Reconciler close gate (#309)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
RECONCILER_CLOSE_REPORT_FIELDS = (
|
||||||
|
"identity/profile",
|
||||||
|
"close capability proof",
|
||||||
|
"PR live state",
|
||||||
|
"candidate head SHA",
|
||||||
|
"target branch SHA",
|
||||||
|
"ancestor proof",
|
||||||
|
"linked issue status",
|
||||||
|
"PR close result",
|
||||||
|
"issue close result",
|
||||||
|
"no review/merge confirmation",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def assess_reconciler_close_gate(
|
||||||
|
*,
|
||||||
|
pr_number: int | None,
|
||||||
|
pr_state: str | None,
|
||||||
|
candidate_head_sha: str | None,
|
||||||
|
live_head_sha: str | None,
|
||||||
|
target_branch: str | None,
|
||||||
|
target_branch_sha: str | None,
|
||||||
|
head_is_ancestor_of_target: bool | None,
|
||||||
|
close_pr_capability: bool,
|
||||||
|
close_issue_capability: bool = False,
|
||||||
|
linked_issue_state: str | None = None,
|
||||||
|
issue_resolved_by_landing: bool = False,
|
||||||
|
) -> dict:
|
||||||
|
"""#309: gate the dedicated reconciler close path for landed PRs.
|
||||||
|
|
||||||
|
The reconciler path may close a PR only with exact ``gitea.pr.close``
|
||||||
|
capability and full already-landed proof: live open PR, pinned head,
|
||||||
|
freshly fetched target branch SHA, and ancestry of the head in the
|
||||||
|
target. Non-landed PRs are never closable here, and the gate never
|
||||||
|
grants review, approval, request-changes, or merge.
|
||||||
|
|
||||||
|
Linked-issue closure: skipped when the issue is already closed;
|
||||||
|
allowed only when the issue is open, resolved by the landed commits,
|
||||||
|
and exact ``gitea.issue.close`` capability is proven.
|
||||||
|
"""
|
||||||
|
reasons: list[str] = []
|
||||||
|
|
||||||
|
if not isinstance(pr_number, int) or pr_number <= 0:
|
||||||
|
reasons.append("PR number missing or invalid (#309)")
|
||||||
|
if (pr_state or "").strip().lower() != "open":
|
||||||
|
reasons.append(
|
||||||
|
"PR is not live-verified open at mutation time; re-fetch the "
|
||||||
|
"PR before any reconciler close (#309)"
|
||||||
|
)
|
||||||
|
if not _FULL_SHA.match((candidate_head_sha or "").strip()):
|
||||||
|
reasons.append(
|
||||||
|
"candidate head SHA is not a full 40-hex commit SHA (#309)"
|
||||||
|
)
|
||||||
|
if live_head_sha is not None and candidate_head_sha and (
|
||||||
|
live_head_sha.strip() != candidate_head_sha.strip()
|
||||||
|
):
|
||||||
|
reasons.append(
|
||||||
|
"PR head changed since the ancestry proof was pinned; re-run "
|
||||||
|
"the already-landed check (#309)"
|
||||||
|
)
|
||||||
|
if not (target_branch or "").strip():
|
||||||
|
reasons.append("target branch missing (#309)")
|
||||||
|
if not _FULL_SHA.match((target_branch_sha or "").strip()):
|
||||||
|
reasons.append(
|
||||||
|
"target branch SHA missing or not a full 40-hex SHA; fetch the "
|
||||||
|
"target branch freshly before the ancestry check (#309)"
|
||||||
|
)
|
||||||
|
if head_is_ancestor_of_target is None:
|
||||||
|
reasons.append(
|
||||||
|
"ancestry of the PR head against the target branch was not "
|
||||||
|
"checked (#309)"
|
||||||
|
)
|
||||||
|
|
||||||
|
never_allowed = {
|
||||||
|
"review_allowed": False,
|
||||||
|
"approve_allowed": False,
|
||||||
|
"request_changes_allowed": False,
|
||||||
|
"merge_allowed": False,
|
||||||
|
}
|
||||||
|
|
||||||
|
if reasons:
|
||||||
|
return {
|
||||||
|
"outcome": "GATE_NOT_PROVEN",
|
||||||
|
"pr_close_allowed": False,
|
||||||
|
"issue_close_allowed": False,
|
||||||
|
"reasons": reasons,
|
||||||
|
"required_report_fields": RECONCILER_CLOSE_REPORT_FIELDS,
|
||||||
|
"safe_next_action": (
|
||||||
|
"re-fetch the PR and target branch, pin SHAs, run the "
|
||||||
|
"ancestry check, then re-run this gate"
|
||||||
|
),
|
||||||
|
**never_allowed,
|
||||||
|
}
|
||||||
|
|
||||||
|
if head_is_ancestor_of_target is False:
|
||||||
|
return {
|
||||||
|
"outcome": "NOT_LANDED_CLOSE_BLOCKED",
|
||||||
|
"pr_close_allowed": False,
|
||||||
|
"issue_close_allowed": False,
|
||||||
|
"reasons": [
|
||||||
|
f"PR #{pr_number} head is not an ancestor of "
|
||||||
|
f"{target_branch}; non-landed PRs cannot be closed through "
|
||||||
|
"the reconciler path (#309)"
|
||||||
|
],
|
||||||
|
"required_report_fields": RECONCILER_CLOSE_REPORT_FIELDS,
|
||||||
|
"safe_next_action": (
|
||||||
|
"route the PR through the normal review workflow; the "
|
||||||
|
"reconciler path only closes already-landed PRs"
|
||||||
|
),
|
||||||
|
**never_allowed,
|
||||||
|
}
|
||||||
|
|
||||||
|
if close_pr_capability is not True:
|
||||||
|
return {
|
||||||
|
"outcome": "RECOVERY_HANDOFF_REQUIRED",
|
||||||
|
"pr_close_allowed": False,
|
||||||
|
"issue_close_allowed": False,
|
||||||
|
"reasons": [
|
||||||
|
"exact gitea.pr.close capability not proven; produce a "
|
||||||
|
"recovery handoff instead of closing (#309)"
|
||||||
|
],
|
||||||
|
"required_report_fields": RECONCILER_CLOSE_REPORT_FIELDS,
|
||||||
|
"safe_next_action": (
|
||||||
|
"produce a recovery handoff naming the missing "
|
||||||
|
"gitea.pr.close capability and the completed ancestor proof"
|
||||||
|
),
|
||||||
|
**never_allowed,
|
||||||
|
}
|
||||||
|
|
||||||
|
issue_close_allowed = (
|
||||||
|
(linked_issue_state or "").strip().lower() == "open"
|
||||||
|
and issue_resolved_by_landing is True
|
||||||
|
and close_issue_capability is True
|
||||||
|
)
|
||||||
|
issue_close_reasons = []
|
||||||
|
if (linked_issue_state or "").strip().lower() == "closed":
|
||||||
|
issue_close_reasons.append(
|
||||||
|
"linked issue already closed; no issue close attempted (#309)"
|
||||||
|
)
|
||||||
|
elif not issue_close_allowed:
|
||||||
|
issue_close_reasons.append(
|
||||||
|
"issue close requires an open linked issue resolved by the "
|
||||||
|
"landed commits and exact gitea.issue.close capability (#309)"
|
||||||
|
)
|
||||||
|
|
||||||
|
return {
|
||||||
|
"outcome": "CLOSE_ALLOWED",
|
||||||
|
"pr_close_allowed": True,
|
||||||
|
"issue_close_allowed": issue_close_allowed,
|
||||||
|
"reasons": issue_close_reasons,
|
||||||
|
"required_report_fields": RECONCILER_CLOSE_REPORT_FIELDS,
|
||||||
|
"safe_next_action": (
|
||||||
|
"close the already-landed PR, report the close result, and "
|
||||||
|
"handle the linked issue per the proven capability"
|
||||||
|
),
|
||||||
|
**never_allowed,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Identity disclosure (#305)
|
# Identity disclosure (#305)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -4360,6 +4909,19 @@ _QUEUE_STATUS_REPORT_HINT = re.compile(
|
|||||||
re.I,
|
re.I,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
_PR_QUEUE_CLEANUP_REPORT_HINT = re.compile(
|
||||||
|
r"workflows/pr-queue-cleanup\.md|task:\s*pr-queue-cleanup|"
|
||||||
|
r"pr[- ]only queue cleanup",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def assess_pr_queue_cleanup_report(report_text: str | None) -> dict:
|
||||||
|
"""#390: validate PR-only cleanup final reports (one PR per run)."""
|
||||||
|
from pr_queue_cleanup import assess_pr_queue_cleanup_report as _assess
|
||||||
|
|
||||||
|
return _assess(report_text or "")
|
||||||
|
|
||||||
_GATE_PASSED_VALUE = re.compile(r"\bpassed\b", re.I)
|
_GATE_PASSED_VALUE = re.compile(r"\bpassed\b", re.I)
|
||||||
|
|
||||||
_NOT_APPLICABLE_VALUE = re.compile(
|
_NOT_APPLICABLE_VALUE = re.compile(
|
||||||
@@ -4783,6 +5345,34 @@ def assess_validation_worktree_edit_report(report_text, **kwargs):
|
|||||||
return _assess(report_text, **kwargs)
|
return _assess(report_text, **kwargs)
|
||||||
|
|
||||||
|
|
||||||
|
def assess_already_landed_handoff_report(report_text, **kwargs):
|
||||||
|
"""#299: reject stale fields after the already-landed gate fires."""
|
||||||
|
from reviewer_already_landed_handoff import assess_already_landed_handoff_report as _assess
|
||||||
|
|
||||||
|
return _assess(report_text, **kwargs)
|
||||||
|
|
||||||
|
|
||||||
|
def assess_inventory_worktree_report(report_text, **kwargs):
|
||||||
|
"""#293: prove inventory pagination and consistent worktree reporting."""
|
||||||
|
from reviewer_inventory_worktree import assess_inventory_worktree_report as _assess
|
||||||
|
|
||||||
|
return _assess(report_text, **kwargs)
|
||||||
|
|
||||||
|
|
||||||
|
def assess_infra_stop_handoff_report(report_text, **kwargs):
|
||||||
|
"""#289: repair handoff purity when infra_stop blocks reviewer capability."""
|
||||||
|
from reviewer_infra_stop_handoff import assess_infra_stop_handoff_report as _assess
|
||||||
|
|
||||||
|
return _assess(report_text, **kwargs)
|
||||||
|
|
||||||
|
|
||||||
|
def assess_mutation_categories_report(report_text, **kwargs):
|
||||||
|
"""#319: require precise mutation categories in reviewer controller handoffs."""
|
||||||
|
from reviewer_mutation_categories import assess_mutation_categories_report as _assess
|
||||||
|
|
||||||
|
return _assess(report_text, **kwargs)
|
||||||
|
|
||||||
|
|
||||||
def assess_worktree_ownership_report(report_text, **kwargs):
|
def assess_worktree_ownership_report(report_text, **kwargs):
|
||||||
"""#312: prove session-owned or safe-reuse worktree before reset/validation."""
|
"""#312: prove session-owned or safe-reuse worktree before reset/validation."""
|
||||||
from reviewer_worktree_ownership import assess_worktree_ownership_report as _assess
|
from reviewer_worktree_ownership import assess_worktree_ownership_report as _assess
|
||||||
|
|||||||
@@ -0,0 +1,216 @@
|
|||||||
|
"""Already-landed controller handoff consistency verifier (#299)."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
from review_proofs import git_ref_mutating_commands
|
||||||
|
|
||||||
|
ALREADY_LANDED_STATE = "ALREADY_LANDED_RECONCILE_REQUIRED"
|
||||||
|
|
||||||
|
_HANDOFF_SECTION_RE = re.compile(r"^##\s*Controller Handoff\s*$", re.I | re.M)
|
||||||
|
|
||||||
|
_STALE_HANDOFF_FIELDS = (
|
||||||
|
"pinned reviewed head",
|
||||||
|
"scratch worktree used",
|
||||||
|
)
|
||||||
|
|
||||||
|
_LEGACY_MUTATIONS_NONE_RE = re.compile(
|
||||||
|
r"^\s*[-*]?\s*mutations\s*:\s*none\s*$",
|
||||||
|
re.I | re.M,
|
||||||
|
)
|
||||||
|
_LEGACY_WORKSPACE_NONE_RE = re.compile(
|
||||||
|
r"^\s*[-*]?\s*workspace\s+mutations\s*:\s*none\s*$",
|
||||||
|
re.I | re.M,
|
||||||
|
)
|
||||||
|
|
||||||
|
_FORBIDDEN_REVIEW_STATES_RE = re.compile(
|
||||||
|
r"review decision\s*:\s*approved?\b|"
|
||||||
|
r"merge result\s*:\s*merged\b|"
|
||||||
|
r"\bready[_ ]to[_ ]merge\b",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
_NARRATIVE_ELIGIBILITY_RE = re.compile(
|
||||||
|
r"eligibility class\s*:\s*([^\n]+)",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
_NARRATIVE_REVIEWED_SHA_RE = re.compile(
|
||||||
|
r"reviewed head sha\s*:\s*([^\n]+)",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
_NARRATIVE_WORKTREE_RE = re.compile(
|
||||||
|
r"review worktree used\s*:\s*([^\n]+)",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _handoff_field_map(report_text: str) -> dict[str, str]:
|
||||||
|
text = report_text or ""
|
||||||
|
match = _HANDOFF_SECTION_RE.search(text)
|
||||||
|
if not match:
|
||||||
|
return {}
|
||||||
|
fields: dict[str, str] = {}
|
||||||
|
for line in text[match.end() :].splitlines():
|
||||||
|
stripped = line.strip().lstrip("-*").strip()
|
||||||
|
if ":" not in stripped:
|
||||||
|
continue
|
||||||
|
key, value = stripped.split(":", 1)
|
||||||
|
fields[key.strip().lower()] = value.strip()
|
||||||
|
return fields
|
||||||
|
|
||||||
|
|
||||||
|
def _narrative_before_handoff(report_text: str) -> str:
|
||||||
|
text = report_text or ""
|
||||||
|
match = _HANDOFF_SECTION_RE.search(text)
|
||||||
|
if not match:
|
||||||
|
return text
|
||||||
|
return text[: match.start()]
|
||||||
|
|
||||||
|
|
||||||
|
def _is_truthy(value: str) -> bool:
|
||||||
|
lowered = (value or "").strip().lower()
|
||||||
|
return lowered not in {"", "none", "n/a", "not applicable", "false", "no", "—", "-"}
|
||||||
|
|
||||||
|
|
||||||
|
def _gate_active(text: str, fields: dict[str, str], session: dict) -> bool:
|
||||||
|
if session.get("gate_fired") or session.get("already_landed_gate_fired"):
|
||||||
|
return True
|
||||||
|
eligibility = (fields.get("eligibility class") or "").upper()
|
||||||
|
if ALREADY_LANDED_STATE in eligibility:
|
||||||
|
return True
|
||||||
|
return ALREADY_LANDED_STATE.lower() in text.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def assess_already_landed_handoff_report(
|
||||||
|
report_text: str,
|
||||||
|
*,
|
||||||
|
handoff_session: dict | None = None,
|
||||||
|
command_log: list | None = None,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Reject stale handoff fields and narrative/handoff drift after the gate (#299)."""
|
||||||
|
text = report_text or ""
|
||||||
|
session = dict(handoff_session or {})
|
||||||
|
fields = _handoff_field_map(text)
|
||||||
|
reasons: list[str] = []
|
||||||
|
|
||||||
|
if not _gate_active(text, fields, session):
|
||||||
|
return {
|
||||||
|
"proven": True,
|
||||||
|
"block": False,
|
||||||
|
"reasons": [],
|
||||||
|
"gate_active": False,
|
||||||
|
"safe_next_action": "proceed",
|
||||||
|
}
|
||||||
|
|
||||||
|
for stale_field in _STALE_HANDOFF_FIELDS:
|
||||||
|
if stale_field in fields:
|
||||||
|
reasons.append(
|
||||||
|
f"already-landed handoff must not include stale field "
|
||||||
|
f"'{stale_field.title()}' (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _LEGACY_WORKSPACE_NONE_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"already-landed handoff must not include legacy "
|
||||||
|
"'Workspace mutations: None' (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
ref_commands = git_ref_mutating_commands(command_log or session.get("command_log"))
|
||||||
|
mutations_observed = bool(
|
||||||
|
ref_commands
|
||||||
|
or session.get("mutations_observed")
|
||||||
|
or session.get("mcp_mutations")
|
||||||
|
or session.get("review_mutations")
|
||||||
|
)
|
||||||
|
if mutations_observed and _LEGACY_MUTATIONS_NONE_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"already-landed handoff must not claim 'Mutations: None' when "
|
||||||
|
"git ref, MCP, review, merge, or cleanup mutations occurred (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
git_ref_value = fields.get("git ref mutations", "").strip().lower()
|
||||||
|
if ref_commands and (not git_ref_value or git_ref_value == "none"):
|
||||||
|
reasons.append(
|
||||||
|
"git fetch/ref updates must be reported under Git ref mutations (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
reviewed_sha = fields.get("reviewed head sha", "")
|
||||||
|
if reviewed_sha and _is_truthy(reviewed_sha):
|
||||||
|
reasons.append(
|
||||||
|
"already-landed handoff must use 'Reviewed head SHA: none' (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
worktree_used = fields.get("review worktree used", "")
|
||||||
|
if worktree_used and _is_truthy(worktree_used):
|
||||||
|
reasons.append(
|
||||||
|
"already-landed handoff must set Review worktree used: false (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
candidate_sha = fields.get("candidate head sha", "")
|
||||||
|
if not candidate_sha or candidate_sha.lower() in {"none", "n/a", "unknown"}:
|
||||||
|
reasons.append(
|
||||||
|
"already-landed handoff must include Candidate head SHA (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _FORBIDDEN_REVIEW_STATES_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"already-landed handoff must not claim approved/merged/ready-to-merge (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
narrative = _narrative_before_handoff(text)
|
||||||
|
handoff_eligibility = (fields.get("eligibility class") or "").strip()
|
||||||
|
narrative_eligibility = (
|
||||||
|
_NARRATIVE_ELIGIBILITY_RE.search(narrative).group(1).strip()
|
||||||
|
if _NARRATIVE_ELIGIBILITY_RE.search(narrative)
|
||||||
|
else ""
|
||||||
|
)
|
||||||
|
if handoff_eligibility and narrative_eligibility:
|
||||||
|
if handoff_eligibility.upper() != narrative_eligibility.upper():
|
||||||
|
reasons.append(
|
||||||
|
"narrative Eligibility class disagrees with controller handoff (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
narrative_reviewed = (
|
||||||
|
_NARRATIVE_REVIEWED_SHA_RE.search(narrative).group(1).strip()
|
||||||
|
if _NARRATIVE_REVIEWED_SHA_RE.search(narrative)
|
||||||
|
else ""
|
||||||
|
)
|
||||||
|
if narrative_reviewed and reviewed_sha:
|
||||||
|
if narrative_reviewed.lower() != reviewed_sha.lower():
|
||||||
|
reasons.append(
|
||||||
|
"narrative Reviewed head SHA disagrees with controller handoff (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
narrative_worktree = (
|
||||||
|
_NARRATIVE_WORKTREE_RE.search(narrative).group(1).strip()
|
||||||
|
if _NARRATIVE_WORKTREE_RE.search(narrative)
|
||||||
|
else ""
|
||||||
|
)
|
||||||
|
if narrative_worktree and worktree_used:
|
||||||
|
if narrative_worktree.lower() != worktree_used.lower():
|
||||||
|
reasons.append(
|
||||||
|
"narrative Review worktree used disagrees with controller handoff (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
git_ref_narrative = "git ref mutations" in narrative.lower()
|
||||||
|
if git_ref_narrative and git_ref_value:
|
||||||
|
if "none" in git_ref_value and ref_commands:
|
||||||
|
reasons.append(
|
||||||
|
"narrative and handoff disagree on git ref mutation reporting (#299)"
|
||||||
|
)
|
||||||
|
|
||||||
|
proven = not reasons
|
||||||
|
return {
|
||||||
|
"proven": proven,
|
||||||
|
"block": not proven,
|
||||||
|
"reasons": list(dict.fromkeys(reasons)),
|
||||||
|
"gate_active": True,
|
||||||
|
"safe_next_action": (
|
||||||
|
"emit canonical ALREADY_LANDED_RECONCILE_REQUIRED handoff without "
|
||||||
|
"stale review/merge fields; align narrative and controller handoff"
|
||||||
|
if reasons
|
||||||
|
else "proceed"
|
||||||
|
),
|
||||||
|
}
|
||||||
@@ -0,0 +1,162 @@
|
|||||||
|
"""Infra-stop repair handoff verifier for blocked reviewer workflows (#289)."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
from capability_stop_terminal import (
|
||||||
|
TERMINAL_REPORT_HEADING,
|
||||||
|
assess_capability_stop_report,
|
||||||
|
)
|
||||||
|
|
||||||
|
_REPAIR_HANDOFF_RE = re.compile(
|
||||||
|
r"repair handoff|control-checkout repair mode",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_CONTROL_REPAIR_RE = re.compile(r"control-checkout repair mode", re.IGNORECASE)
|
||||||
|
_PR_QUEUE_ADVANCE_RE = re.compile(
|
||||||
|
r"(?:selected pr|pr #\d+ (?:to review|selected)|eligible pr|"
|
||||||
|
r"next (?:eligible )?pr(?: to review)?|oldest eligible pr|pinned (?:review )?head)",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_REVIEW_MUTATION_RE = re.compile(
|
||||||
|
r"(?:gitea_review_pr|gitea_merge_pr|request_changes|submitted\s+approve|"
|
||||||
|
r"merge result|review decision\s*:\s*approve)",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_BACKGROUND_TOOL_RE = re.compile(r"\b(?:schedule|manage_task)\b", re.IGNORECASE)
|
||||||
|
_PR_REVIEW_REPORT_RE = re.compile(
|
||||||
|
r"(?:review summary|merge recommendation|validation result\s*:\s*pass|"
|
||||||
|
r"queue status report with selected pr)",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
|
||||||
|
_DIAGNOSTIC_FIELDS = {
|
||||||
|
"mcp process root": re.compile(r"mcp process root\s*:", re.IGNORECASE),
|
||||||
|
"inspected git root": re.compile(r"inspected git root\s*:", re.IGNORECASE),
|
||||||
|
"conflict marker path": re.compile(
|
||||||
|
r"(?:conflict marker path|exact conflict marker path)\s*:",
|
||||||
|
re.IGNORECASE,
|
||||||
|
),
|
||||||
|
"merge/rebase control path": re.compile(
|
||||||
|
r"(?:merge/rebase control path|exact merge/rebase control path)\s*:",
|
||||||
|
re.IGNORECASE,
|
||||||
|
),
|
||||||
|
"safe next repair action": re.compile(
|
||||||
|
r"safe next (?:repair )?action\s*:",
|
||||||
|
re.IGNORECASE,
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def assess_infra_stop_handoff_report(
|
||||||
|
report_text: str,
|
||||||
|
*,
|
||||||
|
stop_session: dict | None = None,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Validate repair handoff purity when infra_stop blocks reviewer capability (#289)."""
|
||||||
|
text = report_text or ""
|
||||||
|
session = dict(stop_session or {})
|
||||||
|
reasons: list[str] = []
|
||||||
|
|
||||||
|
infra_stop = bool(
|
||||||
|
session.get("infra_stop")
|
||||||
|
or session.get("infra_stop_blocked")
|
||||||
|
or session.get("route_result") == "infra_stop"
|
||||||
|
)
|
||||||
|
if not infra_stop:
|
||||||
|
return {
|
||||||
|
"proven": True,
|
||||||
|
"block": False,
|
||||||
|
"reasons": [],
|
||||||
|
"infra_stop": False,
|
||||||
|
"safe_next_action": "proceed",
|
||||||
|
}
|
||||||
|
|
||||||
|
lower = text.lower()
|
||||||
|
repair_handoff = bool(
|
||||||
|
_REPAIR_HANDOFF_RE.search(text)
|
||||||
|
or TERMINAL_REPORT_HEADING.lower() in lower
|
||||||
|
)
|
||||||
|
if not repair_handoff:
|
||||||
|
reasons.append(
|
||||||
|
"infra_stop requires a repair handoff, not a PR review report"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _PR_REVIEW_REPORT_RE.search(text) and not _REPAIR_HANDOFF_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"final output must be a repair handoff instead of stale PR review state"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _PR_QUEUE_ADVANCE_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"infra_stop blocks PR queue advancement; do not select or advance next PR"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _REVIEW_MUTATION_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"review, approval, request-changes, merge, or comment mutations "
|
||||||
|
"forbidden while infra_stop is active"
|
||||||
|
)
|
||||||
|
|
||||||
|
if session.get("pinned_head_sha") or re.search(
|
||||||
|
r"pinned (?:review )?head sha\s*:", text, re.IGNORECASE
|
||||||
|
):
|
||||||
|
if session.get("capability_cleared") is not True:
|
||||||
|
reasons.append(
|
||||||
|
"cannot pin PR head SHA while review capability never cleared"
|
||||||
|
)
|
||||||
|
|
||||||
|
if session.get("main_checkout_diagnostics") and not _CONTROL_REPAIR_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"main-checkout diagnostics must be labeled CONTROL-CHECKOUT REPAIR MODE"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _BACKGROUND_TOOL_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"background schedule/manage_task tools must not be used during "
|
||||||
|
"blocked infra_stop recovery"
|
||||||
|
)
|
||||||
|
|
||||||
|
assessment = session.get("infra_stop_assessment") or {}
|
||||||
|
if assessment.get("infra_stop") or infra_stop:
|
||||||
|
missing = [
|
||||||
|
label
|
||||||
|
for label, pattern in _DIAGNOSTIC_FIELDS.items()
|
||||||
|
if not pattern.search(text)
|
||||||
|
]
|
||||||
|
if missing and session.get("require_infra_diagnostics", True):
|
||||||
|
reasons.append(
|
||||||
|
"infra_stop repair handoff missing diagnostic fields: "
|
||||||
|
+ ", ".join(missing)
|
||||||
|
)
|
||||||
|
conflict_file = assessment.get("conflict_file")
|
||||||
|
if conflict_file and conflict_file.lower() not in lower:
|
||||||
|
reasons.append(
|
||||||
|
f"infra_stop assessment conflict file {conflict_file!r} "
|
||||||
|
"not reflected in repair handoff"
|
||||||
|
)
|
||||||
|
|
||||||
|
capability = assess_capability_stop_report(
|
||||||
|
text,
|
||||||
|
trust_gate_status=session.get("trust_gate_status"),
|
||||||
|
capability_denied=True,
|
||||||
|
)
|
||||||
|
if not capability.get("pure"):
|
||||||
|
reasons.extend(capability.get("reasons") or [])
|
||||||
|
|
||||||
|
proven = not reasons
|
||||||
|
return {
|
||||||
|
"proven": proven,
|
||||||
|
"block": not proven,
|
||||||
|
"reasons": list(dict.fromkeys(reasons)),
|
||||||
|
"infra_stop": True,
|
||||||
|
"repair_handoff": repair_handoff,
|
||||||
|
"safe_next_action": (
|
||||||
|
"stop PR queue work; emit CONTROL-CHECKOUT REPAIR MODE handoff "
|
||||||
|
"with infra diagnostics and safe next repair action"
|
||||||
|
if reasons
|
||||||
|
else "proceed"
|
||||||
|
),
|
||||||
|
}
|
||||||
@@ -0,0 +1,308 @@
|
|||||||
|
"""Reviewer inventory completeness and worktree state verifier (#293)."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
_PAGINATION_FINALITY_EVIDENCE = re.compile(
|
||||||
|
r"pagination_complete\s*:\s*true|inventory_complete\s*:\s*true|"
|
||||||
|
r"is_final_page\s*:\s*true|pages_fetched|has_more\s*:\s*false|"
|
||||||
|
r"no next page|final[- ]page|pr_inventory_trust_gate\.status|"
|
||||||
|
r"total_count\s*:|pagination.*(?:final|complete)",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
_INCOMPLETE_PAGINATION_PROOF = re.compile(
|
||||||
|
r"first page only|partial page|page 1 only|truncated|"
|
||||||
|
r"returned \d+ open prs?(?:\s*$|\s*[,;])",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
_DEFAULT_PAGE_SIZE_ASSUMPTION = re.compile(
|
||||||
|
r"(?:less|fewer) than (?:the )?(?:default )?(?:gitea )?page[- ]?(?:size|limit)|"
|
||||||
|
r"default (?:gitea )?page[- ]?size|under (?:the )?50[- ]?(?:item )?limit|"
|
||||||
|
r"(?:complete|exhaustive).*(?:default )?page[- ]?size|"
|
||||||
|
r"page[- ]?size assumption|assumed complete",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
_ELIGIBILITY_CLAIM_RE = re.compile(
|
||||||
|
r"\b(?:oldest eligible pr|next eligible pr|next pr to review|"
|
||||||
|
r"inventory (?:is )?complete|inventory exhaustive|exhaustive inventory)\b",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
_EXACT_PAGE_LIMIT_RE = re.compile(
|
||||||
|
r"(?:returned|listed|fetched)\s+(\d+)\s+open prs?|"
|
||||||
|
r"open pr count\s*:\s*(\d+)|"
|
||||||
|
r"page[- ]?size\s*(?:=|:)\s*(\d+)",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
_LEGACY_SCRATCH_FALSE_RE = re.compile(
|
||||||
|
r"scratch worktree used\s*:\s*false",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
_BRANCHES_WORKTREE_RE = re.compile(r"\bbranches/", re.I)
|
||||||
|
|
||||||
|
_CONTRADICTORY_HEAD_RE = re.compile(
|
||||||
|
r"detached head\s*/\s*branch\s+master|"
|
||||||
|
r"branch\s+master\s*/\s*detached head|"
|
||||||
|
r"detached head.*branch\s+(?:master|main|dev)\b.*detached|"
|
||||||
|
r"checkout\s*:\s*detached head\s*/\s*branch",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
_MAIN_CHECKOUT_BRANCH_RE = re.compile(
|
||||||
|
r"main checkout branch\s*:\s*(\S+)",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
_REVIEW_HEAD_STATE_RE = re.compile(
|
||||||
|
r"review worktree head state\s*:\s*(\S+)",
|
||||||
|
re.I,
|
||||||
|
)
|
||||||
|
|
||||||
|
_HANDOFF_SECTION_RE = re.compile(
|
||||||
|
r"^##\s*Controller Handoff\s*$",
|
||||||
|
re.I | re.M,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _handoff_field_map(report_text: str) -> dict[str, str]:
|
||||||
|
"""Parse ``- Field: value`` lines from the Controller Handoff section."""
|
||||||
|
text = report_text or ""
|
||||||
|
match = _HANDOFF_SECTION_RE.search(text)
|
||||||
|
if not match:
|
||||||
|
return {}
|
||||||
|
section = text[match.end() :]
|
||||||
|
fields: dict[str, str] = {}
|
||||||
|
for line in section.splitlines():
|
||||||
|
stripped = line.strip().lstrip("-*").strip()
|
||||||
|
if ":" not in stripped:
|
||||||
|
continue
|
||||||
|
key, value = stripped.split(":", 1)
|
||||||
|
fields[key.strip().lower()] = value.strip()
|
||||||
|
return fields
|
||||||
|
|
||||||
|
|
||||||
|
def _truthy_field(value: str) -> bool:
|
||||||
|
lowered = (value or "").strip().lower()
|
||||||
|
if not lowered:
|
||||||
|
return False
|
||||||
|
if lowered in {"false", "no", "none", "not applicable", "n/a", "—", "-"}:
|
||||||
|
return False
|
||||||
|
return True
|
||||||
|
|
||||||
|
|
||||||
|
def _field_proves_pagination(value: str) -> bool:
|
||||||
|
proof = (value or "").strip()
|
||||||
|
if not proof or proof.lower() in {"none", "n/a", "not applicable", "unknown"}:
|
||||||
|
return False
|
||||||
|
if _DEFAULT_PAGE_SIZE_ASSUMPTION.search(proof):
|
||||||
|
return False
|
||||||
|
if _INCOMPLETE_PAGINATION_PROOF.search(proof):
|
||||||
|
return False
|
||||||
|
return bool(_PAGINATION_FINALITY_EVIDENCE.search(proof))
|
||||||
|
|
||||||
|
|
||||||
|
def _pagination_proven(text: str, session: dict, *, pagination_field: str = "") -> bool:
|
||||||
|
if session.get("pagination_complete") or session.get("inventory_complete"):
|
||||||
|
return True
|
||||||
|
session_proof = session.get("inventory_pagination_proof") or ""
|
||||||
|
if _field_proves_pagination(str(session_proof)):
|
||||||
|
return True
|
||||||
|
if _field_proves_pagination(pagination_field):
|
||||||
|
return True
|
||||||
|
if _PAGINATION_FINALITY_EVIDENCE.search(text):
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def _exact_page_limit_unproven(
|
||||||
|
text: str, session: dict, *, pagination_proven: bool = False
|
||||||
|
) -> bool:
|
||||||
|
"""True when a full page was returned without final-page proof."""
|
||||||
|
if pagination_proven:
|
||||||
|
return False
|
||||||
|
requested = session.get("requested_page_size")
|
||||||
|
returned = session.get("returned_page_size")
|
||||||
|
if isinstance(requested, int) and isinstance(returned, int):
|
||||||
|
if returned >= requested > 0:
|
||||||
|
return True
|
||||||
|
for match in _EXACT_PAGE_LIMIT_RE.finditer(text):
|
||||||
|
count = next((g for g in match.groups() if g), None)
|
||||||
|
if not count:
|
||||||
|
continue
|
||||||
|
try:
|
||||||
|
n = int(count)
|
||||||
|
except ValueError:
|
||||||
|
continue
|
||||||
|
if n in {10, 20, 50}:
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def assess_inventory_worktree_report(
|
||||||
|
report_text: str,
|
||||||
|
*,
|
||||||
|
inventory_session: dict | None = None,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Validate PR inventory pagination proof and worktree field consistency (#293)."""
|
||||||
|
text = report_text or ""
|
||||||
|
session = dict(inventory_session or {})
|
||||||
|
reasons: list[str] = []
|
||||||
|
|
||||||
|
fields = _handoff_field_map(text)
|
||||||
|
pagination_proof_field = fields.get("inventory pagination proof", "")
|
||||||
|
|
||||||
|
pagination_proven = _pagination_proven(
|
||||||
|
text, session, pagination_field=pagination_proof_field
|
||||||
|
)
|
||||||
|
|
||||||
|
if _ELIGIBILITY_CLAIM_RE.search(text):
|
||||||
|
if not pagination_proven:
|
||||||
|
reasons.append(
|
||||||
|
"oldest/next eligible PR or inventory-complete claim requires "
|
||||||
|
"final-page/no-next-page/pagination_complete proof"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _DEFAULT_PAGE_SIZE_ASSUMPTION.search(text):
|
||||||
|
if not pagination_proven:
|
||||||
|
reasons.append(
|
||||||
|
"inventory pagination assumed from default page size without "
|
||||||
|
"final-page/no-next-page/total-count/traversal proof"
|
||||||
|
)
|
||||||
|
|
||||||
|
if pagination_proof_field:
|
||||||
|
if _DEFAULT_PAGE_SIZE_ASSUMPTION.search(pagination_proof_field):
|
||||||
|
reasons.append(
|
||||||
|
"Inventory pagination proof field relies on page-size assumption"
|
||||||
|
)
|
||||||
|
elif _INCOMPLETE_PAGINATION_PROOF.search(pagination_proof_field):
|
||||||
|
reasons.append(
|
||||||
|
"Inventory pagination proof field does not prove final page"
|
||||||
|
)
|
||||||
|
elif not _field_proves_pagination(pagination_proof_field):
|
||||||
|
if _ELIGIBILITY_CLAIM_RE.search(text) or _EXACT_PAGE_LIMIT_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"Inventory pagination proof field missing final-page metadata"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _exact_page_limit_unproven(text, session, pagination_proven=pagination_proven):
|
||||||
|
reasons.append(
|
||||||
|
"exactly-full first page returned without final-page or "
|
||||||
|
"no-next-page proof"
|
||||||
|
)
|
||||||
|
|
||||||
|
review_worktree_used = fields.get("review worktree used", "")
|
||||||
|
review_worktree_path = fields.get("review worktree path", "")
|
||||||
|
scratch_used = fields.get("scratch worktree used", "")
|
||||||
|
inside_branches = fields.get("review worktree inside branches:", "") or fields.get(
|
||||||
|
"review worktree inside branches", ""
|
||||||
|
)
|
||||||
|
|
||||||
|
branches_path_in_text = bool(
|
||||||
|
_BRANCHES_WORKTREE_RE.search(review_worktree_path)
|
||||||
|
or _BRANCHES_WORKTREE_RE.search(text)
|
||||||
|
)
|
||||||
|
|
||||||
|
if branches_path_in_text:
|
||||||
|
if review_worktree_used and not _truthy_field(review_worktree_used):
|
||||||
|
reasons.append(
|
||||||
|
"reports using a branches/ review worktree must set "
|
||||||
|
"Review worktree used: true"
|
||||||
|
)
|
||||||
|
if scratch_used and re.search(r"\bfalse\b", scratch_used, re.I):
|
||||||
|
reasons.append(
|
||||||
|
"Scratch worktree used: false rejected when a branches/ "
|
||||||
|
"review worktree was created or used"
|
||||||
|
)
|
||||||
|
if _LEGACY_SCRATCH_FALSE_RE.search(text) and not _truthy_field(
|
||||||
|
review_worktree_used
|
||||||
|
):
|
||||||
|
reasons.append(
|
||||||
|
"legacy Scratch worktree used: false contradicts branches/ "
|
||||||
|
"review worktree usage"
|
||||||
|
)
|
||||||
|
|
||||||
|
if review_worktree_path and _truthy_field(review_worktree_used):
|
||||||
|
if "branches/" not in review_worktree_path.replace("\\", "/").lower():
|
||||||
|
reasons.append(
|
||||||
|
"Review worktree path must be under branches/ when "
|
||||||
|
"Review worktree used is true"
|
||||||
|
)
|
||||||
|
if inside_branches and re.search(r"\bfalse\b", inside_branches, re.I):
|
||||||
|
reasons.append(
|
||||||
|
"Review worktree inside branches must be true when path is "
|
||||||
|
"under branches/"
|
||||||
|
)
|
||||||
|
|
||||||
|
main_branch = (
|
||||||
|
fields.get("main checkout branch", "")
|
||||||
|
or _MAIN_CHECKOUT_BRANCH_RE.search(text).group(1)
|
||||||
|
if _MAIN_CHECKOUT_BRANCH_RE.search(text)
|
||||||
|
else ""
|
||||||
|
)
|
||||||
|
head_state = (
|
||||||
|
fields.get("review worktree head state", "")
|
||||||
|
or (
|
||||||
|
_REVIEW_HEAD_STATE_RE.search(text).group(1)
|
||||||
|
if _REVIEW_HEAD_STATE_RE.search(text)
|
||||||
|
else ""
|
||||||
|
)
|
||||||
|
)
|
||||||
|
if main_branch and head_state:
|
||||||
|
main_norm = main_branch.strip().lower()
|
||||||
|
head_norm = head_state.strip().lower()
|
||||||
|
if head_norm in {"branch", "on branch"} and main_norm in {
|
||||||
|
"master",
|
||||||
|
"main",
|
||||||
|
"dev",
|
||||||
|
}:
|
||||||
|
if "detached" in text.lower() and "review worktree" in text.lower():
|
||||||
|
reasons.append(
|
||||||
|
"review worktree HEAD state must not reuse main checkout "
|
||||||
|
"branch wording"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _CONTRADICTORY_HEAD_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"contradictory checkout wording such as 'Detached HEAD / Branch master'"
|
||||||
|
)
|
||||||
|
|
||||||
|
if review_worktree_used and _truthy_field(review_worktree_used):
|
||||||
|
if not review_worktree_path or review_worktree_path.lower() in {
|
||||||
|
"none",
|
||||||
|
"n/a",
|
||||||
|
"not applicable",
|
||||||
|
}:
|
||||||
|
reasons.append(
|
||||||
|
"Review worktree used: true requires Review worktree path"
|
||||||
|
)
|
||||||
|
if not head_state or head_state.lower() in {
|
||||||
|
"none",
|
||||||
|
"n/a",
|
||||||
|
"not applicable",
|
||||||
|
"unknown",
|
||||||
|
}:
|
||||||
|
reasons.append(
|
||||||
|
"Review worktree used: true requires Review worktree HEAD state"
|
||||||
|
)
|
||||||
|
|
||||||
|
proven = not reasons
|
||||||
|
return {
|
||||||
|
"proven": proven,
|
||||||
|
"block": not proven,
|
||||||
|
"reasons": list(dict.fromkeys(reasons)),
|
||||||
|
"pagination_proven": pagination_proven,
|
||||||
|
"branches_worktree_used": branches_path_in_text,
|
||||||
|
"safe_next_action": (
|
||||||
|
"prove inventory pagination with final-page metadata; align "
|
||||||
|
"Review worktree used/path/HEAD state with branches/ usage"
|
||||||
|
if reasons
|
||||||
|
else "proceed"
|
||||||
|
),
|
||||||
|
}
|
||||||
@@ -0,0 +1,130 @@
|
|||||||
|
"""Precise mutation category verifier for reviewer controller handoffs (#319)."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
_CONTROLLER_HANDOFF_RE = re.compile(r"controller handoff", re.IGNORECASE)
|
||||||
|
_WORKSPACE_MUTATIONS_RE = re.compile(
|
||||||
|
r"^\s*[-*]?\s*workspace\s+mutations\s*:",
|
||||||
|
re.IGNORECASE | re.MULTILINE,
|
||||||
|
)
|
||||||
|
_VAGUE_MUTATIONS_NONE_RE = re.compile(
|
||||||
|
r"^\s*[-*]?\s*mutations\s*:\s*none\s*$",
|
||||||
|
re.IGNORECASE | re.MULTILINE,
|
||||||
|
)
|
||||||
|
|
||||||
|
_REQUIRED_CATEGORIES = (
|
||||||
|
"file edits by reviewer",
|
||||||
|
"worktree/index mutations",
|
||||||
|
"git ref mutations",
|
||||||
|
)
|
||||||
|
_WORKTREE_FIELD_ALIASES = (
|
||||||
|
"worktree/index mutations",
|
||||||
|
"worktree mutations",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _has_category(text: str, category: str) -> bool:
|
||||||
|
pattern = re.compile(
|
||||||
|
rf"^\s*[-*]?\s*{re.escape(category)}\s*:",
|
||||||
|
re.IGNORECASE | re.MULTILINE,
|
||||||
|
)
|
||||||
|
return bool(pattern.search(text))
|
||||||
|
|
||||||
|
|
||||||
|
def _has_worktree_category(text: str) -> bool:
|
||||||
|
return any(_has_category(text, alias) for alias in _WORKTREE_FIELD_ALIASES)
|
||||||
|
|
||||||
|
|
||||||
|
def _normalize_for_consistency_check(text: str) -> str:
|
||||||
|
"""Map #319 precise labels to #313 field names for consistency delegation."""
|
||||||
|
normalized = text
|
||||||
|
if _has_category(text, "worktree/index mutations") and not _has_category(
|
||||||
|
text, "worktree mutations"
|
||||||
|
):
|
||||||
|
normalized = re.sub(
|
||||||
|
r"(worktree/index mutations\s*:)",
|
||||||
|
"Worktree mutations:",
|
||||||
|
normalized,
|
||||||
|
flags=re.IGNORECASE,
|
||||||
|
)
|
||||||
|
return normalized
|
||||||
|
|
||||||
|
|
||||||
|
def assess_mutation_categories_report(
|
||||||
|
report_text: str,
|
||||||
|
*,
|
||||||
|
handoff_session: dict | None = None,
|
||||||
|
observed_commands: list | None = None,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Require precise mutation categories in reviewer controller handoffs (#319)."""
|
||||||
|
text = report_text or ""
|
||||||
|
session = dict(handoff_session or {})
|
||||||
|
reasons: list[str] = []
|
||||||
|
lower = text.lower()
|
||||||
|
|
||||||
|
is_controller = bool(
|
||||||
|
session.get("controller_handoff")
|
||||||
|
or _CONTROLLER_HANDOFF_RE.search(text)
|
||||||
|
)
|
||||||
|
if not is_controller and not session.get("require_categories"):
|
||||||
|
return {
|
||||||
|
"proven": True,
|
||||||
|
"block": False,
|
||||||
|
"reasons": [],
|
||||||
|
"controller_handoff": False,
|
||||||
|
"safe_next_action": "proceed",
|
||||||
|
}
|
||||||
|
|
||||||
|
if _WORKSPACE_MUTATIONS_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"controller handoffs must not use legacy 'Workspace mutations'; "
|
||||||
|
"use precise mutation category fields"
|
||||||
|
)
|
||||||
|
|
||||||
|
if _VAGUE_MUTATIONS_NONE_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"controller handoffs must not use vague 'Mutations: none'; "
|
||||||
|
"report each precise category separately"
|
||||||
|
)
|
||||||
|
|
||||||
|
missing = [
|
||||||
|
category
|
||||||
|
for category in _REQUIRED_CATEGORIES
|
||||||
|
if category != "worktree/index mutations" and not _has_category(text, category)
|
||||||
|
]
|
||||||
|
if not _has_worktree_category(text):
|
||||||
|
missing.append("worktree/index mutations")
|
||||||
|
|
||||||
|
if missing:
|
||||||
|
reasons.append(
|
||||||
|
"controller handoff missing precise mutation categories: "
|
||||||
|
+ ", ".join(missing)
|
||||||
|
)
|
||||||
|
|
||||||
|
commands = observed_commands or session.get("observed_commands")
|
||||||
|
if commands:
|
||||||
|
from review_proofs import assess_workspace_mutation_consistency
|
||||||
|
|
||||||
|
consistency = assess_workspace_mutation_consistency(
|
||||||
|
_normalize_for_consistency_check(text),
|
||||||
|
commands,
|
||||||
|
)
|
||||||
|
if not consistency.get("complete"):
|
||||||
|
reasons.extend(consistency.get("reasons") or [])
|
||||||
|
|
||||||
|
proven = not reasons
|
||||||
|
return {
|
||||||
|
"proven": proven,
|
||||||
|
"block": not proven,
|
||||||
|
"reasons": reasons,
|
||||||
|
"controller_handoff": True,
|
||||||
|
"safe_next_action": (
|
||||||
|
"replace Workspace mutations with file edits, worktree/index, git ref, "
|
||||||
|
"and other precise mutation category fields"
|
||||||
|
if reasons
|
||||||
|
else "proceed"
|
||||||
|
),
|
||||||
|
}
|
||||||
@@ -57,6 +57,8 @@ REVIEWER_TASKS = frozenset({
|
|||||||
"review_pr",
|
"review_pr",
|
||||||
"merge_pr",
|
"merge_pr",
|
||||||
"blind_pr_queue_review",
|
"blind_pr_queue_review",
|
||||||
|
"pr_queue_cleanup",
|
||||||
|
"pr-queue-cleanup",
|
||||||
"request_changes_pr",
|
"request_changes_pr",
|
||||||
"approve_pr",
|
"approve_pr",
|
||||||
})
|
})
|
||||||
@@ -88,14 +90,24 @@ TASK_REQUIRED_ROLE = {
|
|||||||
"review_pr": "reviewer",
|
"review_pr": "reviewer",
|
||||||
"merge_pr": "reviewer",
|
"merge_pr": "reviewer",
|
||||||
"blind_pr_queue_review": "reviewer",
|
"blind_pr_queue_review": "reviewer",
|
||||||
|
"pr_queue_cleanup": "reviewer",
|
||||||
|
"pr-queue-cleanup": "reviewer",
|
||||||
"request_changes_pr": "reviewer",
|
"request_changes_pr": "reviewer",
|
||||||
"approve_pr": "reviewer",
|
"approve_pr": "reviewer",
|
||||||
|
# #309: reconciler tasks close already-landed PRs/issues only.
|
||||||
|
"reconcile_close_landed_pr": "reconciler",
|
||||||
|
"reconcile_close_landed_issue": "reconciler",
|
||||||
}
|
}
|
||||||
|
|
||||||
WRONG_ROLE_REVIEWER_MSG = (
|
WRONG_ROLE_REVIEWER_MSG = (
|
||||||
"Wrong role/session for reviewer task. Launch reviewer MCP namespace."
|
"Wrong role/session for reviewer task. Launch reviewer MCP namespace."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
WRONG_ROLE_RECONCILER_MSG = (
|
||||||
|
"Wrong role/session for reconciler task. Launch a reconciler-capable "
|
||||||
|
"MCP namespace/profile with exact close capability."
|
||||||
|
)
|
||||||
|
|
||||||
_session_last_route: dict | None = None
|
_session_last_route: dict | None = None
|
||||||
|
|
||||||
|
|
||||||
@@ -191,6 +203,26 @@ def route_task_session(
|
|||||||
_record_route(result)
|
_record_route(result)
|
||||||
return result
|
return result
|
||||||
|
|
||||||
|
if required_role == "reconciler":
|
||||||
|
result = {
|
||||||
|
"task_type": task_type,
|
||||||
|
"required_role": required_role,
|
||||||
|
"active_role": active_role_kind,
|
||||||
|
"active_profile": active_profile,
|
||||||
|
"route_result": ROUTE_WRONG_ROLE,
|
||||||
|
"downstream_allowed": False,
|
||||||
|
"reasons": [
|
||||||
|
WRONG_ROLE_RECONCILER_MSG,
|
||||||
|
"Reconciler tasks cannot run in author or reviewer "
|
||||||
|
"sessions without exact close capability.",
|
||||||
|
],
|
||||||
|
"message": WRONG_ROLE_RECONCILER_MSG,
|
||||||
|
"runtime_switching_supported": runtime_switching_supported,
|
||||||
|
"profile_switch_blocked": not runtime_switching_supported,
|
||||||
|
}
|
||||||
|
_record_route(result)
|
||||||
|
return result
|
||||||
|
|
||||||
if required_role == "author":
|
if required_role == "author":
|
||||||
route = ROUTE_TO_AUTHOR
|
route = ROUTE_TO_AUTHOR
|
||||||
message = (
|
message = (
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ workflow file.
|
|||||||
| Reconcile already-landed open PRs | [`workflows/reconcile-landed-pr.md`](workflows/reconcile-landed-pr.md) | [`schemas/reconcile-landed-final-report.md`](schemas/reconcile-landed-final-report.md) |
|
| Reconcile already-landed open PRs | [`workflows/reconcile-landed-pr.md`](workflows/reconcile-landed-pr.md) | [`schemas/reconcile-landed-final-report.md`](schemas/reconcile-landed-final-report.md) |
|
||||||
| Create or update Gitea issues | [`workflows/create-issue.md`](workflows/create-issue.md) | [`schemas/create-issue-final-report.md`](schemas/create-issue-final-report.md) |
|
| Create or update Gitea issues | [`workflows/create-issue.md`](workflows/create-issue.md) | [`schemas/create-issue-final-report.md`](schemas/create-issue-final-report.md) |
|
||||||
| Work on an assigned issue / author code | [`workflows/work-issue.md`](workflows/work-issue.md) | [`schemas/work-issue-final-report.md`](schemas/work-issue-final-report.md) |
|
| Work on an assigned issue / author code | [`workflows/work-issue.md`](workflows/work-issue.md) | [`schemas/work-issue-final-report.md`](schemas/work-issue-final-report.md) |
|
||||||
|
| PR-only queue cleanup (one canonical review per PR) | [`workflows/pr-queue-cleanup.md`](workflows/pr-queue-cleanup.md) | [`schemas/pr-queue-cleanup-final-report.md`](schemas/pr-queue-cleanup-final-report.md) |
|
||||||
|
|
||||||
## Universal rules
|
## Universal rules
|
||||||
|
|
||||||
@@ -50,6 +51,10 @@ changes, merge, implement fixes, create branches, commit, push, or create PRs.
|
|||||||
A run that starts in `work-issue` mode may not review, approve, request changes,
|
A run that starts in `work-issue` mode may not review, approve, request changes,
|
||||||
merge, close PRs, or act as reviewer.
|
merge, close PRs, or act as reviewer.
|
||||||
|
|
||||||
|
A run that starts in `pr-queue-cleanup` mode may not claim issues, create
|
||||||
|
branches, edit implementation files, file new issues, or review a second PR
|
||||||
|
after any terminal review mutation.
|
||||||
|
|
||||||
If the task requires a different mode, stop and produce a handoff for the
|
If the task requires a different mode, stop and produce a handoff for the
|
||||||
correct workflow.
|
correct workflow.
|
||||||
|
|
||||||
@@ -156,6 +161,7 @@ Ready-to-copy task prompts live in [`templates/`](templates/):
|
|||||||
|
|
||||||
- [`start-issue.md`](templates/start-issue.md) — author work (loads `work-issue.md`)
|
- [`start-issue.md`](templates/start-issue.md) — author work (loads `work-issue.md`)
|
||||||
- [`review-pr.md`](templates/review-pr.md) — review (loads `review-merge-pr.md`)
|
- [`review-pr.md`](templates/review-pr.md) — review (loads `review-merge-pr.md`)
|
||||||
|
- [`pr-queue-cleanup.md`](templates/pr-queue-cleanup.md) — one PR per cleanup run
|
||||||
- [`merge-pr.md`](templates/merge-pr.md) — merge (loads `review-merge-pr.md`)
|
- [`merge-pr.md`](templates/merge-pr.md) — merge (loads `review-merge-pr.md`)
|
||||||
- [`recover-bad-state.md`](templates/recover-bad-state.md)
|
- [`recover-bad-state.md`](templates/recover-bad-state.md)
|
||||||
- [`reconcile-closed-not-merged-pr.md`](templates/reconcile-closed-not-merged-pr.md)
|
- [`reconcile-closed-not-merged-pr.md`](templates/reconcile-closed-not-merged-pr.md)
|
||||||
|
|||||||
@@ -0,0 +1,31 @@
|
|||||||
|
# PR-queue-cleanup final report schema
|
||||||
|
|
||||||
|
One report per cleanup run (one run = one PR). Fields must all be present;
|
||||||
|
use `none` where nothing occurred. Validated by
|
||||||
|
`pr_queue_cleanup.assess_pr_queue_cleanup_report` (fail closed).
|
||||||
|
|
||||||
|
* Task: pr-queue-cleanup
|
||||||
|
* Workflow source: workflows/pr-queue-cleanup.md (+ version/commit/hash)
|
||||||
|
* Repo:
|
||||||
|
* Role/profile:
|
||||||
|
* Identity:
|
||||||
|
* PR inventory pagination proof: (inventory_complete / final page / total_count)
|
||||||
|
* Queue ordering proof:
|
||||||
|
* Earlier PRs skipped: (with live per-PR proof)
|
||||||
|
* Selected PR: (exactly one)
|
||||||
|
* Pinned head SHA:
|
||||||
|
* Review decision: (single terminal decision)
|
||||||
|
* Merge authorized for PR: true/false (explicit per-PR operator authorization)
|
||||||
|
* Merge gates result:
|
||||||
|
* Merge result: (none / not attempted / merged SHA / blocker)
|
||||||
|
* Run stop point: (which §4 chain rule ended the run)
|
||||||
|
* Next suggested PR: (named, not continued to)
|
||||||
|
* File edits by reviewer:
|
||||||
|
* Worktree/index mutations:
|
||||||
|
* Git ref mutations:
|
||||||
|
* MCP/Gitea mutations:
|
||||||
|
* Issue mutations: none (required — forbidden in cleanup mode)
|
||||||
|
* Branch mutations: none (required — forbidden in cleanup mode)
|
||||||
|
* Read-only diagnostics:
|
||||||
|
* Blockers:
|
||||||
|
* Safe next action: (fresh run for the next PR)
|
||||||
@@ -0,0 +1,25 @@
|
|||||||
|
# Template: PR-only queue cleanup (one PR per run)
|
||||||
|
|
||||||
|
Copy, fill the `<...>` fields, and paste as the task prompt.
|
||||||
|
|
||||||
|
```text
|
||||||
|
Task: PR-only queue cleanup for <repo>.
|
||||||
|
|
||||||
|
Load workflows/pr-queue-cleanup.md and workflows/review-merge-pr.md before any
|
||||||
|
mutation. Route pr_queue_cleanup through gitea_route_task_session (reviewer
|
||||||
|
profile required).
|
||||||
|
|
||||||
|
Rules:
|
||||||
|
- One run = exactly one selected PR = one terminal review decision.
|
||||||
|
- Build full open-PR inventory with pagination proof before selection.
|
||||||
|
- Forbidden: issue claiming, branch creation, implementation edits, issue filing,
|
||||||
|
reviewing a second PR after a terminal mutation in this run.
|
||||||
|
- After REQUEST_CHANGES: stop. After APPROVED: merge only this PR and only if
|
||||||
|
operator explicitly authorized merge for this PR in this run.
|
||||||
|
- Report Next suggested PR without continuing to it.
|
||||||
|
|
||||||
|
Operator PR list (optional): <pr numbers or "oldest eligible from inventory">
|
||||||
|
Merge authorized for selected PR in this run: <true|false>
|
||||||
|
|
||||||
|
End with the pr-queue-cleanup final report schema.
|
||||||
|
```
|
||||||
@@ -0,0 +1,86 @@
|
|||||||
|
---
|
||||||
|
task_mode: pr-queue-cleanup
|
||||||
|
canonical: true
|
||||||
|
final_report_schema: ../schemas/pr-queue-cleanup-final-report.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# PR-only queue cleanup workflow (canonical)
|
||||||
|
|
||||||
|
**Task mode:** `pr-queue-cleanup`
|
||||||
|
|
||||||
|
Reviewer-role mode for cleanup periods when the queue holds many open PRs.
|
||||||
|
Each run dispatches **exactly one** canonical review for **exactly one** PR,
|
||||||
|
then stops after any terminal review mutation. The next PR always requires a
|
||||||
|
new run with fresh identity, capability, and inventory proof.
|
||||||
|
|
||||||
|
This mode composes with the canonical review workflow: for the selected PR,
|
||||||
|
load and follow [`workflows/review-merge-pr.md`](review-merge-pr.md) in full.
|
||||||
|
This file adds the cleanup-mode boundaries around that per-PR run; it does
|
||||||
|
not replace the review workflow.
|
||||||
|
|
||||||
|
## 0. Load the canonical workflow first
|
||||||
|
|
||||||
|
Load this file and `workflows/review-merge-pr.md` before any mutation and
|
||||||
|
report source, version/commit/hash, and any conflict with the operator
|
||||||
|
prompt. If either cannot be loaded, stop and produce a recovery handoff.
|
||||||
|
|
||||||
|
## 1. Mode isolation — forbidden actions
|
||||||
|
|
||||||
|
PR-only cleanup mode forbids, with no exceptions:
|
||||||
|
|
||||||
|
* issue claiming (`claim_issue`, `mark_issue`, `lock_issue`)
|
||||||
|
* branch creation or push (`create_branch`, `push_branch`)
|
||||||
|
* implementation edits of any kind
|
||||||
|
* new issue filing (`create_issue`)
|
||||||
|
* PR creation (`create_pr`) and commit tools (`commit_files`)
|
||||||
|
* reviewing a second PR after any terminal review mutation
|
||||||
|
|
||||||
|
`pr_queue_cleanup.check_cleanup_task_allowed` fails closed on these tasks.
|
||||||
|
If author-side work is needed, stop and hand off to `work-issue` mode in a
|
||||||
|
separate session.
|
||||||
|
|
||||||
|
## 2. Identity, capability, and routing
|
||||||
|
|
||||||
|
Route `pr_queue_cleanup` through `gitea_route_task_session` — reviewer role
|
||||||
|
required; author sessions receive `wrong_role_stop`. Prove `gitea.pr.review`
|
||||||
|
capability before selection. Merge additionally requires `gitea.pr.merge`
|
||||||
|
plus the explicit per-PR authorization in §4.
|
||||||
|
|
||||||
|
## 3. Inventory and selection
|
||||||
|
|
||||||
|
Build the complete open-PR inventory with pagination proof
|
||||||
|
(`inventory_complete`, final page, `total_count`) before any selection
|
||||||
|
claim. Select exactly one PR according to project queue ordering rules
|
||||||
|
(oldest-first unless the operator queue says otherwise), skipping earlier
|
||||||
|
PRs only with live per-PR proof. No multi-PR validation and no batch report
|
||||||
|
may substitute for per-PR proof.
|
||||||
|
|
||||||
|
## 4. Terminal mutation chain
|
||||||
|
|
||||||
|
`pr_queue_cleanup.resolve_cleanup_run_state` is the authority:
|
||||||
|
|
||||||
|
* After `REQUEST_CHANGES` → the run stops.
|
||||||
|
* After `COMMENT` or a proof-backed skip → the run stops.
|
||||||
|
* After `APPROVED` → the run may continue **only** to same-PR merge, and only
|
||||||
|
when the operator explicitly authorized merge for that specific PR in this
|
||||||
|
run (`Merge authorized: true`) and every merge gate passes.
|
||||||
|
* After merge, or on any merge blocker → the run stops.
|
||||||
|
* Any terminal mutation targeting a PR other than the selected PR is a hard
|
||||||
|
stop and must be reported as a violation.
|
||||||
|
|
||||||
|
## 5. Fresh run per PR
|
||||||
|
|
||||||
|
The next PR requires a new run with fresh identity, capability, inventory,
|
||||||
|
and ordering proof. Do not carry pinned SHAs, eligibility classes, or
|
||||||
|
validation results across runs.
|
||||||
|
|
||||||
|
## 6. Final report
|
||||||
|
|
||||||
|
Use the schema in
|
||||||
|
[`schemas/pr-queue-cleanup-final-report.md`](../schemas/pr-queue-cleanup-final-report.md).
|
||||||
|
The report must include the **Next suggested PR** (from the proven ordering)
|
||||||
|
without continuing to it, exactly one **Selected PR**, the single terminal
|
||||||
|
decision, pagination proof, and `Issue mutations: none` / `Branch mutations:
|
||||||
|
none`. `pr_queue_cleanup.assess_pr_queue_cleanup_report` validates these
|
||||||
|
fields and fails closed on batch reviews, missing pagination proof, missing
|
||||||
|
next-suggested-PR, unauthorized merges, or any issue/branch mutation.
|
||||||
@@ -248,6 +248,28 @@ Post a reconciliation comment only if:
|
|||||||
If comment capability is missing, record the intended comment in the final
|
If comment capability is missing, record the intended comment in the final
|
||||||
handoff only.
|
handoff only.
|
||||||
|
|
||||||
|
## 12A. Partial reconciliation policy (#302)
|
||||||
|
|
||||||
|
The durable policy when `close_pr` capability is missing is
|
||||||
|
**comment-then-stop** (`resolve_partial_reconciliation_plan` in
|
||||||
|
`review_proofs.py` enforces it):
|
||||||
|
|
||||||
|
* `close_pr` proven → full reconciliation: close the PR and report the
|
||||||
|
close result (`FULL_RECONCILE_CLOSE_ALLOWED`).
|
||||||
|
* `close_pr` missing, `comment_pr` proven → post exactly one
|
||||||
|
reconciliation comment carrying PR head SHA, target branch SHA,
|
||||||
|
ancestor proof, linked issue status, and the required missing
|
||||||
|
capability, then stop for a human or authorized close
|
||||||
|
(`PARTIAL_RECONCILE_COMMENT_THEN_STOP`).
|
||||||
|
* `comment_pr` also missing → no Gitea mutation; produce a recovery
|
||||||
|
handoff recording the intended comment and required capabilities
|
||||||
|
(`RECOVERY_HANDOFF_ONLY`).
|
||||||
|
* Ancestry not proven → no mutation regardless of capability
|
||||||
|
(`GATE_NOT_PROVEN`).
|
||||||
|
|
||||||
|
Final reports must explain why the comment was or was not posted and
|
||||||
|
name the missing capability (`assess_partial_reconciliation_report`).
|
||||||
|
|
||||||
## 13. PR close rules
|
## 13. PR close rules
|
||||||
|
|
||||||
Close a PR only if:
|
Close a PR only if:
|
||||||
|
|||||||
@@ -72,6 +72,14 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
|
|||||||
"permission": "gitea.pr.review",
|
"permission": "gitea.pr.review",
|
||||||
"role": "reviewer",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
|
"pr_queue_cleanup": {
|
||||||
|
"permission": "gitea.pr.review",
|
||||||
|
"role": "reviewer",
|
||||||
|
},
|
||||||
|
"pr-queue-cleanup": {
|
||||||
|
"permission": "gitea.pr.review",
|
||||||
|
"role": "reviewer",
|
||||||
|
},
|
||||||
"request_changes_pr": {
|
"request_changes_pr": {
|
||||||
"permission": "gitea.pr.request_changes",
|
"permission": "gitea.pr.request_changes",
|
||||||
"role": "reviewer",
|
"role": "reviewer",
|
||||||
@@ -96,6 +104,28 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
|
|||||||
"permission": "gitea.read",
|
"permission": "gitea.read",
|
||||||
"role": "author",
|
"role": "author",
|
||||||
},
|
},
|
||||||
|
# #309: dedicated reconciler path for already-landed open PRs. Exact
|
||||||
|
# close capabilities only — never review/approve/request_changes/merge.
|
||||||
|
"reconcile_close_landed_pr": {
|
||||||
|
"permission": "gitea.pr.close",
|
||||||
|
"role": "reconciler",
|
||||||
|
},
|
||||||
|
"reconcile_close_landed_issue": {
|
||||||
|
"permission": "gitea.issue.close",
|
||||||
|
"role": "reconciler",
|
||||||
|
},
|
||||||
|
"post_heartbeat": {
|
||||||
|
"permission": "gitea.issue.comment",
|
||||||
|
"role": "author",
|
||||||
|
},
|
||||||
|
"reconcile_issue_claims": {
|
||||||
|
"permission": "gitea.read",
|
||||||
|
"role": "author",
|
||||||
|
},
|
||||||
|
"cleanup_stale_claims": {
|
||||||
|
"permission": "gitea.issue.comment",
|
||||||
|
"role": "author",
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
# Issue-mutating MCP tools and their resolver task keys.
|
# Issue-mutating MCP tools and their resolver task keys.
|
||||||
|
|||||||
@@ -0,0 +1,116 @@
|
|||||||
|
"""Tests for issue claim heartbeat leases (#268)."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from datetime import datetime, timedelta, timezone
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
import issue_claim_heartbeat as ich # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
class TestHeartbeatFormatting(unittest.TestCase):
|
||||||
|
def test_format_and_parse_roundtrip(self):
|
||||||
|
body = ich.format_heartbeat_body(
|
||||||
|
kind="claim",
|
||||||
|
issue_number=268,
|
||||||
|
branch="feat/issue-268-heartbeat-leases",
|
||||||
|
phase="claimed",
|
||||||
|
profile="prgs-author",
|
||||||
|
next_action="implement module",
|
||||||
|
)
|
||||||
|
parsed = ich.parse_heartbeat_comment(body)
|
||||||
|
self.assertIsNotNone(parsed)
|
||||||
|
assert parsed is not None
|
||||||
|
self.assertEqual(parsed["issue_number"], 268)
|
||||||
|
self.assertEqual(parsed["branch"], "feat/issue-268-heartbeat-leases")
|
||||||
|
self.assertEqual(parsed["phase"], "claimed")
|
||||||
|
|
||||||
|
|
||||||
|
class TestClaimClassification(unittest.TestCase):
|
||||||
|
NOW = datetime(2026, 7, 7, 12, 0, tzinfo=timezone.utc)
|
||||||
|
|
||||||
|
def _comment(self, *, minutes_ago: int, kind: str = "progress") -> dict:
|
||||||
|
ts = (self.NOW - timedelta(minutes=minutes_ago)).isoformat()
|
||||||
|
body = ich.format_heartbeat_body(
|
||||||
|
kind=kind,
|
||||||
|
issue_number=268,
|
||||||
|
branch="feat/issue-268-heartbeat-leases",
|
||||||
|
phase="implementation",
|
||||||
|
profile="prgs-author",
|
||||||
|
)
|
||||||
|
return {"id": 1, "body": body, "created_at": ts, "updated_at": ts}
|
||||||
|
|
||||||
|
def test_active_claim_within_lease(self):
|
||||||
|
issue = {"number": 268, "title": "t", "labels": [{"name": "status:in-progress"}]}
|
||||||
|
result = ich.classify_issue_claim(
|
||||||
|
issue=issue,
|
||||||
|
comments=[self._comment(minutes_ago=5)],
|
||||||
|
now=self.NOW,
|
||||||
|
)
|
||||||
|
self.assertEqual(result["status"], "active")
|
||||||
|
|
||||||
|
def test_stale_claim_without_branch(self):
|
||||||
|
issue = {"number": 268, "title": "t", "labels": [{"name": "status:in-progress"}]}
|
||||||
|
result = ich.classify_issue_claim(
|
||||||
|
issue=issue,
|
||||||
|
comments=[self._comment(minutes_ago=45)],
|
||||||
|
now=self.NOW,
|
||||||
|
)
|
||||||
|
self.assertEqual(result["status"], "stale")
|
||||||
|
|
||||||
|
def test_reclaimable_after_sixty_minutes_without_branch(self):
|
||||||
|
issue = {"number": 268, "title": "t", "labels": [{"name": "status:in-progress"}]}
|
||||||
|
result = ich.classify_issue_claim(
|
||||||
|
issue=issue,
|
||||||
|
comments=[self._comment(minutes_ago=90)],
|
||||||
|
now=self.NOW,
|
||||||
|
)
|
||||||
|
self.assertEqual(result["status"], "reclaimable")
|
||||||
|
|
||||||
|
def test_awaiting_review_when_open_pr_exists(self):
|
||||||
|
issue = {"number": 268, "title": "t", "labels": [{"name": "status:in-progress"}]}
|
||||||
|
open_prs = [
|
||||||
|
{
|
||||||
|
"number": 400,
|
||||||
|
"title": "feat",
|
||||||
|
"body": "Closes #268",
|
||||||
|
"head": {"ref": "feat/issue-268-heartbeat-leases"},
|
||||||
|
}
|
||||||
|
]
|
||||||
|
result = ich.classify_issue_claim(
|
||||||
|
issue=issue,
|
||||||
|
comments=[self._comment(minutes_ago=120)],
|
||||||
|
open_prs=open_prs,
|
||||||
|
now=self.NOW,
|
||||||
|
)
|
||||||
|
self.assertEqual(result["status"], "awaiting_review")
|
||||||
|
|
||||||
|
def test_phantom_claim_without_heartbeat(self):
|
||||||
|
issue = {"number": 268, "title": "t", "labels": [{"name": "status:in-progress"}]}
|
||||||
|
result = ich.classify_issue_claim(
|
||||||
|
issue=issue,
|
||||||
|
comments=[{"id": 1, "body": "working on it"}],
|
||||||
|
now=self.NOW,
|
||||||
|
)
|
||||||
|
self.assertEqual(result["status"], "phantom")
|
||||||
|
|
||||||
|
|
||||||
|
class TestInventory(unittest.TestCase):
|
||||||
|
def test_build_cleanup_plan_flags_phantom(self):
|
||||||
|
inventory = {
|
||||||
|
"entries": [
|
||||||
|
{"issue_number": 1, "status": "phantom", "reasons": ["no heartbeat"]},
|
||||||
|
{"issue_number": 2, "status": "active", "reasons": []},
|
||||||
|
]
|
||||||
|
}
|
||||||
|
plan = ich.build_cleanup_plan(inventory)
|
||||||
|
self.assertEqual(len(plan), 1)
|
||||||
|
self.assertEqual(plan[0]["issue_number"], 1)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -172,6 +172,8 @@ class TestAllowedIssueWritesProceed(IssueWriteGateBase):
|
|||||||
return [{"id": 10, "name": "status:in-progress"}]
|
return [{"id": 10, "name": "status:in-progress"}]
|
||||||
if method == "POST" and "/issues/9/labels" in url:
|
if method == "POST" and "/issues/9/labels" in url:
|
||||||
return [{"name": "status:in-progress"}]
|
return [{"name": "status:in-progress"}]
|
||||||
|
if method == "POST" and "/issues/9/comments" in url:
|
||||||
|
return {"id": 501}
|
||||||
if method == "GET" and url.endswith("/user"):
|
if method == "GET" and url.endswith("/user"):
|
||||||
return {"login": "author-user"}
|
return {"login": "author-user"}
|
||||||
return {}
|
return {}
|
||||||
|
|||||||
@@ -16,6 +16,7 @@ def test_all_workflow_files_exist():
|
|||||||
"reconcile-landed-pr.md",
|
"reconcile-landed-pr.md",
|
||||||
"create-issue.md",
|
"create-issue.md",
|
||||||
"work-issue.md",
|
"work-issue.md",
|
||||||
|
"pr-queue-cleanup.md",
|
||||||
):
|
):
|
||||||
assert (SKILL_DIR / "workflows" / name).is_file(), name
|
assert (SKILL_DIR / "workflows" / name).is_file(), name
|
||||||
|
|
||||||
@@ -26,6 +27,7 @@ def test_skill_references_all_workflow_files():
|
|||||||
"workflows/reconcile-landed-pr.md",
|
"workflows/reconcile-landed-pr.md",
|
||||||
"workflows/create-issue.md",
|
"workflows/create-issue.md",
|
||||||
"workflows/work-issue.md",
|
"workflows/work-issue.md",
|
||||||
|
"workflows/pr-queue-cleanup.md",
|
||||||
):
|
):
|
||||||
assert name in SKILL
|
assert name in SKILL
|
||||||
|
|
||||||
@@ -36,6 +38,7 @@ def test_skill_contains_mode_isolation_language():
|
|||||||
assert "reconcile-landed-pr" in SKILL
|
assert "reconcile-landed-pr" in SKILL
|
||||||
assert "create-issue" in SKILL
|
assert "create-issue" in SKILL
|
||||||
assert "work-issue" in SKILL
|
assert "work-issue" in SKILL
|
||||||
|
assert "pr-queue-cleanup" in SKILL
|
||||||
assert "Do not mix modes" in SKILL or "do not mix modes" in SKILL.lower()
|
assert "Do not mix modes" in SKILL or "do not mix modes" in SKILL.lower()
|
||||||
|
|
||||||
|
|
||||||
@@ -78,6 +81,12 @@ def test_reconcile_landed_workflow_contract():
|
|||||||
assert "canonical: true" in text
|
assert "canonical: true" in text
|
||||||
assert "Do not review or merge normal PRs" in text
|
assert "Do not review or merge normal PRs" in text
|
||||||
assert "Already-landed proof" in text
|
assert "Already-landed proof" in text
|
||||||
|
# Issue #302: partial reconciliation policy must stay documented.
|
||||||
|
assert "## 12A. Partial reconciliation policy (#302)" in text
|
||||||
|
assert "comment-then-stop" in text
|
||||||
|
assert "PARTIAL_RECONCILE_COMMENT_THEN_STOP" in text
|
||||||
|
assert "RECOVERY_HANDOFF_ONLY" in text
|
||||||
|
assert "resolve_partial_reconciliation_plan" in text
|
||||||
|
|
||||||
|
|
||||||
def test_create_issue_workflow_contract():
|
def test_create_issue_workflow_contract():
|
||||||
@@ -93,12 +102,24 @@ def test_work_issue_workflow_contract():
|
|||||||
assert "## 21. PR creation rules" in text
|
assert "## 21. PR creation rules" in text
|
||||||
|
|
||||||
|
|
||||||
|
def test_pr_queue_cleanup_workflow_contract():
|
||||||
|
text = (SKILL_DIR / "workflows" / "pr-queue-cleanup.md").read_text(
|
||||||
|
encoding="utf-8"
|
||||||
|
)
|
||||||
|
assert "canonical: true" in text
|
||||||
|
assert "exactly one" in text
|
||||||
|
assert "check_cleanup_task_allowed" in text
|
||||||
|
assert "resolve_cleanup_run_state" in text
|
||||||
|
assert "Next suggested PR" in text
|
||||||
|
|
||||||
|
|
||||||
def test_final_report_schemas_exist():
|
def test_final_report_schemas_exist():
|
||||||
for name in (
|
for name in (
|
||||||
"review-merge-final-report.md",
|
"review-merge-final-report.md",
|
||||||
"reconcile-landed-final-report.md",
|
"reconcile-landed-final-report.md",
|
||||||
"create-issue-final-report.md",
|
"create-issue-final-report.md",
|
||||||
"work-issue-final-report.md",
|
"work-issue-final-report.md",
|
||||||
|
"pr-queue-cleanup-final-report.md",
|
||||||
):
|
):
|
||||||
assert (SKILL_DIR / "schemas" / name).is_file(), name
|
assert (SKILL_DIR / "schemas" / name).is_file(), name
|
||||||
|
|
||||||
@@ -118,6 +139,20 @@ def test_start_issue_template_references_work_issue_workflow():
|
|||||||
assert "workflows/work-issue.md" in text
|
assert "workflows/work-issue.md" in text
|
||||||
|
|
||||||
|
|
||||||
|
def test_pr_queue_cleanup_template_references_workflow():
|
||||||
|
text = (SKILL_DIR / "templates" / "pr-queue-cleanup.md").read_text(
|
||||||
|
encoding="utf-8"
|
||||||
|
)
|
||||||
|
assert "workflows/pr-queue-cleanup.md" in text
|
||||||
|
assert "pr_queue_cleanup" in text
|
||||||
|
|
||||||
|
|
||||||
|
def test_pr_queue_cleanup_verifier_exported():
|
||||||
|
from review_proofs import assess_pr_queue_cleanup_report
|
||||||
|
|
||||||
|
assert callable(assess_pr_queue_cleanup_report)
|
||||||
|
|
||||||
|
|
||||||
def test_reviewer_fallback_verifier_exported():
|
def test_reviewer_fallback_verifier_exported():
|
||||||
from review_proofs import assess_reviewer_fallback_report
|
from review_proofs import assess_reviewer_fallback_report
|
||||||
|
|
||||||
@@ -166,6 +201,30 @@ def test_validation_worktree_edit_verifier_exported():
|
|||||||
assert callable(assess_validation_worktree_edit_report)
|
assert callable(assess_validation_worktree_edit_report)
|
||||||
|
|
||||||
|
|
||||||
|
def test_already_landed_handoff_verifier_exported():
|
||||||
|
from review_proofs import assess_already_landed_handoff_report
|
||||||
|
|
||||||
|
assert callable(assess_already_landed_handoff_report)
|
||||||
|
|
||||||
|
|
||||||
|
def test_inventory_worktree_verifier_exported():
|
||||||
|
from review_proofs import assess_inventory_worktree_report
|
||||||
|
|
||||||
|
assert callable(assess_inventory_worktree_report)
|
||||||
|
|
||||||
|
|
||||||
|
def test_infra_stop_handoff_verifier_exported():
|
||||||
|
from review_proofs import assess_infra_stop_handoff_report
|
||||||
|
|
||||||
|
assert callable(assess_infra_stop_handoff_report)
|
||||||
|
|
||||||
|
|
||||||
|
def test_mutation_categories_verifier_exported():
|
||||||
|
from review_proofs import assess_mutation_categories_report
|
||||||
|
|
||||||
|
assert callable(assess_mutation_categories_report)
|
||||||
|
|
||||||
|
|
||||||
def test_worktree_ownership_verifier_exported():
|
def test_worktree_ownership_verifier_exported():
|
||||||
from review_proofs import assess_worktree_ownership_report
|
from review_proofs import assess_worktree_ownership_report
|
||||||
|
|
||||||
@@ -176,3 +235,22 @@ def test_already_landed_classification_verifier_exported():
|
|||||||
from review_proofs import assess_already_landed_classification_report
|
from review_proofs import assess_already_landed_classification_report
|
||||||
|
|
||||||
assert callable(assess_already_landed_classification_report)
|
assert callable(assess_already_landed_classification_report)
|
||||||
|
|
||||||
|
|
||||||
|
def test_pr_queue_cleanup_verifier_exported():
|
||||||
|
from review_proofs import assess_pr_queue_cleanup_report
|
||||||
|
|
||||||
|
assert callable(assess_pr_queue_cleanup_report)
|
||||||
|
|
||||||
|
|
||||||
|
def test_pr_queue_cleanup_workflow_contract():
|
||||||
|
text = (SKILL_DIR / "workflows" / "pr-queue-cleanup.md").read_text(encoding="utf-8")
|
||||||
|
assert "canonical: true" in text
|
||||||
|
assert "task_mode: pr-queue-cleanup" in text
|
||||||
|
assert "## 1. Mode isolation" in text
|
||||||
|
assert "## 4. Terminal mutation chain" in text
|
||||||
|
assert "## 5. Fresh run per PR" in text
|
||||||
|
assert "exactly one" in text.lower()
|
||||||
|
assert "review-merge-pr.md" in text
|
||||||
|
assert "Next suggested PR" in text
|
||||||
|
assert "schemas/pr-queue-cleanup-final-report.md" in text
|
||||||
|
|||||||
@@ -321,15 +321,17 @@ class TestMarkIssue(unittest.TestCase):
|
|||||||
@patch("mcp_server.api_request")
|
@patch("mcp_server.api_request")
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_start_adds_label(self, _auth, mock_api):
|
def test_start_adds_label(self, _auth, mock_api):
|
||||||
# First call: get labels; second call: add label
|
# labels, add label, post claim heartbeat comment
|
||||||
mock_api.side_effect = [
|
mock_api.side_effect = [
|
||||||
[{"id": 10, "name": "status:in-progress"}],
|
[{"id": 10, "name": "status:in-progress"}],
|
||||||
[{"name": "status:in-progress"}],
|
[{"name": "status:in-progress"}],
|
||||||
|
{"id": 99},
|
||||||
]
|
]
|
||||||
with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True):
|
with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True):
|
||||||
result = gitea_mark_issue(issue_number=5, action="start")
|
result = gitea_mark_issue(issue_number=5, action="start")
|
||||||
self.assertTrue(result["success"])
|
self.assertTrue(result["success"])
|
||||||
self.assertIn("claimed", result["message"])
|
self.assertIn("claimed", result["message"])
|
||||||
|
self.assertTrue(result.get("heartbeat_posted"))
|
||||||
|
|
||||||
@patch("mcp_server.api_request")
|
@patch("mcp_server.api_request")
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
|
|||||||
@@ -0,0 +1,121 @@
|
|||||||
|
"""Tests for native MCP preference gate and shell circuit breaker (#270)."""
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
import native_mcp_preference as nmp # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
class TestShellHealthCircuitBreaker(unittest.TestCase):
|
||||||
|
def setUp(self):
|
||||||
|
nmp.clear_shell_health_for_tests()
|
||||||
|
|
||||||
|
def test_two_spawn_failures_hard_stop(self):
|
||||||
|
nmp.record_shell_spawn_outcome(exit_code=-1, stdout="", stderr="")
|
||||||
|
status = nmp.record_shell_spawn_outcome(exit_code=-1, stdout="", stderr="")
|
||||||
|
self.assertTrue(status["hard_stopped"])
|
||||||
|
self.assertFalse(status["shell_use_allowed"])
|
||||||
|
self.assertEqual(status["consecutive_spawn_failures"], 2)
|
||||||
|
|
||||||
|
def test_success_resets_breaker(self):
|
||||||
|
nmp.record_shell_spawn_outcome(exit_code=-1, stdout="", stderr="")
|
||||||
|
status = nmp.record_shell_spawn_outcome(exit_code=0, stdout="ok", stderr="")
|
||||||
|
self.assertFalse(status["hard_stopped"])
|
||||||
|
self.assertEqual(status["consecutive_spawn_failures"], 0)
|
||||||
|
|
||||||
|
|
||||||
|
class TestNativeMcpPreferenceGate(unittest.TestCase):
|
||||||
|
def setUp(self):
|
||||||
|
nmp.clear_shell_health_for_tests()
|
||||||
|
|
||||||
|
def test_mcp_available_blocks_local_script(self):
|
||||||
|
result = nmp.assess_gitea_operation_path(
|
||||||
|
task="create_pr",
|
||||||
|
command_or_detail="python create_pr.py --remote prgs --title T --head h",
|
||||||
|
mcp_available=True,
|
||||||
|
mcp_tool_visible=True,
|
||||||
|
)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertEqual(result["path_kind"], "shell_script")
|
||||||
|
|
||||||
|
def test_mcp_native_path_allowed(self):
|
||||||
|
result = nmp.assess_gitea_operation_path(
|
||||||
|
task="comment_issue",
|
||||||
|
path_kind="mcp_native",
|
||||||
|
mcp_available=True,
|
||||||
|
mcp_tool_visible=True,
|
||||||
|
)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
self.assertTrue(result["allowed"])
|
||||||
|
|
||||||
|
def test_mcp_unavailable_requires_terminal_report(self):
|
||||||
|
result = nmp.assess_gitea_operation_path(
|
||||||
|
task="merge_pr",
|
||||||
|
path_kind="shell_script",
|
||||||
|
mcp_available=False,
|
||||||
|
mcp_tool_visible=False,
|
||||||
|
)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("terminal recovery report" in r for r in result["reasons"])
|
||||||
|
)
|
||||||
|
self.assertIn(nmp.TERMINAL_REPORT_HEADING, result["terminal_report"])
|
||||||
|
|
||||||
|
def test_recovery_fallback_allowed_with_proof(self):
|
||||||
|
result = nmp.assess_gitea_operation_path(
|
||||||
|
task="merge_pr",
|
||||||
|
path_kind="shell_script",
|
||||||
|
command_or_detail="Recovery mode: MCP tools unavailable in this session.",
|
||||||
|
mcp_available=False,
|
||||||
|
recovery_mode=True,
|
||||||
|
recovery_proof_complete=True,
|
||||||
|
)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
|
||||||
|
def test_cli_auth_divergence_blocked(self):
|
||||||
|
result = nmp.assess_gitea_operation_path(
|
||||||
|
task="create_pr",
|
||||||
|
command_or_detail="GITEA_MCP_PROFILE=prgs-reviewer python create_pr.py",
|
||||||
|
mcp_available=True,
|
||||||
|
active_profile="prgs-author",
|
||||||
|
)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertTrue(any("CLI auth divergence" in r for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_unsafe_helper_fallback_denied(self):
|
||||||
|
result = nmp.assess_gitea_operation_path(
|
||||||
|
task="commit_files",
|
||||||
|
command_or_detail="python _encode_payload.py",
|
||||||
|
mcp_available=True,
|
||||||
|
mcp_tool_visible=True,
|
||||||
|
)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertEqual(result["path_kind"], "unsafe_helper")
|
||||||
|
|
||||||
|
def test_reviewer_mcp_server_touch_blocked(self):
|
||||||
|
result = nmp.assess_gitea_operation_path(
|
||||||
|
task="review_pr",
|
||||||
|
command_or_detail="pkill -f gitea_mcp_server.py",
|
||||||
|
mcp_available=True,
|
||||||
|
role="reviewer",
|
||||||
|
active_profile="prgs-reviewer",
|
||||||
|
)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertEqual(result["path_kind"], "mcp_server_touch")
|
||||||
|
|
||||||
|
def test_hard_stopped_shell_blocks_non_mcp_path(self):
|
||||||
|
nmp.record_shell_spawn_outcome(exit_code=-1, stdout="", stderr="")
|
||||||
|
nmp.record_shell_spawn_outcome(exit_code=-1, stdout="", stderr="")
|
||||||
|
result = nmp.assess_gitea_operation_path(
|
||||||
|
task="lock_issue",
|
||||||
|
path_kind="shell_script",
|
||||||
|
mcp_available=True,
|
||||||
|
)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertTrue(any("circuit breaker" in r for r in result["reasons"]))
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -0,0 +1,269 @@
|
|||||||
|
"""Tests for PR-only queue cleanup mode (#390)."""
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
from pr_queue_cleanup import (
|
||||||
|
CLEANUP_FORBIDDEN_TASKS,
|
||||||
|
CLEANUP_WORKFLOW_PATH,
|
||||||
|
CONTINUE_TO_SAME_PR_MERGE,
|
||||||
|
STOP_AFTER_DECISION,
|
||||||
|
STOP_AFTER_MERGE,
|
||||||
|
STOP_AFTER_MERGE_BLOCKER,
|
||||||
|
STOP_AFTER_REQUEST_CHANGES,
|
||||||
|
STOP_APPROVED_MERGE_GATES_FAILED,
|
||||||
|
STOP_APPROVED_NO_MERGE_AUTH,
|
||||||
|
STOP_GATE_NOT_PROVEN,
|
||||||
|
assess_pr_queue_cleanup_report,
|
||||||
|
check_cleanup_task_allowed,
|
||||||
|
resolve_cleanup_run_state,
|
||||||
|
)
|
||||||
|
from review_proofs import assess_pr_queue_cleanup_report as proofs_assess
|
||||||
|
from role_session_router import REVIEWER_TASKS, route_task_session
|
||||||
|
from task_capability_map import required_permission, required_role
|
||||||
|
|
||||||
|
|
||||||
|
class TestCleanupCapabilityMapping(unittest.TestCase):
|
||||||
|
def test_cleanup_task_is_reviewer_role(self):
|
||||||
|
for key in ("pr_queue_cleanup", "pr-queue-cleanup"):
|
||||||
|
self.assertEqual(required_permission(key), "gitea.pr.review")
|
||||||
|
self.assertEqual(required_role(key), "reviewer")
|
||||||
|
|
||||||
|
def test_cleanup_task_registered_as_reviewer_route(self):
|
||||||
|
self.assertIn("pr_queue_cleanup", REVIEWER_TASKS)
|
||||||
|
|
||||||
|
def test_author_session_gets_wrong_role_stop(self):
|
||||||
|
result = route_task_session(
|
||||||
|
"pr_queue_cleanup",
|
||||||
|
active_profile="prgs-author",
|
||||||
|
active_role_kind="author",
|
||||||
|
allowed_in_current_session=False,
|
||||||
|
)
|
||||||
|
self.assertEqual(result["route_result"], "wrong_role_stop")
|
||||||
|
self.assertFalse(result["downstream_allowed"])
|
||||||
|
|
||||||
|
def test_reviewer_session_allowed(self):
|
||||||
|
result = route_task_session(
|
||||||
|
"pr_queue_cleanup",
|
||||||
|
active_profile="prgs-reviewer",
|
||||||
|
active_role_kind="reviewer",
|
||||||
|
allowed_in_current_session=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(result["route_result"], "allowed_current_session")
|
||||||
|
self.assertTrue(result["downstream_allowed"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestCleanupTaskGate(unittest.TestCase):
|
||||||
|
def test_author_tasks_blocked(self):
|
||||||
|
for task in (
|
||||||
|
"claim_issue",
|
||||||
|
"mark_issue",
|
||||||
|
"lock_issue",
|
||||||
|
"create_issue",
|
||||||
|
"create_branch",
|
||||||
|
"push_branch",
|
||||||
|
"create_pr",
|
||||||
|
"commit_files",
|
||||||
|
):
|
||||||
|
allowed, reasons = check_cleanup_task_allowed(task)
|
||||||
|
self.assertFalse(allowed, task)
|
||||||
|
self.assertTrue(reasons, task)
|
||||||
|
|
||||||
|
def test_review_task_allowed(self):
|
||||||
|
allowed, reasons = check_cleanup_task_allowed("review_pr")
|
||||||
|
self.assertTrue(allowed)
|
||||||
|
self.assertEqual(reasons, [])
|
||||||
|
|
||||||
|
def test_forbidden_set_covers_issue_filing_and_impl(self):
|
||||||
|
self.assertIn("create_issue", CLEANUP_FORBIDDEN_TASKS)
|
||||||
|
self.assertIn("create_branch", CLEANUP_FORBIDDEN_TASKS)
|
||||||
|
self.assertIn("commit_files", CLEANUP_FORBIDDEN_TASKS)
|
||||||
|
|
||||||
|
|
||||||
|
class TestCleanupRunState(unittest.TestCase):
|
||||||
|
def test_request_changes_stops_run(self):
|
||||||
|
state = resolve_cleanup_run_state("request_changes")
|
||||||
|
self.assertEqual(state["outcome"], STOP_AFTER_REQUEST_CHANGES)
|
||||||
|
self.assertFalse(state["further_mutation_allowed"])
|
||||||
|
|
||||||
|
def test_comment_and_skip_stop_run(self):
|
||||||
|
for decision in ("comment", "skip"):
|
||||||
|
state = resolve_cleanup_run_state(decision)
|
||||||
|
self.assertEqual(state["outcome"], STOP_AFTER_DECISION)
|
||||||
|
self.assertFalse(state["further_mutation_allowed"])
|
||||||
|
|
||||||
|
def test_approved_without_merge_auth_stops(self):
|
||||||
|
state = resolve_cleanup_run_state("approved", merge_gates_passed=True)
|
||||||
|
self.assertEqual(state["outcome"], STOP_APPROVED_NO_MERGE_AUTH)
|
||||||
|
self.assertFalse(state["further_mutation_allowed"])
|
||||||
|
|
||||||
|
def test_approved_with_merge_auth_continues_to_merge(self):
|
||||||
|
state = resolve_cleanup_run_state(
|
||||||
|
"approved",
|
||||||
|
merge_authorized_for_pr=True,
|
||||||
|
merge_gates_passed=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(state["outcome"], CONTINUE_TO_SAME_PR_MERGE)
|
||||||
|
self.assertTrue(state["further_mutation_allowed"])
|
||||||
|
self.assertEqual(state["allowed_mutation"], "merge same PR only")
|
||||||
|
|
||||||
|
def test_approved_with_auth_but_failed_gates_stops(self):
|
||||||
|
state = resolve_cleanup_run_state(
|
||||||
|
"approved",
|
||||||
|
merge_authorized_for_pr=True,
|
||||||
|
merge_gates_passed=False,
|
||||||
|
)
|
||||||
|
self.assertEqual(state["outcome"], STOP_APPROVED_MERGE_GATES_FAILED)
|
||||||
|
self.assertFalse(state["further_mutation_allowed"])
|
||||||
|
|
||||||
|
def test_merge_completed_stops(self):
|
||||||
|
state = resolve_cleanup_run_state(
|
||||||
|
"approved",
|
||||||
|
merge_authorized_for_pr=True,
|
||||||
|
merge_gates_passed=True,
|
||||||
|
merge_completed=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(state["outcome"], STOP_AFTER_MERGE)
|
||||||
|
self.assertFalse(state["further_mutation_allowed"])
|
||||||
|
|
||||||
|
def test_merge_blocker_stops(self):
|
||||||
|
state = resolve_cleanup_run_state(
|
||||||
|
"approved",
|
||||||
|
merge_authorized_for_pr=True,
|
||||||
|
merge_gates_passed=True,
|
||||||
|
merge_blocker=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(state["outcome"], STOP_AFTER_MERGE_BLOCKER)
|
||||||
|
self.assertFalse(state["further_mutation_allowed"])
|
||||||
|
|
||||||
|
def test_unknown_decision_fails_closed(self):
|
||||||
|
state = resolve_cleanup_run_state("ship_it")
|
||||||
|
self.assertEqual(state["outcome"], STOP_GATE_NOT_PROVEN)
|
||||||
|
self.assertFalse(state["further_mutation_allowed"])
|
||||||
|
|
||||||
|
|
||||||
|
def _clean_report(**overrides):
|
||||||
|
lines = {
|
||||||
|
"task": "Task: pr-queue-cleanup",
|
||||||
|
"workflow": f"Workflow source: {CLEANUP_WORKFLOW_PATH} (hash abc123def456)",
|
||||||
|
"pagination": (
|
||||||
|
"PR inventory pagination proof: inventory_complete=true, final "
|
||||||
|
"page, total_count 15"
|
||||||
|
),
|
||||||
|
"selected": "Selected PR: #371",
|
||||||
|
"decision": "Review decision: approved",
|
||||||
|
"merge_auth": "Merge authorized for PR: false",
|
||||||
|
"merge_result": "Merge result: not attempted",
|
||||||
|
"next": "Next suggested PR: #372",
|
||||||
|
"issue_mut": "Issue mutations: none",
|
||||||
|
"branch_mut": "Branch mutations: none",
|
||||||
|
}
|
||||||
|
lines.update(overrides)
|
||||||
|
return "\n".join(v for v in lines.values() if v)
|
||||||
|
|
||||||
|
|
||||||
|
class TestCleanupReportVerifier(unittest.TestCase):
|
||||||
|
def test_clean_single_pr_report_passes(self):
|
||||||
|
result = assess_pr_queue_cleanup_report(_clean_report())
|
||||||
|
self.assertTrue(result["proven"], result["reasons"])
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
|
||||||
|
def test_approved_and_merged_with_authorization_passes(self):
|
||||||
|
report = _clean_report(
|
||||||
|
merge_auth="Merge authorized for PR: true",
|
||||||
|
merge_result="Merge result: merged 0123456789ab",
|
||||||
|
)
|
||||||
|
result = assess_pr_queue_cleanup_report(report)
|
||||||
|
self.assertTrue(result["proven"], result["reasons"])
|
||||||
|
|
||||||
|
def test_request_changes_then_stop_passes(self):
|
||||||
|
report = _clean_report(decision="Review decision: request_changes")
|
||||||
|
result = assess_pr_queue_cleanup_report(report)
|
||||||
|
self.assertTrue(result["proven"], result["reasons"])
|
||||||
|
|
||||||
|
def test_multi_pr_selection_blocked(self):
|
||||||
|
report = _clean_report() + "\nSelected PR: #403\n"
|
||||||
|
result = assess_pr_queue_cleanup_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("multiple prs" in r.lower() for r in result["reasons"]),
|
||||||
|
result["reasons"],
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_two_terminal_mutations_blocked(self):
|
||||||
|
report = _clean_report() + "\nReview decision: request_changes"
|
||||||
|
result = assess_pr_queue_cleanup_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("terminal review mutations" in r for r in result["reasons"]),
|
||||||
|
result["reasons"],
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_missing_pagination_proof_blocked(self):
|
||||||
|
report = _clean_report(
|
||||||
|
pagination="PR inventory pagination proof: inventory listed"
|
||||||
|
)
|
||||||
|
result = assess_pr_queue_cleanup_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("pagination" in r.lower() for r in result["reasons"]),
|
||||||
|
result["reasons"],
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_missing_next_suggested_pr_blocked(self):
|
||||||
|
report = _clean_report(next="")
|
||||||
|
result = assess_pr_queue_cleanup_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("Next suggested PR" in r for r in result["reasons"]),
|
||||||
|
result["reasons"],
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_merge_without_authorization_blocked(self):
|
||||||
|
report = _clean_report(
|
||||||
|
merge_result="Merge result: merged abc123abc123",
|
||||||
|
)
|
||||||
|
result = assess_pr_queue_cleanup_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("authorization" in r.lower() for r in result["reasons"]),
|
||||||
|
result["reasons"],
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_issue_mutations_forbidden(self):
|
||||||
|
report = _clean_report(issue_mut="Issue mutations: gitea_mark_issue")
|
||||||
|
result = assess_pr_queue_cleanup_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("issue mutations" in r.lower() for r in result["reasons"]),
|
||||||
|
result["reasons"],
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_branch_mutations_forbidden(self):
|
||||||
|
report = _clean_report(branch_mut="Branch mutations: created feat/x")
|
||||||
|
result = assess_pr_queue_cleanup_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("branch mutations" in r.lower() for r in result["reasons"]),
|
||||||
|
result["reasons"],
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_missing_workflow_citation_blocked(self):
|
||||||
|
report = _clean_report(workflow="Workflow source: none")
|
||||||
|
result = assess_pr_queue_cleanup_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("pr-queue-cleanup.md" in r for r in result["reasons"]),
|
||||||
|
result["reasons"],
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_review_proofs_wrapper_matches_module(self):
|
||||||
|
direct = assess_pr_queue_cleanup_report(_clean_report())
|
||||||
|
wrapped = proofs_assess(_clean_report())
|
||||||
|
self.assertEqual(direct["proven"], wrapped["proven"])
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -0,0 +1,188 @@
|
|||||||
|
"""Issue #309: dedicated reconciler capability to close already-landed PRs.
|
||||||
|
|
||||||
|
The reconciler path must prove exact ``gitea.pr.close`` capability, must
|
||||||
|
never imply review/approve/request-changes/merge capability, and may close
|
||||||
|
a PR only after full already-landed proof.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
|
||||||
|
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
import role_session_router # noqa: E402
|
||||||
|
import task_capability_map # noqa: E402
|
||||||
|
from review_proofs import assess_reconciler_close_gate # noqa: E402
|
||||||
|
|
||||||
|
PINNED = "0fdc8f582026b72a229d59a172c0a63ac4aaeaf9"
|
||||||
|
OTHER = "a4060c5de00f2b1c9e88f4f6f0f3f9a7b2c1d0e9"
|
||||||
|
|
||||||
|
|
||||||
|
class TestReconcilerCapabilityMap(unittest.TestCase):
|
||||||
|
def test_reconcile_close_pr_task_maps_to_pr_close(self):
|
||||||
|
self.assertEqual(
|
||||||
|
task_capability_map.required_permission(
|
||||||
|
"reconcile_close_landed_pr"),
|
||||||
|
"gitea.pr.close",
|
||||||
|
)
|
||||||
|
self.assertEqual(
|
||||||
|
task_capability_map.required_role("reconcile_close_landed_pr"),
|
||||||
|
"reconciler",
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_reconcile_close_issue_task_maps_to_issue_close(self):
|
||||||
|
self.assertEqual(
|
||||||
|
task_capability_map.required_permission(
|
||||||
|
"reconcile_close_landed_issue"),
|
||||||
|
"gitea.issue.close",
|
||||||
|
)
|
||||||
|
self.assertEqual(
|
||||||
|
task_capability_map.required_role(
|
||||||
|
"reconcile_close_landed_issue"),
|
||||||
|
"reconciler",
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_reconciler_tasks_do_not_grant_review_permissions(self):
|
||||||
|
for task in ("reconcile_close_landed_pr",
|
||||||
|
"reconcile_close_landed_issue"):
|
||||||
|
permission = task_capability_map.required_permission(task)
|
||||||
|
self.assertNotIn("review", permission)
|
||||||
|
self.assertNotIn("approve", permission)
|
||||||
|
self.assertNotIn("merge", permission)
|
||||||
|
self.assertNotIn("request_changes", permission)
|
||||||
|
|
||||||
|
|
||||||
|
class TestReconcilerRouting(unittest.TestCase):
|
||||||
|
def test_reconciler_task_requires_reconciler_role(self):
|
||||||
|
self.assertEqual(
|
||||||
|
role_session_router.required_role_for_task(
|
||||||
|
"reconcile_close_landed_pr"),
|
||||||
|
"reconciler",
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_reconciler_task_blocked_in_author_session(self):
|
||||||
|
result = role_session_router.route_task_session(
|
||||||
|
"reconcile_close_landed_pr",
|
||||||
|
active_profile="prgs-author",
|
||||||
|
active_role_kind="author",
|
||||||
|
allowed_in_current_session=False,
|
||||||
|
)
|
||||||
|
self.assertEqual(result["route_result"], "wrong_role_stop")
|
||||||
|
self.assertFalse(result["downstream_allowed"])
|
||||||
|
|
||||||
|
def test_reconciler_task_allowed_in_matching_session(self):
|
||||||
|
result = role_session_router.route_task_session(
|
||||||
|
"reconcile_close_landed_pr",
|
||||||
|
active_profile="prgs-reconciler",
|
||||||
|
active_role_kind="reconciler",
|
||||||
|
allowed_in_current_session=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(result["route_result"], "allowed_current_session")
|
||||||
|
self.assertTrue(result["downstream_allowed"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestReconcilerCloseGate(unittest.TestCase):
|
||||||
|
def _gate(self, **overrides):
|
||||||
|
kwargs = {
|
||||||
|
"pr_number": 278,
|
||||||
|
"pr_state": "open",
|
||||||
|
"candidate_head_sha": PINNED,
|
||||||
|
"live_head_sha": PINNED,
|
||||||
|
"target_branch": "master",
|
||||||
|
"target_branch_sha": OTHER,
|
||||||
|
"head_is_ancestor_of_target": True,
|
||||||
|
"close_pr_capability": True,
|
||||||
|
"close_issue_capability": False,
|
||||||
|
"linked_issue_state": "closed",
|
||||||
|
}
|
||||||
|
kwargs.update(overrides)
|
||||||
|
return assess_reconciler_close_gate(**kwargs)
|
||||||
|
|
||||||
|
def test_already_landed_open_pr_close_allowed(self):
|
||||||
|
result = self._gate()
|
||||||
|
self.assertEqual(result["outcome"], "CLOSE_ALLOWED")
|
||||||
|
self.assertTrue(result["pr_close_allowed"])
|
||||||
|
|
||||||
|
def test_not_landed_pr_cannot_be_closed(self):
|
||||||
|
result = self._gate(head_is_ancestor_of_target=False)
|
||||||
|
self.assertEqual(result["outcome"], "NOT_LANDED_CLOSE_BLOCKED")
|
||||||
|
self.assertFalse(result["pr_close_allowed"])
|
||||||
|
|
||||||
|
def test_unchecked_ancestry_blocks_close(self):
|
||||||
|
result = self._gate(head_is_ancestor_of_target=None)
|
||||||
|
self.assertEqual(result["outcome"], "GATE_NOT_PROVEN")
|
||||||
|
self.assertFalse(result["pr_close_allowed"])
|
||||||
|
|
||||||
|
def test_stale_target_branch_sha_blocks_close(self):
|
||||||
|
result = self._gate(target_branch_sha=None)
|
||||||
|
self.assertEqual(result["outcome"], "GATE_NOT_PROVEN")
|
||||||
|
self.assertFalse(result["pr_close_allowed"])
|
||||||
|
|
||||||
|
def test_closed_pr_state_blocks_close(self):
|
||||||
|
result = self._gate(pr_state="closed")
|
||||||
|
self.assertEqual(result["outcome"], "GATE_NOT_PROVEN")
|
||||||
|
self.assertFalse(result["pr_close_allowed"])
|
||||||
|
|
||||||
|
def test_changed_head_blocks_close(self):
|
||||||
|
result = self._gate(live_head_sha=OTHER)
|
||||||
|
self.assertEqual(result["outcome"], "GATE_NOT_PROVEN")
|
||||||
|
self.assertFalse(result["pr_close_allowed"])
|
||||||
|
|
||||||
|
def test_missing_close_capability_produces_recovery_handoff(self):
|
||||||
|
result = self._gate(close_pr_capability=False)
|
||||||
|
self.assertEqual(result["outcome"], "RECOVERY_HANDOFF_REQUIRED")
|
||||||
|
self.assertFalse(result["pr_close_allowed"])
|
||||||
|
self.assertTrue(any(
|
||||||
|
"gitea.pr.close" in reason for reason in result["reasons"]
|
||||||
|
))
|
||||||
|
|
||||||
|
def test_linked_issue_already_closed_skips_issue_close(self):
|
||||||
|
result = self._gate(linked_issue_state="closed",
|
||||||
|
close_issue_capability=True)
|
||||||
|
self.assertFalse(result["issue_close_allowed"])
|
||||||
|
|
||||||
|
def test_linked_open_issue_close_requires_capability(self):
|
||||||
|
allowed = self._gate(
|
||||||
|
linked_issue_state="open",
|
||||||
|
issue_resolved_by_landing=True,
|
||||||
|
close_issue_capability=True,
|
||||||
|
)
|
||||||
|
self.assertTrue(allowed["issue_close_allowed"])
|
||||||
|
|
||||||
|
blocked = self._gate(
|
||||||
|
linked_issue_state="open",
|
||||||
|
issue_resolved_by_landing=True,
|
||||||
|
close_issue_capability=False,
|
||||||
|
)
|
||||||
|
self.assertFalse(blocked["issue_close_allowed"])
|
||||||
|
|
||||||
|
def test_gate_never_allows_review_mutations(self):
|
||||||
|
for result in (
|
||||||
|
self._gate(),
|
||||||
|
self._gate(head_is_ancestor_of_target=False),
|
||||||
|
self._gate(close_pr_capability=False),
|
||||||
|
):
|
||||||
|
self.assertFalse(result["review_allowed"])
|
||||||
|
self.assertFalse(result["approve_allowed"])
|
||||||
|
self.assertFalse(result["request_changes_allowed"])
|
||||||
|
self.assertFalse(result["merge_allowed"])
|
||||||
|
|
||||||
|
def test_report_fields_listed_for_close(self):
|
||||||
|
result = self._gate()
|
||||||
|
for field in (
|
||||||
|
"identity/profile",
|
||||||
|
"close capability proof",
|
||||||
|
"PR live state",
|
||||||
|
"candidate head SHA",
|
||||||
|
"target branch SHA",
|
||||||
|
"ancestor proof",
|
||||||
|
"linked issue status",
|
||||||
|
"PR close result",
|
||||||
|
"issue close result",
|
||||||
|
"no review/merge confirmation",
|
||||||
|
):
|
||||||
|
self.assertIn(field, result["required_report_fields"])
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -0,0 +1,162 @@
|
|||||||
|
"""Tests for the dedicated reconciliation controller-handoff schema (#303).
|
||||||
|
|
||||||
|
Already-landed PR reconciliation runs must emit the reconciliation
|
||||||
|
schema, not stale author/reviewer handoff fields; observed git ref
|
||||||
|
mutations (fetch) must be reported, and legacy fields fail validation.
|
||||||
|
"""
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
from review_proofs import assess_reconciliation_handoff # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
def _good_handoff(**overrides):
|
||||||
|
fields = {
|
||||||
|
"Task": "Reconcile already-landed open PRs",
|
||||||
|
"Repo": "prgs/Scaled-Tech-Consulting/Gitea-Tools",
|
||||||
|
"Role/profile": "prgs-reconciler",
|
||||||
|
"Identity": "jcwalker3",
|
||||||
|
"Selected PR": "278",
|
||||||
|
"PR live state": "open",
|
||||||
|
"Candidate head SHA": "2a544b7",
|
||||||
|
"Target branch": "master",
|
||||||
|
"Target branch SHA": "fa0cb12",
|
||||||
|
"Ancestor proof": "candidate head is ancestor of target branch SHA",
|
||||||
|
"Linked issue": "263",
|
||||||
|
"Linked issue live status": "open (fetched live this session)",
|
||||||
|
"Eligibility class": "ALREADY_LANDED_RECONCILE_REQUIRED",
|
||||||
|
"Capabilities proven": "gitea.pr.comment",
|
||||||
|
"Missing capabilities": "gitea.pr.close",
|
||||||
|
"PR comments posted": "1 (PR #278 reconciliation notice)",
|
||||||
|
"Issue comments posted": "none",
|
||||||
|
"PRs closed": "none",
|
||||||
|
"Issues closed": "none",
|
||||||
|
"Git ref mutations": "git fetch prgs master",
|
||||||
|
"Worktree mutations": "none",
|
||||||
|
"MCP/Gitea mutations": "PR comment on #278",
|
||||||
|
"External-state mutations": "none",
|
||||||
|
"Read-only diagnostics": "gitea_view_pr 278, gitea_view_issue 263",
|
||||||
|
"Blocker": "PR close capability missing",
|
||||||
|
"Safe next action": "operator closes PR #278 or grants close capability",
|
||||||
|
"No review/merge confirmation": "no review or merge mutations performed",
|
||||||
|
}
|
||||||
|
fields.update(overrides)
|
||||||
|
lines = ["Controller Handoff"]
|
||||||
|
lines += [f"- {k}: {v}" for k, v in fields.items() if v is not None]
|
||||||
|
return "\n".join(lines) + "\n"
|
||||||
|
|
||||||
|
|
||||||
|
class TestReconciliationSchemaPasses(unittest.TestCase):
|
||||||
|
def test_blocked_reconciliation_passes(self):
|
||||||
|
result = assess_reconciliation_handoff(
|
||||||
|
_good_handoff(),
|
||||||
|
observed_commands=["git fetch prgs master"],
|
||||||
|
)
|
||||||
|
self.assertTrue(result["complete"])
|
||||||
|
self.assertEqual(result["reasons"], [])
|
||||||
|
|
||||||
|
def test_successful_pr_close_passes(self):
|
||||||
|
report = _good_handoff(**{
|
||||||
|
"Missing capabilities": "none",
|
||||||
|
"PRs closed": "PR #278",
|
||||||
|
"Blocker": "none",
|
||||||
|
"Safe next action": "none; reconciliation complete",
|
||||||
|
})
|
||||||
|
result = assess_reconciliation_handoff(
|
||||||
|
report, observed_commands=["git fetch prgs master"]
|
||||||
|
)
|
||||||
|
self.assertTrue(result["complete"])
|
||||||
|
|
||||||
|
def test_comment_only_reconciliation_passes(self):
|
||||||
|
report = _good_handoff(**{
|
||||||
|
"Git ref mutations": "none",
|
||||||
|
})
|
||||||
|
result = assess_reconciliation_handoff(report, observed_commands=[])
|
||||||
|
self.assertTrue(result["complete"])
|
||||||
|
|
||||||
|
def test_noop_already_closed_passes(self):
|
||||||
|
report = _good_handoff(**{
|
||||||
|
"PR live state": "closed",
|
||||||
|
"PR comments posted": "none",
|
||||||
|
"MCP/Gitea mutations": "none",
|
||||||
|
"Git ref mutations": "none",
|
||||||
|
"Blocker": "none",
|
||||||
|
"Safe next action": "none; PR already closed",
|
||||||
|
})
|
||||||
|
result = assess_reconciliation_handoff(report, observed_commands=[])
|
||||||
|
self.assertTrue(result["complete"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestSchemaViolationsFail(unittest.TestCase):
|
||||||
|
def test_missing_linked_issue_proof_fails(self):
|
||||||
|
report = _good_handoff(**{"Linked issue live status": None})
|
||||||
|
result = assess_reconciliation_handoff(report, observed_commands=[])
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("linked issue live status" in r.lower()
|
||||||
|
for r in result["reasons"])
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_wrong_eligibility_class_fails(self):
|
||||||
|
report = _good_handoff(**{"Eligibility class": "REVIEW_ELIGIBLE"})
|
||||||
|
result = assess_reconciliation_handoff(report, observed_commands=[])
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
|
||||||
|
def test_missing_handoff_section_fails(self):
|
||||||
|
result = assess_reconciliation_handoff(
|
||||||
|
"no handoff here", observed_commands=[]
|
||||||
|
)
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestStaleFieldsFail(unittest.TestCase):
|
||||||
|
def test_pr_number_opened_fails(self):
|
||||||
|
report = _good_handoff() + "- PR number opened: 278\n"
|
||||||
|
result = assess_reconciliation_handoff(report, observed_commands=[])
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("pr number opened" in r.lower() for r in result["reasons"])
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_pinned_reviewed_head_fails(self):
|
||||||
|
report = _good_handoff() + "- Pinned reviewed head: 2a544b7\n"
|
||||||
|
result = assess_reconciliation_handoff(report, observed_commands=[])
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
|
||||||
|
def test_scratch_worktree_used_fails(self):
|
||||||
|
report = _good_handoff() + "- Scratch worktree used: False\n"
|
||||||
|
result = assess_reconciliation_handoff(report, observed_commands=[])
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
|
||||||
|
def test_workspace_mutations_fails(self):
|
||||||
|
report = _good_handoff() + "- Workspace mutations: None\n"
|
||||||
|
result = assess_reconciliation_handoff(report, observed_commands=[])
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
|
||||||
|
def test_author_lock_fields_fail(self):
|
||||||
|
report = (
|
||||||
|
_good_handoff()
|
||||||
|
+ "- Issue lock proof: locked before diff\n"
|
||||||
|
+ "- Claim/comment status: claimed\n"
|
||||||
|
)
|
||||||
|
result = assess_reconciliation_handoff(report, observed_commands=[])
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestObservedFetchMustBeReported(unittest.TestCase):
|
||||||
|
def test_fetch_with_ref_mutations_none_fails(self):
|
||||||
|
report = _good_handoff(**{"Git ref mutations": "none"})
|
||||||
|
result = assess_reconciliation_handoff(
|
||||||
|
report, observed_commands=["git fetch prgs master"]
|
||||||
|
)
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("git ref mutation" in r.lower() for r in result["reasons"])
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -275,6 +275,28 @@ class TestResolveTaskCapability(unittest.TestCase):
|
|||||||
self.assertEqual(res["required_role_kind"], "author")
|
self.assertEqual(res["required_role_kind"], "author")
|
||||||
self.assertIn("author", res["exact_safe_next_action"].lower())
|
self.assertIn("author", res["exact_safe_next_action"].lower())
|
||||||
|
|
||||||
|
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
||||||
|
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||||
|
def test_resolve_pr_queue_cleanup_author_profile_blocked(self, _auth, _api):
|
||||||
|
with patch.dict(os.environ, self._env("author-profile")):
|
||||||
|
res = mcp_server.gitea_resolve_task_capability(
|
||||||
|
task="pr_queue_cleanup", remote="prgs"
|
||||||
|
)
|
||||||
|
self.assertEqual(res["required_role_kind"], "reviewer")
|
||||||
|
self.assertFalse(res["allowed_in_current_session"])
|
||||||
|
self.assertTrue(res["stop_required"])
|
||||||
|
|
||||||
|
@patch("mcp_server.api_request", return_value={"login": "reviewer-user"})
|
||||||
|
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
|
||||||
|
def test_resolve_pr_queue_cleanup_reviewer_profile_allowed(self, _auth, _api):
|
||||||
|
with patch.dict(os.environ, self._env("reviewer-profile")):
|
||||||
|
res = mcp_server.gitea_resolve_task_capability(
|
||||||
|
task="pr_queue_cleanup", remote="prgs"
|
||||||
|
)
|
||||||
|
self.assertEqual(res["required_operation_permission"], "gitea.pr.review")
|
||||||
|
self.assertTrue(res["allowed_in_current_session"])
|
||||||
|
self.assertFalse(res["stop_required"])
|
||||||
|
|
||||||
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
||||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||||
def test_close_pr_known_and_lookalike_tasks_still_fail_closed(self, _auth, _api):
|
def test_close_pr_known_and_lookalike_tasks_still_fail_closed(self, _auth, _api):
|
||||||
|
|||||||
@@ -0,0 +1,121 @@
|
|||||||
|
"""Tests for enforced PR review/merge state machine (#290)."""
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
import review_merge_state_machine as rmsm # noqa: E402
|
||||||
|
|
||||||
|
FULL_REVIEW_COMPLETION = {state: True for state in rmsm.REVIEW_MERGE_STATES}
|
||||||
|
|
||||||
|
|
||||||
|
def _completion_through(state_name: str) -> dict[str, bool]:
|
||||||
|
idx = rmsm.state_index(state_name)
|
||||||
|
return {state: i <= idx for i, state in enumerate(rmsm.REVIEW_MERGE_STATES)}
|
||||||
|
|
||||||
|
|
||||||
|
class TestWorkflowBlockers(unittest.TestCase):
|
||||||
|
def test_infra_stop_blocks_all_states(self):
|
||||||
|
result = rmsm.assess_workflow_blockers(infra_stop=True)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertEqual(result["forbidden_states"], list(rmsm.REVIEW_MERGE_STATES))
|
||||||
|
|
||||||
|
def test_infra_stop_forbids_advancement(self):
|
||||||
|
result = rmsm.assess_state_advancement(
|
||||||
|
{},
|
||||||
|
target_state="INVENTORY",
|
||||||
|
infra_stop=True,
|
||||||
|
)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertFalse(result["allowed"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestStateAdvancement(unittest.TestCase):
|
||||||
|
def test_cannot_skip_inventory(self):
|
||||||
|
result = rmsm.assess_state_advancement(
|
||||||
|
{"PRECHECK": True},
|
||||||
|
target_state="SELECT_PR",
|
||||||
|
)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertEqual(result["next_allowed_state"], "INVENTORY")
|
||||||
|
|
||||||
|
def test_sequential_advance_allowed(self):
|
||||||
|
completion = _completion_through("INVENTORY")
|
||||||
|
result = rmsm.assess_state_advancement(
|
||||||
|
completion,
|
||||||
|
target_state="SELECT_PR",
|
||||||
|
)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
self.assertTrue(result["allowed"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestApproveAndMergeGates(unittest.TestCase):
|
||||||
|
def test_approve_blocked_without_validate(self):
|
||||||
|
completion = _completion_through("PIN_HEAD_SHA")
|
||||||
|
result = rmsm.can_approve(completion)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertIn("VALIDATE", ",".join(result["missing_states"]))
|
||||||
|
|
||||||
|
def test_approve_allowed_when_review_path_complete(self):
|
||||||
|
completion = _completion_through("REVIEW_DECISION")
|
||||||
|
result = rmsm.can_approve(completion)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
self.assertTrue(result["allowed"])
|
||||||
|
|
||||||
|
def test_merge_blocked_without_pre_merge_gates(self):
|
||||||
|
completion = _completion_through("APPROVE_OR_REQUEST_CHANGES")
|
||||||
|
result = rmsm.can_merge(completion)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertTrue(result["missing_pre_merge_gates"])
|
||||||
|
|
||||||
|
def test_merge_allowed_with_pre_merge_gates(self):
|
||||||
|
completion = _completion_through("PRE_MERGE_RECHECK")
|
||||||
|
gates = {gate: True for gate in rmsm._PRE_MERGE_REQUIRED_GATES}
|
||||||
|
result = rmsm.can_merge(completion, pre_merge_gates=gates)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
self.assertTrue(result["allowed"])
|
||||||
|
|
||||||
|
def test_merge_blocked_when_head_sha_gate_fails(self):
|
||||||
|
completion = _completion_through("PRE_MERGE_RECHECK")
|
||||||
|
gates = {gate: True for gate in rmsm._PRE_MERGE_REQUIRED_GATES}
|
||||||
|
gates["reviewed_head_sha_unchanged"] = False
|
||||||
|
result = rmsm.can_merge(completion, pre_merge_gates=gates)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertIn("reviewed_head_sha_unchanged", result["missing_pre_merge_gates"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestRecoveryHandoff(unittest.TestCase):
|
||||||
|
def test_blocked_handoff_without_replay_passes(self):
|
||||||
|
report = (
|
||||||
|
"Blocked recovery handoff.\n"
|
||||||
|
"Restart the full workflow after infra_stop clears.\n"
|
||||||
|
"No approve/merge performed."
|
||||||
|
)
|
||||||
|
result = rmsm.assess_blocked_recovery_handoff(report)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
|
||||||
|
def test_blocked_handoff_with_merge_replay_fails(self):
|
||||||
|
report = "Please merge PR #12 now using gitea_merge_pr."
|
||||||
|
result = rmsm.assess_blocked_recovery_handoff(report)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestFinalReportClaims(unittest.TestCase):
|
||||||
|
def test_ready_to_merge_claim_without_gates_fails(self):
|
||||||
|
result = rmsm.assess_final_report_state_claims(
|
||||||
|
"PR is ready to merge after validation.",
|
||||||
|
state_completion=_completion_through("VALIDATE"),
|
||||||
|
)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
|
||||||
|
def test_reviewed_claim_without_validate_fails(self):
|
||||||
|
result = rmsm.assess_final_report_state_claims(
|
||||||
|
"PR reviewed and looks good.",
|
||||||
|
state_completion={"PRECHECK": True},
|
||||||
|
)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -26,6 +26,8 @@ from review_proofs import ( # noqa: E402
|
|||||||
assess_already_landed_report_state,
|
assess_already_landed_report_state,
|
||||||
assess_already_landed_review_gate,
|
assess_already_landed_review_gate,
|
||||||
assess_author_pr_report,
|
assess_author_pr_report,
|
||||||
|
assess_partial_reconciliation_report,
|
||||||
|
resolve_partial_reconciliation_plan,
|
||||||
assess_validation_environment_proof,
|
assess_validation_environment_proof,
|
||||||
assess_full_suite_failure_approval_gate,
|
assess_full_suite_failure_approval_gate,
|
||||||
assess_queue_ordering_proof,
|
assess_queue_ordering_proof,
|
||||||
@@ -2641,6 +2643,133 @@ class TestReviewerBaselineValidationProof(unittest.TestCase):
|
|||||||
self.assertFalse(report["baseline_validation_proven"])
|
self.assertFalse(report["baseline_validation_proven"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestPartialReconciliationPlan(unittest.TestCase):
|
||||||
|
"""Issue #302: policy when PR-close capability is missing (Option B)."""
|
||||||
|
|
||||||
|
def _plan(self, **overrides):
|
||||||
|
kwargs = {
|
||||||
|
"ancestor_proven": True,
|
||||||
|
"capabilities": {
|
||||||
|
"close_pr": False,
|
||||||
|
"comment_pr": True,
|
||||||
|
"close_issue": True,
|
||||||
|
"comment_issue": True,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
kwargs.update(overrides)
|
||||||
|
return resolve_partial_reconciliation_plan(**kwargs)
|
||||||
|
|
||||||
|
def test_close_capability_present_allows_full_reconcile(self):
|
||||||
|
plan = self._plan(capabilities={
|
||||||
|
"close_pr": True, "comment_pr": True,
|
||||||
|
"close_issue": True, "comment_issue": True,
|
||||||
|
})
|
||||||
|
self.assertEqual(plan["outcome"], "FULL_RECONCILE_CLOSE_ALLOWED")
|
||||||
|
self.assertIn("close_pr", plan["allowed_mutations"])
|
||||||
|
|
||||||
|
def test_close_missing_with_comment_posts_comment_then_stops(self):
|
||||||
|
plan = self._plan()
|
||||||
|
self.assertEqual(
|
||||||
|
plan["outcome"], "PARTIAL_RECONCILE_COMMENT_THEN_STOP")
|
||||||
|
self.assertEqual(plan["allowed_mutations"], ("comment_pr",))
|
||||||
|
for field in (
|
||||||
|
"PR head SHA",
|
||||||
|
"target branch SHA",
|
||||||
|
"ancestor proof",
|
||||||
|
"linked issue status",
|
||||||
|
"required missing capability",
|
||||||
|
):
|
||||||
|
self.assertIn(field, plan["required_comment_fields"])
|
||||||
|
|
||||||
|
def test_comment_capability_missing_produces_handoff_only(self):
|
||||||
|
plan = self._plan(capabilities={
|
||||||
|
"close_pr": False, "comment_pr": False,
|
||||||
|
"close_issue": True, "comment_issue": True,
|
||||||
|
})
|
||||||
|
self.assertEqual(plan["outcome"], "RECOVERY_HANDOFF_ONLY")
|
||||||
|
self.assertEqual(plan["allowed_mutations"], ())
|
||||||
|
|
||||||
|
def test_all_capabilities_missing_produces_handoff_only(self):
|
||||||
|
plan = self._plan(capabilities={
|
||||||
|
"close_pr": False, "comment_pr": False,
|
||||||
|
"close_issue": False, "comment_issue": False,
|
||||||
|
})
|
||||||
|
self.assertEqual(plan["outcome"], "RECOVERY_HANDOFF_ONLY")
|
||||||
|
self.assertEqual(plan["allowed_mutations"], ())
|
||||||
|
|
||||||
|
def test_unproven_ancestry_blocks_all_mutations(self):
|
||||||
|
plan = self._plan(ancestor_proven=False, capabilities={
|
||||||
|
"close_pr": True, "comment_pr": True,
|
||||||
|
"close_issue": True, "comment_issue": True,
|
||||||
|
})
|
||||||
|
self.assertEqual(plan["outcome"], "GATE_NOT_PROVEN")
|
||||||
|
self.assertEqual(plan["allowed_mutations"], ())
|
||||||
|
|
||||||
|
def test_missing_capability_dict_fails_closed(self):
|
||||||
|
plan = self._plan(capabilities=None)
|
||||||
|
self.assertEqual(plan["outcome"], "RECOVERY_HANDOFF_ONLY")
|
||||||
|
self.assertEqual(plan["allowed_mutations"], ())
|
||||||
|
|
||||||
|
|
||||||
|
class TestPartialReconciliationReport(unittest.TestCase):
|
||||||
|
"""Issue #302: reports must explain why comments were or were not posted."""
|
||||||
|
|
||||||
|
def _plan(self, outcome):
|
||||||
|
capabilities = {
|
||||||
|
"FULL_RECONCILE_CLOSE_ALLOWED": {
|
||||||
|
"close_pr": True, "comment_pr": True,
|
||||||
|
"close_issue": True, "comment_issue": True,
|
||||||
|
},
|
||||||
|
"PARTIAL_RECONCILE_COMMENT_THEN_STOP": {
|
||||||
|
"close_pr": False, "comment_pr": True,
|
||||||
|
"close_issue": True, "comment_issue": True,
|
||||||
|
},
|
||||||
|
"RECOVERY_HANDOFF_ONLY": {
|
||||||
|
"close_pr": False, "comment_pr": False,
|
||||||
|
"close_issue": False, "comment_issue": False,
|
||||||
|
},
|
||||||
|
}[outcome]
|
||||||
|
return resolve_partial_reconciliation_plan(
|
||||||
|
ancestor_proven=True, capabilities=capabilities)
|
||||||
|
|
||||||
|
def test_partial_report_requires_comment_and_missing_capability(self):
|
||||||
|
plan = self._plan("PARTIAL_RECONCILE_COMMENT_THEN_STOP")
|
||||||
|
good = (
|
||||||
|
"Reconciliation comment posted on PR #278 with ancestor proof. "
|
||||||
|
"PR close skipped: close capability gitea.pr.close missing; "
|
||||||
|
"stopped for authorized close."
|
||||||
|
)
|
||||||
|
result = assess_partial_reconciliation_report(good, plan=plan)
|
||||||
|
self.assertTrue(result["complete"])
|
||||||
|
|
||||||
|
bad = "Stopped. Nothing done."
|
||||||
|
result = assess_partial_reconciliation_report(bad, plan=plan)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
|
||||||
|
def test_handoff_only_report_requires_no_comment_explanation(self):
|
||||||
|
plan = self._plan("RECOVERY_HANDOFF_ONLY")
|
||||||
|
good = (
|
||||||
|
"No reconciliation comment posted: comment capability missing. "
|
||||||
|
"No Gitea mutation performed; recovery handoff produced."
|
||||||
|
)
|
||||||
|
result = assess_partial_reconciliation_report(good, plan=plan)
|
||||||
|
self.assertTrue(result["complete"])
|
||||||
|
|
||||||
|
bad = "Recovery handoff produced."
|
||||||
|
result = assess_partial_reconciliation_report(bad, plan=plan)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
|
||||||
|
def test_full_reconcile_report_requires_close_result(self):
|
||||||
|
plan = self._plan("FULL_RECONCILE_CLOSE_ALLOWED")
|
||||||
|
good = "PR close result: closed PR #278 after ancestor proof."
|
||||||
|
result = assess_partial_reconciliation_report(good, plan=plan)
|
||||||
|
self.assertTrue(result["complete"])
|
||||||
|
|
||||||
|
bad = "Reconciliation done."
|
||||||
|
result = assess_partial_reconciliation_report(bad, plan=plan)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
|
||||||
|
|
||||||
class TestAlreadyLandedReviewGate(unittest.TestCase):
|
class TestAlreadyLandedReviewGate(unittest.TestCase):
|
||||||
"""Issue #292: PR head already on target blocks approval and merge."""
|
"""Issue #292: PR head already on target blocks approval and merge."""
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,108 @@
|
|||||||
|
"""Tests for canonical review-workflow citation and version proof (#296).
|
||||||
|
|
||||||
|
PR review/merge reports must prove they loaded the canonical workflow
|
||||||
|
(workflows/review-merge-pr.md) and cite the workflow version/hash used;
|
||||||
|
a stale or missing hash fails so prompt drift is caught.
|
||||||
|
"""
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
from review_proofs import ( # noqa: E402
|
||||||
|
assess_review_workflow_source,
|
||||||
|
compute_workflow_hash,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
CANONICAL_TEXT = "# review-merge-pr workflow v1\nstep one\n"
|
||||||
|
CANONICAL_HASH = compute_workflow_hash(CANONICAL_TEXT)
|
||||||
|
|
||||||
|
|
||||||
|
def _report(citation=True, version=CANONICAL_HASH):
|
||||||
|
lines = ["Task: review next eligible PR"]
|
||||||
|
if citation:
|
||||||
|
lines.append(
|
||||||
|
"Workflow source: skills/llm-project-workflow/"
|
||||||
|
"workflows/review-merge-pr.md (loaded via mcp_get_skill_guide)"
|
||||||
|
)
|
||||||
|
if version is not None:
|
||||||
|
lines.append(f"- Workflow version: {version}")
|
||||||
|
return "\n".join(lines) + "\n"
|
||||||
|
|
||||||
|
|
||||||
|
class TestComputeWorkflowHash(unittest.TestCase):
|
||||||
|
def test_deterministic(self):
|
||||||
|
self.assertEqual(
|
||||||
|
compute_workflow_hash(CANONICAL_TEXT),
|
||||||
|
compute_workflow_hash(CANONICAL_TEXT),
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_changes_with_content(self):
|
||||||
|
self.assertNotEqual(
|
||||||
|
compute_workflow_hash(CANONICAL_TEXT),
|
||||||
|
compute_workflow_hash(CANONICAL_TEXT + "extra rule\n"),
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_short_hex(self):
|
||||||
|
h = compute_workflow_hash(CANONICAL_TEXT)
|
||||||
|
self.assertEqual(len(h), 12)
|
||||||
|
int(h, 16) # raises if not hex
|
||||||
|
|
||||||
|
|
||||||
|
class TestReviewWorkflowSource(unittest.TestCase):
|
||||||
|
def test_citation_and_matching_hash_passes(self):
|
||||||
|
result = assess_review_workflow_source(
|
||||||
|
_report(), canonical_hash=CANONICAL_HASH
|
||||||
|
)
|
||||||
|
self.assertTrue(result["complete"])
|
||||||
|
self.assertEqual(result["reasons"], [])
|
||||||
|
|
||||||
|
def test_missing_citation_fails(self):
|
||||||
|
result = assess_review_workflow_source(
|
||||||
|
_report(citation=False), canonical_hash=CANONICAL_HASH
|
||||||
|
)
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("review-merge-pr" in r.lower() for r in result["reasons"])
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_missing_version_field_fails(self):
|
||||||
|
result = assess_review_workflow_source(
|
||||||
|
_report(version=None), canonical_hash=CANONICAL_HASH
|
||||||
|
)
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("workflow version" in r.lower() for r in result["reasons"])
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_stale_hash_fails(self):
|
||||||
|
result = assess_review_workflow_source(
|
||||||
|
_report(version="deadbeef0000"), canonical_hash=CANONICAL_HASH
|
||||||
|
)
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("stale" in r.lower() or "does not match" in r.lower()
|
||||||
|
for r in result["reasons"])
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_version_none_fails(self):
|
||||||
|
result = assess_review_workflow_source(
|
||||||
|
_report(version="none"), canonical_hash=CANONICAL_HASH
|
||||||
|
)
|
||||||
|
self.assertFalse(result["complete"])
|
||||||
|
|
||||||
|
def test_without_canonical_hash_field_still_required(self):
|
||||||
|
# No canonical hash supplied (e.g. offline validation): citation
|
||||||
|
# and a populated version field are still required.
|
||||||
|
self.assertTrue(
|
||||||
|
assess_review_workflow_source(_report())["complete"]
|
||||||
|
)
|
||||||
|
self.assertFalse(
|
||||||
|
assess_review_workflow_source(_report(version=None))["complete"]
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -0,0 +1,131 @@
|
|||||||
|
"""Tests for already-landed handoff verifier (#299)."""
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
from reviewer_already_landed_handoff import ( # noqa: E402
|
||||||
|
ALREADY_LANDED_STATE,
|
||||||
|
assess_already_landed_handoff_report,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _good_report() -> str:
|
||||||
|
return "\n".join([
|
||||||
|
"## PR reconciliation summary",
|
||||||
|
f"Eligibility class: {ALREADY_LANDED_STATE}",
|
||||||
|
"Candidate head SHA: 2a544f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||||
|
"Reviewed head SHA: none",
|
||||||
|
"Review worktree used: false",
|
||||||
|
"Ancestor proof: PR head is ancestor of prgs/master",
|
||||||
|
"",
|
||||||
|
"## Controller Handoff",
|
||||||
|
"- Selected PR: #278",
|
||||||
|
f"- Eligibility class: {ALREADY_LANDED_STATE}",
|
||||||
|
"- Candidate head SHA: 2a544f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||||
|
"- Reviewed head SHA: none",
|
||||||
|
"- Target branch: master",
|
||||||
|
"- Target branch SHA: abc123def4567890abcdef1234567890abcdef12",
|
||||||
|
"- Review worktree used: false",
|
||||||
|
"- Git ref mutations: git fetch prgs master",
|
||||||
|
"- Review decision: none",
|
||||||
|
"- Merge result: none",
|
||||||
|
"- Linked issue status: open",
|
||||||
|
"- Safe next action: reconcile open PR and linked issue",
|
||||||
|
])
|
||||||
|
|
||||||
|
|
||||||
|
class TestAlreadyLandedHandoff(unittest.TestCase):
|
||||||
|
def test_clean_handoff_passes(self):
|
||||||
|
result = assess_already_landed_handoff_report(_good_report())
|
||||||
|
self.assertTrue(result["proven"], result["reasons"])
|
||||||
|
self.assertTrue(result["gate_active"])
|
||||||
|
|
||||||
|
def test_inactive_gate_skips_checks(self):
|
||||||
|
report = "## Controller Handoff\n- Selected PR: #12\n- Review decision: approve"
|
||||||
|
result = assess_already_landed_handoff_report(report)
|
||||||
|
self.assertTrue(result["proven"])
|
||||||
|
self.assertFalse(result["gate_active"])
|
||||||
|
|
||||||
|
def test_rejects_stale_pinned_reviewed_head(self):
|
||||||
|
report = _good_report().replace(
|
||||||
|
"- Candidate head SHA:",
|
||||||
|
"- Pinned reviewed head: 2a544f582026b72a229d59a172c0a63ac4aaeaf9\n"
|
||||||
|
"- Candidate head SHA:",
|
||||||
|
)
|
||||||
|
result = assess_already_landed_handoff_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("pinned reviewed head" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_rejects_scratch_worktree_used(self):
|
||||||
|
report = _good_report() + "\n- Scratch worktree used: false"
|
||||||
|
result = assess_already_landed_handoff_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("scratch worktree" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_rejects_mutations_none_with_git_fetch(self):
|
||||||
|
report = _good_report().replace(
|
||||||
|
"- Git ref mutations: git fetch prgs master",
|
||||||
|
"- Git ref mutations: none\n- Mutations: None",
|
||||||
|
)
|
||||||
|
result = assess_already_landed_handoff_report(
|
||||||
|
report,
|
||||||
|
command_log=[{"command": "git fetch prgs master", "performed": True}],
|
||||||
|
)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
joined = " ".join(result["reasons"]).lower()
|
||||||
|
self.assertIn("mutations", joined)
|
||||||
|
self.assertIn("git ref", joined)
|
||||||
|
|
||||||
|
def test_rejects_workspace_mutations_none(self):
|
||||||
|
report = _good_report() + "\n- Workspace mutations: None"
|
||||||
|
result = assess_already_landed_handoff_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("workspace mutations" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_rejects_reviewed_head_sha_when_gate_fired(self):
|
||||||
|
report = _good_report().replace("- Reviewed head SHA: none", "- Reviewed head SHA: abc123")
|
||||||
|
result = assess_already_landed_handoff_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("reviewed head sha" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_rejects_narrative_handoff_eligibility_mismatch(self):
|
||||||
|
report = _good_report().replace(
|
||||||
|
f"Eligibility class: {ALREADY_LANDED_STATE}",
|
||||||
|
"Eligibility class: NORMAL_REVIEW_CANDIDATE",
|
||||||
|
1,
|
||||||
|
)
|
||||||
|
result = assess_already_landed_handoff_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("eligibility class" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_session_gate_fired_without_text_marker(self):
|
||||||
|
report = "## Controller Handoff\n- Selected PR: #1\n- Pinned reviewed head: abc"
|
||||||
|
result = assess_already_landed_handoff_report(
|
||||||
|
report,
|
||||||
|
handoff_session={"gate_fired": True},
|
||||||
|
)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(result["gate_active"])
|
||||||
|
|
||||||
|
def test_infra_stop_style_normal_handoff_not_blocked(self):
|
||||||
|
report = "\n".join([
|
||||||
|
"## Controller Handoff",
|
||||||
|
"- Selected PR: #12",
|
||||||
|
"- Review decision: request_changes",
|
||||||
|
"- Pinned reviewed head: abc123def4567890abcdef1234567890abcdef12",
|
||||||
|
])
|
||||||
|
result = assess_already_landed_handoff_report(report)
|
||||||
|
self.assertTrue(result["proven"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestExport(unittest.TestCase):
|
||||||
|
def test_review_proofs_reexport(self):
|
||||||
|
from review_proofs import assess_already_landed_handoff_report as exported
|
||||||
|
|
||||||
|
self.assertTrue(callable(exported))
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -0,0 +1,106 @@
|
|||||||
|
"""Tests for infra-stop repair handoff verifier (#289)."""
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
from capability_stop_terminal import TERMINAL_REPORT_HEADING
|
||||||
|
from reviewer_infra_stop_handoff import assess_infra_stop_handoff_report # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
def _repair_handoff() -> str:
|
||||||
|
return "\n".join([
|
||||||
|
TERMINAL_REPORT_HEADING,
|
||||||
|
"Repair handoff for infra_stop blocked reviewer workflow.",
|
||||||
|
"CONTROL-CHECKOUT REPAIR MODE",
|
||||||
|
"MCP process root: /Users/dev/Gitea-Tools",
|
||||||
|
"Inspected git root: /Users/dev/Gitea-Tools",
|
||||||
|
"Conflict marker path: gitea_mcp_server.py",
|
||||||
|
"Merge/rebase control path: none",
|
||||||
|
"Safe next repair action: resolve conflict markers and restart MCP",
|
||||||
|
"infra_stop: true",
|
||||||
|
])
|
||||||
|
|
||||||
|
|
||||||
|
class TestInfraStopHandoff(unittest.TestCase):
|
||||||
|
def test_repair_handoff_passes(self):
|
||||||
|
result = assess_infra_stop_handoff_report(
|
||||||
|
_repair_handoff(),
|
||||||
|
stop_session={
|
||||||
|
"infra_stop": True,
|
||||||
|
"infra_stop_assessment": {
|
||||||
|
"infra_stop": True,
|
||||||
|
"conflict_file": "gitea_mcp_server.py",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
self.assertTrue(result["proven"], result["reasons"])
|
||||||
|
|
||||||
|
def test_next_pr_selection_blocks(self):
|
||||||
|
report = "\n".join([
|
||||||
|
TERMINAL_REPORT_HEADING,
|
||||||
|
"Next eligible PR to review: PR #276",
|
||||||
|
"Pinned review head SHA: abc123",
|
||||||
|
])
|
||||||
|
result = assess_infra_stop_handoff_report(
|
||||||
|
report,
|
||||||
|
stop_session={"infra_stop": True, "require_infra_diagnostics": False},
|
||||||
|
)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("queue" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_missing_repair_handoff_blocks(self):
|
||||||
|
result = assess_infra_stop_handoff_report(
|
||||||
|
"Validation result: pass\nReview summary for PR #276",
|
||||||
|
stop_session={"infra_stop": True},
|
||||||
|
)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
|
||||||
|
def test_main_checkout_diagnostics_without_label_blocks(self):
|
||||||
|
report = "\n".join([
|
||||||
|
TERMINAL_REPORT_HEADING,
|
||||||
|
"Repair handoff",
|
||||||
|
"Ran git status in main checkout for MCP diagnostics.",
|
||||||
|
])
|
||||||
|
result = assess_infra_stop_handoff_report(
|
||||||
|
report,
|
||||||
|
stop_session={
|
||||||
|
"infra_stop": True,
|
||||||
|
"main_checkout_diagnostics": True,
|
||||||
|
"require_infra_diagnostics": False,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("control-checkout" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_background_scheduling_blocks(self):
|
||||||
|
report = "\n".join([
|
||||||
|
TERMINAL_REPORT_HEADING,
|
||||||
|
"Repair handoff",
|
||||||
|
"Used schedule/manage_task while waiting for MCP recovery.",
|
||||||
|
])
|
||||||
|
result = assess_infra_stop_handoff_report(
|
||||||
|
report,
|
||||||
|
stop_session={"infra_stop": True, "require_infra_diagnostics": False},
|
||||||
|
)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("schedule" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_non_infra_stop_skips(self):
|
||||||
|
result = assess_infra_stop_handoff_report(
|
||||||
|
"Selected PR #1 for review",
|
||||||
|
stop_session={"infra_stop": False},
|
||||||
|
)
|
||||||
|
self.assertTrue(result["proven"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestExport(unittest.TestCase):
|
||||||
|
def test_review_proofs_reexport(self):
|
||||||
|
from review_proofs import assess_infra_stop_handoff_report as exported
|
||||||
|
|
||||||
|
self.assertTrue(callable(exported))
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -0,0 +1,126 @@
|
|||||||
|
"""Tests for reviewer inventory/worktree verifier (#293)."""
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
from reviewer_inventory_worktree import assess_inventory_worktree_report # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
def _good_handoff() -> str:
|
||||||
|
return "\n".join([
|
||||||
|
"## Controller Handoff",
|
||||||
|
"- Selected PR: 280",
|
||||||
|
"- Inventory pagination proof: is_final_page: true, has_more: false, total_count: 6",
|
||||||
|
"- Review worktree used: true",
|
||||||
|
"- Review worktree path: branches/review-pr280-feat-issue-275-claim",
|
||||||
|
"- Review worktree inside branches: true",
|
||||||
|
"- Review worktree HEAD state: detached",
|
||||||
|
"- Main checkout branch: master",
|
||||||
|
"- Current status: reviewed PR #280",
|
||||||
|
])
|
||||||
|
|
||||||
|
|
||||||
|
class TestInventoryPaginationProof(unittest.TestCase):
|
||||||
|
def test_no_next_page_proof_passes(self):
|
||||||
|
result = assess_inventory_worktree_report(_good_handoff())
|
||||||
|
self.assertTrue(result["proven"], result["reasons"])
|
||||||
|
self.assertTrue(result["pagination_proven"])
|
||||||
|
|
||||||
|
def test_rejects_partial_first_page_eligibility_claim(self):
|
||||||
|
report = "\n".join([
|
||||||
|
"Oldest eligible PR is #280.",
|
||||||
|
"gitea_list_prs returned 10 open PRs on the first page.",
|
||||||
|
"## Controller Handoff",
|
||||||
|
"- Inventory pagination proof: first page only",
|
||||||
|
])
|
||||||
|
result = assess_inventory_worktree_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertTrue(any("eligible" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_rejects_default_page_size_assumption(self):
|
||||||
|
report = (
|
||||||
|
"Inventory exhaustive because fewer than the default Gitea page size "
|
||||||
|
"were returned."
|
||||||
|
)
|
||||||
|
result = assess_inventory_worktree_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("page size" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_rejects_exactly_full_first_page_without_finality(self):
|
||||||
|
report = "\n".join([
|
||||||
|
"Next eligible PR: #290 based on complete inventory.",
|
||||||
|
"gitea_list_prs returned 50 open PRs.",
|
||||||
|
"## Controller Handoff",
|
||||||
|
"- Inventory pagination proof: returned 50 open PRs",
|
||||||
|
])
|
||||||
|
result = assess_inventory_worktree_report(
|
||||||
|
report,
|
||||||
|
inventory_session={"requested_page_size": 50, "returned_page_size": 50},
|
||||||
|
)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("full first page" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_allows_exact_page_with_session_pagination_complete(self):
|
||||||
|
report = "Oldest eligible PR: #12 after queue inventory."
|
||||||
|
result = assess_inventory_worktree_report(
|
||||||
|
report,
|
||||||
|
inventory_session={"pagination_complete": True},
|
||||||
|
)
|
||||||
|
self.assertTrue(result["proven"], result["reasons"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestWorktreeConsistency(unittest.TestCase):
|
||||||
|
def test_branches_worktree_requires_review_worktree_used_true(self):
|
||||||
|
report = "\n".join([
|
||||||
|
"## Controller Handoff",
|
||||||
|
"- Inventory pagination proof: is_final_page: true",
|
||||||
|
"- Review worktree used: false",
|
||||||
|
"- Review worktree path: branches/review-pr280-feat",
|
||||||
|
"- Scratch worktree used: false",
|
||||||
|
])
|
||||||
|
result = assess_inventory_worktree_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
joined = " ".join(result["reasons"]).lower()
|
||||||
|
self.assertIn("review worktree used", joined)
|
||||||
|
self.assertIn("scratch worktree", joined)
|
||||||
|
|
||||||
|
def test_rejects_contradictory_detached_and_branch_wording(self):
|
||||||
|
report = "\n".join([
|
||||||
|
"Checkout: Detached HEAD / Branch master",
|
||||||
|
"## Controller Handoff",
|
||||||
|
"- Inventory pagination proof: is_final_page: true",
|
||||||
|
"- Review worktree used: true",
|
||||||
|
"- Review worktree path: branches/review-pr280-feat",
|
||||||
|
"- Review worktree HEAD state: detached",
|
||||||
|
"- Main checkout branch: master",
|
||||||
|
])
|
||||||
|
result = assess_inventory_worktree_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("contradictory" in r.lower() for r in result["reasons"])
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_requires_head_state_when_review_worktree_used(self):
|
||||||
|
report = "\n".join([
|
||||||
|
"## Controller Handoff",
|
||||||
|
"- Inventory pagination proof: pagination_complete: true",
|
||||||
|
"- Review worktree used: true",
|
||||||
|
"- Review worktree path: branches/review-pr280-feat",
|
||||||
|
])
|
||||||
|
result = assess_inventory_worktree_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("head state" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
|
||||||
|
class TestExport(unittest.TestCase):
|
||||||
|
def test_review_proofs_reexport(self):
|
||||||
|
from review_proofs import assess_inventory_worktree_report as exported
|
||||||
|
|
||||||
|
self.assertTrue(callable(exported))
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -0,0 +1,88 @@
|
|||||||
|
"""Tests for precise mutation category verifier (#319)."""
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
from reviewer_mutation_categories import assess_mutation_categories_report # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
def _precise_handoff() -> str:
|
||||||
|
return "\n".join([
|
||||||
|
"Controller Handoff",
|
||||||
|
"File edits by reviewer: none",
|
||||||
|
"Worktree/index mutations: git worktree add branches/review-pr1",
|
||||||
|
"Git ref mutations: git fetch prgs master",
|
||||||
|
"MCP/Gitea mutations: gitea_view_pr",
|
||||||
|
"Review mutations: gitea_review_pr request_changes",
|
||||||
|
"Read-only diagnostics: git status, git diff",
|
||||||
|
])
|
||||||
|
|
||||||
|
|
||||||
|
class TestMutationCategories(unittest.TestCase):
|
||||||
|
def test_precise_categories_pass(self):
|
||||||
|
result = assess_mutation_categories_report(_precise_handoff())
|
||||||
|
self.assertTrue(result["proven"], result["reasons"])
|
||||||
|
|
||||||
|
def test_legacy_workspace_mutations_blocks(self):
|
||||||
|
report = "\n".join([
|
||||||
|
"Controller Handoff",
|
||||||
|
"Workspace mutations: none (no local file changes)",
|
||||||
|
"File edits by reviewer: none",
|
||||||
|
"Worktree/index mutations: none",
|
||||||
|
"Git ref mutations: none",
|
||||||
|
])
|
||||||
|
result = assess_mutation_categories_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("workspace mutations" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_workspace_none_with_worktree_command_blocks(self):
|
||||||
|
report = "\n".join([
|
||||||
|
"Controller Handoff",
|
||||||
|
"Workspace mutations: none (no local file changes)",
|
||||||
|
"File edits by reviewer: none",
|
||||||
|
"Worktree/index mutations: none",
|
||||||
|
"Git ref mutations: none",
|
||||||
|
])
|
||||||
|
result = assess_mutation_categories_report(
|
||||||
|
report,
|
||||||
|
observed_commands=["git reset --hard FETCH_HEAD"],
|
||||||
|
)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
|
||||||
|
def test_file_edits_none_with_worktree_mutation_passes(self):
|
||||||
|
report = "\n".join([
|
||||||
|
"Controller Handoff",
|
||||||
|
"File edits by reviewer: none",
|
||||||
|
"Worktree/index mutations: git reset --hard FETCH_HEAD",
|
||||||
|
"Git ref mutations: git fetch prgs",
|
||||||
|
])
|
||||||
|
result = assess_mutation_categories_report(
|
||||||
|
report,
|
||||||
|
observed_commands=["git reset --hard FETCH_HEAD", "git fetch prgs"],
|
||||||
|
)
|
||||||
|
self.assertTrue(result["proven"], result["reasons"])
|
||||||
|
|
||||||
|
def test_missing_required_categories_blocks(self):
|
||||||
|
report = "Controller Handoff\nFile edits by reviewer: none\n"
|
||||||
|
result = assess_mutation_categories_report(report)
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertTrue(any("missing" in r.lower() for r in result["reasons"]))
|
||||||
|
|
||||||
|
def test_non_controller_handoff_skips(self):
|
||||||
|
result = assess_mutation_categories_report(
|
||||||
|
"Workspace mutations: none\n",
|
||||||
|
)
|
||||||
|
self.assertTrue(result["proven"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestExport(unittest.TestCase):
|
||||||
|
def test_review_proofs_reexport(self):
|
||||||
|
from review_proofs import assess_mutation_categories_report as exported
|
||||||
|
|
||||||
|
self.assertTrue(callable(exported))
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -99,6 +99,24 @@ class TestRoleSessionRouter(unittest.TestCase):
|
|||||||
self.assertFalse(route["downstream_allowed"])
|
self.assertFalse(route["downstream_allowed"])
|
||||||
self.assertIn("Wrong role/session for reviewer task", route["message"])
|
self.assertIn("Wrong role/session for reviewer task", route["message"])
|
||||||
|
|
||||||
|
def test_pr_queue_cleanup_under_author_profile_wrong_role_stop(self):
|
||||||
|
with patch.dict(os.environ, self._env("prgs-author")):
|
||||||
|
route = mcp_server.gitea_route_task_session(
|
||||||
|
task_type="pr_queue_cleanup", remote="prgs"
|
||||||
|
)
|
||||||
|
self.assertEqual(route["route_result"], role_session_router.ROUTE_WRONG_ROLE)
|
||||||
|
self.assertFalse(route["downstream_allowed"])
|
||||||
|
|
||||||
|
@patch("mcp_server.api_request", return_value={"login": "sysadmin"})
|
||||||
|
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
|
||||||
|
def test_pr_queue_cleanup_under_reviewer_profile_allowed(self, _auth, _api):
|
||||||
|
with patch.dict(os.environ, self._env("prgs-reviewer")):
|
||||||
|
route = mcp_server.gitea_route_task_session(
|
||||||
|
task_type="pr_queue_cleanup", remote="prgs"
|
||||||
|
)
|
||||||
|
self.assertEqual(route["route_result"], role_session_router.ROUTE_ALLOWED)
|
||||||
|
self.assertTrue(route["downstream_allowed"])
|
||||||
|
|
||||||
@patch("mcp_server.api_request")
|
@patch("mcp_server.api_request")
|
||||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||||
def test_issue_creation_blocked_after_reviewer_wrong_role_stop(
|
def test_issue_creation_blocked_after_reviewer_wrong_role_stop(
|
||||||
|
|||||||
Reference in New Issue
Block a user