diff --git a/already_landed_reconcile.py b/already_landed_reconcile.py new file mode 100644 index 0000000..b2e195c --- /dev/null +++ b/already_landed_reconcile.py @@ -0,0 +1,142 @@ +"""Already-landed open PR reconciliation gates (#310). + +Reconciler workflows may close an open PR only when the PR head SHA is proven +an ancestor of a freshly fetched target branch. Arbitrary PR closure is denied. +""" + +from __future__ import annotations + +import subprocess +from typing import Any + +from merged_cleanup_reconcile import extract_linked_issue, is_head_ancestor_of_ref + +ELIGIBILITY_ALREADY_LANDED = "ALREADY_LANDED_RECONCILE_REQUIRED" +ELIGIBILITY_NOT_LANDED = "NOT_ALREADY_LANDED" +ELIGIBILITY_STALE_TARGET = "TARGET_BRANCH_UNVERIFIED" +ELIGIBILITY_PR_NOT_OPEN = "PR_NOT_OPEN" + + +def fetch_target_branch(project_root: str, remote: str, branch: str) -> dict[str, Any]: + """Fetch *branch* from *remote* and return the resolved SHA.""" + ref = f"{remote}/{branch}" + fetch = subprocess.run( + ["git", "-C", project_root, "fetch", remote, branch], + capture_output=True, + text=True, + check=False, + ) + if fetch.returncode != 0: + return { + "success": False, + "target_branch": branch, + "target_ref": ref, + "target_branch_sha": None, + "reasons": [ + f"git fetch {remote} {branch} failed: " + f"{(fetch.stderr or fetch.stdout or '').strip()}" + ], + } + + rev = subprocess.run( + ["git", "-C", project_root, "rev-parse", ref], + capture_output=True, + text=True, + check=False, + ) + if rev.returncode != 0: + return { + "success": False, + "target_branch": branch, + "target_ref": ref, + "target_branch_sha": None, + "reasons": [ + f"git rev-parse {ref} failed: " + f"{(rev.stderr or rev.stdout or '').strip()}" + ], + } + + return { + "success": True, + "target_branch": branch, + "target_ref": ref, + "target_branch_sha": (rev.stdout or "").strip(), + "reasons": [], + "git_fetch_command": f"git fetch {remote} {branch}", + } + + +def assess_already_landed_reconciliation( + *, + pr: dict[str, Any], + project_root: str, + remote: str, + target_branch: str, + target_fetch: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Return eligibility and proof for reconciling an open PR.""" + pr_number = int(pr.get("number") or 0) + pr_state = (pr.get("state") or "").strip().lower() + head_sha = (pr.get("head") or {}).get("sha") if isinstance(pr.get("head"), dict) else None + if not head_sha and isinstance(pr.get("head"), str): + head_sha = None + head_ref = (pr.get("head") or {}).get("ref") if isinstance(pr.get("head"), dict) else pr.get("head") + base_ref = (pr.get("base") or {}).get("ref") if isinstance(pr.get("base"), dict) else pr.get("base") + title = pr.get("title") or "" + body = pr.get("body") or "" + + fetch_result = target_fetch or fetch_target_branch(project_root, remote, target_branch) + linked_issue = extract_linked_issue(title, body) + + result: dict[str, Any] = { + "pr_number": pr_number, + "pr_state": pr_state, + "candidate_head_sha": head_sha, + "head_ref": head_ref, + "base_ref": base_ref or target_branch, + "target_branch": target_branch, + "target_branch_sha": fetch_result.get("target_branch_sha"), + "linked_issue": linked_issue, + "git_ref_mutations": [], + "reasons": [], + } + if fetch_result.get("git_fetch_command"): + result["git_ref_mutations"].append(fetch_result["git_fetch_command"]) + + if pr_state != "open": + result["eligibility_class"] = ELIGIBILITY_PR_NOT_OPEN + result["ancestor_proof"] = None + result["close_allowed"] = False + result["reasons"].append(f"PR #{pr_number} state is {pr_state!r}, not open") + return result + + if not fetch_result.get("success"): + result["eligibility_class"] = ELIGIBILITY_STALE_TARGET + result["ancestor_proof"] = None + result["close_allowed"] = False + result["reasons"].extend(fetch_result.get("reasons") or []) + return result + + target_ref = fetch_result.get("target_ref") or f"{remote}/{target_branch}" + ancestor = is_head_ancestor_of_ref(project_root, head_sha, target_ref) + result["ancestor_proof"] = ancestor + + if ancestor is None: + result["eligibility_class"] = ELIGIBILITY_STALE_TARGET + result["close_allowed"] = False + result["reasons"].append( + f"ancestor check failed for head {head_sha!r} against {target_ref}" + ) + return result + + if ancestor: + result["eligibility_class"] = ELIGIBILITY_ALREADY_LANDED + result["close_allowed"] = True + return result + + result["eligibility_class"] = ELIGIBILITY_NOT_LANDED + result["close_allowed"] = False + result["reasons"].append( + f"PR head {head_sha} is not an ancestor of {target_ref}" + ) + return result \ No newline at end of file diff --git a/author_mutation_worktree.py b/author_mutation_worktree.py new file mode 100644 index 0000000..04b6933 --- /dev/null +++ b/author_mutation_worktree.py @@ -0,0 +1,105 @@ +"""Branches-only author mutation worktree guard (#274). + +Author/coder mutations must run from a session-owned worktree under the +project's ``branches/`` directory, never from the stable control checkout. +""" + +from __future__ import annotations + +import os + +BASE_BRANCHES = frozenset({"master", "main", "dev"}) + + +def _normalize_path(path: str) -> str: + return (path or "").replace("\\", "/").rstrip("/") + + +def is_path_under_branches(path: str, project_root: str | None = None) -> bool: + """True when *path* resolves inside ``/branches/``.""" + normalized = _normalize_path(path) + if not normalized: + return False + if "/branches/" in f"{normalized}/": + return True + if normalized.endswith("/branches"): + return True + if project_root: + root = _normalize_path(os.path.realpath(project_root)) + real = _normalize_path(os.path.realpath(path)) + if real.startswith(f"{root}/"): + rel = real[len(root) + 1 :] + return rel == "branches" or rel.startswith("branches/") + return False + + +def resolve_mutation_workspace( + worktree_path: str | None, + project_root: str, + *, + active_worktree_env: str | None = None, + author_worktree_env: str | None = None, +) -> str: + """Resolve the workspace path inspected before author mutations.""" + for candidate in (worktree_path, active_worktree_env, author_worktree_env): + text = (candidate or "").strip() + if text: + return os.path.realpath(os.path.abspath(text)) + return os.path.realpath(project_root) + + +def assess_author_mutation_worktree( + *, + workspace_path: str, + project_root: str, + current_branch: str | None = None, + base_branches: frozenset[str] | None = None, +) -> dict: + """Fail closed when author mutations are not rooted under ``branches/``.""" + bases = base_branches or BASE_BRANCHES + reasons: list[str] = [] + root = os.path.realpath(project_root) + workspace = os.path.realpath(workspace_path) + branch = (current_branch or "").strip() + + under_branches = is_path_under_branches(workspace, root) + if not under_branches: + if workspace == root: + reasons.append( + "author mutation blocked: workspace is the stable control checkout; " + "create or switch to a session-owned worktree under branches/" + ) + else: + reasons.append( + f"author mutation blocked: workspace '{workspace}' is not under " + f"'{root}/branches/'; create a branches/ worktree first" + ) + + if not under_branches and workspace == root and branch and branch not in bases: + reasons.append( + f"control checkout drift: branch '{branch}' is not a stable base " + f"branch ({'/'.join(sorted(bases))})" + ) + + proven = not reasons + return { + "proven": proven, + "block": not proven, + "reasons": reasons, + "project_root": root, + "workspace_path": workspace, + "under_branches": is_path_under_branches(workspace, root), + "current_branch": branch or None, + } + + +def format_author_mutation_worktree_error(assessment: dict) -> str: + """Single RuntimeError message for MCP preflight gates.""" + workspace = assessment.get("workspace_path") or "(unknown)" + root = assessment.get("project_root") or "(unknown)" + reasons = "; ".join(assessment.get("reasons") or ["unknown branches-only violation"]) + return ( + f"Branches-only mutation guard (#274): {reasons}. " + f"project root: {root}; workspace: {workspace}. " + "Create a session-owned worktree under branches/ before mutating." + ) \ No newline at end of file diff --git a/capability_stop_terminal.py b/capability_stop_terminal.py index 70ff264..d7f9751 100644 --- a/capability_stop_terminal.py +++ b/capability_stop_terminal.py @@ -13,6 +13,8 @@ REVIEWER_CAPABILITY_TASKS = frozenset({ "review_pr", "merge_pr", "blind_pr_queue_review", + "pr_queue_cleanup", + "pr-queue-cleanup", "request_changes_pr", "approve_pr", }) diff --git a/docs/gitea-dual-namespace-deployment.md b/docs/gitea-dual-namespace-deployment.md index aa4c03d..6ee7e8e 100644 --- a/docs/gitea-dual-namespace-deployment.md +++ b/docs/gitea-dual-namespace-deployment.md @@ -23,6 +23,7 @@ launched with exactly one static execution profile: |-----------------------------|----------------|-------------| | `gitea-author` | an author profile | implement issues, push branches, open PRs, comment | | `gitea-reviewer` | a reviewer profile | review, approve/request changes, merge | +| `gitea-reconciler` | a reconciler profile | close already-landed open PRs after ancestry proof (#304 profile; #310 close tool) | Properties: diff --git a/docs/gitea-execution-profiles.md b/docs/gitea-execution-profiles.md index 061ae70..3d9958f 100644 --- a/docs/gitea-execution-profiles.md +++ b/docs/gitea-execution-profiles.md @@ -226,6 +226,31 @@ explicit operator-directed closure of a contaminated PR). If `close_pr` ever resolves as unknown, agents must fail closed rather than fall back to the edit path. +## Reconciler profile for already-landed open PRs (#304 / #310) + +Normal author and reviewer profiles must not gain broad `gitea.pr.close` +authority. Already-landed open PRs (head SHA is an ancestor of the target +branch) need a dedicated reconciler profile such as `prgs-reconciler` with a +narrow operation set: + +- `gitea.read` +- `gitea.pr.comment` +- `gitea.issue.comment` +- `gitea.issue.close` +- `gitea.pr.close` + +Forbidden on reconciler profiles: `gitea.pr.approve`, `gitea.pr.merge`, +`gitea.pr.review`, `gitea.pr.create`, `gitea.branch.push`, and +`gitea.repo.commit`. + +Launch a static `gitea-reconciler` MCP namespace with +`GITEA_MCP_PROFILE=prgs-reconciler`. Profile shape is validated by +`reconciler_profile.assess_reconciler_profile` (#304). Use the +`gitea_reconcile_already_landed_pr` tool (#310). The resolver task is +`reconcile_already_landed_pr`. PR close is allowed only after live PR fetch, +fresh target-branch fetch, recorded target SHA, and ancestor proof. PRs whose +heads are not already landed cannot be closed through this path. + ## Identity and fail-closed rules Before **any** mutating action, a workflow must know both: diff --git a/docs/llm-workflow-runbooks.md b/docs/llm-workflow-runbooks.md index a5a51b2..f3d82e3 100644 --- a/docs/llm-workflow-runbooks.md +++ b/docs/llm-workflow-runbooks.md @@ -169,6 +169,25 @@ To avoid the bottleneck of relaunching/restarting the MCP server to switch betwe * `mcp__gitea-reviewer__*` (for reviewing PRs, approving, requesting changes, merging) * **Trust Model:** Separate tokens remain separate in the keychain/environment. Each instance operates under its own `GITEA_MCP_PROFILE` and enforces its own `allowed_operations`. A runtime `whoami` identity check is still performed independently, and self-review/self-merge checks remain strictly mandatory. The dual-server pattern is a operational convenience and never a security bypass. * **Reviewer-Identity PR Creation Deadlock:** Reviewer/merge identities must not create PRs or push branches. Doing so makes the reviewer identity the PR author in Gitea, blocking subsequent independent review and causing a review deadlock. Normally, PRs must be created by the author/work identity (`gitea-author`), leaving the reviewer identity (`gitea-reviewer`) clean and available for independent review and merge. +* **Reconciler namespace (#310):** Register a third static instance for + already-landed PR cleanup when review queues block on open PRs whose heads + already landed on `master`: + +```json +"gitea-reconciler": { + "command": "/path/to/Gitea-Tools/venv/bin/python3", + "args": ["/path/to/Gitea-Tools/mcp_server.py"], + "env": { + "GITEA_MCP_CONFIG": "/path/to/.config/gitea-tools/profiles.json", + "GITEA_MCP_PROFILE": "prgs-reconciler" + } +} +``` + + The reconciler profile grants `gitea.pr.close` only for + `gitea_reconcile_already_landed_pr` after ancestry proof — not for normal + review or author workflows. + * **Fallback:** If the dual-profile MCP launcher pattern is not supported or configured in the client, the LLM must relaunch or restart the client/MCP with the correct profile environment variable before claiming or working on any tasks. ## Setup runbook — interactive menu @@ -254,7 +273,20 @@ Never create a parallel branch or PR for the same issue unless the old branch is proven abandoned and the takeover is recorded. Gitea-Tools lease gates: `gitea_lock_issue` (fail-closed before author -mutations), `status:in-progress`, and claim comments. Full portable wording: +mutations), `status:in-progress`, and claim comments. `gitea_lock_issue` +records an `author_issue_work` lease in the issue-lock payload with issue +number, optional PR number, branch, worktree path, claimant identity/profile, +created timestamp, expiry timestamp, and last heartbeat timestamp. An active +same-issue/same-operation lease blocks duplicate work. An expired lease still +blocks takeover until a recovery review records why the prior work is abandoned, +completed, or unsafe to continue. + +Remote branches matching the issue number are also treated as active work unless +the recovery review proves the branch is abandoned or superseded. Never delete +or clean up a branch when it has an active lease, dirty worktree, open PR, or is +the only copy of unmerged work. + +Full portable wording: [`skills/llm-project-workflow/SKILL.md`](../skills/llm-project-workflow/SKILL.md). ## Global LLM Worktree Rule @@ -749,3 +781,22 @@ scripts/release-tag v0.4.0 --notes-file /tmp/release-notes.md --push - [`credential-isolation.md`](credential-isolation.md) — credential handling. - [`release-workflows.md`](release-workflows.md) — release/merge workflow. - [`../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. diff --git a/docs/wiki/Workflow.md b/docs/wiki/Workflow.md index 2514dda..2e05526 100644 --- a/docs/wiki/Workflow.md +++ b/docs/wiki/Workflow.md @@ -17,6 +17,10 @@ 0. Work Selection Rule — verify a work lease before any mutations (open PRs, issue-linked PRs, branches, worktrees, dirty worktrees, active leases/ handoffs, merged-PR completion). Stop if another session owns the lease. + `gitea_lock_issue` records an operation-scoped `author_issue_work` lease + with issue, branch, worktree, claimant, created, expiry, and heartbeat + fields; active or expired same-operation leases require recovery review + before takeover. 0b. Global LLM Worktree Rule — main checkout on `master`/`main`/`dev` only; mutate only from a `branches/` worktree after proving root, cwd, branch, stable main-checkout branch, and session worktree path (no exceptions). @@ -31,4 +35,4 @@ Wiki publication, credential provisioning, and cross-repo mirror operations are operator-confirmed actions — see [Runbooks](Runbooks.md). -Full Gitea-specific runbooks: `docs/llm-workflow-runbooks.md` in the repository. \ No newline at end of file +Full Gitea-specific runbooks: `docs/llm-workflow-runbooks.md` in the repository. diff --git a/final_report_validator.py b/final_report_validator.py index bff9467..3fbac0f 100644 --- a/final_report_validator.py +++ b/final_report_validator.py @@ -87,25 +87,72 @@ _ALREADY_LANDED_RE = re.compile( r"already[- ]landed|ancestor proof\s*:\s*passed|eligibility class\s*:\s*already_landed", re.IGNORECASE, ) +_ALREADY_LANDED_GATE_FIRED_RE = re.compile( + r"\b(?:fired|passed|true|yes|already_landed_reconcile_required)\b", + re.IGNORECASE, +) _ELIGIBLE_REVIEW_RE = re.compile( - r"eligible for (?:review|merge)|ready for (?:review|merge)|next eligible pr", + r"eligible for (?:review|merge)|ready for (?:review|merge)|" + r"(?:next|oldest)\s+eligible\s+pr", + re.IGNORECASE, +) +_APPROVED_STATE_RE = re.compile(r"\bapproved?\b", re.IGNORECASE) +_MERGED_STATE_RE = re.compile(r"\bmerged\b", re.IGNORECASE) +_READY_TO_MERGE_STATE_RE = re.compile(r"\bready_to_merge\b|\bready to merge\b", re.IGNORECASE) +_NEGATED_STATE_RE = re.compile( + r"\b(?:not|no|none|n/a|never)\b|request_changes|not attempted", re.IGNORECASE, ) _REVIEWED_LANDED_RE = re.compile( r"(?:reviewed|approved).{0,40}already[- ]landed|already[- ]landed.{0,40}(?:reviewed|eligible)", re.IGNORECASE | re.DOTALL, ) +_TARGET_BRANCH_UP_TO_DATE_RE = re.compile( + r"\btarget branch (?:is )?up[- ]to[- ]date\b", + re.IGNORECASE, +) +_TARGET_BRANCH_SHA_RE = re.compile( + r"target branch sha\s*:\s*[0-9a-f]{40}", + re.IGNORECASE, +) +_FULL_SHA_RE = re.compile(r"\b[0-9a-f]{40}\b", re.IGNORECASE) _RECONCILE_STALE_FIELDS = ( "pr number opened", "pinned reviewed head", "scratch worktree used", "review decision", "merge result", + "issue lock proof", + "claim/comment status", + "workspace mutations", ) +# Issue #307: reconciliation handoffs must carry the full canonical schema. _RECONCILE_REQUIRED_FIELDS = ( + "Task", + "Repo", + "Role/profile", + "Identity", "Selected PR", - "Eligibility class", + "PR live state", + "Candidate head SHA", + "Target branch", + "Target branch SHA", + "Ancestor proof", + "Linked issue", "Linked issue live status", + "Eligibility class", + "Capabilities proven", + "Missing capabilities", + "PR comments posted", + "Issue comments posted", + "PRs closed", + "Issues closed", + "Git ref mutations", + "Worktree mutations", + "MCP/Gitea mutations", + "External-state mutations", + "Read-only diagnostics", + "Blocker", "Safe next action", "No review/merge confirmation", ) @@ -190,6 +237,54 @@ def _handoff_fields(report_text: str) -> dict[str, str]: return fields +def _already_landed_gate_fired(report_text: str) -> bool: + text = report_text or "" + fields = _handoff_fields(text) + + gate_value = fields.get("already-landed gate", "") + if gate_value and _ALREADY_LANDED_GATE_FIRED_RE.search(gate_value): + return True + + ancestor_value = fields.get("ancestor proof", "") + if re.search(r"\bpassed\b", ancestor_value, re.IGNORECASE): + return True + + eligibility_value = fields.get("eligibility class", "") + if "already_landed" in eligibility_value.lower(): + return True + + if re.search( + r"\bpr\b.{0,60}\balready[- ]landed\b|\balready[- ]landed\b.{0,60}\bpr\b", + text, + re.IGNORECASE | re.DOTALL, + ): + return True + + return bool(_ALREADY_LANDED_RE.search(text)) + + +def _positive_review_state_terms(fields: dict[str, str]) -> list[str]: + terms: list[str] = [] + for key, value in fields.items(): + lowered_key = key.lower() + lowered_value = value.lower() + if _NEGATED_STATE_RE.search(value): + continue + if lowered_key == "review decision" and _APPROVED_STATE_RE.search(value): + terms.append("APPROVED") + elif lowered_key == "merge result" and _MERGED_STATE_RE.search(value): + terms.append("MERGED") + elif _READY_TO_MERGE_STATE_RE.search(value): + terms.append("READY_TO_MERGE") + elif lowered_value in {"approved", "approve"}: + terms.append("APPROVED") + elif lowered_value == "merged": + terms.append("MERGED") + elif lowered_value == "ready_to_merge": + terms.append("READY_TO_MERGE") + return sorted(set(terms)) + + def _rule_shared_controller_handoff( report_text: str, task_kind: str, @@ -346,19 +441,55 @@ def _rule_reviewer_git_fetch_readonly( "classify git fetch under Git ref mutations, not read-only diagnostics", ) ] - if not git_ref_reported and _GIT_FETCH_RE.search(text): + if not git_ref_reported: + # Issue #307: an observed git fetch (report text or action log) + # must be listed under Git ref mutations. return [ validator_finding( "reviewer.git_fetch_readonly", "downgrade", "Git ref mutations", - "git fetch mentioned without Git ref mutation classification", + "git fetch occurred without Git ref mutation classification", "record git fetch under Git ref mutations", ) ] return [] +def _rule_reviewer_validation_failure_history( + report_text: str, + *, + validation_session: dict | None = None, +) -> list[dict[str, str]]: + from reviewer_validation_failure_history import ( + assess_validation_failure_history_report, + ) + + session = validation_session or {} + observed = session.get("observed_failures") or [] + if not observed: + return [] + + result = assess_validation_failure_history_report( + report_text, + validation_session=session, + ) + if result.get("proven"): + return [] + severity = "block" if result.get("violations") else "downgrade" + return [ + validator_finding( + "reviewer.validation_failure_history", + severity, + "Validation failure history", + reason, + result.get("safe_next_action") + or "document every observed validation failure before claiming pass", + ) + for reason in (result.get("violations") or result.get("reasons") or ["incomplete"]) + ] + + def _rule_reviewer_validation_command(report_text: str) -> list[dict[str, str]]: text = report_text or "" if not _BARE_PYTEST_RE.search(text): @@ -492,7 +623,7 @@ def _rule_reviewer_main_checkout_path(report_text: str) -> list[dict[str, str]]: def _rule_reviewer_already_landed_eligible(report_text: str) -> list[dict[str, str]]: text = report_text or "" - if not _ALREADY_LANDED_RE.search(text): + if not _already_landed_gate_fired(text): return [] if not _ELIGIBLE_REVIEW_RE.search(text): return [] @@ -507,6 +638,65 @@ def _rule_reviewer_already_landed_eligible(report_text: str) -> list[dict[str, s ] +def _rule_reviewer_already_landed_state(report_text: str) -> list[dict[str, str]]: + text = report_text or "" + if not _already_landed_gate_fired(text): + return [] + + terms = _positive_review_state_terms(_handoff_fields(text)) + if not terms: + return [] + + return [ + validator_finding( + "reviewer.already_landed_review_state", + "block", + "Review decision", + "already-landed gate fired but final report claims " + f"{', '.join(terms)} state", + "stop normal review/merge; classify as ALREADY_LANDED_RECONCILE_REQUIRED", + ) + ] + + +def _rule_reviewer_target_branch_freshness( + report_text: str, + *, + action_log: list[dict] | None = None, +) -> list[dict[str, str]]: + text = report_text or "" + if not _TARGET_BRANCH_UP_TO_DATE_RE.search(text): + return [] + + fields = _handoff_fields(text) + fetch_reported = bool(_GIT_FETCH_RE.search(text)) or any( + _GIT_FETCH_RE.search(str(e.get("command") or e.get("action") or "")) + for e in (action_log or []) + ) + target_sha_reported = bool(_TARGET_BRANCH_SHA_RE.search(text)) or any( + "target branch" in key and "sha" in key and _FULL_SHA_RE.search(value) + for key, value in fields.items() + ) + if fetch_reported and target_sha_reported: + return [] + + missing = [] + if not fetch_reported: + missing.append("fresh git fetch") + if not target_sha_reported: + missing.append("target branch SHA") + return [ + validator_finding( + "reviewer.target_branch_freshness_proof", + "block", + "Target branch SHA", + "target branch up-to-date claim lacks " + " and ".join(missing), + "fetch the target branch and report the exact target branch SHA before " + "claiming it is up to date", + ) + ] + + def _rule_reconcile_controller_handoff(report_text: str) -> list[dict[str, str]]: from review_proofs import _handoff_section_lines @@ -541,10 +731,18 @@ def _rule_reconcile_controller_handoff(report_text: str) -> list[dict[str, str]] return [] -def _rule_reconcile_stale_author_fields(report_text: str) -> list[dict[str, str]]: +def _rule_reconcile_stale_author_fields( + report_text: str, + *, + session_pr_opened: bool = False, +) -> list[dict[str, str]]: text = (report_text or "").lower() findings = [] for field in _RECONCILE_STALE_FIELDS: + if field == "pr number opened" and session_pr_opened: + # Issue #307: allowed only when a PR was actually opened + # in this session. + continue if f"{field}:" in text: findings.append( validator_finding( @@ -700,12 +898,15 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = { _rule_reviewer_mutation_categories, _rule_reviewer_git_fetch_readonly, _rule_reviewer_validation_command, + _rule_reviewer_validation_failure_history, _rule_reviewer_validation_structured, _rule_reviewer_linked_issue, _rule_reviewer_baseline_on_failure, _rule_reviewer_main_checkout_baseline, _rule_reviewer_main_checkout_path, _rule_reviewer_already_landed_eligible, + _rule_reviewer_already_landed_state, + _rule_reviewer_target_branch_freshness, _rule_reviewer_mutation_ledger, _rule_reviewer_review_mutation, ], @@ -803,6 +1004,8 @@ def assess_final_report_validator( mutations_observed: bool = False, local_edits: bool = False, issue_filing_lock: dict | None = None, + session_pr_opened: bool = False, + validation_session: dict | None = None, ) -> dict[str, Any]: """Validate final-report text against task-specific proof rules (#327). @@ -857,6 +1060,8 @@ def assess_final_report_validator( "action_log": action_log, "mutations_observed": mutations_observed, "local_edits": local_edits, + "session_pr_opened": session_pr_opened, + "validation_session": validation_session, } for rule in _RULES_BY_TASK.get(normalized_kind, ()): @@ -875,4 +1080,4 @@ def assess_final_report_validator( "safe_next_action": _primary_safe_next_action(findings), "complete": grade == "A", "handoff_heading": HANDOFF_HEADING, - } \ No newline at end of file + } diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 02635c7..902046c 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -21,6 +21,7 @@ import json import functools import contextlib import subprocess +from datetime import datetime, timedelta, timezone # Mutation-authority record (#199, refs #194). Deliberately in-process, NOT a @@ -397,6 +398,28 @@ def record_preflight_check(type_name: str, resolved_role: str | None = None): _preflight_resolved_role = resolved_role +def _enforce_branches_only_author_mutation(worktree_path: str | None = None) -> None: + """#274: author mutations must run from a branches/ session worktree.""" + if _preflight_resolved_role == "reviewer": + return + workspace = author_mutation_worktree.resolve_mutation_workspace( + worktree_path, + PROJECT_ROOT, + active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV), + author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV), + ) + git_state = issue_lock_worktree.read_worktree_git_state(workspace) + assessment = author_mutation_worktree.assess_author_mutation_worktree( + workspace_path=workspace, + project_root=PROJECT_ROOT, + current_branch=git_state.get("current_branch"), + ) + if assessment["block"]: + raise RuntimeError( + author_mutation_worktree.format_author_mutation_worktree_error(assessment) + ) + + def verify_preflight_purity(remote: str | None = None, worktree_path: str | None = None): """Verify that identity and capability were verified prior to session edits.""" global _preflight_reviewer_violation_files @@ -426,32 +449,33 @@ def verify_preflight_purity(remote: str | None = None, worktree_path: str | None "file edits before mutation (fail closed). " f"{_format_preflight_workspace_details(details)}" ) - return - - if _preflight_whoami_violation: - raise RuntimeError( - "Pre-flight order violation: Workspace file edits occurred before " - f"gitea_whoami verification (fail closed). Offending files: " - f"{_format_preflight_files(_preflight_whoami_violation_files)}" - ) - if _preflight_capability_violation: - raise RuntimeError( - "Pre-flight order violation: Workspace file edits occurred before " - f"gitea_resolve_task_capability verification (fail closed). Offending files: " - f"{_format_preflight_files(_preflight_capability_violation_files)}" - ) - - if _preflight_resolved_role == "reviewer": - current = _get_workspace_porcelain() - baseline = _preflight_capability_baseline_porcelain or "" - reviewer_delta = _new_tracked_changes_since(baseline, current) - _preflight_reviewer_violation_files = reviewer_delta - if reviewer_delta: + else: + if _preflight_whoami_violation: raise RuntimeError( - "Reviewer role violation: Reviewer profile is forbidden from modifying " - "tracked workspace files (fail closed). Offending files: " - f"{_format_preflight_files(reviewer_delta)}" + "Pre-flight order violation: Workspace file edits occurred before " + f"gitea_whoami verification (fail closed). Offending files: " + f"{_format_preflight_files(_preflight_whoami_violation_files)}" ) + if _preflight_capability_violation: + raise RuntimeError( + "Pre-flight order violation: Workspace file edits occurred before " + f"gitea_resolve_task_capability verification (fail closed). Offending files: " + f"{_format_preflight_files(_preflight_capability_violation_files)}" + ) + + if _preflight_resolved_role == "reviewer": + current = _get_workspace_porcelain() + baseline = _preflight_capability_baseline_porcelain or "" + reviewer_delta = _new_tracked_changes_since(baseline, current) + _preflight_reviewer_violation_files = reviewer_delta + if reviewer_delta: + raise RuntimeError( + "Reviewer role violation: Reviewer profile is forbidden from modifying " + "tracked workspace files (fail closed). Offending files: " + f"{_format_preflight_files(reviewer_delta)}" + ) + + _enforce_branches_only_author_mutation(worktree_path) from mcp.server.fastmcp import FastMCP # noqa: E402 @@ -477,14 +501,145 @@ import review_proofs # noqa: E402 import review_workflow_load # noqa: E402 import agent_temp_artifacts import issue_lock_worktree # noqa: E402 +import already_landed_reconcile # noqa: E402 +import author_mutation_worktree # noqa: E402 import issue_claim_heartbeat # noqa: E402 import merged_cleanup_reconcile # noqa: E402 +import reconciler_profile # noqa: E402 +import reconciliation_workflow # 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, # consumed by gitea_create_pr and scripts/worktree-start. ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json" +WORK_LEASE_TTL_HOURS = 4 +AUTHOR_ISSUE_WORK_LEASE = "author_issue_work" +VALID_WORK_LEASE_OPERATIONS = frozenset({ + AUTHOR_ISSUE_WORK_LEASE, + "review_pr_work", + "fix_pr_changes", + "cleanup_branch_work", + "issue_filing_work", + "recovery_work", +}) + + +def _work_lease_now() -> datetime: + return datetime.now(timezone.utc) + + +def _work_lease_timestamp(value: datetime) -> str: + return value.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace("+00:00", "Z") + + +def _parse_work_lease_timestamp(value: str | None) -> datetime | None: + text = (value or "").strip() + if not text: + return None + try: + return datetime.fromisoformat(text.replace("Z", "+00:00")).astimezone(timezone.utc) + except ValueError: + return None + + +def _load_existing_issue_lock() -> dict | None: + if not os.path.exists(ISSUE_LOCK_FILE): + return None + try: + with open(ISSUE_LOCK_FILE, encoding="utf-8") as f: + data = json.load(f) + return data if isinstance(data, dict) else None + except Exception: + return None + + +def _work_lease_claimant(host: str | None) -> dict: + profile = get_profile() + username = _IDENTITY_CACHE.get(host) if host else None + if username is None and host and not _preflight_in_test_mode(): + username = _authenticated_username(host) + return { + "username": username, + "profile": profile.get("profile_name"), + } + + +def _build_author_issue_work_lease( + *, + issue_number: int, + branch_name: str, + worktree_path: str, + host: str | None, +) -> dict: + created = _work_lease_now() + expires = created + timedelta(hours=WORK_LEASE_TTL_HOURS) + return { + "operation_type": AUTHOR_ISSUE_WORK_LEASE, + "issue_number": issue_number, + "pr_number": None, + "branch": branch_name, + "worktree_path": worktree_path, + "claimant": _work_lease_claimant(host), + "created_at": _work_lease_timestamp(created), + "expires_at": _work_lease_timestamp(expires), + "last_heartbeat_at": _work_lease_timestamp(created), + } + + +def _active_work_lease_block( + existing_lock: dict | None, + *, + issue_number: int, + branch_name: str, + worktree_path: str, + operation_type: str, +) -> str | None: + if not existing_lock: + return None + existing_issue = existing_lock.get("issue_number") + existing_branch = existing_lock.get("branch_name") + existing_worktree = existing_lock.get("worktree_path") + lease = existing_lock.get("work_lease") + existing_operation = ( + lease.get("operation_type") + if isinstance(lease, dict) + else AUTHOR_ISSUE_WORK_LEASE + ) + if existing_issue != issue_number or existing_operation != operation_type: + return None + + same_owner = ( + existing_branch == branch_name + and os.path.realpath(str(existing_worktree or "")) == os.path.realpath(worktree_path) + ) + expires_at = ( + _parse_work_lease_timestamp(lease.get("expires_at")) + if isinstance(lease, dict) + else None + ) + if expires_at and expires_at <= _work_lease_now(): + return ( + f"Issue #{issue_number} has an expired {operation_type} lease on " + f"branch '{existing_branch}' from worktree '{existing_worktree}'. " + "Recovery review is required before takeover (fail closed)" + ) + if same_owner: + return None + return ( + f"Issue #{issue_number} already has an active {operation_type} lease on " + f"branch '{existing_branch}' from worktree '{existing_worktree}' " + "(fail closed)" + ) + + +def _branch_entry_name(branch: dict | str) -> str: + if isinstance(branch, str): + return branch + if not isinstance(branch, dict): + return "" + return str(branch.get("name") or branch.get("ref") or "") def _reveal_endpoints() -> bool: @@ -932,6 +1087,16 @@ def gitea_lock_issue( resolved_worktree = issue_lock_worktree.resolve_author_worktree_path( worktree_path, PROJECT_ROOT ) + active_lease_block = _active_work_lease_block( + _load_existing_issue_lock(), + issue_number=issue_number, + branch_name=branch_name, + worktree_path=resolved_worktree, + operation_type=AUTHOR_ISSUE_WORK_LEASE, + ) + if active_lease_block: + raise RuntimeError(active_lease_block) + git_state = issue_lock_worktree.read_worktree_git_state(resolved_worktree) verify_preflight_purity(remote, worktree_path=resolved_worktree) lock_assessment = issue_lock_worktree.assess_issue_lock_worktree( @@ -977,6 +1142,25 @@ def gitea_lock_issue( f"Issue #{issue_number} is already tied to an open PR (PR #{pr.get('number')}) via Closes/Fixes reference (fail closed)" ) + branch_url = f"{repo_api_url(h, o, r)}/branches" + try: + branches = api_get_all(branch_url, auth) + except Exception as e: + raise RuntimeError(f"Could not list branches to verify issue lock: {e}") + for branch in branches: + name = _branch_entry_name(branch) + if expected_pattern in name: + raise ValueError( + f"Issue #{issue_number} already has matching branch '{name}' " + "(fail closed)" + ) + + work_lease = _build_author_issue_work_lease( + issue_number=issue_number, + branch_name=branch_name, + worktree_path=resolved_worktree, + host=h, + ) data = { "issue_number": issue_number, "branch_name": branch_name, @@ -984,6 +1168,7 @@ def gitea_lock_issue( "org": o, "repo": r, "worktree_path": resolved_worktree, + "work_lease": work_lease, } try: @@ -1004,6 +1189,7 @@ def gitea_lock_issue( "issue_number": issue_number, "branch_name": branch_name, "worktree_path": resolved_worktree, + "work_lease": work_lease, } if agent_artifacts: result["warnings"] = [ @@ -1060,7 +1246,7 @@ def gitea_create_pr( ) if blocked: return blocked - verify_preflight_purity(remote) + verify_preflight_purity(remote, worktree_path=worktree_path) h, o, r = _resolve(remote, host, org, repo) # ── Issue Lock Validation (Issue #194 / #196) ── @@ -3305,16 +3491,32 @@ def gitea_delete_branch( repo: Override the repository name. Returns: - dict with 'success' and 'message'. + dict with 'success' and 'message'; on a permission block, + 'success'/'performed' False, 'reasons', and a structured + 'permission_report' (#142) with no API call made. """ + gate_reasons = _profile_operation_gate("gitea.branch.delete") + if gate_reasons: + return { + "success": False, + "performed": False, + "required_permission": "gitea.branch.delete", + "reasons": gate_reasons, + "permission_report": _permission_block_report("gitea.branch.delete"), + } + verify_preflight_purity(remote) h, o, r = _resolve(remote, host, org, repo) auth = _auth(h) import urllib.parse encoded_branch = urllib.parse.quote(branch, safe="") url = f"{repo_api_url(h, o, r)}/branches/{encoded_branch}" + request_metadata = { + "branch": branch, + "required_permission": "gitea.branch.delete", + } with _audited("delete_branch", host=h, remote=remote, org=o, repo=r, - target_branch=branch, request_metadata={"branch": branch}): + target_branch=branch, request_metadata=request_metadata): api_request("DELETE", url, auth) return {"success": True, "message": f"Remote branch '{branch}' deleted."} @@ -3454,6 +3656,357 @@ def gitea_reconcile_merged_cleanups( return {"success": True, "performed": True, **report} +@mcp.tool() +def gitea_assess_already_landed_reconciliation( + pr_number: int, + target_branch: str = "master", + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, +) -> dict: + """Read-only: assess already-landed reconciliation for one open PR (#301). + + Fetches the PR live, refreshes the target branch, checks ancestor proof, + and returns a capability plan for the active profile. Does not review, + approve, request changes, merge, or mutate Gitea state. + + Args: + pr_number: Open PR to assess. + target_branch: Branch used for ancestry proof (default master). + remote: Known instance — 'dadeschools' or 'prgs'. + host: Override the Gitea host. + org: Override the owner/organization. + repo: Override the repository name. + + Returns: + dict with eligibility proof, capability plan, and safe next action. + """ + read_block = _profile_operation_gate("gitea.read") + if read_block: + return { + "success": False, + "performed": False, + "reasons": read_block, + "permission_report": _permission_block_report("gitea.read"), + } + + h, o, r = _resolve(remote, host, org, repo) + auth = _auth(h) + pr = api_request("GET", f"{repo_api_url(h, o, r)}/pulls/{pr_number}", auth) + + assessment = reconciliation_workflow.assess_open_pr_reconciliation( + pr=pr, + project_root=PROJECT_ROOT, + remote=remote, + target_branch=target_branch, + ) + + profile = get_profile() + capabilities = reconciliation_workflow.profile_reconciliation_capabilities( + profile.get("allowed_operations"), + profile.get("forbidden_operations"), + ) + plan = reconciliation_workflow.resolve_reconciliation_plan( + assessment=assessment, + capabilities=capabilities, + ) + + return { + "success": True, + "performed": False, + "pr_number": pr_number, + "assessment": assessment, + "capabilities": capabilities, + "plan": plan, + "workflow_source": "skills/llm-project-workflow/workflows/reconcile-landed-pr.md", + "task_mode": "reconcile-landed-pr", + "review_merge_allowed": False, + } + + +@mcp.tool() +def gitea_scan_already_landed_open_prs( + target_branch: str = "master", + limit: int = 50, + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, +) -> dict: + """Read-only: list open PRs already landed on the target branch (#301). + + Traverses open PR inventory with pagination proof, assesses each candidate, + and returns PRs classified as ``ALREADY_LANDED_RECONCILE_REQUIRED``. Does + not review, approve, merge, or mutate Gitea state. + + Args: + target_branch: Branch used for ancestry proof (default master). + limit: Max open PRs to inspect across pages. + remote: Known instance — 'dadeschools' or 'prgs'. + host: Override the Gitea host. + org: Override the owner/organization. + repo: Override the repository name. + + Returns: + dict with candidates, pagination metadata, and target branch SHA proof. + """ + read_block = _profile_operation_gate("gitea.read") + if read_block: + return { + "success": False, + "performed": False, + "reasons": read_block, + "permission_report": _permission_block_report("gitea.read"), + } + + h, o, r = _resolve(remote, host, org, repo) + auth = _auth(h) + url = f"{repo_api_url(h, o, r)}/pulls?state=open" + + open_prs: list[dict] = [] + current_page = 1 + pages_fetched = 0 + last_meta: dict = {} + per_page = min(50, max(1, int(limit))) + while pages_fetched < 100 and len(open_prs) < limit: + raw_page, last_meta = api_fetch_page( + url, auth, page=current_page, limit=per_page + ) + pages_fetched += 1 + remaining = limit - len(open_prs) + open_prs.extend(raw_page[:remaining]) + if last_meta["is_final_page"] or len(open_prs) >= limit: + break + current_page += 1 + + pagination = { + "page": 1, + "per_page": per_page, + "returned_count": len(open_prs), + "has_more": False, + "next_page": None, + "is_final_page": bool(last_meta.get("is_final_page")), + "pages_fetched": pages_fetched, + "inventory_complete": bool(last_meta.get("is_final_page")), + "total_count": len(open_prs) if last_meta.get("is_final_page") else None, + } + + target_fetch = reconciliation_workflow.fetch_target_branch( + PROJECT_ROOT, remote, target_branch + ) + + candidates: list[dict] = [] + for pr in open_prs: + assessment = reconciliation_workflow.assess_open_pr_reconciliation( + pr=pr, + project_root=PROJECT_ROOT, + remote=remote, + target_branch=target_branch, + target_fetch=target_fetch, + ) + if assessment.get("eligibility_class") == ( + reconciliation_workflow.ELIGIBILITY_ALREADY_LANDED + ): + candidates.append(assessment) + + return { + "success": True, + "performed": False, + "target_branch": target_branch, + "target_branch_sha": target_fetch.get("target_branch_sha"), + "git_ref_mutations": ( + [target_fetch["git_fetch_command"]] + if target_fetch.get("git_fetch_command") + else [] + ), + "candidates": candidates, + "candidate_count": len(candidates), + "pagination": pagination, + "task_mode": "reconcile-landed-pr", + "review_merge_allowed": False, + } + + +@mcp.tool() +def gitea_reconcile_already_landed_pr( + pr_number: int, + close_pr: bool = False, + close_linked_issue: bool = False, + post_comment: bool = False, + comment_body: str = "", + target_branch: str = "master", + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, +) -> dict: + """Reconcile an open PR whose head is already on the target branch (#310). + + Read-only assessment always runs. PR close requires ``gitea.pr.close`` and + proof that the pinned PR head SHA is an ancestor of a freshly fetched + target branch. Arbitrary PR closure is denied. + + Args: + pr_number: Open PR to reconcile. + close_pr: When True, close the PR after already-landed proof passes. + close_linked_issue: When True, close a linked issue after live fetch + when the issue is open and ``gitea.issue.close`` is allowed. + post_comment: When True, post ``comment_body`` on the PR thread. + comment_body: PR comment text when ``post_comment`` is True. + target_branch: Target branch used for ancestry proof (default master). + remote: Known instance — 'dadeschools' or 'prgs'. + host: Override the Gitea host. + org: Override the owner/organization. + repo: Override the repository name. + + Returns: + dict with eligibility proof, optional mutations, and recovery guidance. + """ + read_block = _profile_operation_gate("gitea.read") + if read_block: + return { + "success": False, + "performed": False, + "reasons": read_block, + "permission_report": _permission_block_report("gitea.read"), + } + + if close_pr: + close_block = _profile_operation_gate("gitea.pr.close") + if close_block: + return { + "success": False, + "performed": False, + "pr_number": pr_number, + "required_permission": "gitea.pr.close", + "reasons": close_block, + "permission_report": _permission_block_report("gitea.pr.close"), + "safe_next_action": ( + "Launch the prgs-reconciler MCP namespace or configure a " + "profile with gitea.pr.close for already-landed cleanup." + ), + } + + h, o, r = _resolve(remote, host, org, repo) + auth = _auth(h) + base = repo_api_url(h, o, r) + pr_url = f"{base}/pulls/{pr_number}" + pr = api_request("GET", pr_url, auth) + + assessment = already_landed_reconcile.assess_already_landed_reconciliation( + pr=pr, + project_root=PROJECT_ROOT, + remote=remote, + target_branch=target_branch, + ) + + result: dict = { + "success": True, + "performed": False, + "pr_number": pr_number, + "pr_state": assessment.get("pr_state"), + "candidate_head_sha": assessment.get("candidate_head_sha"), + "target_branch": assessment.get("target_branch"), + "target_branch_sha": assessment.get("target_branch_sha"), + "ancestor_proof": assessment.get("ancestor_proof"), + "eligibility_class": assessment.get("eligibility_class"), + "linked_issue": assessment.get("linked_issue"), + "close_allowed": assessment.get("close_allowed"), + "git_ref_mutations": assessment.get("git_ref_mutations") or [], + "reasons": list(assessment.get("reasons") or []), + "pr_closed": False, + "issue_closed": False, + "pr_comment_posted": False, + } + + if not assessment.get("close_allowed"): + result["success"] = False + result["safe_next_action"] = ( + "Do not close this PR via the reconciler path until " + "already-landed proof passes." + ) + return result + + verify_preflight_purity(remote) + + if post_comment and comment_body.strip(): + comment_block = _profile_operation_gate("gitea.pr.comment") + if comment_block: + result["success"] = False + result["reasons"].extend(comment_block) + result["permission_report"] = _permission_block_report( + "gitea.pr.comment" + ) + return result + comment_url = f"{base}/issues/{pr_number}/comments" + with _audited( + "comment_pr", + host=h, + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + request_metadata={"source": "reconcile_already_landed_pr"}, + ): + api_request("POST", comment_url, auth, {"body": comment_body.strip()}) + result["pr_comment_posted"] = True + result["performed"] = True + + if close_pr: + patch_url = f"{base}/pulls/{pr_number}" + request_metadata = { + "source": "reconcile_already_landed_pr", + "required_permission": "gitea.pr.close", + "candidate_head_sha": assessment.get("candidate_head_sha"), + "target_branch_sha": assessment.get("target_branch_sha"), + "ancestor_proof": assessment.get("ancestor_proof"), + } + with _audited( + "close_pr", + host=h, + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + request_metadata=request_metadata, + ): + closed = api_request("PATCH", patch_url, auth, {"state": "closed"}) + result["pr_closed"] = True + result["performed"] = True + result["pr_state"] = closed.get("state", "closed") + + linked_issue = assessment.get("linked_issue") + if close_linked_issue and linked_issue: + issue_block = _profile_operation_gate("gitea.issue.close") + if issue_block: + result["reasons"].extend(issue_block) + result["issue_close_blocked"] = True + return result + issue_url = f"{base}/issues/{linked_issue}" + issue = api_request("GET", issue_url, auth) + result["linked_issue_live_status"] = issue.get("state") + if (issue.get("state") or "").lower() == "open": + with _audited( + "close_issue", + host=h, + remote=remote, + org=o, + repo=r, + issue_number=linked_issue, + request_metadata={"source": "reconcile_already_landed_pr"}, + ): + api_request( + "PATCH", + issue_url, + auth, + {"state": "closed"}, + ) + result["issue_closed"] = True + result["performed"] = True + + return result + @mcp.tool() def gitea_close_issue( issue_number: int, @@ -3963,18 +4516,30 @@ def gitea_create_issue_comment( def _role_kind(allowed, forbidden) -> str: """Classify the active profile from its normalized operations. + 'reconciler' can close already-landed PRs without review/author powers; 'author' can create PRs / push branches; 'reviewer' can approve/merge; - 'mixed' can do both (a config smell — the reviewer-deadlock invariant - forbids it in v2 configs); 'limited' can do neither. + 'reconciler' can close already-landed PRs via ``gitea.pr.close`` without + review/author powers; 'mixed' can do both (a config smell — the + reviewer-deadlock invariant forbids it in v2 configs); 'limited' can do + neither. """ + if reconciler_profile.is_reconciler_profile(allowed, forbidden): + return "reconciler" def can(op): return gitea_config.check_operation(op, allowed, forbidden)[0] review = can("gitea.pr.approve") or can("gitea.pr.merge") author = can("gitea.pr.create") or can("gitea.branch.push") + reconciler = ( + can("gitea.pr.close") + and not review + and not author + ) if review and author: return "mixed" if review: return "reviewer" + if reconciler: + return "reconciler" if author: return "author" return "limited" @@ -4111,6 +4676,14 @@ _GUIDE_RULES = { "inventory, summarize) needs no explicit authorization. Enforced " "fail closed via subagent_gate.assess_subagent_delegation and " "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, " @@ -4221,6 +4794,28 @@ _PROJECT_SKILLS = { "Do not merge unless the operator explicitly authorizes it.", ], }, + "gitea-reconcile-landed-pr": { + "description": "Reconcile open PRs whose heads are already on the " + "target branch without review or merge.", + "when_to_use": "An open PR blocks the review queue but its head SHA " + "is already an ancestor of master; use the dedicated " + "reconciliation path instead of review/merge.", + "required_operations": ["gitea.read"], + "status": "available", + "notes": "Load skills/llm-project-workflow/workflows/reconcile-landed-pr.md " + "first. PR close requires exact gitea.pr.close; review/merge " + "capabilities must not be used on this path.", + "steps": [ + "Resolve task: gitea_resolve_task_capability(task='reconcile_landed_pr').", + "Load canonical workflow via mcp_get_skill_guide or " + "skills/llm-project-workflow/workflows/reconcile-landed-pr.md.", + "Scan queue: gitea_scan_already_landed_open_prs (pagination proof).", + "Assess one PR: gitea_assess_already_landed_reconciliation.", + "Follow the plan outcome: full close only with gitea.pr.close, " + "comment-then-stop when close is missing, recovery handoff otherwise.", + "Never approve, request changes, or merge on this path.", + ], + }, "gitea-pr-merge": { "description": "Gated merge of an approved pull request.", "when_to_use": "Reviewer/merger profile with explicit operator " @@ -4745,9 +5340,11 @@ _RUNTIME_CAPABILITY_TASKS = ( "create_issue", "comment_issue", "create_pr", + "work_issue", "review_pr", "merge_pr", "close_issue", + "reconcile_already_landed_pr", ) @@ -4796,9 +5393,11 @@ def _build_runtime_task_capabilities( "create_issue": "can_create_issues", "comment_issue": "can_comment_on_issues", "create_pr": "can_author_prs", + "work_issue": "can_work_issues", "review_pr": "can_review_prs", "merge_pr": "can_merge_prs", "close_issue": "can_close_issues", + "reconcile_already_landed_pr": "can_reconcile_already_landed_prs", } for task in _RUNTIME_CAPABILITY_TASKS: permission = task_capability_map.required_permission(task) @@ -4826,6 +5425,65 @@ 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, @@ -4888,6 +5546,37 @@ def gitea_get_shell_health() -> dict: return native_mcp_preference.shell_health_status() +@mcp.tool() +def gitea_validate_review_final_report( + report_text: str, + review_decision_lock: dict | None = None, + linked_issue_lock: dict | None = None, + validation_report: dict | None = None, + action_log: list[dict] | None = None, + mutations_observed: bool = False, + local_edits: bool = False, + validation_session: dict | None = None, +) -> dict: + """Read-only: machine-validate reviewer final report schema before output (#391). + + Composes the composable final-report validator with review-specific schema + gates (legacy fields, mutation categories, merge/issue claims, blocked + handoff replay, and narrative/handoff consistency). + """ + from review_final_report_schema import assess_review_final_report_schema + + return assess_review_final_report_schema( + report_text, + review_decision_lock=review_decision_lock, + linked_issue_lock=linked_issue_lock, + validation_report=validation_report, + action_log=action_log, + mutations_observed=mutations_observed, + local_edits=local_edits, + validation_session=validation_session, + ) + + @mcp.tool() def gitea_get_runtime_context( remote: str = "dadeschools", @@ -5006,6 +5695,10 @@ def gitea_get_runtime_context( "preflight_block_reasons": preflight["preflight_block_reasons"], "preflight_workspace": preflight.get("preflight_workspace"), "session_capabilities": session_capabilities, + "reconciler_profile_assessment": reconciler_profile.assess_reconciler_profile( + allowed, forbidden + ), + "role_kind": _role_kind(allowed, forbidden), "shell_health": native_mcp_preference.shell_health_status(), "workflow_load_proof": review_workflow_load.workflow_load_status( PROJECT_ROOT), diff --git a/migrate_profiles.py b/migrate_profiles.py index 3613c04..6bf150c 100755 --- a/migrate_profiles.py +++ b/migrate_profiles.py @@ -31,6 +31,8 @@ REVIEWER_DEFAULT_FORBIDDEN = ["branch", "commit", "push", "open_pr"] def infer_role(name, execution_profile): """Return the unambiguous role for a legacy profile name, or None.""" haystack = f"{name} {execution_profile or ''}".lower() + if "reconciler" in haystack: + return "reconciler" has_author = "author" in haystack has_reviewer = "reviewer" in haystack if has_author == has_reviewer: diff --git a/pr_queue_cleanup.py b/pr_queue_cleanup.py new file mode 100644 index 0000000..c1237aa --- /dev/null +++ b/pr_queue_cleanup.py @@ -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" + ), + } diff --git a/reconciler_profile.py b/reconciler_profile.py new file mode 100644 index 0000000..4d9d589 --- /dev/null +++ b/reconciler_profile.py @@ -0,0 +1,94 @@ +"""Reconciler profile model for already-landed PR closure (#304). + +Defines the narrowly scoped operation set for a dedicated reconciler profile +such as ``prgs-reconciler``. Close MCP tooling and ancestry gates ship in the +#310 stack; this module validates profile shape only. +""" + +from __future__ import annotations + +import gitea_config + +RECONCILER_REQUIRED_OPERATIONS = ( + "gitea.read", + "gitea.pr.close", +) + +RECONCILER_RECOMMENDED_OPERATIONS = ( + "gitea.pr.comment", + "gitea.issue.comment", + "gitea.issue.close", +) + +RECONCILER_FORBIDDEN_OPERATIONS = ( + "gitea.pr.approve", + "gitea.pr.merge", + "gitea.pr.review", + "gitea.pr.create", + "gitea.branch.push", + "gitea.repo.commit", +) + + +def _normalized_allowed(allowed: list[str]) -> set[str]: + normalized: set[str] = set() + for entry in allowed or []: + try: + normalized.add(gitea_config.normalize_operation(entry)) + except gitea_config.ConfigError: + continue + return normalized + + +def _forbidden_in_allowed(allowed: list[str], ops: tuple[str, ...]) -> list[str]: + """Return forbidden ops that are explicitly listed in *allowed*.""" + allowed_n = _normalized_allowed(allowed) + present: list[str] = [] + for op in ops: + try: + if gitea_config.normalize_operation(op) in allowed_n: + present.append(op) + except gitea_config.ConfigError: + continue + return present + + +def is_reconciler_profile(allowed: list[str], forbidden: list[str]) -> bool: + """Return True when *allowed*/*forbidden* describe a reconciler profile.""" + def can(op: str) -> bool: + return gitea_config.check_operation(op, allowed, forbidden)[0] + + if not can("gitea.pr.close"): + return False + if _forbidden_in_allowed(allowed, RECONCILER_FORBIDDEN_OPERATIONS): + return False + return True + + +def assess_reconciler_profile(allowed: list[str], forbidden: list[str]) -> dict: + """Validate reconciler profile operations (read-only, fail closed).""" + allowed = list(allowed or []) + forbidden = list(forbidden or []) + reasons: list[str] = [] + + for op in RECONCILER_REQUIRED_OPERATIONS: + ok, _ = gitea_config.check_operation(op, allowed, forbidden) + if not ok: + reasons.append(f"missing required operation {op}") + + for op in _forbidden_in_allowed(allowed, RECONCILER_FORBIDDEN_OPERATIONS): + reasons.append(f"forbidden operation must not be allowed: {op}") + + missing_recommended = [ + op for op in RECONCILER_RECOMMENDED_OPERATIONS + if not gitea_config.check_operation(op, allowed, forbidden)[0] + ] + + return { + "is_reconciler_profile": is_reconciler_profile(allowed, forbidden), + "valid": not reasons and is_reconciler_profile(allowed, forbidden), + "reasons": reasons, + "missing_recommended_operations": missing_recommended, + "required_operations": list(RECONCILER_REQUIRED_OPERATIONS), + "forbidden_operations": list(RECONCILER_FORBIDDEN_OPERATIONS), + } \ No newline at end of file diff --git a/reconciliation_workflow.py b/reconciliation_workflow.py new file mode 100644 index 0000000..48867b7 --- /dev/null +++ b/reconciliation_workflow.py @@ -0,0 +1,292 @@ +"""Already-landed PR reconciliation workflow helpers (#301). + +Read-only assessment and capability planning for reconciling open PRs whose +heads are already ancestors of the target branch. Does not invoke review or +merge paths. +""" + +from __future__ import annotations + +import subprocess +from typing import Any + +import gitea_config +from merged_cleanup_reconcile import extract_linked_issue, is_head_ancestor_of_ref + +ELIGIBILITY_ALREADY_LANDED = "ALREADY_LANDED_RECONCILE_REQUIRED" +ELIGIBILITY_NOT_LANDED = "NOT_ALREADY_LANDED" +ELIGIBILITY_STALE_TARGET = "TARGET_BRANCH_UNVERIFIED" +ELIGIBILITY_PR_NOT_OPEN = "PR_NOT_OPEN" + +OUTCOME_FULL_RECONCILE = "FULL_RECONCILE_CLOSE_ALLOWED" +OUTCOME_PARTIAL_COMMENT = "PARTIAL_RECONCILE_COMMENT_THEN_STOP" +OUTCOME_RECOVERY_HANDOFF = "RECOVERY_HANDOFF_ONLY" +OUTCOME_NOT_LANDED = "NOT_LANDED_NO_ACTION" +OUTCOME_GATE_NOT_PROVEN = "GATE_NOT_PROVEN" + +RECONCILE_WORKFLOW_MARKERS = ( + "workflows/reconcile-landed-pr.md", + "reconcile-landed-pr.md", +) + +RECONCILE_TASK_MARKERS = ( + "reconcile-landed-pr", + "reconcile already-landed", + "reconcile_already_landed", +) + + +def fetch_target_branch(project_root: str, remote: str, branch: str) -> dict[str, Any]: + """Fetch *branch* from *remote* and return the resolved SHA.""" + ref = f"{remote}/{branch}" + fetch = subprocess.run( + ["git", "-C", project_root, "fetch", remote, branch], + capture_output=True, + text=True, + check=False, + ) + if fetch.returncode != 0: + return { + "success": False, + "target_branch": branch, + "target_ref": ref, + "target_branch_sha": None, + "reasons": [ + f"git fetch {remote} {branch} failed: " + f"{(fetch.stderr or fetch.stdout or '').strip()}" + ], + } + + rev = subprocess.run( + ["git", "-C", project_root, "rev-parse", ref], + capture_output=True, + text=True, + check=False, + ) + if rev.returncode != 0: + return { + "success": False, + "target_branch": branch, + "target_ref": ref, + "target_branch_sha": None, + "reasons": [ + f"git rev-parse {ref} failed: " + f"{(rev.stderr or rev.stdout or '').strip()}" + ], + } + + return { + "success": True, + "target_branch": branch, + "target_ref": ref, + "target_branch_sha": (rev.stdout or "").strip(), + "reasons": [], + "git_fetch_command": f"git fetch {remote} {branch}", + } + + +def profile_reconciliation_capabilities( + allowed: list[str] | None, + forbidden: list[str] | None, +) -> dict[str, bool]: + """Map active profile operations to reconciliation capabilities.""" + allowed = allowed or [] + forbidden = forbidden or [] + + def can(op: str) -> bool: + return gitea_config.check_operation(op, allowed, forbidden)[0] + + return { + "read": can("gitea.read"), + "comment_pr": can("gitea.pr.comment"), + "comment_issue": can("gitea.issue.comment"), + "close_pr": can("gitea.pr.close"), + "close_issue": can("gitea.issue.close"), + "review_pr": can("gitea.pr.review") or can("gitea.pr.approve"), + "merge_pr": can("gitea.pr.merge"), + } + + +def assess_open_pr_reconciliation( + *, + pr: dict[str, Any], + project_root: str, + remote: str, + target_branch: str, + target_fetch: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Assess whether an open PR is eligible for already-landed reconciliation.""" + pr_number = int(pr.get("number") or 0) + pr_state = (pr.get("state") or "").strip().lower() + head = pr.get("head") or {} + head_sha = head.get("sha") if isinstance(head, dict) else None + head_ref = head.get("ref") if isinstance(head, dict) else None + base = pr.get("base") or {} + base_ref = base.get("ref") if isinstance(base, dict) else None + title = pr.get("title") or "" + body = pr.get("body") or "" + + fetch_result = target_fetch or fetch_target_branch( + project_root, remote, target_branch + ) + linked_issue = extract_linked_issue(title, body) + + result: dict[str, Any] = { + "pr_number": pr_number, + "pr_state": pr_state, + "candidate_head_sha": head_sha, + "head_ref": head_ref, + "base_ref": base_ref or target_branch, + "target_branch": target_branch, + "target_branch_sha": fetch_result.get("target_branch_sha"), + "linked_issue": linked_issue, + "git_ref_mutations": [], + "reasons": [], + "review_merge_allowed": False, + } + if fetch_result.get("git_fetch_command"): + result["git_ref_mutations"].append(fetch_result["git_fetch_command"]) + + if pr_state != "open": + result["eligibility_class"] = ELIGIBILITY_PR_NOT_OPEN + result["ancestor_proof"] = None + result["reconciliation_allowed"] = False + result["reasons"].append(f"PR #{pr_number} state is {pr_state!r}, not open") + return result + + if not fetch_result.get("success"): + result["eligibility_class"] = ELIGIBILITY_STALE_TARGET + result["ancestor_proof"] = None + result["reconciliation_allowed"] = False + result["reasons"].extend(fetch_result.get("reasons") or []) + return result + + target_ref = fetch_result.get("target_ref") or f"{remote}/{target_branch}" + ancestor = is_head_ancestor_of_ref(project_root, head_sha, target_ref) + result["ancestor_proof"] = ancestor + + if ancestor is None: + result["eligibility_class"] = ELIGIBILITY_STALE_TARGET + result["reconciliation_allowed"] = False + result["reasons"].append( + f"ancestor check failed for head {head_sha!r} against {target_ref}" + ) + return result + + if ancestor: + result["eligibility_class"] = ELIGIBILITY_ALREADY_LANDED + result["reconciliation_allowed"] = True + return result + + result["eligibility_class"] = ELIGIBILITY_NOT_LANDED + result["reconciliation_allowed"] = False + result["reasons"].append( + f"PR head {head_sha} is not an ancestor of {target_ref}" + ) + return result + + +def resolve_reconciliation_plan( + *, + assessment: dict[str, Any], + capabilities: dict[str, bool] | None, +) -> dict[str, Any]: + """Map eligibility + profile capabilities to a reconciliation outcome.""" + caps = capabilities or {} + reasons: list[str] = [] + + if not assessment.get("reconciliation_allowed"): + outcome = OUTCOME_NOT_LANDED + if assessment.get("eligibility_class") in { + ELIGIBILITY_STALE_TARGET, + ELIGIBILITY_PR_NOT_OPEN, + }: + outcome = OUTCOME_GATE_NOT_PROVEN + reasons.extend(assessment.get("reasons") or []) + return { + "outcome": outcome, + "close_pr_allowed": False, + "comment_pr_allowed": False, + "close_issue_allowed": False, + "comment_issue_allowed": False, + "review_merge_allowed": False, + "missing_capabilities": _missing_reconciliation_capabilities(caps), + "safe_next_action": ( + "Do not reconcile via review/merge; repair missing proof first." + ), + "reasons": reasons, + } + + close_pr = bool(caps.get("close_pr")) + comment_pr = bool(caps.get("comment_pr")) + close_issue = bool(caps.get("close_issue")) + comment_issue = bool(caps.get("comment_issue")) + + if caps.get("review_pr") or caps.get("merge_pr"): + reasons.append( + "reconciliation path must not use review/merge capabilities" + ) + + if close_pr: + outcome = OUTCOME_FULL_RECONCILE + safe_next_action = ( + "Run reconciliation close via exact gitea.pr.close after proof; " + "do not approve or merge." + ) + elif comment_pr: + outcome = OUTCOME_PARTIAL_COMMENT + safe_next_action = ( + "Post one reconciliation comment with ancestor proof, then stop " + "for an authorized close profile." + ) + else: + outcome = OUTCOME_RECOVERY_HANDOFF + safe_next_action = ( + "Produce a recovery handoff naming missing gitea.pr.close and/or " + "gitea.pr.comment; do not loop through review/merge." + ) + reasons.append("gitea.pr.close is not available in the active profile") + + return { + "outcome": outcome, + "close_pr_allowed": close_pr, + "comment_pr_allowed": comment_pr, + "close_issue_allowed": close_issue, + "comment_issue_allowed": comment_issue, + "review_merge_allowed": False, + "missing_capabilities": _missing_reconciliation_capabilities(caps), + "safe_next_action": safe_next_action, + "reasons": reasons, + } + + +def _missing_reconciliation_capabilities(caps: dict[str, bool]) -> list[str]: + missing = [] + if not caps.get("read"): + missing.append("gitea.read") + if not caps.get("close_pr"): + missing.append("gitea.pr.close") + if not caps.get("comment_pr"): + missing.append("gitea.pr.comment") + return missing + + +def assess_reconcile_workflow_source(report_text: str) -> dict[str, Any]: + """Reconciliation reports must cite the canonical workflow file.""" + lower = (report_text or "").lower() + reasons = [] + if not any(marker in lower for marker in RECONCILE_WORKFLOW_MARKERS): + reasons.append( + "reconciliation report missing workflow source " + "(workflows/reconcile-landed-pr.md)" + ) + if not any(marker in lower for marker in RECONCILE_TASK_MARKERS): + reasons.append( + "reconciliation report missing task mode declaration " + "(reconcile-landed-pr)" + ) + return { + "complete": not reasons, + "downgraded": bool(reasons), + "reasons": reasons, + } \ No newline at end of file diff --git a/review_final_report_schema.py b/review_final_report_schema.py new file mode 100644 index 0000000..e428015 --- /dev/null +++ b/review_final_report_schema.py @@ -0,0 +1,321 @@ +"""Reviewer final-report schema verification before session output (#391). + +Composes the composable ``assess_final_report_validator`` (#327) with +review-specific schema gates required before a reviewer session completes. +""" + +from __future__ import annotations + +import re +from typing import Any + +from final_report_validator import ( + assess_final_report_validator, + validator_finding, + _handoff_fields, +) + +_LEGACY_STALE_FIELDS = ( + "pinned reviewed head", + "scratch worktree used", + "workspace mutations", +) + +_REVIEWED_HEAD_RE = re.compile( + r"(?:pinned reviewed head|reviewed head sha)\s*:\s*([0-9a-f]{7,40})", + re.IGNORECASE, +) +_VALIDATION_PASS_RE = re.compile( + r"validation\s*:\s*(?:pass|passed|strong|ok|green)", + re.IGNORECASE, +) +_MERGED_CLAIM_RE = re.compile( + r"\b(?:merged|merge result\s*:\s*merged|pr\s+#\d+\s+merged)\b", + re.IGNORECASE, +) +_MERGE_RESULT_RE = re.compile(r"merge result\s*:", re.IGNORECASE) +_ANCESTRY_PROOF_RE = re.compile( + r"(?:ancestor proof|target branch sha|merge commit)", + re.IGNORECASE, +) +_ISSUE_CLOSED_RE = re.compile( + r"(?:linked issue(?:\s+live)?\s+status\s*:\s*closed|issue\s+#\d+\s+closed)", + re.IGNORECASE, +) +_LIVE_ISSUE_PROOF_RE = re.compile( + r"gitea_view_issue|(?:issue|linked issue).{0,40}fetched live|live fetch proof", + re.IGNORECASE, +) +_FULL_SUITE_PASS_RE = re.compile( + r"(?:full suite passed|full test suite passed|\d+\s+passed,\s*0\s+failed)", + re.IGNORECASE, +) +_TESTS_IGNORED_RE = re.compile( + r"(?:ignored tests|tests?\s+ignored|skipped tests|not run|tests?\s+skipped)", + re.IGNORECASE, +) +_BLOCKED_HANDOFF_RE = re.compile( + r"(?:blocker\s*:|recovery handoff|infra_stop|capability stop|mutation blocked)", + re.IGNORECASE, +) +_REPLAY_COMMAND_RE = re.compile( + r"(?:gitea_submit_pr_review|gitea_merge_pr|gitea_review_pr|" + r"submitted\s+['\"]approve['\"]|replay approve|replay merge)", + re.IGNORECASE, +) +_PENDING_RE = re.compile(r"\bPENDING\b", re.IGNORECASE) +_APPROVED_RE = re.compile( + r"(?:review decision\s*:\s*approve|submitted\s+approve|official review submitted)", + re.IGNORECASE, +) +_DRY_RUN_RE = re.compile(r"dry[- ]run", re.IGNORECASE) +_FINDING_READY_RE = re.compile( + r"(?:finding ready|ready to submit|not yet submitted)", + re.IGNORECASE, +) +_NARRATIVE_APPROVE_RE = re.compile( + r"(?:^|\n)\s*(?:##\s+)?(?:summary|verdict|recommendation)\b[^\n]*\bapprove\b", + re.IGNORECASE | re.MULTILINE, +) +_HANDOFF_DECISION_RE = re.compile( + r"review decision\s*:\s*(\w+)", + re.IGNORECASE, +) + + +def _rule_legacy_stale_fields(report_text: str) -> list[dict[str, str]]: + fields = _handoff_fields(report_text) + findings: list[dict[str, str]] = [] + for stale in _LEGACY_STALE_FIELDS: + if stale in fields and fields[stale].lower() not in {"", "none", "n/a"}: + findings.append( + validator_finding( + "reviewer.legacy_stale_field", + "block", + stale.title(), + f"legacy or stale handoff field '{stale}' must not appear in " + "canonical reviewer final reports", + "remove stale fields; use Candidate head SHA and precise " + "mutation categories instead", + ) + ) + return findings + + +def _rule_reviewed_head_without_validation(report_text: str) -> list[dict[str, str]]: + text = report_text or "" + if not _REVIEWED_HEAD_RE.search(text): + return [] + if _VALIDATION_PASS_RE.search(text): + return [] + return [ + validator_finding( + "reviewer.reviewed_head_without_validation", + "block", + "Pinned reviewed head", + "reviewed/pinned head SHA reported without validation pass proof", + "document validation command, cwd, and pass result before pinning head SHA", + ) + ] + + +def _rule_merged_without_proof(report_text: str) -> list[dict[str, str]]: + text = report_text or "" + if not _MERGED_CLAIM_RE.search(text): + return [] + if _MERGE_RESULT_RE.search(text) and _ANCESTRY_PROOF_RE.search(text): + return [] + return [ + validator_finding( + "reviewer.merged_without_proof", + "block", + "Merge result", + "merged outcome claimed without merge result and target ancestry proof", + "include Merge result, target branch SHA, and ancestor proof before claiming merged", + ) + ] + + +def _rule_issue_closed_without_live_proof(report_text: str) -> list[dict[str, str]]: + text = report_text or "" + if not _ISSUE_CLOSED_RE.search(text): + return [] + fields = _handoff_fields(text) + status_value = fields.get("linked issue live status", "") + readonly_value = fields.get("read-only diagnostics", "") + proof_blob = f"{status_value} {readonly_value}".lower() + if _LIVE_ISSUE_PROOF_RE.search(proof_blob): + return [] + return [ + validator_finding( + "reviewer.issue_closed_without_live_proof", + "block", + "Linked issue", + "issue closed claimed without live linked-issue verification proof", + "fetch linked issue live (gitea_view_issue) and record Linked issue live status", + ) + ] + + +def _rule_full_suite_pass_ignored_tests(report_text: str) -> list[dict[str, str]]: + text = report_text or "" + if not (_FULL_SUITE_PASS_RE.search(text) and _TESTS_IGNORED_RE.search(text)): + return [] + if re.search(r"explicitly\s+(?:ignored|skipped)", text, re.IGNORECASE): + return [] + return [ + validator_finding( + "reviewer.full_suite_pass_ignored_tests", + "block", + "Validation", + "full suite pass claimed while tests were ignored or skipped without " + "explicit disclosure", + "state which tests were ignored/skipped or downgrade validation verdict", + ) + ] + + +def _rule_blocked_handoff_replay(report_text: str) -> list[dict[str, str]]: + text = report_text or "" + if not _BLOCKED_HANDOFF_RE.search(text): + return [] + if not _REPLAY_COMMAND_RE.search(text): + return [] + return [ + validator_finding( + "reviewer.blocked_handoff_replay", + "block", + "Safe next action", + "blocked recovery handoff includes direct approve or merge replay commands", + "restart the full workflow after the blocker clears; do not replay mutations", + ) + ] + + +def _rule_narrative_handoff_drift(report_text: str) -> list[dict[str, str]]: + text = report_text or "" + narrative_approve = bool(_NARRATIVE_APPROVE_RE.search(text)) + decision_match = _HANDOFF_DECISION_RE.search(text) + if not narrative_approve or not decision_match: + return [] + handoff_decision = decision_match.group(1).strip().lower() + if handoff_decision in {"approve", "approved"}: + return [] + return [ + validator_finding( + "reviewer.narrative_handoff_drift", + "block", + "Review decision", + f"narrative approves PR but controller handoff says '{handoff_decision}'", + "align narrative summary with controller handoff Review decision field", + ) + ] + + +def _rule_review_state_ambiguous(report_text: str) -> list[dict[str, str]]: + text = report_text or "" + findings: list[dict[str, str]] = [] + if _PENDING_RE.search(text) and _APPROVED_RE.search(text): + findings.append( + validator_finding( + "reviewer.pending_vs_approved", + "block", + "Review decision", + "report conflates PENDING review state with APPROVED outcome", + "distinguish official submitted review from pending or ready-to-submit state", + ) + ) + if _DRY_RUN_RE.search(text) and _APPROVED_RE.search(text): + if "dry-run only" not in text.lower(): + findings.append( + validator_finding( + "reviewer.dry_run_vs_submitted", + "block", + "Review mutations", + "dry-run language mixed with official approve/submitted claims", + "mark dry-run explicitly or document the live review mutation proof", + ) + ) + if _FINDING_READY_RE.search(text) and _APPROVED_RE.search(text): + findings.append( + validator_finding( + "reviewer.finding_ready_vs_submitted", + "downgrade", + "Review decision", + "finding-ready wording combined with submitted-approve claims", + "state whether review was submitted live or only prepared for submission", + ) + ) + return findings + + +_SCHEMA_RULES = ( + _rule_legacy_stale_fields, + _rule_reviewed_head_without_validation, + _rule_merged_without_proof, + _rule_issue_closed_without_live_proof, + _rule_full_suite_pass_ignored_tests, + _rule_blocked_handoff_replay, + _rule_narrative_handoff_drift, + _rule_review_state_ambiguous, +) + + +def _merge_validator_results( + base: dict[str, Any], + extra_findings: list[dict[str, str]], +) -> dict[str, Any]: + findings = list(base.get("findings") or []) + extra_findings + blocked = base.get("blocked") or any(f["severity"] == "block" for f in extra_findings) + downgraded = base.get("downgraded") or any( + f["severity"] == "downgrade" for f in extra_findings + ) + grade = base.get("grade", "A") + if blocked: + grade = "blocked" + elif downgraded and grade == "A": + grade = "downgraded" + reasons = [f"{f['rule_id']}: {f['reason']}" for f in findings] + safe_next = base.get("safe_next_action") or "none" + if extra_findings: + safe_next = extra_findings[0].get("safe_next_action", safe_next) + return { + **base, + "grade": grade, + "blocked": blocked, + "downgraded": downgraded, + "findings": findings, + "reasons": reasons, + "safe_next_action": safe_next, + "complete": grade == "A", + "schema_rules_applied": len(_SCHEMA_RULES), + } + + +def assess_review_final_report_schema( + report_text: str, + *, + review_decision_lock: dict | None = None, + linked_issue_lock: dict | None = None, + validation_report: dict | None = None, + action_log: list[dict] | None = None, + mutations_observed: bool = False, + local_edits: bool = False, + validation_session: dict | None = None, +) -> dict[str, Any]: + """Validate reviewer final report text before session completion (#391).""" + base = assess_final_report_validator( + report_text, + "review_pr", + review_decision_lock=review_decision_lock, + linked_issue_lock=linked_issue_lock, + validation_report=validation_report, + action_log=action_log, + mutations_observed=mutations_observed, + local_edits=local_edits, + validation_session=validation_session, + ) + extra: list[dict[str, str]] = [] + for rule in _SCHEMA_RULES: + extra.extend(rule(report_text)) + return _merge_validator_results(base, extra) \ No newline at end of file diff --git a/review_merge_state_machine.py b/review_merge_state_machine.py new file mode 100644 index 0000000..33c9705 --- /dev/null +++ b/review_merge_state_machine.py @@ -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, + } \ No newline at end of file diff --git a/review_proofs.py b/review_proofs.py index 17f1527..20cf89b 100644 --- a/review_proofs.py +++ b/review_proofs.py @@ -14,9 +14,15 @@ is usable from prompts, harness assertions, and tests. The MCP-level gates here weakens or replaces them. """ +import hashlib import re import issue_duplicate_gate +from reconciliation_workflow import ( + RECONCILE_TASK_MARKERS, + RECONCILE_WORKFLOW_MARKERS, + assess_reconcile_workflow_source, +) from reviewer_worktree import assess_reviewer_worktree_proof _FULL_SHA = re.compile(r"^[0-9a-f]{40}$") @@ -1699,16 +1705,41 @@ def build_final_report(checkout_proof, inventory, validation, contamination, if report_text and _QUEUE_STATUS_REPORT_HINT.search(report_text) 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 = ( assess_proof_wording(report_text) if report_text else {"proven": True, "block": False, "reasons": [], "violations": []} ) + reconcile_inventory = ( + assess_reconcile_inventory_report(report_text) + if report_text and _RECONCILE_INVENTORY_HINT.search(report_text) + else {"proven": True, "block": False, "reasons": []} + ) + reconcile_linked_issue = ( + assess_reconcile_linked_issue_report(report_text) + if report_text and _RECONCILE_LINKED_ISSUE_HINT.search(report_text) + else {"proven": True, "block": False, "reasons": []} + ) + 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": []} ) + proof_backed_handoff = ( + assess_proof_backed_handoff_report(report_text) + if report_text and _PROOF_BACKED_HANDOFF_HINT.search(report_text) + else {"proven": True, "block": False, "reasons": []} + ) if baseline_validation is None and report_text: baseline_validation = assess_reviewer_baseline_validation_proof( report_text=report_text, @@ -1721,6 +1752,17 @@ def build_final_report(checkout_proof, inventory, validation, contamination, "reasons": [], "violations": [], } + if report_text: + already_landed_classification = assess_already_landed_classification_report( + report_text + ) + else: + already_landed_classification = { + "proven": True, + "block": False, + "reasons": [], + "violations": [], + } contamination_status = contamination.get("status", "unknown") checkout_proven = bool(checkout_proof.get("proven")) @@ -1870,6 +1912,26 @@ def build_final_report(checkout_proof, inventory, validation, contamination, "queue-status report violates loaded workflow proof gates (#339)" ) downgrade_reasons.extend(queue_status_report.get("reasons", [])) + if not reconcile_inventory.get("proven"): + downgrade_reasons.append( + "reconciliation PR inventory lacks pagination proof (#308)" + ) + downgrade_reasons.extend(reconcile_inventory.get("reasons", [])) + if not reconcile_linked_issue.get("proven"): + downgrade_reasons.append( + "reconciliation linked issue status lacks live fetch proof (#300)" + ) + downgrade_reasons.extend(reconcile_linked_issue.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)" @@ -1880,6 +1942,16 @@ def build_final_report(checkout_proof, inventory, validation, contamination, "reviewer baseline validation proof missing or failed (#325)" ) downgrade_reasons.extend(baseline_validation.get("reasons", [])) + if not proof_backed_handoff.get("proven"): + downgrade_reasons.append( + "proof-sensitive handoff claims lack command/tool evidence (#395)" + ) + downgrade_reasons.extend(proof_backed_handoff.get("reasons", [])) + if not already_landed_classification.get("proven"): + downgrade_reasons.append( + "already-landed classification wording missing or invalid (#295)" + ) + downgrade_reasons.extend(already_landed_classification.get("reasons", [])) merge_allowed = ( identity_eligible @@ -1968,6 +2040,24 @@ def build_final_report(checkout_proof, inventory, validation, contamination, "queue_status_violations": list( queue_status_report.get("violations") or [] ), + "reconcile_inventory_proven": bool(reconcile_inventory.get("proven")), + "reconcile_inventory_violations": list( + reconcile_inventory.get("reasons") or [] + ), + "reconcile_linked_issue_proven": bool(reconcile_linked_issue.get("proven")), + "reconcile_linked_issue_violations": list( + reconcile_linked_issue.get("reasons") 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 [] @@ -1976,6 +2066,12 @@ def build_final_report(checkout_proof, inventory, validation, contamination, "baseline_validation_violations": list( baseline_validation.get("violations") or [] ), + "already_landed_classification_proven": bool( + already_landed_classification.get("proven") + ), + "already_landed_classification_violations": list( + already_landed_classification.get("reasons") or [] + ), } @@ -2242,6 +2338,30 @@ CREATE_ISSUE_FORBIDDEN_CROSS_MODE_FIELDS = ( ("Terminal review mutation", ("terminal review mutation",)), ) +WORK_ISSUE_WORKFLOW_MARKERS = ( + "workflows/work-issue.md", + "work-issue.md", +) + +WORK_ISSUE_TASK_MARKERS = ( + "work-issue", + "work issue", +) + +WORK_ISSUE_FORBIDDEN_CROSS_MODE_FIELDS = ( + ("Selected PR", ("selected pr",)), + ("Review decision", ("review decision",)), + ("Merge result", ("merge result",)), + ("Merge preflight", ("merge preflight",)), + ("Already-landed gate", ("already-landed gate",)), + ("Pinned reviewed head", ("pinned reviewed head",)), + ("Terminal review mutation", ("terminal review mutation",)), + ("Duplicate search terms", ("duplicate search terms",)), + ("Issues created", ("issues created",)), + ("Issues skipped as duplicates", ("issues skipped as duplicates",)), + ("Requested issue task", ("requested issue task",)), +) + def _handoff_section_lines(report_text): """Return the lines of the Controller Handoff section, or None.""" @@ -3352,6 +3472,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: """#337: create-issue reports must cite the canonical workflow file.""" lower = (report_text or "").lower() @@ -3373,6 +3554,104 @@ def assess_create_issue_workflow_source(report_text: str) -> dict: } +def assess_work_issue_workflow_source(report_text: str) -> dict: + """#139: work-issue reports must cite the canonical workflow file.""" + lower = (report_text or "").lower() + reasons = [] + if not any(marker in lower for marker in WORK_ISSUE_WORKFLOW_MARKERS): + reasons.append( + "work-issue report missing workflow source " + "(workflows/work-issue.md)" + ) + if not any(marker in lower for marker in WORK_ISSUE_TASK_MARKERS): + reasons.append( + "work-issue report missing task mode declaration (work-issue)" + ) + return { + "complete": not reasons, + "downgraded": bool(reasons), + "reasons": reasons, + } + + +def assess_work_issue_mode_isolation(report_text: str) -> dict: + """#139: reject review-merge/create-issue handoff fields in work-issue runs.""" + section = _handoff_section_lines(report_text) + reasons = [] + if section is None: + return { + "complete": True, + "downgraded": False, + "reasons": [], + } + + labels = [] + for line in section: + stripped = line.strip().lstrip("-*").strip() + if ":" in stripped: + labels.append(stripped.split(":", 1)[0].strip().lower()) + + for name, aliases in WORK_ISSUE_FORBIDDEN_CROSS_MODE_FIELDS: + if any( + label.startswith(alias) + for label in labels + for alias in aliases + ): + reasons.append( + f"work-issue handoff must not include cross-mode field " + f"'{name}'" + ) + + legacy_workspace = any( + label.startswith("workspace mutations") for label in labels + ) + precise_categories = any( + label.startswith("file edits by author") + for label in labels + ) + if legacy_workspace and not precise_categories: + reasons.append( + "work-issue handoff must use precise mutation categories " + "instead of legacy 'Workspace mutations'" + ) + + return { + "complete": not reasons, + "downgraded": bool(reasons), + "reasons": reasons, + } + + +def assess_work_issue_final_report(report_text: str) -> dict: + """#139: composite verifier for work-issue final reports.""" + checks = { + "workflow_source": assess_work_issue_workflow_source(report_text), + "mode_isolation": assess_work_issue_mode_isolation(report_text), + } + + reasons = [] + downgraded = False + for name, result in checks.items(): + verdict = result.get("verdict") + if verdict in ("missing", "incomplete"): + downgraded = True + reasons.extend(result.get("reasons") or []) + elif result.get("downgraded") or not result.get("complete", True): + downgraded = True + reasons.extend( + f"{name}: {r}" for r in (result.get("reasons") or []) + ) + + grade = "A" if not downgraded else "downgraded" + return { + "grade": grade, + "downgraded": downgraded, + "checks": checks, + "reasons": reasons, + "complete": not downgraded, + } + + def assess_create_issue_mode_isolation(report_text: str) -> dict: """#337: reject work-issue/review-merge handoff fields in create-issue runs.""" section = _handoff_section_lines(report_text) @@ -4789,12 +5068,50 @@ def assess_final_report_validator(report_text, task_kind, **kwargs): return _validate(report_text, task_kind, **kwargs) +def assess_review_final_report_schema(report_text, **kwargs): + """#391: reviewer final-report schema verification before session output.""" + from review_final_report_schema import assess_review_final_report_schema as _assess + + return _assess(report_text, **kwargs) + + _QUEUE_STATUS_REPORT_HINT = re.compile( r"queue[- ]status|selected pr:\s*none|no pr selected|" r"queue[- ]status[- ]only", re.I, ) +_RECONCILE_INVENTORY_HINT = re.compile( + r"already[- ]landed reconciliation|reconcile[- ]landed|" + r"open pr inventory|inventory pagination proof", + re.I, +) + +_RECONCILE_LINKED_ISSUE_HINT = re.compile( + r"already[- ]landed|reconcil|linked issue(?:\s+live)?\s+status", + 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, +) + +_PROOF_BACKED_HANDOFF_HINT = re.compile( + r"task:\s*review-merge-pr|task mode:\s*review-merge-pr|" + r"review-merge-pr|controller handoff", + 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) _NOT_APPLICABLE_VALUE = re.compile( @@ -5186,6 +5503,24 @@ def assess_validation_integrity_report(report_text, **kwargs): return _assess(report_text, **kwargs) +def assess_validation_failure_history_report(report_text, **kwargs): + """#396: final reports must account for transient validation failures.""" + from reviewer_validation_failure_history import ( + assess_validation_failure_history_report as _assess, + ) + + return _assess(report_text, **kwargs) + + +def assess_already_landed_classification_report(report_text, **kwargs): + """#295: already-landed PRs are reconciliation-only, not review eligible.""" + from reviewer_already_landed_classification import ( + assess_already_landed_classification_report as _assess, + ) + + return _assess(report_text, **kwargs) + + def assess_prior_blocker_skip_proof(report_text, **kwargs): """#318: require live blocker proof before skipping earlier open PRs.""" from reviewer_blocker_skip import assess_prior_blocker_skip_proof as _assess @@ -5209,6 +5544,27 @@ def assess_validation_worktree_edit_report(report_text, **kwargs): return _assess(report_text, **kwargs) +def assess_reconcile_inventory_report(report_text, **kwargs): + """#308: prove PR inventory pagination in reconciliation reports.""" + from reviewer_reconcile_inventory import assess_reconcile_inventory_report as _assess + + return _assess(report_text, **kwargs) + + +def assess_reconcile_linked_issue_report(report_text, **kwargs): + """#300: prove linked issue status was fetched live in reconciliation handoffs.""" + from reviewer_reconcile_linked_issue import assess_reconcile_linked_issue_report as _assess + + 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 @@ -5235,3 +5591,10 @@ def assess_worktree_ownership_report(report_text, **kwargs): from reviewer_worktree_ownership import assess_worktree_ownership_report as _assess return _assess(report_text, **kwargs) + + +def assess_proof_backed_handoff_report(report_text, **kwargs): + """#395: proof-sensitive review handoff claims require explicit evidence.""" + from reviewer_proof_backed_handoff import assess_proof_backed_handoff_report as _assess + + return _assess(report_text, **kwargs) diff --git a/reviewer_already_landed_classification.py b/reviewer_already_landed_classification.py new file mode 100644 index 0000000..b5c5f10 --- /dev/null +++ b/reviewer_already_landed_classification.py @@ -0,0 +1,143 @@ +"""Already-landed PR classification verifier for reviewer reports (#295).""" + +from __future__ import annotations + +import re +from typing import Any + +ALREADY_LANDED_ELIGIBILITY_CLASS = "ALREADY_LANDED_RECONCILE_REQUIRED" + +_LANDED_CONTEXT_RE = re.compile( + r"already[- ]landed|ancestor proof\s*:\s*passed|" + r"eligibility class\s*:\s*already_landed", + re.IGNORECASE, +) +_ELIGIBILITY_CLASS_LINE_RE = re.compile( + r"eligibility class\s*:\s*(.+)$", + re.IGNORECASE | re.MULTILINE, +) +_INELIGIBLE_SELECTION_RE = re.compile( + r"\b(?:next|oldest)\s+eligible\s+pr\b", + re.IGNORECASE, +) +_REVIEW_ELIGIBLE_RE = re.compile( + r"\beligible for (?:review|merge)\b|\bready for (?:review|merge)\b", + re.IGNORECASE, +) +_PINNED_REVIEWED_HEAD_RE = re.compile( + r"\bpinned reviewed head\s*:", + re.IGNORECASE, +) +_CANDIDATE_HEAD_RE = re.compile( + r"\bcandidate head sha\s*:", + re.IGNORECASE, +) +_REVIEWED_HEAD_NONE_RE = re.compile( + r"\breviewed head sha\s*:\s*none\b", + re.IGNORECASE, +) +_VALIDATION_PROOF_RE = re.compile( + r"(?:validation passed|pytest.*passed|diff review passed|" + r"validated on pinned head)", + re.IGNORECASE, +) +_QUEUE_BLOCKED_RE = re.compile( + r"queue blocked by already[- ]landed|" + r"reconciliation(?:-only)?\s+(?:next step|required)", + re.IGNORECASE, +) + + +def _landed_context(report_text: str, eligibility_class: str | None) -> bool: + if (eligibility_class or "").strip().upper() == ALREADY_LANDED_ELIGIBILITY_CLASS: + return True + return bool(_LANDED_CONTEXT_RE.search(report_text or "")) + + +def assess_already_landed_classification_report( + report_text: str, + *, + eligibility_class: str | None = None, + selected_pr_already_landed: bool | None = None, +) -> dict[str, Any]: + """#295: already-landed PRs are reconciliation-only, not review eligible.""" + text = report_text or "" + reasons: list[str] = [] + + landed = _landed_context(text, eligibility_class) + if selected_pr_already_landed is True: + landed = True + if not landed: + return { + "proven": True, + "block": False, + "landed_context": False, + "reasons": [], + "safe_next_action": "proceed", + } + + if _INELIGIBLE_SELECTION_RE.search(text): + reasons.append( + "already-landed PR must not be described as oldest/next eligible PR; " + "use 'Oldest open PR requiring action' and " + f"'Eligibility class: {ALREADY_LANDED_ELIGIBILITY_CLASS}'" + ) + + if _REVIEW_ELIGIBLE_RE.search(text): + reasons.append( + "already-landed PR must not be described as eligible for review or merge" + ) + + class_match = _ELIGIBILITY_CLASS_LINE_RE.search(text) + if class_match: + declared = class_match.group(1).strip().upper() + if declared != ALREADY_LANDED_ELIGIBILITY_CLASS: + reasons.append( + "already-landed PR eligibility class must be " + f"{ALREADY_LANDED_ELIGIBILITY_CLASS}" + ) + elif selected_pr_already_landed is True: + reasons.append( + f"already-landed selected PR must declare Eligibility class: " + f"{ALREADY_LANDED_ELIGIBILITY_CLASS}" + ) + + if _PINNED_REVIEWED_HEAD_RE.search(text): + if not _VALIDATION_PROOF_RE.search(text): + reasons.append( + "already-landed pre-review report must use Candidate head SHA, " + "not Pinned reviewed head, unless validation and diff review passed" + ) + + if selected_pr_already_landed is True and not _CANDIDATE_HEAD_RE.search(text): + if _PINNED_REVIEWED_HEAD_RE.search(text) or "head sha" in text.lower(): + reasons.append( + "already-landed ancestry check must report Candidate head SHA" + ) + + if selected_pr_already_landed is True and not _REVIEWED_HEAD_NONE_RE.search(text): + if _PINNED_REVIEWED_HEAD_RE.search(text) and not _VALIDATION_PROOF_RE.search(text): + reasons.append( + "already-landed report must state 'Reviewed head SHA: none' " + "when validation did not run" + ) + + if selected_pr_already_landed is True and not _QUEUE_BLOCKED_RE.search(text): + reasons.append( + "already-landed queue blocker must state reconciliation requirement " + "or queue blocked wording" + ) + + proven = not reasons + return { + "proven": proven, + "block": not proven, + "landed_context": True, + "reasons": reasons, + "safe_next_action": ( + "classify as ALREADY_LANDED_RECONCILE_REQUIRED, use Candidate head SHA, " + "and stop normal review" + if not proven + else "proceed" + ), + } \ No newline at end of file diff --git a/reviewer_already_landed_handoff.py b/reviewer_already_landed_handoff.py new file mode 100644 index 0000000..0642644 --- /dev/null +++ b/reviewer_already_landed_handoff.py @@ -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" + ), + } \ No newline at end of file diff --git a/reviewer_proof_backed_handoff.py b/reviewer_proof_backed_handoff.py new file mode 100644 index 0000000..0cf0a79 --- /dev/null +++ b/reviewer_proof_backed_handoff.py @@ -0,0 +1,217 @@ +"""Proof-backed reviewer handoff claim verifier (#395).""" + +from __future__ import annotations + +import re +from typing import Any + +_PAGINATION_FINALITY = re.compile( + r"has_more\s*:\s*false|is_final_page\s*:\s*true|" + r"inventory_complete\s*:\s*true|pages_fetched|total_count\s*:", + re.I, +) +_PAGE_SIZE_ONLY = re.compile( + r"(?:less|fewer) than (?:the )?(?:default )?page[- ]?size|" + r"under (?:the )?50[- ]?(?:item )?limit|" + r"returned \d+ (?:open )?prs?.*less than 50", + re.I, +) +_INVENTORY_COMPLETE_CLAIM = re.compile( + r"\b(?:inventory (?:is )?complete|inventory exhaustive|" + r"complete (?:pr )?inventory)\b", + re.I, +) +_SKIP_CLAIM = re.compile( + r"\b(?:earlier prs? skipped|skipped (?:earlier )?pr|" + r"skip(?:ped)? pr #?\d+|non-mergeable|prior request.changes)\b", + re.I, +) +_CONFLICT_PROOF = re.compile( + r"merge simulation|git merge --no-commit|conflicting files|" + r"conflict proof|merge_exit|non-mergeable", + re.I, +) +_BASELINE_CLAIM = re.compile( + r"\b(?:baseline (?:validation|worktree|comparison)|" + r"same as master|pre-existing (?:on )?master|" + r"failure signatures match)\b", + re.I, +) +_BASELINE_PATH = re.compile(r"baseline worktree path\s*:\s*(\S+)", re.I) +_BASELINE_SHA = re.compile(r"baseline (?:target )?sha\s*:\s*([0-9a-f]{7,40})", re.I) +_BASELINE_DIRTY_BEFORE = re.compile( + r"baseline.*dirty before|dirty before.*baseline", re.I +) +_BASELINE_DIRTY_AFTER = re.compile( + r"baseline.*dirty after|dirty after.*baseline", re.I +) +_BASELINE_COMMAND = re.compile( + r"baseline.*(?:validation )?command|pytest.*baseline", re.I +) +_BASELINE_RESULT = re.compile( + r"baseline.*(?:validation )?result|baseline_exit|baseline failures", re.I +) +_MASTER_INTEGRATION = re.compile( + r"\b(?:merged master into|merge(?:d)? (?:remote[- ]tracking )?branch.*master|" + r"master integration|integrated master|rebase.*master)\b", + re.I, +) +_CLEANUP_CLAIM = re.compile( + r"\b(?:worktree(?:s)? (?:were )?cleaned|cleanup (?:result|mutations)|" + r"removed (?:session[- ]owned )?worktree|git worktree remove)\b", + re.I, +) +_WORKTREE_LIST = re.compile(r"git worktree list|worktree list proof", re.I) +_PROOF_SOURCE = re.compile( + r"proof source\s*:\s*(command|mcp metadata|prior blocker|not checked)", + re.I, +) +_HANDOFF_SECTION = re.compile(r"^##\s*Controller Handoff\s*$", re.I | re.M) + + +def _handoff_fields(report_text: str) -> dict[str, str]: + text = report_text or "" + match = _HANDOFF_SECTION.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 _command_text(entry: Any) -> str: + if isinstance(entry, dict): + return str(entry.get("command") or entry.get("tool") or "").strip() + return str(entry or "").strip() + + +def _action_log_has(action_log: list | None, pattern: re.Pattern[str]) -> bool: + for entry in action_log or []: + blob = " ".join( + filter( + None, + [ + _command_text(entry), + str(entry.get("result") or "") if isinstance(entry, dict) else "", + str(entry.get("reason") or "") if isinstance(entry, dict) else "", + ], + ) + ) + if pattern.search(blob): + return True + return False + + +def assess_proof_backed_handoff_report( + report_text: str, + *, + action_log: list | None = None, + inventory_session: dict | None = None, + baseline_session: dict | None = None, + skip_session: list[dict] | None = None, +) -> dict[str, Any]: + """Require explicit command/tool evidence for proof-sensitive review claims (#395).""" + text = report_text or "" + fields = _handoff_fields(text) + reasons: list[str] = [] + inventory = dict(inventory_session or {}) + baseline = dict(baseline_session or {}) + skips = list(skip_session or []) + + pagination_field = fields.get("inventory pagination proof", "") + if _INVENTORY_COMPLETE_CLAIM.search(text) or _PAGE_SIZE_ONLY.search(text): + has_meta = bool( + inventory.get("inventory_complete") + or inventory.get("pagination_complete") + or _PAGINATION_FINALITY.search(pagination_field) + or _PAGINATION_FINALITY.search(text) + or _action_log_has(action_log, re.compile(r"gitea_list_prs", re.I)) + ) + if _PAGE_SIZE_ONLY.search(text) and not has_meta: + reasons.append( + "inventory completeness claimed from page-size assumption only; " + "require has_more=false, is_final_page=true, or inventory_complete=true" + ) + elif _INVENTORY_COMPLETE_CLAIM.search(text) and not has_meta: + reasons.append( + "inventory complete claim lacks explicit pagination metadata proof" + ) + + if _SKIP_CLAIM.search(text) or fields.get("earlier prs skipped", "").lower() not in { + "", + "none", + "n/a", + }: + skip_proof = bool( + skips + or _CONFLICT_PROOF.search(text) + or _action_log_has( + action_log, re.compile(r"git merge --no-commit|gitea_get_pr_review_feedback", re.I) + ) + ) + if not skip_proof: + reasons.append( + "earlier PR skip claim lacks command/tool conflict or blocker proof" + ) + + if _BASELINE_CLAIM.search(text) or fields.get("baseline worktree used", "").lower() == "true": + baseline_ok = bool( + baseline.get("complete") + or ( + _BASELINE_PATH.search(text) + and _BASELINE_SHA.search(text) + and (_BASELINE_DIRTY_BEFORE.search(text) or baseline.get("clean_before")) + and (_BASELINE_COMMAND.search(text) or baseline.get("command")) + and (_BASELINE_RESULT.search(text) or baseline.get("result")) + and (_BASELINE_DIRTY_AFTER.search(text) or baseline.get("clean_after")) + ) + ) + if not baseline_ok: + reasons.append( + "baseline validation claim missing worktree path, target SHA, " + "dirty-before/after status, command, or result proof" + ) + + if _MASTER_INTEGRATION.search(text): + if not _action_log_has( + action_log, + re.compile(r"git merge.*master|git rebase.*master", re.I), + ) and not re.search(r"merge_exit\s*=\s*\d+|merge simulation", text, re.I): + reasons.append( + "master integration claim lacks exact merge/rebase command and result" + ) + + if _CLEANUP_CLAIM.search(text) or fields.get("cleanup mutations", "").lower() not in { + "", + "none", + "n/a", + }: + if not (_WORKTREE_LIST.search(text) or _action_log_has(action_log, _WORKTREE_LIST)): + reasons.append( + "cleanup claim lacks final git worktree list or equivalent proof" + ) + + if re.search(r"\blive proof\b", text, re.I) and not _PROOF_SOURCE.search(text): + if not action_log: + reasons.append( + "live proof wording requires proof source classification " + "(command, MCP metadata, prior blocker, or not checked)" + ) + + proven = not reasons + return { + "proven": proven, + "block": not proven, + "reasons": reasons, + "safe_next_action": ( + "cite explicit command/tool evidence or structured MCP pagination metadata " + "for each proof-sensitive claim" + if not proven + else "proceed" + ), + } \ No newline at end of file diff --git a/reviewer_reconcile_inventory.py b/reviewer_reconcile_inventory.py new file mode 100644 index 0000000..9dfc0e5 --- /dev/null +++ b/reviewer_reconcile_inventory.py @@ -0,0 +1,162 @@ +"""Reconciliation PR inventory pagination proof verifier (#308).""" + +from __future__ import annotations + +import re +from typing import Any + +_COMPLETE_SCAN_CLAIM_RE = re.compile( + r"\b(?:all already[- ]landed(?:\s+prs?)?(?:\s+were)?\s+found|" + r"complete queue scan|all open prs? (?:were )?(?:found|checked|inventoried|listed)|" + r"inventory (?:is )?complete|exhaustive (?:pr )?inventory|" + r"no additional already[- ]landed|scanned (?:the )?(?:full )?open pr queue)\b", + re.I, +) + +_FINALITY_RE = re.compile( + r"is_final_page\s*:\s*true|has_more\s*:\s*false|no next page|final[- ]page|" + r"pagination_complete\s*:\s*true|inventory pagination proof\s*:\s*(?!assumed|none\b).+", + re.I, +) + +_REQUESTED_PAGE_SIZE_RE = re.compile( + r"requested page size\s*:\s*(\d+)|page[- ]?size\s*(?:=|:)\s*(\d+)", + re.I, +) +_PAGES_FETCHED_RE = re.compile( + r"pages? fetched\s*:\s*(\d+)|page count\s*:\s*(\d+)", + re.I, +) +_RETURNED_PER_PAGE_RE = re.compile( + r"returned pr count(?: per page)?\s*:|open pr count per page|" + r"returned \d+ open prs? per page|per[- ]page counts?\s*:", + re.I, +) +_EXACT_LIMIT_RETURN_RE = re.compile( + r"returned\s+(\d+)\s+open prs?|listed\s+(\d+)\s+open prs?", + re.I, +) + +_DEFAULT_PAGE_SIZE_ASSUMPTION = re.compile( + r"(?:less|fewer) than (?:the )?(?:default )?(?:gitea )?page[- ]?(?:size|limit)|" + r"default (?:gitea )?page[- ]?size|assumed complete", + re.I, +) + + +def _int_from_groups(match: re.Match[str] | None) -> int | None: + if not match: + return None + for group in match.groups(): + if group: + return int(group) + return None + + +def _pagination_proven(text: str, session: dict) -> bool: + if session.get("pagination_complete") or session.get("inventory_complete"): + return True + if _FINALITY_RE.search(text): + return True + pages = session.get("pages_fetched") + if isinstance(pages, int) and pages >= 1 and session.get("is_final_page"): + return True + return False + + +def _metadata_present(text: str, session: dict) -> list[str]: + missing: list[str] = [] + has_page_size = bool( + _REQUESTED_PAGE_SIZE_RE.search(text) + or session.get("requested_page_size") + ) + has_pages_fetched = bool( + _PAGES_FETCHED_RE.search(text) or session.get("pages_fetched") + ) + has_returned_counts = bool( + _RETURNED_PER_PAGE_RE.search(text) or session.get("returned_per_page") + ) + if not has_page_size: + missing.append("requested page size") + if not has_pages_fetched: + missing.append("page count fetched") + if not has_returned_counts: + missing.append("returned PR count per page") + if not _pagination_proven(text, session): + missing.append("final-page/no-next-page proof") + return missing + + +def assess_reconcile_inventory_report( + report_text: str, + *, + inventory_session: dict | None = None, +) -> dict[str, Any]: + """Validate PR inventory pagination proof in reconciliation reports (#308).""" + text = report_text or "" + session = dict(inventory_session or {}) + reasons: list[str] = [] + + claims_scan = bool(_COMPLETE_SCAN_CLAIM_RE.search(text)) + lists_open_prs = bool( + re.search(r"open prs? listed|listed open prs?|pr inventory", text, re.I) + ) + + if not claims_scan and not lists_open_prs and not session.get("inventory_required"): + return { + "proven": True, + "block": False, + "reasons": [], + "inventory_claimed": False, + "safe_next_action": "proceed", + } + + if _DEFAULT_PAGE_SIZE_ASSUMPTION.search(text) and not _pagination_proven(text, session): + reasons.append( + "reconciliation inventory assumed complete from page-size guess " + "without final-page proof (#308)" + ) + + if claims_scan and not _pagination_proven(text, session): + reasons.append( + "complete queue scan or all already-landed claim requires " + "pagination finality proof (#308)" + ) + + missing = _metadata_present(text, session) + if (claims_scan or lists_open_prs) and missing: + reasons.append( + "reconciliation inventory missing pagination metadata: " + + ", ".join(missing) + ) + + requested = session.get("requested_page_size") + returned = session.get("returned_page_size") + if isinstance(requested, int) and isinstance(returned, int): + if returned >= requested > 0 and not _pagination_proven(text, session): + reasons.append( + "exactly-full first reconciliation page without final-page proof (#308)" + ) + else: + for match in _EXACT_LIMIT_RETURN_RE.finditer(text): + count = _int_from_groups(match) + if count in {10, 20, 50} and not _pagination_proven(text, session): + reasons.append( + "exactly-full first reconciliation page without final-page proof (#308)" + ) + break + + proven = not reasons + return { + "proven": proven, + "block": not proven, + "reasons": list(dict.fromkeys(reasons)), + "inventory_claimed": claims_scan or lists_open_prs, + "pagination_proven": _pagination_proven(text, session), + "safe_next_action": ( + "include requested page size, pages fetched, per-page counts, and " + "final-page/no-next-page proof before claiming complete scan" + if reasons + else "proceed" + ), + } \ No newline at end of file diff --git a/reviewer_reconcile_linked_issue.py b/reviewer_reconcile_linked_issue.py new file mode 100644 index 0000000..8a01ee0 --- /dev/null +++ b/reviewer_reconcile_linked_issue.py @@ -0,0 +1,191 @@ +"""Reconciliation linked-issue live proof verifier (#300).""" + +from __future__ import annotations + +import re +from typing import Any + +_HANDOFF_SECTION_RE = re.compile(r"^##\s*Controller Handoff\s*$", re.I | re.M) + +_LINKED_ISSUE_FIELD_RE = re.compile( + r"linked issue\s*:\s*(.+)$", + re.I | re.M, +) +_LINKED_ISSUE_STATUS_RE = re.compile( + r"linked issue(?:\s+live)?\s+status\s*:\s*(.+)$", + re.I | re.M, +) + +_STATUS_CLAIM_RE = re.compile( + r"\b(?:issue\s+#?\d+\s*\()?(open|closed|resolved)\b|" + r"issue\s+(?:is\s+)?(open|closed|resolved)\b|" + r"linked issue.*\b(open|closed|resolved)\b", + re.I, +) +_NOT_VERIFIED_RE = re.compile(r"not verified in this session", re.I) + +_LIVE_FETCH_PROOF_RE = re.compile( + r"gitea_view_issue|issue fetched live|live issue fetch|" + r"fetched linked issue.*(?:current|this) session|" + r"linked issue (?:fetch|proof).*(?:current|this) session|" + r"live linked issue proof", + re.I, +) + +_ISSUE_ACTION_RECOMMEND_RE = re.compile( + r"(?:recommend(?:ed)?|should|must)\s+(?:close|reopen|comment on)\s+" + r"(?:the\s+)?(?:linked\s+)?issue|" + r"(?:close|reopen|comment on)\s+linked issue\s+#?\d+", + re.I, +) +_CAPABILITY_PROOF_RE = re.compile( + r"capability proof|exact capability|gitea_close_issue|" + r"gitea_mark_issue|gitea_create_issue_comment|gitea_edit_issue", + re.I, +) + +_NO_LINKED_ISSUE_VALUES = frozenset({ + "", + "none", + "n/a", + "not applicable", + "no linked issue", + "unknown", +}) + + +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 _extract_linked_issue_number(text: str, fields: dict[str, str]) -> int | None: + linked = fields.get("linked issue", "") + if linked.lower() in _NO_LINKED_ISSUE_VALUES: + return None + match = re.search(r"#?(\d+)", linked) + if match: + return int(match.group(1)) + field_match = _LINKED_ISSUE_FIELD_RE.search(text) + if field_match: + num = re.search(r"#?(\d+)", field_match.group(1)) + if num: + return int(num.group(1)) + return None + + +def _status_value(text: str, fields: dict[str, str]) -> str: + for key in ("linked issue live status", "linked issue status"): + if fields.get(key): + return fields[key] + match = _LINKED_ISSUE_STATUS_RE.search(text) + return match.group(1).strip() if match else "" + + +def _live_proof_present(text: str, session: dict, issue_number: int | None) -> bool: + if session.get("live_fetched") or session.get("verified_live"): + return True + if session.get("issue_fetch_proof"): + return True + if _LIVE_FETCH_PROOF_RE.search(text): + return True + if issue_number is not None: + pattern = re.compile( + rf"gitea_view_issue.*#?{issue_number}|" + rf"issue\s+#?{issue_number}.*(?:fetched|viewed).*(?:current|this) session", + re.I, + ) + if pattern.search(text): + return True + return False + + +def assess_reconcile_linked_issue_report( + report_text: str, + *, + reconcile_session: dict | None = None, +) -> dict[str, Any]: + """Validate linked issue status claims in reconciliation handoffs (#300).""" + text = report_text or "" + session = dict(reconcile_session or {}) + fields = _handoff_field_map(text) + reasons: list[str] = [] + + issue_number = session.get("linked_issue_number") + if issue_number is None: + issue_number = _extract_linked_issue_number(text, fields) + + if issue_number is None: + status = _status_value(text, fields) + if status and status.lower() not in _NO_LINKED_ISSUE_VALUES: + if _STATUS_CLAIM_RE.search(status): + reasons.append( + "linked issue status reported without identifying the linked issue (#300)" + ) + if not reasons: + return { + "proven": True, + "block": False, + "reasons": [], + "linked_issue_number": None, + "safe_next_action": "proceed", + } + + status = _status_value(text, fields) + status_lower = status.lower() + + if session.get("issue_missing"): + if status_lower and "not verified" not in status_lower: + reasons.append( + "missing linked issue must be reported as " + "'not verified in this session' (#300)" + ) + elif status: + if _NOT_VERIFIED_RE.search(status): + pass + elif _STATUS_CLAIM_RE.search(status): + if not _live_proof_present(text, session, issue_number): + reasons.append( + "linked issue open/closed/resolved status requires live " + "gitea_view_issue proof in the current session (#300)" + ) + elif status_lower not in _NO_LINKED_ISSUE_VALUES: + if not _live_proof_present(text, session, issue_number): + reasons.append( + "linked issue status claim requires live fetch proof or " + "'not verified in this session' (#300)" + ) + + if _ISSUE_ACTION_RECOMMEND_RE.search(text): + if not _CAPABILITY_PROOF_RE.search(text) and not session.get( + "issue_action_capability_proven" + ): + reasons.append( + "recommended linked issue close/reopen/comment requires " + "exact capability proof (#300)" + ) + + proven = not reasons + return { + "proven": proven, + "block": not proven, + "reasons": list(dict.fromkeys(reasons)), + "linked_issue_number": issue_number, + "live_proof_present": _live_proof_present(text, session, issue_number), + "safe_next_action": ( + "fetch linked issue with gitea_view_issue in this session or " + "report 'Linked issue status: not verified in this session'" + if reasons + else "proceed" + ), + } \ No newline at end of file diff --git a/reviewer_validation_failure_history.py b/reviewer_validation_failure_history.py new file mode 100644 index 0000000..80da5c3 --- /dev/null +++ b/reviewer_validation_failure_history.py @@ -0,0 +1,198 @@ +"""Transient validation failure history verifier (#396). + +Reviewer sessions may observe validation failures that later pass on rerun. +Final reports must document every failure observed during the session, not +only the last passing result. +""" + +from __future__ import annotations + +import re +from typing import Any + +_SECTION_RE = re.compile( + r"validation failure history", + re.IGNORECASE, +) +_FAILURE_ENTRY_RE = re.compile( + r"(?:failure\s*(?:#|entry)?\s*\d+|validation failure)\s*:", + re.IGNORECASE, +) +_COMMAND_RE = re.compile( + r"(?:command|validation command)\s*:\s*(.+)", + re.IGNORECASE, +) +_FAILING_TEST_RE = re.compile( + r"(?:failing test|failure|error)\s*:\s*(.+)", + re.IGNORECASE, +) +_CAUSE_RE = re.compile( + r"(?:suspected cause|cause)\s*:\s*(.+)", + re.IGNORECASE, +) +_REPRODUCED_RE = re.compile( + r"(?:reproduced|reproduces)\s*:\s*(yes|no|unknown)", + re.IGNORECASE, +) +_BASELINE_RE = re.compile( + r"(?:on baseline master|baseline master|exists on baseline)\s*:\s*(yes|no|unknown|not checked)", + re.IGNORECASE, +) +_PR_CAUSED_RE = re.compile( + r"(?:pr[- ]caused|pr caused)\s*:\s*(yes|no|unknown|not proven)", + re.IGNORECASE, +) +_TRANSIENT_STATUS_RE = re.compile( + r"(?:passed after transient failure investigation|transient failure investigation)", + re.IGNORECASE, +) +_PLAIN_PASS_RE = re.compile( + r"validation\s*:\s*(?:pass|passed|strong|ok|green)\b", + re.IGNORECASE, +) +_ENV_CLEANUP_RE = re.compile( + r"(?:environmental cleanup|state cleaned|what changed between runs)\s*:", + re.IGNORECASE, +) +_UNKNOWN_CAUSE_RE = re.compile( + r"(?:suspected cause|cause)\s*:\s*(?:unknown|unexplained|not determined)", + re.IGNORECASE, +) + + +def _failure_documented_in_text(text: str, failure: dict[str, Any]) -> bool: + """Return True when *failure* appears documented in free-form report text.""" + command = (failure.get("command") or "").strip() + failing = (failure.get("failing_test") or failure.get("error") or "").strip() + if command and command not in text: + return False + if failing and failing not in text: + return False + return bool(command or failing) + + +def _section_has_structured_fields(text: str) -> bool: + if not _SECTION_RE.search(text): + return False + section_start = _SECTION_RE.search(text).start() + section = text[section_start:] + has_command = bool(_COMMAND_RE.search(section)) + has_failure = bool(_FAILING_TEST_RE.search(section)) + return has_command and has_failure + + +def assess_validation_failure_history_report( + report_text: str, + *, + validation_session: dict | None = None, +) -> dict[str, Any]: + """Require final reports to account for transient validation failures (#396).""" + text = report_text or "" + session = dict(validation_session or {}) + reasons: list[str] = [] + violations: list[str] = [] + + observed = list(session.get("observed_failures") or []) + final_status = (session.get("final_validation_status") or "").strip().lower() + cause_unknown = any( + (f.get("suspected_cause") or "").strip().lower() in { + "unknown", "unexplained", "not determined", "" + } + and not (f.get("pr_caused") or "").strip().lower() in {"yes", "no"} + for f in observed + ) or bool(session.get("cause_unknown")) + + if not observed: + return { + "proven": True, + "block": False, + "reasons": [], + "violations": [], + "observed_failure_count": 0, + "safe_next_action": "proceed", + } + + documented = _section_has_structured_fields(text) + if not documented: + for failure in observed: + if _failure_documented_in_text(text, failure): + documented = True + break + + if not documented: + violations.append( + "session observed validation failure(s) but final report omits " + "Validation failure history" + ) + reasons.append( + "every validation failure observed during the session must appear " + "in a Validation failure history section" + ) + + if documented and not _SECTION_RE.search(text): + reasons.append( + "failure details must appear under an explicit " + "'Validation failure history' heading" + ) + + for idx, failure in enumerate(observed, start=1): + prefix = f"failure #{idx}" + if not _failure_documented_in_text(text, failure) and documented: + reasons.append( + f"{prefix}: command and failing test/error not documented in report" + ) + command = (failure.get("command") or "").strip() + failing = (failure.get("failing_test") or failure.get("error") or "").strip() + if not command: + reasons.append(f"{prefix}: missing command in session failure record") + if not failing: + reasons.append( + f"{prefix}: missing failing test or error in session failure record" + ) + + plain_pass = bool(_PLAIN_PASS_RE.search(text)) + transient_wording = bool(_TRANSIENT_STATUS_RE.search(text)) + final_passed = final_status in { + "passed", + "pass", + "passed_after_transient_failure_investigation", + } + + if (final_passed or plain_pass) and observed: + if cause_unknown and not transient_wording: + violations.append( + "unknown transient failure cause cannot be erased as plain 'passed'" + ) + reasons.append( + "when cause is unknown, use status " + "'passed after transient failure investigation'" + ) + elif not transient_wording and final_status != "passed_after_transient_failure_investigation": + if not _ENV_CLEANUP_RE.search(text): + reasons.append( + "later passing rerun must document what changed between runs " + "or environmental cleanup performed" + ) + + if session.get("environmental_contamination") and not _ENV_CLEANUP_RE.search(text): + reasons.append( + "environmental /tmp state contamination must document cleanup or " + "what changed before the passing rerun" + ) + + proven = not reasons and not violations + return { + "proven": proven, + "block": bool(violations) or not proven, + "reasons": reasons, + "violations": violations, + "observed_failure_count": len(observed), + "documented": documented, + "safe_next_action": ( + "add Validation failure history with command, failing test, cause, " + "reproduction, baseline comparison, and PR-caused evidence; use " + "'passed after transient failure investigation' when appropriate" + if not proven + else "proceed" + ), + } \ No newline at end of file diff --git a/role_session_router.py b/role_session_router.py index ff75025..8f30dc5 100644 --- a/role_session_router.py +++ b/role_session_router.py @@ -57,6 +57,8 @@ REVIEWER_TASKS = frozenset({ "review_pr", "merge_pr", "blind_pr_queue_review", + "pr_queue_cleanup", + "pr-queue-cleanup", "request_changes_pr", "approve_pr", }) @@ -72,6 +74,15 @@ AUTHOR_TASKS = frozenset({ "comment_pr", "address_pr_change_requests", "delete_branch", + "work_issue", + "work-issue", + "reconcile_landed_pr", +}) + +RECONCILER_TASKS = frozenset({ + "reconcile_already_landed_pr", + "reconcile_already_landed", + "reconcile-landed-pr", }) TASK_REQUIRED_ROLE = { @@ -88,8 +99,16 @@ TASK_REQUIRED_ROLE = { "review_pr": "reviewer", "merge_pr": "reviewer", "blind_pr_queue_review": "reviewer", + "pr_queue_cleanup": "reviewer", + "pr-queue-cleanup": "reviewer", "request_changes_pr": "reviewer", "approve_pr": "reviewer", + "work_issue": "author", + "work-issue": "author", + "reconcile_landed_pr": "author", + "reconcile_already_landed_pr": "reconciler", + "reconcile_already_landed": "reconciler", + "reconcile-landed-pr": "reconciler", # #309: reconciler tasks close already-landed PRs/issues only. "reconcile_close_landed_pr": "reconciler", "reconcile_close_landed_issue": "reconciler", diff --git a/skills/llm-project-workflow/SKILL.md b/skills/llm-project-workflow/SKILL.md index 0ef7db1..dc2f961 100644 --- a/skills/llm-project-workflow/SKILL.md +++ b/skills/llm-project-workflow/SKILL.md @@ -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) | | 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) | +| 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 @@ -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, 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 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`) - [`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`) - [`recover-bad-state.md`](templates/recover-bad-state.md) - [`reconcile-closed-not-merged-pr.md`](templates/reconcile-closed-not-merged-pr.md) diff --git a/skills/llm-project-workflow/schemas/pr-queue-cleanup-final-report.md b/skills/llm-project-workflow/schemas/pr-queue-cleanup-final-report.md new file mode 100644 index 0000000..c7abe8c --- /dev/null +++ b/skills/llm-project-workflow/schemas/pr-queue-cleanup-final-report.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) diff --git a/skills/llm-project-workflow/schemas/review-merge-final-report.md b/skills/llm-project-workflow/schemas/review-merge-final-report.md index 08c094b..9eb3592 100644 --- a/skills/llm-project-workflow/schemas/review-merge-final-report.md +++ b/skills/llm-project-workflow/schemas/review-merge-final-report.md @@ -91,4 +91,23 @@ Verifier: `review_proofs.assess_queue_status_report()`. Narrative final report and controller handoff must agree on eligibility class, candidate/reviewed head SHA, mutation state, worktree usage, review decision, -terminal review mutation, merge result, and linked issue status. \ No newline at end of file +terminal review mutation, merge result, and linked issue status. + +### Proof-backed claims (#395) + +Proof-sensitive claims must cite explicit command/tool evidence in the report +or structured MCP metadata — not narrative alone: + +- **Inventory complete:** `has_more=false`, `is_final_page=true`, + `inventory_complete=true`, and/or `total_count` from `gitea_list_prs`. +- **Skipped earlier PRs:** merge-simulation command output, conflict file list, + or live `gitea_get_pr_review_feedback` / mergeability fields. +- **Baseline validation:** baseline worktree path, baseline target SHA, + dirty-before/after, exact command, exact result. +- **Master integration:** exact `git merge` / `git rebase` command and exit status. +- **Cleanup:** final `git worktree list` (or remove commands) proving session + worktrees were removed. + +When a claim relies on prior-session blocker state or MCP metadata only, label +the proof source explicitly (`command`, `MCP metadata`, `prior blocker`, +`not checked`). Do not use `live proof` without that classification. \ No newline at end of file diff --git a/skills/llm-project-workflow/templates/pr-queue-cleanup.md b/skills/llm-project-workflow/templates/pr-queue-cleanup.md new file mode 100644 index 0000000..d3913f1 --- /dev/null +++ b/skills/llm-project-workflow/templates/pr-queue-cleanup.md @@ -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 . + +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): +Merge authorized for selected PR in this run: + +End with the pr-queue-cleanup final report schema. +``` \ No newline at end of file diff --git a/skills/llm-project-workflow/workflows/pr-queue-cleanup.md b/skills/llm-project-workflow/workflows/pr-queue-cleanup.md new file mode 100644 index 0000000..8a1be98 --- /dev/null +++ b/skills/llm-project-workflow/workflows/pr-queue-cleanup.md @@ -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. diff --git a/skills/llm-project-workflow/workflows/review-merge-pr.md b/skills/llm-project-workflow/workflows/review-merge-pr.md index 86eb359..5b0e10a 100644 --- a/skills/llm-project-workflow/workflows/review-merge-pr.md +++ b/skills/llm-project-workflow/workflows/review-merge-pr.md @@ -538,6 +538,36 @@ Official validation integrity status must be one of: Do not invent a softer status. +## 21A. Validation failure history rule + +Every validation failure observed during the review session must appear in the +final report, even if a later rerun passes. + +Before claiming `Validation: passed`, list every failed validation command from +the session under `Validation failure history`. + +For each failure, report: + +* command +* failing test or error +* suspected cause +* whether it reproduced +* whether it exists on baseline master +* evidence for whether it is or is not PR-caused + +If a later rerun passes, also report: + +* what changed between runs +* whether environmental state was cleaned (for example `/tmp` lock files) +* why the pass is trustworthy + +If the cause is unknown, do not erase the earlier failure with plain +`Validation: passed`. Use a status such as +`passed after transient failure investigation`. + +`gitea_validate_review_final_report` rejects reports that omit known earlier +validation failures when `validation_session.observed_failures` is supplied. + ## 22. Baseline validation rule Do not run tests in the main checkout. @@ -1096,6 +1126,7 @@ Controller Handoff: * Baseline worktree path: * Files reviewed: * Validation: +* Validation failure history: * Official validation integrity status: * Terminal review mutation: * Review decision: diff --git a/skills/llm-project-workflow/workflows/work-issue.md b/skills/llm-project-workflow/workflows/work-issue.md index 303b4cb..f450cd7 100644 --- a/skills/llm-project-workflow/workflows/work-issue.md +++ b/skills/llm-project-workflow/workflows/work-issue.md @@ -83,6 +83,20 @@ Examples: If capability cannot be proven, stop and produce a recovery handoff only. +### 2A. Pre-task role routing (#139) + +Before issue selection or any mutation, resolve the composite author task: + +* `gitea_route_task_session(task_type="work-issue", remote=…)` — must return + `route_result: allowed_current_session` and `downstream_allowed: true` +* `gitea_resolve_task_capability(task="work_issue", remote=…)` — must show + `required_role_kind: author` and `allowed_in_current_session: true` + +Hyphen (`work-issue`) and underscore (`work_issue`) aliases are equivalent. +If routing returns `ambiguous_task_stop`, `wrong_role_stop`, or +`route_to_author_session`, stop and produce a recovery handoff only — do not +fall back to reviewer tools or guess a different profile. + ## 3. Stop immediately on blocked infrastructure If any of the following appears, stop immediately: diff --git a/task_capability_map.py b/task_capability_map.py index 8453255..1d40a78 100644 --- a/task_capability_map.py +++ b/task_capability_map.py @@ -72,6 +72,14 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = { "permission": "gitea.pr.review", "role": "reviewer", }, + "pr_queue_cleanup": { + "permission": "gitea.pr.review", + "role": "reviewer", + }, + "pr-queue-cleanup": { + "permission": "gitea.pr.review", + "role": "reviewer", + }, "request_changes_pr": { "permission": "gitea.pr.request_changes", "role": "reviewer", @@ -96,6 +104,26 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = { "permission": "gitea.read", "role": "author", }, + "work_issue": { + "permission": "gitea.pr.create", + "role": "author", + }, + "work-issue": { + "permission": "gitea.pr.create", + "role": "author", + }, + "reconcile_landed_pr": { + "permission": "gitea.read", + "role": "author", + }, + "reconcile-landed-pr": { + "permission": "gitea.read", + "role": "author", + }, + "reconcile_already_landed_pr": { + "permission": "gitea.pr.close", + "role": "reconciler", + }, # #309: dedicated reconciler path for already-landed open PRs. Exact # close capabilities only — never review/approve/request_changes/merge. "reconcile_close_landed_pr": { diff --git a/tests/test_already_landed_reconcile.py b/tests/test_already_landed_reconcile.py new file mode 100644 index 0000000..72a99a9 --- /dev/null +++ b/tests/test_already_landed_reconcile.py @@ -0,0 +1,131 @@ +"""Tests for already-landed PR reconciliation gates (#310).""" +import os +import sys +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) + +import already_landed_reconcile + + +class TestAssessAlreadyLandedReconciliation(unittest.TestCase): + def test_open_pr_already_landed_allows_close(self): + with patch( + "already_landed_reconcile.is_head_ancestor_of_ref", + return_value=True, + ): + assessment = already_landed_reconcile.assess_already_landed_reconciliation( + pr={ + "number": 278, + "state": "open", + "title": "Fix thing (Closes #263)", + "body": "", + "head": {"ref": "feat/x", "sha": "abc123"}, + "base": {"ref": "master"}, + }, + project_root="/tmp/repo", + remote="prgs", + target_branch="master", + target_fetch={ + "success": True, + "target_branch": "master", + "target_ref": "prgs/master", + "target_branch_sha": "deadbeef", + "git_fetch_command": "git fetch prgs master", + }, + ) + self.assertTrue(assessment["close_allowed"]) + self.assertEqual( + assessment["eligibility_class"], + already_landed_reconcile.ELIGIBILITY_ALREADY_LANDED, + ) + self.assertEqual(assessment["linked_issue"], 263) + self.assertTrue(assessment["ancestor_proof"]) + + def test_not_landed_pr_denies_close(self): + with patch( + "already_landed_reconcile.is_head_ancestor_of_ref", + return_value=False, + ): + assessment = already_landed_reconcile.assess_already_landed_reconciliation( + pr={ + "number": 99, + "state": "open", + "title": "WIP", + "body": "", + "head": {"ref": "feat/y", "sha": "fff111"}, + "base": {"ref": "master"}, + }, + project_root="/tmp/repo", + remote="prgs", + target_branch="master", + target_fetch={ + "success": True, + "target_branch": "master", + "target_ref": "prgs/master", + "target_branch_sha": "deadbeef", + }, + ) + self.assertFalse(assessment["close_allowed"]) + self.assertEqual( + assessment["eligibility_class"], + already_landed_reconcile.ELIGIBILITY_NOT_LANDED, + ) + + def test_stale_target_branch_denies_close(self): + assessment = already_landed_reconcile.assess_already_landed_reconciliation( + pr={ + "number": 99, + "state": "open", + "title": "WIP", + "body": "", + "head": {"ref": "feat/y", "sha": "fff111"}, + "base": {"ref": "master"}, + }, + project_root="/tmp/repo", + remote="prgs", + target_branch="master", + target_fetch={ + "success": False, + "target_branch": "master", + "target_ref": "prgs/master", + "target_branch_sha": None, + "reasons": ["git fetch failed"], + }, + ) + self.assertFalse(assessment["close_allowed"]) + self.assertEqual( + assessment["eligibility_class"], + already_landed_reconcile.ELIGIBILITY_STALE_TARGET, + ) + + def test_closed_pr_denies_close(self): + assessment = already_landed_reconcile.assess_already_landed_reconciliation( + pr={ + "number": 50, + "state": "closed", + "title": "Done", + "body": "", + "head": {"ref": "feat/z", "sha": "aaa"}, + "base": {"ref": "master"}, + }, + project_root="/tmp/repo", + remote="prgs", + target_branch="master", + target_fetch={ + "success": True, + "target_branch": "master", + "target_ref": "prgs/master", + "target_branch_sha": "deadbeef", + }, + ) + self.assertFalse(assessment["close_allowed"]) + self.assertEqual( + assessment["eligibility_class"], + already_landed_reconcile.ELIGIBILITY_PR_NOT_OPEN, + ) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_author_mutation_worktree.py b/tests/test_author_mutation_worktree.py new file mode 100644 index 0000000..aaf9fa5 --- /dev/null +++ b/tests/test_author_mutation_worktree.py @@ -0,0 +1,109 @@ +"""Tests for branches-only author mutation worktree guard (#274).""" + +from __future__ import annotations + +import sys +import unittest +from pathlib import Path +from unittest import mock + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import author_mutation_worktree as amw # noqa: E402 + + +class TestPathUnderBranches(unittest.TestCase): + ROOT = "/repo/Gitea-Tools" + + def test_branches_subdirectory_passes(self): + self.assertTrue( + amw.is_path_under_branches(f"{self.ROOT}/branches/issue-274", self.ROOT) + ) + + def test_control_checkout_fails(self): + self.assertFalse(amw.is_path_under_branches(self.ROOT, self.ROOT)) + + def test_sibling_directory_fails(self): + self.assertFalse( + amw.is_path_under_branches("/repo/other-checkout", self.ROOT) + ) + + +class TestAssessAuthorMutationWorktree(unittest.TestCase): + ROOT = "/repo/Gitea-Tools" + + def test_branches_worktree_passes(self): + result = amw.assess_author_mutation_worktree( + workspace_path=f"{self.ROOT}/branches/issue-274", + project_root=self.ROOT, + current_branch="feat/issue-274-branches-only-worktrees", + ) + self.assertTrue(result["proven"]) + self.assertFalse(result["block"]) + + def test_control_checkout_blocks(self): + result = amw.assess_author_mutation_worktree( + workspace_path=self.ROOT, + project_root=self.ROOT, + current_branch="master", + ) + self.assertFalse(result["proven"]) + self.assertTrue(result["block"]) + self.assertIn("control checkout", result["reasons"][0]) + + def test_drifted_control_branch_blocks(self): + result = amw.assess_author_mutation_worktree( + workspace_path=self.ROOT, + project_root=self.ROOT, + current_branch="feat/some-task", + ) + self.assertTrue(result["block"]) + self.assertTrue(any("drift" in r for r in result["reasons"])) + + def test_branches_worktree_as_project_root_passes(self): + """MCP launched from a branches/ worktree uses that path as PROJECT_ROOT.""" + worktree_root = f"{self.ROOT}/branches/issue-274" + result = amw.assess_author_mutation_worktree( + workspace_path=worktree_root, + project_root=worktree_root, + current_branch="feat/issue-274-branches-only-worktrees", + ) + self.assertTrue(result["proven"]) + self.assertFalse(result["block"]) + + +class TestPreflightIntegration(unittest.TestCase): + def test_verify_preflight_blocks_control_checkout_with_test_porcelain(self): + import mcp_server + + mcp_server._preflight_whoami_called = True + mcp_server._preflight_capability_called = True + mcp_server._preflight_resolved_role = "author" + control_root = "/repo/Gitea-Tools" + with mock.patch.object(mcp_server, "PROJECT_ROOT", control_root): + with mock.patch.dict( + "os.environ", + {"GITEA_TEST_PORCELAIN": ""}, + clear=False, + ): + with self.assertRaises(RuntimeError) as ctx: + mcp_server.verify_preflight_purity() + self.assertIn("Branches-only mutation guard", str(ctx.exception)) + + def test_verify_preflight_allows_branches_worktree(self): + import mcp_server + + mcp_server._preflight_whoami_called = True + mcp_server._preflight_capability_called = True + mcp_server._preflight_resolved_role = "author" + worktree = "/repo/Gitea-Tools/branches/issue-274" + with mock.patch.dict( + "os.environ", + {"GITEA_TEST_PORCELAIN": ""}, + clear=False, + ): + mcp_server.verify_preflight_purity(worktree_path=worktree) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_commit_payloads.py b/tests/test_commit_payloads.py index 1b3a80d..e8e4123 100644 --- a/tests/test_commit_payloads.py +++ b/tests/test_commit_payloads.py @@ -55,7 +55,15 @@ class TestCommitPayloads(unittest.TestCase): with open(self.config_path, "w", encoding="utf-8") as fh: fh.write(json.dumps(CONFIG)) - self.locked_worktree_dir = tempfile.TemporaryDirectory() + repo_root = os.path.realpath( + os.path.join(os.path.dirname(__file__), os.pardir) + ) + branches_root = os.path.join(repo_root, "branches") + os.makedirs(branches_root, exist_ok=True) + self.locked_worktree_dir = tempfile.TemporaryDirectory( + dir=branches_root, + prefix="test-commit-payloads-", + ) self.locked_worktree_path = os.path.realpath(self.locked_worktree_dir.name) self.lock_file_path = "/tmp/gitea_issue_lock.json" @@ -94,6 +102,7 @@ class TestCommitPayloads(unittest.TestCase): "GITEA_MCP_PROFILE": profile, "GITEA_TOKEN_AUTHOR": "author-pass", "GITEA_TEST_PORCELAIN": "", + "GITEA_AUTHOR_WORKTREE": self.locked_worktree_path, } @patch("mcp_server.api_request") diff --git a/tests/test_delete_branch_capability.py b/tests/test_delete_branch_capability.py new file mode 100644 index 0000000..e67d45a --- /dev/null +++ b/tests/test_delete_branch_capability.py @@ -0,0 +1,217 @@ +"""Tests for gitea_delete_branch capability gate (Issue #408). + +``gitea_delete_branch`` requires the exact ``gitea.branch.delete`` operation: +without it the delete fails closed (no preflight, no auth lookup, no API call, +structured permission report). With it, deletion proceeds through existing +preflight and audit unchanged. +""" +import json +import os +import sys +import tempfile +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) + +import mcp_server +from mcp_server import gitea_delete_branch + +FAKE_AUTH = "token fake" + +AUTHOR_NO_DELETE = { + "profile_name": "prgs-author", + "allowed_operations": [ + "gitea.read", "gitea.pr.create", "gitea.pr.comment", + "gitea.branch.push", "gitea.issue.comment", + ], + "forbidden_operations": ["gitea.pr.approve", "gitea.pr.merge"], + "audit_label": "prgs-author", +} + +AUTHOR_WITH_DELETE = { + "profile_name": "prgs-author-deleter", + "allowed_operations": AUTHOR_NO_DELETE["allowed_operations"] + [ + "gitea.branch.delete", + ], + "forbidden_operations": ["gitea.pr.approve", "gitea.pr.merge"], + "audit_label": "prgs-author-deleter", +} + +CONFIG = { + "version": 2, + "contexts": { + "ctx": { + "enabled": True, + "gitea": {"enabled": True, "base_url": "https://gitea.example.com"}, + } + }, + "profiles": { + "author-no-delete": { + "enabled": True, + "context": "ctx", + "role": "author", + "username": "author-user", + "auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"}, + "allowed_operations": AUTHOR_NO_DELETE["allowed_operations"], + "forbidden_operations": [], + "execution_profile": "author-no-delete", + }, + "author-with-delete": { + "enabled": True, + "context": "ctx", + "role": "author", + "username": "deleter-user", + "auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"}, + "allowed_operations": AUTHOR_WITH_DELETE["allowed_operations"], + "forbidden_operations": [], + "execution_profile": "author-with-delete", + }, + "reviewer-profile": { + "enabled": True, + "context": "ctx", + "role": "reviewer", + "username": "reviewer-user", + "auth": {"type": "env", "name": "GITEA_TOKEN_REVIEWER"}, + "allowed_operations": [ + "gitea.read", "gitea.pr.review", "gitea.pr.merge", + ], + "forbidden_operations": [ + "gitea.branch.delete", "gitea.branch.push", "gitea.pr.create", + ], + "execution_profile": "reviewer-profile", + }, + }, + "rules": {"allow_runtime_switching": False}, +} + + +class TestDeleteBranchToolGate(unittest.TestCase): + + def setUp(self): + self._preflight_snapshot = ( + mcp_server._preflight_whoami_called, + mcp_server._preflight_capability_called, + ) + mcp_server._preflight_whoami_called = False + mcp_server._preflight_capability_called = False + self._remotes = patch.dict(mcp_server.REMOTES, { + "prgs": {"host": "gitea.example.com", "org": "Example-Org", + "repo": "Example-Repo"}, + }) + self._remotes.start() + mcp_server._IDENTITY_CACHE.clear() + patch("gitea_config.load_config", return_value={}).start() + patch( + "gitea_config.is_runtime_switching_enabled", return_value=False + ).start() + patch("gitea_audit.audit_enabled", return_value=False).start() + self.mock_api = patch("mcp_server.api_request").start() + self.mock_auth = patch( + "mcp_server.get_auth_header", return_value=FAKE_AUTH + ).start() + + def tearDown(self): + patch.stopall() + mcp_server._IDENTITY_CACHE.clear() + mcp_server._preflight_whoami_called, mcp_server._preflight_capability_called = ( + self._preflight_snapshot + ) + + def _set_profile(self, profile): + patch("mcp_server.get_profile", return_value=profile).start() + + def test_blocked_without_delete_capability(self): + self._set_profile(AUTHOR_NO_DELETE) + res = gitea_delete_branch(branch="feat/branch", remote="prgs") + self.assertFalse(res["success"]) + self.assertFalse(res["performed"]) + self.assertEqual(res["required_permission"], "gitea.branch.delete") + self.assertTrue(res["reasons"]) + self.assertEqual( + res["permission_report"]["missing_permission"], + "gitea.branch.delete", + ) + self.mock_api.assert_not_called() + self.mock_auth.assert_not_called() + + def test_allowed_delete_proceeds(self): + self._set_profile(AUTHOR_WITH_DELETE) + mcp_server.record_preflight_check("whoami") + mcp_server.record_preflight_check("capability", resolved_role="author") + res = gitea_delete_branch(branch="feat/branch", remote="prgs") + self.assertTrue(res["success"]) + self.assertIn("deleted", res["message"]) + delete_calls = [ + c for c in self.mock_api.call_args_list if c.args[0] == "DELETE" + ] + self.assertTrue(delete_calls) + + def test_allowed_delete_audited_with_capability_proof(self): + self._set_profile(AUTHOR_WITH_DELETE) + patch("gitea_audit.audit_enabled", return_value=True).start() + mock_write = patch("gitea_audit.write_event").start() + mcp_server.record_preflight_check("whoami") + mcp_server.record_preflight_check("capability", resolved_role="author") + self.mock_api.return_value = {} + gitea_delete_branch(branch="feat/branch", remote="prgs") + mock_write.assert_called() + event = mock_write.call_args[0][0] + self.assertEqual(event["action"], "delete_branch") + self.assertEqual( + event["request_metadata"]["required_permission"], + "gitea.branch.delete", + ) + + +class TestDeleteBranchResolverParity(unittest.TestCase): + def setUp(self): + self._remotes = patch.dict(mcp_server.REMOTES, { + "prgs": {"host": "gitea.example.com", "org": "Example-Org", + "repo": "Example-Repo"}, + }) + self._remotes.start() + mcp_server._IDENTITY_CACHE.clear() + self._dir = tempfile.TemporaryDirectory() + self.config_path = os.path.join(self._dir.name, "profiles.json") + with open(self.config_path, "w", encoding="utf-8") as fh: + fh.write(json.dumps(CONFIG)) + patch( + "gitea_config.is_runtime_switching_enabled", return_value=False + ).start() + patch("gitea_audit.audit_enabled", return_value=False).start() + self.mock_api = patch("mcp_server.api_request").start() + patch("mcp_server.get_auth_header", return_value=FAKE_AUTH).start() + + def tearDown(self): + patch.stopall() + mcp_server._IDENTITY_CACHE.clear() + self._dir.cleanup() + + def _env(self, profile: str) -> dict: + return { + "GITEA_MCP_CONFIG": self.config_path, + "GITEA_MCP_PROFILE": profile, + "GITEA_TOKEN_AUTHOR": "author-pass", + "GITEA_TOKEN_REVIEWER": "reviewer-pass", + } + + def test_reviewer_resolver_denial_blocks_raw_tool(self): + with patch.dict(os.environ, self._env("reviewer-profile"), clear=True): + resolve = mcp_server.gitea_resolve_task_capability( + task="delete_branch", remote="prgs") + self.assertFalse(resolve["allowed_in_current_session"]) + res = gitea_delete_branch(branch="feat/branch", remote="prgs") + self.assertFalse(res["success"]) + self.assertEqual( + res["permission_report"]["missing_permission"], + resolve["required_operation_permission"], + ) + delete_calls = [ + c for c in self.mock_api.call_args_list if c.args[0] == "DELETE" + ] + self.assertFalse(delete_calls) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_final_report_validator.py b/tests/test_final_report_validator.py index f8e8848..672d82e 100644 --- a/tests/test_final_report_validator.py +++ b/tests/test_final_report_validator.py @@ -25,11 +25,15 @@ def _review_handoff(**overrides): "- Files changed: review_proofs.py", "- Validation: pytest tests/test_review_proofs.py -q in branches/review-203", "- Mutations: review only", - "- Workspace mutations: none", "- File edits by reviewer: none", "- Worktree/index mutations: none", "- Git ref mutations: git fetch prgs master", "- MCP/Gitea mutations: review submitted", + "- Review mutations: submitted request_changes review", + "- Merge mutations: none", + "- Cleanup mutations: none", + "- External-state mutations: none", + "- Read-only diagnostics: issue and PR metadata inspected", "- Current status: PR open", "- Blockers: none", "- Next: await author", @@ -53,26 +57,47 @@ def _review_handoff(**overrides): return text -def _reconcile_handoff(**overrides): +def _reconcile_handoff(drop=(), **overrides): lines = [ "## Controller Handoff", "", "- Task: reconcile already-landed PR #99", "- Repo: Scaled-Tech-Consulting/Gitea-Tools", - "- Role: reconciler", - "- Identity: sysadmin / prgs-author", + "- Role/profile: reconciler / prgs-reconciler", + "- Identity: sysadmin", "- Selected PR: #99", - "- Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED", + "- PR live state: open", + "- Candidate head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", + "- Target branch: master", + "- Target branch SHA: 5e023dc71b0e2b813a0b1eee6b0f42630c47a95c", + "- Ancestor proof: candidate head is ancestor of target branch SHA", "- Linked issue: #98", "- Linked issue live status: not verified in this session", + "- Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED", + "- Capabilities proven: gitea.read, gitea.pr.comment", + "- Missing capabilities: gitea.pr.close", + "- PR comments posted: 1 reconciliation comment on PR #99", + "- Issue comments posted: none", + "- PRs closed: none", + "- Issues closed: none", "- File edits by reconciler: none", "- Git ref mutations: git fetch prgs master", + "- Worktree mutations: none", + "- MCP/Gitea mutations: PR comment posted", + "- External-state mutations: none", + "- Read-only diagnostics: gitea_view_pr, gitea_view_issue", "- Reconciliation mutations: PR comment posted", "- Current status: reconciliation complete", - "- Safe next action: none", + "- Blocker: missing gitea.pr.close capability", + "- Safe next action: hand off to a profile with gitea.pr.close", "- No review/merge confirmation: confirmed", ] - text = "\n".join(lines) + kept = [ + line + for line in lines + if not any(line.startswith(f"- {name}:") for name in drop) + ] + text = "\n".join(kept) for key, value in overrides.items(): if f"- {key}:" in text: text = text.replace(f"- {key}:", f"- {key}: {value}") @@ -190,11 +215,76 @@ class TestReviewPrRules(unittest.TestCase): ) ) + def test_already_landed_oldest_eligible_pr_blocks(self): + report = ( + _review_handoff() + + "\nAlready landed.\nOldest eligible PR: PR #278." + ) + result = assess_final_report_validator(report, "review_pr") + self.assertTrue(result["blocked"]) + self.assertTrue( + any( + f["rule_id"] == "reviewer.already_landed_eligible_wording" + for f in result["findings"] + ) + ) + + def test_already_landed_gate_blocks_review_terminal_states(self): + cases = { + "APPROVED": ("- Review decision: request_changes", "- Review decision: APPROVED"), + "MERGED": ("- Merge result: not attempted", "- Merge result: MERGED"), + "READY_TO_MERGE": ("- Current status: PR open", "- Current status: READY_TO_MERGE"), + } + for state, (old, new) in cases.items(): + with self.subTest(state=state): + report = ( + _review_handoff() + .replace("- Reviewer eligibility: eligible", "- Reviewer eligibility: blocked") + .replace(old, new) + + "\n- Already-landed gate: fired" + + "\n- Ancestor proof: passed" + + "\n- Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED" + ) + result = assess_final_report_validator(report, "review_pr") + self.assertTrue(result["blocked"]) + self.assertTrue( + any( + f["rule_id"] == "reviewer.already_landed_review_state" + for f in result["findings"] + ) + ) + + def test_target_branch_up_to_date_requires_fetch_and_sha(self): + report = ( + _review_handoff() + .replace("- Git ref mutations: git fetch prgs master", "- Git ref mutations: none") + + "\nTarget branch up to date." + ) + result = assess_final_report_validator(report, "review_pr") + self.assertTrue(result["blocked"]) + self.assertTrue( + any( + f["rule_id"] == "reviewer.target_branch_freshness_proof" + for f in result["findings"] + ) + ) + + def test_target_branch_up_to_date_with_fetch_and_sha_passes(self): + report = ( + _review_handoff() + + "\n- Target branch SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9" + + "\nTarget branch up to date." + ) + result = assess_final_report_validator(report, "review_pr") + self.assertEqual(result["grade"], "A") def test_git_fetch_must_not_be_readonly_only(self): report = ( _review_handoff() .replace("- Git ref mutations: git fetch prgs master", "- Git ref mutations: none") - + "\n- Read-only diagnostics: git fetch prgs master" + .replace( + "- Read-only diagnostics: issue and PR metadata inspected", + "- Read-only diagnostics: git fetch prgs master", + ) ) result = assess_final_report_validator( report, @@ -251,6 +341,159 @@ class TestReconciliationRules(unittest.TestCase): ) +class TestCanonicalReconcileSchema(unittest.TestCase): + """Issue #307: reconciliation workflows emit only the canonical schema.""" + + def test_comment_only_reconciliation_passes(self): + result = assess_final_report_validator( + _reconcile_handoff(), + "reconcile_already_landed", + ) + self.assertEqual(result["grade"], "A") + self.assertEqual(result["findings"], []) + + def test_successful_close_reconciliation_passes(self): + report = _reconcile_handoff().replace( + "- Capabilities proven: gitea.read, gitea.pr.comment", + "- Capabilities proven: gitea.read, gitea.pr.comment, gitea.pr.close", + ).replace( + "- Missing capabilities: gitea.pr.close", + "- Missing capabilities: none", + ).replace( + "- PRs closed: none", + "- PRs closed: #99", + ).replace( + "- Blocker: missing gitea.pr.close capability", + "- Blocker: none", + ) + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertEqual(result["grade"], "A") + + def test_blocked_reconciliation_passes(self): + report = _reconcile_handoff().replace( + "- Capabilities proven: gitea.read, gitea.pr.comment", + "- Capabilities proven: gitea.read", + ).replace( + "- Missing capabilities: gitea.pr.close", + "- Missing capabilities: gitea.pr.close, gitea.pr.comment", + ).replace( + "- PR comments posted: 1 reconciliation comment on PR #99", + "- PR comments posted: none", + ).replace( + "- MCP/Gitea mutations: PR comment posted", + "- MCP/Gitea mutations: none", + ).replace( + "- Reconciliation mutations: PR comment posted", + "- Reconciliation mutations: none", + ) + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertEqual(result["grade"], "A") + + def test_missing_capabilities_field_required(self): + report = _reconcile_handoff(drop=("Missing capabilities",)) + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertEqual(result["grade"], "downgraded") + self.assertTrue( + any( + f["rule_id"] == "reconcile.controller_handoff" + and "Missing capabilities" in f["reason"] + for f in result["findings"] + ) + ) + + def test_missing_ancestor_proof_and_candidate_sha_downgrade(self): + report = _reconcile_handoff(drop=("Ancestor proof", "Candidate head SHA")) + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertEqual(result["grade"], "downgraded") + reasons = " ".join(f["reason"] for f in result["findings"]) + self.assertIn("Ancestor proof", reasons) + self.assertIn("Candidate head SHA", reasons) + + def test_stale_issue_lock_proof_blocks(self): + report = _reconcile_handoff() + "\n- Issue lock proof: locked before diff" + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertTrue(result["blocked"]) + self.assertTrue( + any( + f["rule_id"] == "reconcile.stale_author_reviewer_fields" + and f["field"] == "issue lock proof" + for f in result["findings"] + ) + ) + + def test_stale_claim_comment_status_blocks(self): + report = _reconcile_handoff() + "\n- Claim/comment status: claimed" + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertTrue(result["blocked"]) + self.assertTrue( + any( + f["rule_id"] == "reconcile.stale_author_reviewer_fields" + and f["field"] == "claim/comment status" + for f in result["findings"] + ) + ) + + def test_stale_workspace_mutations_blocks_without_observed_mutations(self): + report = _reconcile_handoff() + "\n- Workspace mutations: None" + result = assess_final_report_validator(report, "reconcile_already_landed") + self.assertTrue(result["blocked"]) + self.assertTrue( + any( + f["rule_id"] == "reconcile.stale_author_reviewer_fields" + and f["field"] == "workspace mutations" + for f in result["findings"] + ) + ) + + def test_pr_number_opened_allowed_when_session_opened_pr(self): + report = _reconcile_handoff() + "\n- PR number opened: #12" + result = assess_final_report_validator( + report, + "reconcile_already_landed", + session_pr_opened=True, + ) + self.assertFalse( + any( + f["rule_id"] == "reconcile.stale_author_reviewer_fields" + and f["field"] == "pr number opened" + for f in result["findings"] + ) + ) + + def test_pr_number_opened_still_blocked_without_session_proof(self): + report = _reconcile_handoff() + "\n- PR number opened: #12" + result = assess_final_report_validator( + report, + "reconcile_already_landed", + session_pr_opened=False, + ) + self.assertTrue(result["blocked"]) + self.assertTrue( + any( + f["rule_id"] == "reconcile.stale_author_reviewer_fields" + and f["field"] == "pr number opened" + for f in result["findings"] + ) + ) + + def test_action_log_fetch_requires_git_ref_mutation_entry(self): + report = _reconcile_handoff().replace( + "- Git ref mutations: git fetch prgs master", + "- Git ref mutations: none", + ) + result = assess_final_report_validator( + report, + "reconcile_already_landed", + action_log=[{"command": "git fetch prgs master", "performed": True}], + ) + self.assertTrue( + any( + f["rule_id"] == "reviewer.git_fetch_readonly" + for f in result["findings"] + ) + ) + + class TestEntryPoint(unittest.TestCase): def test_unknown_task_kind_blocks(self): result = assess_final_report_validator("report", "unknown_mode") @@ -270,4 +513,4 @@ class TestEntryPoint(unittest.TestCase): if __name__ == "__main__": - unittest.main() \ No newline at end of file + unittest.main() diff --git a/tests/test_llm_workflow_split.py b/tests/test_llm_workflow_split.py index 6d191c0..44d80ce 100644 --- a/tests/test_llm_workflow_split.py +++ b/tests/test_llm_workflow_split.py @@ -16,6 +16,7 @@ def test_all_workflow_files_exist(): "reconcile-landed-pr.md", "create-issue.md", "work-issue.md", + "pr-queue-cleanup.md", ): 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/create-issue.md", "workflows/work-issue.md", + "workflows/pr-queue-cleanup.md", ): assert name in SKILL @@ -36,6 +38,7 @@ def test_skill_contains_mode_isolation_language(): assert "reconcile-landed-pr" in SKILL assert "create-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() @@ -97,6 +100,19 @@ def test_work_issue_workflow_contract(): assert "canonical: true" in text assert "Do not merge your own PR" in text assert "## 21. PR creation rules" in text + assert "gitea_route_task_session" in text + assert "work-issue" 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(): @@ -105,6 +121,7 @@ def test_final_report_schemas_exist(): "reconcile-landed-final-report.md", "create-issue-final-report.md", "work-issue-final-report.md", + "pr-queue-cleanup-final-report.md", ): assert (SKILL_DIR / "schemas" / name).is_file(), name @@ -124,6 +141,30 @@ def test_start_issue_template_references_work_issue_workflow(): assert "workflows/work-issue.md" in text +def test_reconcile_skill_registered_in_mcp_server(): + from mcp_server import _PROJECT_SKILLS + + assert "gitea-reconcile-landed-pr" in _PROJECT_SKILLS + steps = _PROJECT_SKILLS["gitea-reconcile-landed-pr"]["steps"] + joined = " ".join(steps).lower() + assert "gitea_scan_already_landed_open_prs" in joined + assert "gitea_assess_already_landed_reconciliation" in joined + + +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(): from review_proofs import assess_reviewer_fallback_report @@ -136,12 +177,24 @@ def test_final_report_validator_exported(): assert callable(assess_final_report_validator) +def test_review_final_report_schema_verifier_exported(): + from review_proofs import assess_review_final_report_schema + + assert callable(assess_review_final_report_schema) + + def test_create_issue_final_report_verifier_exported(): from review_proofs import assess_create_issue_final_report assert callable(assess_create_issue_final_report) +def test_work_issue_final_report_verifier_exported(): + from review_proofs import assess_work_issue_final_report + + assert callable(assess_work_issue_final_report) + + def test_merge_simulation_verifier_exported(): from review_proofs import assess_merge_simulation_report @@ -154,6 +207,12 @@ def test_validation_integrity_verifier_exported(): assert callable(assess_validation_integrity_report) +def test_validation_failure_history_verifier_exported(): + from review_proofs import assess_validation_failure_history_report + + assert callable(assess_validation_failure_history_report) + + def test_prior_blocker_skip_verifier_exported(): from review_proofs import assess_prior_blocker_skip_proof @@ -172,12 +231,36 @@ def test_validation_worktree_edit_verifier_exported(): assert callable(assess_validation_worktree_edit_report) +def test_reconcile_inventory_verifier_exported(): + from review_proofs import assess_reconcile_inventory_report + + assert callable(assess_reconcile_inventory_report) + + +def test_reconcile_linked_issue_verifier_exported(): + from review_proofs import assess_reconcile_linked_issue_report + + assert callable(assess_reconcile_linked_issue_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_proof_backed_handoff_verifier_exported(): + from review_proofs import assess_proof_backed_handoff_report + + assert callable(assess_proof_backed_handoff_report) + + def test_infra_stop_handoff_verifier_exported(): from review_proofs import assess_infra_stop_handoff_report @@ -193,4 +276,29 @@ def test_mutation_categories_verifier_exported(): def test_worktree_ownership_verifier_exported(): from review_proofs import assess_worktree_ownership_report - assert callable(assess_worktree_ownership_report) \ No newline at end of file + assert callable(assess_worktree_ownership_report) + + +def test_already_landed_classification_verifier_exported(): + from review_proofs import 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 diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 1928e2a..d9dc789 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -1013,9 +1013,19 @@ class TestReviewPR(unittest.TestCase): # --------------------------------------------------------------------------- class TestDeleteBranch(unittest.TestCase): + DELETE_PROFILE = { + "profile_name": "test-deleter", + "allowed_operations": ["gitea.read", "gitea.branch.delete"], + "forbidden_operations": [], + "audit_label": "test-deleter", + } + + @patch("mcp_server.get_profile", return_value=DELETE_PROFILE) @patch("mcp_server.api_request") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) - def test_delete_branch(self, _auth, mock_api): + def test_delete_branch(self, _auth, mock_api, _profile): + mcp_server.record_preflight_check("whoami") + mcp_server.record_preflight_check("capability", resolved_role="author") mock_api.return_value = {} result = gitea_delete_branch(branch="feat/branch") self.assertTrue(result["success"]) @@ -3059,10 +3069,18 @@ class TestIssueLocking(unittest.TestCase): mock_api.return_value = [] # no open PRs res = gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") self.assertTrue(res["success"]) + self.assertEqual(res["work_lease"]["operation_type"], "author_issue_work") + self.assertEqual(res["work_lease"]["issue_number"], 196) + self.assertEqual(res["work_lease"]["branch"], "feat/issue-196-mutations") + self.assertIn("created_at", res["work_lease"]) + self.assertIn("expires_at", res["work_lease"]) + self.assertIn("last_heartbeat_at", res["work_lease"]) + self.assertEqual(res["work_lease"]["claimant"]["profile"], "gitea-default") self.assertTrue(os.path.exists(ISSUE_LOCK_FILE)) with open(ISSUE_LOCK_FILE, encoding="utf-8") as f: lock = json.load(f) self.assertIn("worktree_path", lock) + self.assertIn("work_lease", lock) def test_lock_issue_mismatch_branch_fails(self): with self.assertRaises(ValueError) as ctx: @@ -3103,6 +3121,51 @@ class TestIssueLocking(unittest.TestCase): gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") self.assertIn("already tied to an open PR", str(ctx.exception)) + @patch( + "mcp_server.issue_lock_worktree.read_worktree_git_state", + return_value=_clean_master_git_state_for_lock(), + ) + @patch("mcp_server.api_get_all") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_lock_issue_reused_by_remote_branch(self, _auth, mock_api, _git_state): + mock_api.side_effect = [ + [], + [{"name": "feat/issue-196-existing-work"}], + ] + with self.assertRaises(ValueError) as ctx: + gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") + self.assertIn("already has matching branch", str(ctx.exception)) + + def test_lock_issue_blocks_active_same_operation_lease(self): + with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: + json.dump({ + "issue_number": 196, + "branch_name": "feat/issue-196-other-work", + "worktree_path": "/tmp/other-worktree", + "work_lease": { + "operation_type": "author_issue_work", + "expires_at": "2999-01-01T00:00:00Z", + }, + }, f) + with self.assertRaises(RuntimeError) as ctx: + gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") + self.assertIn("already has an active author_issue_work lease", str(ctx.exception)) + + def test_lock_issue_blocks_expired_same_operation_lease_for_recovery(self): + with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: + json.dump({ + "issue_number": 196, + "branch_name": "feat/issue-196-other-work", + "worktree_path": "/tmp/other-worktree", + "work_lease": { + "operation_type": "author_issue_work", + "expires_at": "2000-01-01T00:00:00Z", + }, + }, f) + with self.assertRaises(RuntimeError) as ctx: + gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") + self.assertIn("Recovery review is required before takeover", str(ctx.exception)) + @patch("mcp_server.api_get_all", return_value=[]) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_lock_from_clean_scratch_worktree(self, _auth, _api): diff --git a/tests/test_pr_queue_cleanup.py b/tests/test_pr_queue_cleanup.py new file mode 100644 index 0000000..e26a026 --- /dev/null +++ b/tests/test_pr_queue_cleanup.py @@ -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() diff --git a/tests/test_reconcile_already_landed_pr_tool.py b/tests/test_reconcile_already_landed_pr_tool.py new file mode 100644 index 0000000..4f1ee05 --- /dev/null +++ b/tests/test_reconcile_already_landed_pr_tool.py @@ -0,0 +1,149 @@ +"""Tests for gitea_reconcile_already_landed_pr MCP tool (#310).""" +import sys +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) + +import mcp_server +from mcp_server import gitea_reconcile_already_landed_pr + +RECONCILER_PROFILE = { + "profile_name": "prgs-reconciler", + "allowed_operations": [ + "gitea.read", + "gitea.pr.comment", + "gitea.issue.comment", + "gitea.issue.close", + "gitea.pr.close", + ], + "forbidden_operations": [ + "gitea.pr.approve", + "gitea.pr.merge", + "gitea.pr.create", + "gitea.branch.push", + ], + "audit_label": "prgs-reconciler", +} + +AUTHOR_PROFILE = { + "profile_name": "prgs-author", + "allowed_operations": [ + "gitea.read", + "gitea.pr.create", + "gitea.issue.comment", + ], + "forbidden_operations": ["gitea.pr.close", "gitea.pr.approve", "gitea.pr.merge"], + "audit_label": "prgs-author", +} + +OPEN_LANDED_PR = { + "number": 278, + "title": "Already landed (Closes #263)", + "body": "", + "state": "open", + "head": {"ref": "feat/issue-263", "sha": "2a544b78d1371925761d16f1c451bb3b2984470e"}, + "base": {"ref": "master"}, +} + + +class TestReconcileAlreadyLandedPrTool(unittest.TestCase): + def setUp(self): + self.mock_api = patch("mcp_server.api_request").start() + self.mock_auth = patch( + "mcp_server.get_auth_header", return_value="token test" + ).start() + patch("mcp_server.verify_preflight_purity").start() + patch("gitea_audit.audit_enabled", return_value=False).start() + mcp_server._IDENTITY_CACHE.clear() + + def tearDown(self): + patch.stopall() + mcp_server._IDENTITY_CACHE.clear() + + def _set_profile(self, profile): + patch("mcp_server.get_profile", return_value=profile).start() + + def test_close_blocked_without_pr_close_capability(self): + self._set_profile(AUTHOR_PROFILE) + res = gitea_reconcile_already_landed_pr( + pr_number=278, + close_pr=True, + remote="prgs", + ) + self.assertFalse(res["success"]) + self.assertFalse(res["performed"]) + self.assertEqual(res["required_permission"], "gitea.pr.close") + self.mock_api.assert_not_called() + + def test_not_landed_pr_close_denied_after_live_fetch(self): + self._set_profile(RECONCILER_PROFILE) + self.mock_api.return_value = OPEN_LANDED_PR + with patch( + "mcp_server.already_landed_reconcile.assess_already_landed_reconciliation", + return_value={ + "pr_state": "open", + "candidate_head_sha": OPEN_LANDED_PR["head"]["sha"], + "target_branch": "master", + "target_branch_sha": "deadbeef", + "ancestor_proof": False, + "eligibility_class": "NOT_ALREADY_LANDED", + "linked_issue": 263, + "close_allowed": False, + "git_ref_mutations": ["git fetch prgs master"], + "reasons": ["not ancestor"], + }, + ): + res = gitea_reconcile_already_landed_pr( + pr_number=278, + close_pr=True, + remote="prgs", + ) + self.assertFalse(res["success"]) + self.assertFalse(res.get("pr_closed")) + patch_calls = [ + c for c in self.mock_api.call_args_list if c.args[0] == "PATCH" + ] + self.assertEqual(patch_calls, []) + + def test_already_landed_close_allowed_with_capability(self): + self._set_profile(RECONCILER_PROFILE) + + def api_side_effect(method, url, auth, payload=None): + if method == "GET" and "/pulls/278" in url: + return dict(OPEN_LANDED_PR) + if method == "PATCH" and "/pulls/278" in url: + closed = dict(OPEN_LANDED_PR) + closed["state"] = "closed" + return closed + return {} + + self.mock_api.side_effect = api_side_effect + with patch( + "mcp_server.already_landed_reconcile.assess_already_landed_reconciliation", + return_value={ + "pr_state": "open", + "candidate_head_sha": OPEN_LANDED_PR["head"]["sha"], + "target_branch": "master", + "target_branch_sha": "e017d80256217f17b0f421cd051cfa5cbcf5ae0f", + "ancestor_proof": True, + "eligibility_class": "ALREADY_LANDED_RECONCILE_REQUIRED", + "linked_issue": 263, + "close_allowed": True, + "git_ref_mutations": ["git fetch prgs master"], + "reasons": [], + }, + ): + res = gitea_reconcile_already_landed_pr( + pr_number=278, + close_pr=True, + remote="prgs", + ) + self.assertTrue(res["success"]) + self.assertTrue(res["performed"]) + self.assertTrue(res["pr_closed"]) + self.assertEqual(res["eligibility_class"], "ALREADY_LANDED_RECONCILE_REQUIRED") + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_reconciler_profile.py b/tests/test_reconciler_profile.py new file mode 100644 index 0000000..1c3086d --- /dev/null +++ b/tests/test_reconciler_profile.py @@ -0,0 +1,87 @@ +"""Tests for reconciler profile model (#304).""" + +import os +import sys +import unittest + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +import gitea_mcp_server as mcp_server +import migrate_profiles +import reconciler_profile + + +PRGS_RECONCILER_ALLOWED = [ + "gitea.read", + "gitea.pr.comment", + "gitea.issue.comment", + "gitea.issue.close", + "gitea.pr.close", +] +PRGS_RECONCILER_FORBIDDEN = [ + "gitea.pr.approve", + "gitea.pr.merge", + "gitea.pr.create", + "gitea.branch.push", +] + + +class TestReconcilerProfileModel(unittest.TestCase): + def test_prgs_reconciler_shape_valid(self): + result = reconciler_profile.assess_reconciler_profile( + PRGS_RECONCILER_ALLOWED, + PRGS_RECONCILER_FORBIDDEN, + ) + self.assertTrue(result["is_reconciler_profile"]) + self.assertTrue(result["valid"]) + self.assertEqual(result["reasons"], []) + + def test_author_profile_not_reconciler(self): + allowed = [ + "gitea.read", + "gitea.pr.create", + "gitea.branch.push", + "gitea.issue.comment", + ] + forbidden = ["gitea.pr.approve", "gitea.pr.merge"] + self.assertFalse( + reconciler_profile.is_reconciler_profile(allowed, forbidden) + ) + + def test_reviewer_profile_not_reconciler(self): + allowed = [ + "gitea.read", + "gitea.pr.review", + "gitea.pr.approve", + "gitea.pr.merge", + ] + forbidden = ["gitea.pr.create", "gitea.branch.push"] + self.assertFalse( + reconciler_profile.is_reconciler_profile(allowed, forbidden) + ) + + def test_broad_close_on_author_invalid(self): + allowed = PRGS_RECONCILER_ALLOWED + ["gitea.pr.create"] + result = reconciler_profile.assess_reconciler_profile( + allowed, + PRGS_RECONCILER_FORBIDDEN, + ) + self.assertFalse(result["valid"]) + self.assertFalse(result["is_reconciler_profile"]) + + def test_role_kind_detects_reconciler(self): + role = mcp_server._role_kind( + PRGS_RECONCILER_ALLOWED, + PRGS_RECONCILER_FORBIDDEN, + ) + self.assertEqual(role, "reconciler") + + def test_migrate_infer_role_reconciler(self): + self.assertEqual( + migrate_profiles.infer_role("prgs-reconciler", "prgs-reconciler"), + "reconciler", + ) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_reconciliation_mcp_tools.py b/tests/test_reconciliation_mcp_tools.py new file mode 100644 index 0000000..974df92 --- /dev/null +++ b/tests/test_reconciliation_mcp_tools.py @@ -0,0 +1,106 @@ +"""Tests for reconciliation MCP assessment tools (#301).""" +import sys +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) + +import mcp_server +from mcp_server import ( + gitea_assess_already_landed_reconciliation, + gitea_scan_already_landed_open_prs, +) + +AUTHOR_PROFILE = { + "profile_name": "prgs-author", + "allowed_operations": [ + "gitea.read", + "gitea.pr.comment", + "gitea.issue.comment", + "gitea.issue.close", + ], + "forbidden_operations": ["gitea.pr.close"], + "audit_label": "prgs-author", +} + +OPEN_PR = { + "number": 278, + "title": "Landed (Closes #263)", + "body": "", + "state": "open", + "head": {"ref": "feat/x", "sha": "a" * 40}, + "base": {"ref": "master"}, +} + + +class TestReconciliationMcpTools(unittest.TestCase): + def setUp(self): + self.mock_api = patch("mcp_server.api_request").start() + self.mock_auth = patch( + "mcp_server.get_auth_header", return_value="token test" + ).start() + patch("mcp_server.get_profile", return_value=AUTHOR_PROFILE).start() + mcp_server._IDENTITY_CACHE.clear() + + def tearDown(self): + patch.stopall() + mcp_server._IDENTITY_CACHE.clear() + + def test_assess_returns_plan_without_mutations(self): + self.mock_api.return_value = OPEN_PR + with patch( + "mcp_server.reconciliation_workflow.assess_open_pr_reconciliation", + return_value={ + "reconciliation_allowed": True, + "eligibility_class": "ALREADY_LANDED_RECONCILE_REQUIRED", + "candidate_head_sha": OPEN_PR["head"]["sha"], + "target_branch_sha": "deadbeef", + "linked_issue": 263, + "review_merge_allowed": False, + }, + ): + res = gitea_assess_already_landed_reconciliation( + pr_number=278, remote="prgs" + ) + self.assertTrue(res["success"]) + self.assertFalse(res["performed"]) + self.assertEqual(res["plan"]["outcome"], "PARTIAL_RECONCILE_COMMENT_THEN_STOP") + self.assertFalse(res["review_merge_allowed"]) + patch_calls = [ + c for c in self.mock_api.call_args_list if c.args[0] == "PATCH" + ] + self.assertEqual(patch_calls, []) + + def test_scan_returns_candidates_with_pagination(self): + with patch( + "mcp_server.api_fetch_page", + return_value=([OPEN_PR], { + "page": 1, + "per_page": 50, + "is_final_page": True, + "has_more": False, + "next_page": None, + }), + ), patch( + "mcp_server.reconciliation_workflow.fetch_target_branch", + return_value={ + "success": True, + "target_branch_sha": "deadbeef", + "git_fetch_command": "git fetch prgs master", + }, + ), patch( + "mcp_server.reconciliation_workflow.assess_open_pr_reconciliation", + return_value={ + "pr_number": 278, + "eligibility_class": "ALREADY_LANDED_RECONCILE_REQUIRED", + "reconciliation_allowed": True, + }, + ): + res = gitea_scan_already_landed_open_prs(remote="prgs", limit=50) + self.assertTrue(res["success"]) + self.assertEqual(res["candidate_count"], 1) + self.assertTrue(res["pagination"]["inventory_complete"]) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_reconciliation_workflow.py b/tests/test_reconciliation_workflow.py new file mode 100644 index 0000000..e3235c4 --- /dev/null +++ b/tests/test_reconciliation_workflow.py @@ -0,0 +1,149 @@ +"""Tests for already-landed reconciliation workflow helpers (#301).""" +import sys +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) + +import reconciliation_workflow +from review_proofs import assess_reconcile_workflow_source + + +class TestReconciliationAssessment(unittest.TestCase): + def test_already_landed_open_pr(self): + with patch( + "reconciliation_workflow.is_head_ancestor_of_ref", + return_value=True, + ): + assessment = reconciliation_workflow.assess_open_pr_reconciliation( + pr={ + "number": 278, + "state": "open", + "title": "Fix (Closes #263)", + "body": "", + "head": {"ref": "feat/x", "sha": "abc" * 13 + "a"}, + "base": {"ref": "master"}, + }, + project_root="/tmp/repo", + remote="prgs", + target_branch="master", + target_fetch={ + "success": True, + "target_ref": "prgs/master", + "target_branch_sha": "deadbeef", + "git_fetch_command": "git fetch prgs master", + }, + ) + self.assertTrue(assessment["reconciliation_allowed"]) + self.assertEqual( + assessment["eligibility_class"], + reconciliation_workflow.ELIGIBILITY_ALREADY_LANDED, + ) + + def test_not_landed_pr(self): + with patch( + "reconciliation_workflow.is_head_ancestor_of_ref", + return_value=False, + ): + assessment = reconciliation_workflow.assess_open_pr_reconciliation( + pr={ + "number": 1, + "state": "open", + "title": "WIP", + "body": "", + "head": {"ref": "feat/y", "sha": "fff" * 13 + "f"}, + "base": {"ref": "master"}, + }, + project_root="/tmp/repo", + remote="prgs", + target_branch="master", + target_fetch={ + "success": True, + "target_ref": "prgs/master", + "target_branch_sha": "deadbeef", + }, + ) + self.assertFalse(assessment["reconciliation_allowed"]) + + +class TestReconciliationPlan(unittest.TestCase): + def test_full_close_when_close_pr_allowed(self): + plan = reconciliation_workflow.resolve_reconciliation_plan( + assessment={ + "reconciliation_allowed": True, + "eligibility_class": reconciliation_workflow.ELIGIBILITY_ALREADY_LANDED, + }, + capabilities={ + "close_pr": True, + "comment_pr": True, + "close_issue": True, + "comment_issue": True, + }, + ) + self.assertEqual( + plan["outcome"], + reconciliation_workflow.OUTCOME_FULL_RECONCILE, + ) + self.assertTrue(plan["close_pr_allowed"]) + self.assertFalse(plan["review_merge_allowed"]) + + def test_comment_then_stop_without_close(self): + plan = reconciliation_workflow.resolve_reconciliation_plan( + assessment={ + "reconciliation_allowed": True, + "eligibility_class": reconciliation_workflow.ELIGIBILITY_ALREADY_LANDED, + }, + capabilities={ + "close_pr": False, + "comment_pr": True, + }, + ) + self.assertEqual( + plan["outcome"], + reconciliation_workflow.OUTCOME_PARTIAL_COMMENT, + ) + + def test_recovery_handoff_without_comment_or_close(self): + plan = reconciliation_workflow.resolve_reconciliation_plan( + assessment={ + "reconciliation_allowed": True, + "eligibility_class": reconciliation_workflow.ELIGIBILITY_ALREADY_LANDED, + }, + capabilities={"close_pr": False, "comment_pr": False}, + ) + self.assertEqual( + plan["outcome"], + reconciliation_workflow.OUTCOME_RECOVERY_HANDOFF, + ) + + def test_not_landed_no_action(self): + plan = reconciliation_workflow.resolve_reconciliation_plan( + assessment={ + "reconciliation_allowed": False, + "eligibility_class": reconciliation_workflow.ELIGIBILITY_NOT_LANDED, + "reasons": ["not ancestor"], + }, + capabilities={"close_pr": True}, + ) + self.assertEqual( + plan["outcome"], + reconciliation_workflow.OUTCOME_NOT_LANDED, + ) + + +class TestReconcileWorkflowSource(unittest.TestCase): + def test_valid_report_passes(self): + report = ( + "Task mode: reconcile-landed-pr\n" + "Workflow source: workflows/reconcile-landed-pr.md\n" + ) + result = assess_reconcile_workflow_source(report) + self.assertTrue(result["complete"]) + + def test_missing_workflow_fails(self): + result = assess_reconcile_workflow_source("Task mode: reconcile-landed-pr") + self.assertFalse(result["complete"]) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_resolve_task_capability.py b/tests/test_resolve_task_capability.py index 3545675..a34456d 100644 --- a/tests/test_resolve_task_capability.py +++ b/tests/test_resolve_task_capability.py @@ -121,6 +121,32 @@ class TestResolveTaskCapability(unittest.TestCase): self.assertTrue(res["allowed_in_current_session"]) self.assertFalse(res["different_mcp_namespace_required"]) + @patch("mcp_server.api_request", return_value={"login": "author-user"}) + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_resolve_work_issue_author_profile_allowed(self, _auth, _api): + with patch.dict(os.environ, self._env("author-profile")): + for task in ("work_issue", "work-issue"): + res = mcp_server.gitea_resolve_task_capability( + task=task, remote="prgs" + ) + self.assertEqual(res["requested_task"], task) + self.assertEqual( + res["required_operation_permission"], "gitea.pr.create" + ) + self.assertEqual(res["required_role_kind"], "author") + self.assertTrue(res["allowed_in_current_session"]) + + @patch("mcp_server.api_request", return_value={"login": "author-user"}) + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_resolve_work_issue_reviewer_profile_blocked(self, _auth, _api): + with patch.dict(os.environ, self._env("reviewer-profile")): + res = mcp_server.gitea_resolve_task_capability( + task="work_issue", remote="prgs" + ) + self.assertFalse(res["allowed_in_current_session"]) + self.assertEqual(res["required_role_kind"], "author") + self.assertTrue(res["different_mcp_namespace_required"]) + @patch("mcp_server.api_request", return_value={"login": "author-user"}) @patch("mcp_server.get_auth_header", return_value="token author-pass") def test_resolve_issue_comment_task(self, _auth, _api): @@ -275,6 +301,40 @@ class TestResolveTaskCapability(unittest.TestCase): self.assertEqual(res["required_role_kind"], "author") 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_reconcile_already_landed_pr_requires_close(self, _auth, _api): + with patch.dict(os.environ, self._env("author-profile")): + res = mcp_server.gitea_resolve_task_capability( + task="reconcile_already_landed_pr", remote="prgs" + ) + self.assertEqual(res["requested_task"], "reconcile_already_landed_pr") + self.assertEqual(res["required_operation_permission"], "gitea.pr.close") + self.assertEqual(res["required_role_kind"], "reconciler") + self.assertFalse(res["allowed_in_current_session"]) + + @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.get_auth_header", return_value="token author-pass") def test_close_pr_known_and_lookalike_tasks_still_fail_closed(self, _auth, _api): diff --git a/tests/test_review_final_report_schema.py b/tests/test_review_final_report_schema.py new file mode 100644 index 0000000..fbcb5a2 --- /dev/null +++ b/tests/test_review_final_report_schema.py @@ -0,0 +1,158 @@ +"""Tests for reviewer final-report schema verification (#391).""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from review_final_report_schema import assess_review_final_report_schema # noqa: E402 + + +def _minimal_review_report(**overrides): + lines = [ + "## Controller Handoff", + "", + "- Task: review PR #203", + "- Repo: Scaled-Tech-Consulting/Gitea-Tools", + "- Role: reviewer", + "- Identity: sysadmin / prgs-reviewer", + "- Issue/PR: #182 / PR #203", + "- Branch/SHA: feat/x @ 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", + "- Files changed: review_proofs.py", + "- Validation: pytest tests/test_review_proofs.py -q in branches/review-203", + "- Mutations: review only", + "- File edits by reviewer: none", + "- Worktree/index mutations: none", + "- Git ref mutations: git fetch prgs master", + "- MCP/Gitea mutations: review submitted", + "- Review mutations: request_changes submitted", + "- Merge mutations: none", + "- Cleanup mutations: none", + "- External-state mutations: none", + "- Read-only diagnostics: gitea_view_pr, gitea_view_issue", + "- Current status: PR open", + "- Blockers: none", + "- Next: await author", + "- Safety: no self-review; no self-merge", + "- Selected PR: #203", + "- Reviewer eligibility: eligible", + "- Candidate head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", + "- Worktree path: branches/review-203", + "- Worktree dirty: clean", + "- Unrelated local mutations: none", + "- Review decision: request_changes", + "- Merge result: not attempted", + "- Linked issue: #182", + "- Linked issue live status: open (gitea_view_issue)", + "- Cleanup status: none", + ] + text = "\n".join(lines) + for key, value in overrides.items(): + prefix = f"- {key}:" + replaced = False + new_lines = [] + for line in text.splitlines(): + if line.strip().startswith(prefix): + new_lines.append(f"{prefix} {value}") + replaced = True + else: + new_lines.append(line) + text = "\n".join(new_lines) + if not replaced: + text += f"\n{prefix} {value}" + return text + + +class TestReviewFinalReportSchema(unittest.TestCase): + def test_clean_report_passes(self): + result = assess_review_final_report_schema( + _minimal_review_report(), + linked_issue_lock={"issue_number": 182, "pr_number": 203}, + ) + self.assertFalse(result["blocked"]) + self.assertIn(result["grade"], ("A", "downgraded")) + + def test_legacy_pinned_reviewed_head_blocks(self): + report = _minimal_review_report(**{"Pinned reviewed head": "abc123"}) + result = assess_review_final_report_schema(report) + rule_ids = [f["rule_id"] for f in result["findings"]] + self.assertIn("reviewer.legacy_stale_field", rule_ids) + + def test_reviewed_head_without_validation_blocks(self): + report = _minimal_review_report().replace( + "- Validation: pytest tests/test_review_proofs.py -q in branches/review-203", + "- Validation: not run", + ) + report += "\n- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9" + result = assess_review_final_report_schema(report) + rule_ids = [f["rule_id"] for f in result["findings"]] + self.assertIn("reviewer.reviewed_head_without_validation", rule_ids) + + def test_merged_without_proof_blocks(self): + report = _minimal_review_report() + "\n\nPR #203 merged successfully." + result = assess_review_final_report_schema(report) + rule_ids = [f["rule_id"] for f in result["findings"]] + self.assertIn("reviewer.merged_without_proof", rule_ids) + + def test_issue_closed_without_live_proof_blocks(self): + report = _minimal_review_report( + **{ + "Linked issue live status": "closed", + "Read-only diagnostics": "gitea_view_pr only", + } + ) + result = assess_review_final_report_schema(report) + rule_ids = [f["rule_id"] for f in result["findings"]] + self.assertIn("reviewer.issue_closed_without_live_proof", rule_ids) + + def test_full_suite_pass_with_ignored_tests_blocks(self): + report = ( + _minimal_review_report() + + "\nFull test suite passed with 0 failed; some tests ignored." + ) + result = assess_review_final_report_schema(report) + rule_ids = [f["rule_id"] for f in result["findings"]] + self.assertIn("reviewer.full_suite_pass_ignored_tests", rule_ids) + + def test_blocked_handoff_replay_blocks(self): + report = ( + _minimal_review_report(**{"Blockers": "capability stop"}) + + "\nSafe next action: run gitea_merge_pr to finish." + ) + result = assess_review_final_report_schema(report) + rule_ids = [f["rule_id"] for f in result["findings"]] + self.assertIn("reviewer.blocked_handoff_replay", rule_ids) + + def test_narrative_handoff_drift_blocks(self): + report = ( + _minimal_review_report() + + "\n## Summary\nVerdict: APPROVE this PR." + ) + result = assess_review_final_report_schema(report) + rule_ids = [f["rule_id"] for f in result["findings"]] + self.assertIn("reviewer.narrative_handoff_drift", rule_ids) + + def test_pending_vs_approved_blocks(self): + report = _minimal_review_report( + **{"Review decision": "PENDING official review submitted approve"} + ) + result = assess_review_final_report_schema(report) + rule_ids = [f["rule_id"] for f in result["findings"]] + self.assertIn("reviewer.pending_vs_approved", rule_ids) + + def test_exported_from_review_proofs(self): + from review_proofs import assess_review_final_report_schema as exported + + self.assertTrue(callable(exported)) + + +class TestMcpValidateReviewFinalReport(unittest.TestCase): + def test_mcp_tool_callable(self): + import gitea_mcp_server + + result = gitea_mcp_server.gitea_validate_review_final_report( + _minimal_review_report() + ) + self.assertIn("grade", result) + self.assertIn("findings", result) + self.assertFalse(result["blocked"]) \ No newline at end of file diff --git a/tests/test_review_merge_state_machine.py b/tests/test_review_merge_state_machine.py new file mode 100644 index 0000000..b3930d6 --- /dev/null +++ b/tests/test_review_merge_state_machine.py @@ -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() \ No newline at end of file diff --git a/tests/test_review_proofs.py b/tests/test_review_proofs.py index 0558517..f132fc2 100644 --- a/tests/test_review_proofs.py +++ b/tests/test_review_proofs.py @@ -40,6 +40,9 @@ from review_proofs import ( # noqa: E402 assess_create_issue_final_report, assess_create_issue_mode_isolation, assess_create_issue_workflow_source, + assess_work_issue_final_report, + assess_work_issue_mode_isolation, + assess_work_issue_workflow_source, assess_edited_pr_inventory_coverage, assess_empty_queue_report, assess_fresh_issue_selection, @@ -2320,6 +2323,52 @@ class TestCreateIssueFinalReport(unittest.TestCase): self.assertTrue(result["downgraded"]) +class TestWorkIssueFinalReport(unittest.TestCase): + """#139: work-issue final-report verifier coverage.""" + + def _good_report(self): + return "\n".join([ + "Task mode: work-issue", + "Workflow source: skills/llm-project-workflow/workflows/work-issue.md", + "## Controller Handoff", + "- Task: work-issue", + "- Repo: Scaled-Tech-Consulting/Gitea-Tools", + "- Role: author", + "- Identity: jcwalker3 / prgs-author", + "- Active profile: prgs-author", + "- Runtime context: prgs-author on prgs", + "- Selected issue: #139", + "- Branch name: feat/issue-139-role-aware-work-issue-routing", + "- PR number: not created in this session", + "- File edits by author: role_session_router.py", + "- Blockers: none", + "- Current status: implementing routing", + "- Safe next action: open PR", + "- Next: open PR", + "- Safety statement: no review/merge", + ]) + + def test_complete_work_issue_report_earns_a(self): + result = assess_work_issue_final_report(self._good_report()) + self.assertEqual(result["grade"], "A") + self.assertFalse(result["downgraded"]) + + def test_missing_workflow_source_downgrades(self): + report = self._good_report().replace( + "workflows/work-issue.md", "SKILL.md only" + ) + result = assess_work_issue_workflow_source(report) + self.assertTrue(result["downgraded"]) + + def test_review_field_in_handoff_downgrades(self): + report = self._good_report().replace( + "- Safety statement:", + "- Review decision: APPROVE\n- Safety statement:", + ) + result = assess_work_issue_mode_isolation(report) + self.assertTrue(result["downgraded"]) + + class TestValidationEnvironmentProof(unittest.TestCase): """Issue #311: exact validation command and execution environment.""" diff --git a/tests/test_review_workflow_source.py b/tests/test_review_workflow_source.py new file mode 100644 index 0000000..07552ca --- /dev/null +++ b/tests/test_review_workflow_source.py @@ -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() diff --git a/tests/test_reviewer_already_landed_classification.py b/tests/test_reviewer_already_landed_classification.py new file mode 100644 index 0000000..7506eeb --- /dev/null +++ b/tests/test_reviewer_already_landed_classification.py @@ -0,0 +1,81 @@ +import unittest + +from reviewer_already_landed_classification import ( # noqa: E402 + ALREADY_LANDED_ELIGIBILITY_CLASS, + assess_already_landed_classification_report, +) + + +class TestAlreadyLandedClassification(unittest.TestCase): + def test_non_landed_report_passes(self): + result = assess_already_landed_classification_report( + "Selected the next eligible PR: PR #286." + ) + self.assertTrue(result["proven"]) + self.assertFalse(result["landed_context"]) + + def test_oldest_eligible_pr_on_landed_blocks(self): + result = assess_already_landed_classification_report( + "PR is already landed.\n" + "Oldest eligible PR: PR #278." + ) + self.assertFalse(result["proven"]) + self.assertTrue(result["block"]) + + def test_next_eligible_pr_on_landed_blocks(self): + result = assess_already_landed_classification_report( + "Ancestor proof: passed\n" + "Selected the next eligible PR: PR #278." + ) + self.assertFalse(result["proven"]) + + def test_eligible_for_review_on_landed_blocks(self): + result = assess_already_landed_classification_report( + "Already landed on master and eligible for review next." + ) + self.assertFalse(result["proven"]) + + def test_proper_landed_wording_passes(self): + result = assess_already_landed_classification_report( + "Oldest open PR requiring action: PR #278\n" + f"Eligibility class: {ALREADY_LANDED_ELIGIBILITY_CLASS}\n" + "Candidate head SHA: abc123\n" + "Reviewed head SHA: none\n" + "Queue blocked by already-landed PR requiring reconciliation", + selected_pr_already_landed=True, + ) + self.assertTrue(result["proven"]) + + def test_pinned_reviewed_head_without_validation_blocks(self): + result = assess_already_landed_classification_report( + "Already landed.\n" + "Pinned reviewed head: abc123def456", + selected_pr_already_landed=True, + ) + self.assertFalse(result["proven"]) + + def test_wrong_eligibility_class_blocks(self): + result = assess_already_landed_classification_report( + "Already landed.\n" + "Eligibility class: REVIEW_ELIGIBLE", + selected_pr_already_landed=True, + ) + self.assertFalse(result["proven"]) + + def test_missing_queue_blocker_blocks(self): + result = assess_already_landed_classification_report( + f"Eligibility class: {ALREADY_LANDED_ELIGIBILITY_CLASS}\n" + "Candidate head SHA: abc123\n" + "Reviewed head SHA: none", + selected_pr_already_landed=True, + ) + self.assertFalse(result["proven"]) + + def test_exported_from_review_proofs(self): + from review_proofs import assess_already_landed_classification_report as exported + + self.assertTrue(callable(exported)) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_reviewer_already_landed_handoff.py b/tests/test_reviewer_already_landed_handoff.py new file mode 100644 index 0000000..cf1c779 --- /dev/null +++ b/tests/test_reviewer_already_landed_handoff.py @@ -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() \ No newline at end of file diff --git a/tests/test_reviewer_proof_backed_handoff.py b/tests/test_reviewer_proof_backed_handoff.py new file mode 100644 index 0000000..d5f0740 --- /dev/null +++ b/tests/test_reviewer_proof_backed_handoff.py @@ -0,0 +1,117 @@ +"""Tests for proof-backed reviewer handoff verifier (#395).""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from reviewer_proof_backed_handoff import ( # noqa: E402 + assess_proof_backed_handoff_report, +) + + +def _good_report() -> str: + return "\n".join([ + "Inventory pagination proof: is_final_page: true, has_more: false, total_count: 11", + "Earlier PRs skipped: #372 non-mergeable — merge simulation conflicts in review_proofs.py", + "Baseline worktree path: branches/baseline-master-pr383", + "Baseline target SHA: 4243b60ca32d79f1ee5e6b0f10d7570e9dea2937", + "Baseline dirty before validation: false", + "Baseline validation command: venv/bin/python -m pytest tests/ -q", + "Baseline validation result: 1 failed (test_clean_reviewer_report_passes)", + "Baseline dirty after validation: false", + "Cleanup mutations: git worktree list showed session worktrees removed", + "## Controller Handoff", + "- Inventory pagination proof: inventory_complete: true, total_count: 11", + "- Earlier PRs skipped: #372 conflict proof via merge simulation", + "- Baseline worktree used: true", + "- Baseline worktree path: branches/baseline-master-pr383", + "- Cleanup mutations: git worktree remove branches/review-pr383", + ]) + + +class TestProofBackedHandoff(unittest.TestCase): + def test_fully_proof_backed_report_passes(self): + result = assess_proof_backed_handoff_report( + _good_report(), + action_log=[{"command": "gitea_list_prs", "result": "inventory_complete=true"}], + baseline_session={ + "complete": True, + "clean_before": True, + "clean_after": True, + "command": "pytest", + "result": "passed", + }, + ) + self.assertTrue(result["proven"], result["reasons"]) + + def test_inventory_page_size_assumption_blocks(self): + report = ( + "Inventory is complete because 13 results are less than page size 50." + ) + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue(any("page-size" in r.lower() for r in result["reasons"])) + + def test_skip_without_conflict_proof_blocks(self): + report = "\n".join([ + "Earlier PRs skipped: #372, #374", + "## Controller Handoff", + "- Earlier PRs skipped: #372, #374", + ]) + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue(any("skip" in r.lower() for r in result["reasons"])) + + def test_baseline_without_command_blocks(self): + report = "Baseline validation passed; same as master failures." + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue(any("baseline" in r.lower() for r in result["reasons"])) + + def test_cleanup_without_worktree_list_blocks(self): + report = "Worktrees were cleaned after merge." + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue(any("cleanup" in r.lower() for r in result["reasons"])) + + def test_cleanup_with_worktree_list_passes(self): + report = "Cleanup: git worktree list confirmed removal." + result = assess_proof_backed_handoff_report( + report, + action_log=[{"command": "git worktree list"}], + ) + self.assertTrue(result["proven"], result["reasons"]) + + def test_master_integration_requires_command(self): + report = "Merged master into the review worktree before validation." + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue(any("master integration" in r.lower() for r in result["reasons"])) + + def test_master_integration_with_action_log_passes(self): + report = "Merged master into review worktree; merge_exit=0" + result = assess_proof_backed_handoff_report( + report, + action_log=[{"command": "git merge --no-commit prgs/master"}], + ) + self.assertTrue(result["proven"], result["reasons"]) + + def test_live_proof_requires_source_classification(self): + report = "Live proof shows inventory complete." + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue(any("proof source" in r.lower() for r in result["reasons"])) + + +class TestExport(unittest.TestCase): + def test_review_proofs_reexport(self): + from review_proofs import assess_proof_backed_handoff_report as exported + + self.assertTrue(callable(exported)) + result = exported(_good_report(), inventory_session={"inventory_complete": True}) + self.assertTrue(result["proven"], result["reasons"]) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_reviewer_reconcile_inventory.py b/tests/test_reviewer_reconcile_inventory.py new file mode 100644 index 0000000..7dfb6b9 --- /dev/null +++ b/tests/test_reviewer_reconcile_inventory.py @@ -0,0 +1,102 @@ +"""Tests for reconciliation inventory pagination verifier (#308).""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from reviewer_reconcile_inventory import assess_reconcile_inventory_report # noqa: E402 + + +def _good_report() -> str: + return "\n".join([ + "## Already-landed reconciliation", + "Open PR inventory:", + "- Requested page size: 50", + "- Pages fetched: 2", + "- Returned PR count per page: page1=50, page2=6", + "- Inventory pagination proof: is_final_page: true, has_more: false", + "All already-landed PRs in the scanned open queue were found.", + "", + "## Controller Handoff", + "- Selected PR: #278", + "- Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED", + ]) + + +class TestReconcileInventoryPagination(unittest.TestCase): + def test_complete_metadata_passes(self): + result = assess_reconcile_inventory_report(_good_report()) + self.assertTrue(result["proven"], result["reasons"]) + self.assertTrue(result["pagination_proven"]) + + def test_no_inventory_claim_skips_checks(self): + report = "## Controller Handoff\n- Selected PR: #278" + result = assess_reconcile_inventory_report(report) + self.assertTrue(result["proven"]) + self.assertFalse(result["inventory_claimed"]) + + def test_first_page_below_limit_with_finality_passes(self): + report = "\n".join([ + "Open PRs listed for reconciliation.", + "- Requested page size: 50", + "- Pages fetched: 1", + "- Returned PR count per page: page1=6", + "- Inventory pagination proof: is_final_page: true", + ]) + result = assess_reconcile_inventory_report(report) + self.assertTrue(result["proven"], result["reasons"]) + + def test_exactly_full_first_page_without_finality_blocks(self): + report = "\n".join([ + "Complete queue scan of open PRs.", + "- Requested page size: 50", + "- Pages fetched: 1", + "- Returned PR count per page: page1=50", + "Listed 50 open PRs on the first page.", + ]) + result = assess_reconcile_inventory_report( + report, + inventory_session={"requested_page_size": 50, "returned_page_size": 50}, + ) + self.assertFalse(result["proven"]) + self.assertTrue(any("full first" in r.lower() for r in result["reasons"])) + + def test_missing_pagination_metadata_blocks(self): + report = "All open PRs were inventoried. Inventory is complete." + result = assess_reconcile_inventory_report(report) + self.assertFalse(result["proven"]) + self.assertTrue(any("metadata" in r.lower() for r in result["reasons"])) + + def test_default_page_size_assumption_blocks(self): + report = ( + "Exhaustive PR inventory: fewer than the default Gitea page size returned." + ) + result = assess_reconcile_inventory_report(report) + self.assertFalse(result["proven"]) + self.assertTrue(any("page-size" in r.lower() or "page size" in r.lower() + for r in result["reasons"])) + + def test_session_pagination_complete_passes(self): + report = "All already-landed PRs were found after scanning open PRs." + result = assess_reconcile_inventory_report( + report, + inventory_session={ + "pagination_complete": True, + "requested_page_size": 50, + "pages_fetched": 2, + "returned_per_page": [50, 3], + }, + ) + self.assertTrue(result["proven"], result["reasons"]) + + +class TestExport(unittest.TestCase): + def test_review_proofs_reexport(self): + from review_proofs import assess_reconcile_inventory_report as exported + + self.assertTrue(callable(exported)) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_reviewer_reconcile_linked_issue.py b/tests/test_reviewer_reconcile_linked_issue.py new file mode 100644 index 0000000..1640f4f --- /dev/null +++ b/tests/test_reviewer_reconcile_linked_issue.py @@ -0,0 +1,117 @@ +"""Tests for reconciliation linked-issue live proof verifier (#300).""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from reviewer_reconcile_linked_issue import assess_reconcile_linked_issue_report # noqa: E402 + + +def _good_handoff() -> str: + return "\n".join([ + "## PR reconciliation summary", + "Fetched linked issue #263 live with gitea_view_issue in this session.", + "", + "## Controller Handoff", + "- Selected PR: #278", + "- Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED", + "- Linked issue: #263", + "- Linked issue live status: Issue #263 (open)", + "- Safe next action: reconcile open PR", + ]) + + +class TestReconcileLinkedIssueLiveProof(unittest.TestCase): + def test_live_fetch_with_open_status_passes(self): + result = assess_reconcile_linked_issue_report( + _good_handoff(), + reconcile_session={"live_fetched": True, "linked_issue_number": 263}, + ) + self.assertTrue(result["proven"], result["reasons"]) + self.assertEqual(result["linked_issue_number"], 263) + + def test_no_linked_issue_passes(self): + report = "\n".join([ + "## Controller Handoff", + "- Selected PR: #99", + "- Linked issue: none", + "- Linked issue live status: not applicable", + ]) + result = assess_reconcile_linked_issue_report(report) + self.assertTrue(result["proven"]) + self.assertIsNone(result["linked_issue_number"]) + + def test_closed_status_without_live_proof_blocks(self): + report = "\n".join([ + "## Controller Handoff", + "- Linked issue: #263", + "- Linked issue live status: Issue #263 (closed)", + ]) + result = assess_reconcile_linked_issue_report(report) + self.assertFalse(result["proven"]) + self.assertTrue(result["block"]) + self.assertTrue(any("live" in r.lower() for r in result["reasons"])) + + def test_not_verified_wording_passes_without_live_proof(self): + report = "\n".join([ + "## Controller Handoff", + "- Linked issue: #263", + "- Linked issue live status: not verified in this session", + ]) + result = assess_reconcile_linked_issue_report(report) + self.assertTrue(result["proven"], result["reasons"]) + + def test_missing_issue_requires_not_verified(self): + report = "\n".join([ + "## Controller Handoff", + "- Linked issue: #999", + "- Linked issue live status: Issue #999 (open)", + ]) + result = assess_reconcile_linked_issue_report( + report, + reconcile_session={"issue_missing": True, "linked_issue_number": 999}, + ) + self.assertFalse(result["proven"]) + self.assertTrue(any("not verified" in r.lower() for r in result["reasons"])) + + def test_text_live_proof_without_session_lock_passes(self): + report = _good_handoff() + result = assess_reconcile_linked_issue_report(report) + self.assertTrue(result["proven"], result["reasons"]) + self.assertTrue(result["live_proof_present"]) + + def test_issue_action_recommendation_requires_capability_proof(self): + report = "\n".join([ + _good_handoff(), + "Recommended: close linked issue #263 after PR reconciliation.", + ]) + result = assess_reconcile_linked_issue_report( + report, + reconcile_session={"live_fetched": True}, + ) + self.assertFalse(result["proven"]) + self.assertTrue(any("capability" in r.lower() for r in result["reasons"])) + + def test_issue_action_with_capability_proof_passes(self): + report = "\n".join([ + _good_handoff(), + "Recommended: close linked issue #263.", + "Capability proof: gitea_close_issue allowed for prgs-author.", + ]) + result = assess_reconcile_linked_issue_report( + report, + reconcile_session={"live_fetched": True}, + ) + self.assertTrue(result["proven"], result["reasons"]) + + +class TestExport(unittest.TestCase): + def test_review_proofs_reexport(self): + from review_proofs import assess_reconcile_linked_issue_report as exported + + self.assertTrue(callable(exported)) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_reviewer_validation_failure_history.py b/tests/test_reviewer_validation_failure_history.py new file mode 100644 index 0000000..957fd52 --- /dev/null +++ b/tests/test_reviewer_validation_failure_history.py @@ -0,0 +1,147 @@ +"""Tests for transient validation failure history verifier (#396).""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from final_report_validator import assess_final_report_validator # noqa: E402 +from reviewer_validation_failure_history import ( # noqa: E402 + assess_validation_failure_history_report, +) + + +def _full_history_report() -> str: + return "\n".join([ + "Validation failure history", + "Failure #1:", + "Command: venv/bin/python -m pytest tests/test_worktrees.py -q", + "Failing test: tests/test_worktrees.py::TestWorktreeStart::test_rejects_untraceable_branches", + "Suspected cause: stale /tmp/gitea_issue_lock.json from prior session", + "Reproduced: no", + "On baseline master: no", + "PR-caused: no", + "What changed between runs: removed /tmp/gitea_issue_lock.json", + "Environmental cleanup: lock file removed before rerun", + "Validation: passed after transient failure investigation", + "Final validation command: venv/bin/python -m pytest tests/ -q", + "Final result: 1497 passed, 6 skipped", + ]) + + +class TestValidationFailureHistory(unittest.TestCase): + def test_no_failures_observed_passes(self): + result = assess_validation_failure_history_report( + "Validation: passed", + validation_session={"observed_failures": []}, + ) + self.assertTrue(result["proven"]) + + def test_omitted_failure_blocks(self): + result = assess_validation_failure_history_report( + "Validation: passed\n1497 passed, 6 skipped", + validation_session={ + "observed_failures": [{ + "command": "pytest tests/test_worktrees.py -q", + "failing_test": ( + "tests/test_worktrees.py::TestWorktreeStart::" + "test_rejects_untraceable_branches" + ), + }], + "final_validation_status": "passed", + }, + ) + self.assertFalse(result["proven"]) + self.assertTrue(result["block"]) + + def test_full_history_with_transient_investigation_passes(self): + result = assess_validation_failure_history_report( + _full_history_report(), + validation_session={ + "observed_failures": [{ + "command": ( + "venv/bin/python -m pytest tests/test_worktrees.py -q" + ), + "failing_test": ( + "tests/test_worktrees.py::TestWorktreeStart::" + "test_rejects_untraceable_branches" + ), + "suspected_cause": "stale /tmp/gitea_issue_lock.json", + "reproduced": "no", + "on_baseline_master": "no", + "pr_caused": "no", + }], + "final_validation_status": "passed_after_transient_failure_investigation", + "environmental_contamination": True, + }, + ) + self.assertTrue(result["proven"], result["reasons"]) + + def test_baseline_equivalent_failure_documented(self): + report = "\n".join([ + "Validation failure history", + "Command: pytest tests/test_example.py -q", + "Failing test: tests/test_example.py::test_flaky", + "Suspected cause: pre-existing master failure", + "On baseline master: yes", + "PR-caused: no", + "What changed between runs: none; failure matches baseline", + "Validation: passed after transient failure investigation", + ]) + result = assess_validation_failure_history_report( + report, + validation_session={ + "observed_failures": [{ + "command": "pytest tests/test_example.py -q", + "failing_test": "tests/test_example.py::test_flaky", + "on_baseline_master": "yes", + "pr_caused": "no", + }], + "final_validation_status": "passed_after_transient_failure_investigation", + }, + ) + self.assertTrue(result["proven"], result["reasons"]) + + def test_unknown_cause_plain_pass_blocks(self): + result = assess_validation_failure_history_report( + "Validation: passed", + validation_session={ + "observed_failures": [{ + "command": "pytest tests/ -q", + "failing_test": "tests/test_foo.py::test_bar", + "suspected_cause": "unknown", + }], + "final_validation_status": "passed", + "cause_unknown": True, + }, + ) + self.assertFalse(result["proven"]) + self.assertTrue(any("transient" in r.lower() for r in result["reasons"])) + + def test_final_report_validator_rejects_omitted_failure(self): + result = assess_final_report_validator( + "Validation: passed", + "review_pr", + validation_session={ + "observed_failures": [{ + "command": "pytest tests/ -q", + "failing_test": "tests/test_foo.py::test_bar", + }], + }, + ) + self.assertTrue(result["blocked"] or result["downgraded"]) + self.assertTrue( + any( + f.get("rule_id") == "reviewer.validation_failure_history" + for f in result.get("findings") or [] + ) + ) + + def test_exported_from_review_proofs(self): + from review_proofs import assess_validation_failure_history_report as exported + + self.assertTrue(callable(exported)) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_role_session_router.py b/tests/test_role_session_router.py index 6204b6d..f545963 100644 --- a/tests/test_role_session_router.py +++ b/tests/test_role_session_router.py @@ -52,6 +52,22 @@ CONFIG = { ], "execution_profile": "prgs-reviewer", }, + "prgs-reconciler": { + "enabled": True, + "context": "ctx", + "role": "reconciler", + "username": "sysadmin", + "auth": {"type": "env", "name": "GITEA_TOKEN_RECONCILER"}, + "allowed_operations": [ + "gitea.read", "gitea.pr.comment", "gitea.issue.comment", + "gitea.issue.close", "gitea.pr.close", + ], + "forbidden_operations": [ + "gitea.pr.approve", "gitea.pr.merge", "gitea.pr.create", + "gitea.branch.push", + ], + "execution_profile": "prgs-reconciler", + }, }, "rules": {"allow_runtime_switching": False}, } @@ -88,6 +104,7 @@ class TestRoleSessionRouter(unittest.TestCase): "GITEA_MCP_PROFILE": profile, "GITEA_TOKEN_AUTHOR": "author-pass", "GITEA_TOKEN_REVIEWER": "reviewer-pass", + "GITEA_TOKEN_RECONCILER": "reconciler-pass", } def test_reviewer_task_under_author_profile_wrong_role_stop(self): @@ -99,6 +116,24 @@ class TestRoleSessionRouter(unittest.TestCase): self.assertFalse(route["downstream_allowed"]) 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.get_auth_header", return_value="token author-pass") def test_issue_creation_blocked_after_reviewer_wrong_role_stop( @@ -157,6 +192,31 @@ class TestRoleSessionRouter(unittest.TestCase): ) self.assertEqual(result.get("number"), 999) + @patch("mcp_server.api_request", return_value={"login": "jcwalker3"}) + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_work_issue_task_under_author_profile_allowed(self, _auth, _api): + with patch.dict(os.environ, self._env("prgs-author")): + for task_type in ("work_issue", "work-issue"): + route = mcp_server.gitea_route_task_session( + task_type=task_type, remote="prgs" + ) + self.assertEqual( + route["route_result"], role_session_router.ROUTE_ALLOWED + ) + self.assertTrue(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_work_issue_task_under_reviewer_profile_routes_to_author(self, _auth, _api): + with patch.dict(os.environ, self._env("prgs-reviewer")): + route = mcp_server.gitea_route_task_session( + task_type="work-issue", remote="prgs" + ) + self.assertEqual( + route["route_result"], role_session_router.ROUTE_TO_AUTHOR + ) + 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_reviewer_task_under_reviewer_profile_allowed(self, _auth, _api): @@ -179,6 +239,24 @@ class TestRoleSessionRouter(unittest.TestCase): ) self.assertFalse(route["downstream_allowed"]) + @patch("mcp_server.api_request", return_value={"login": "sysadmin"}) + @patch("mcp_server.get_auth_header", return_value="token reconciler-pass") + def test_reconciler_task_under_reconciler_profile_allowed(self, _auth, _api): + with patch.dict(os.environ, self._env("prgs-reconciler")): + route = mcp_server.gitea_route_task_session( + task_type="reconcile_already_landed_pr", remote="prgs" + ) + self.assertEqual(route["route_result"], role_session_router.ROUTE_ALLOWED) + self.assertTrue(route["downstream_allowed"]) + + def test_reconciler_task_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="reconcile_already_landed_pr", remote="prgs" + ) + self.assertEqual(route["route_result"], role_session_router.ROUTE_WRONG_ROLE) + self.assertFalse(route["downstream_allowed"]) + def test_activate_profile_blocked_in_static_mode(self): with patch.dict(os.environ, self._env("prgs-author")): result = mcp_server.gitea_activate_profile(