diff --git a/docs/llm-workflow-runbooks.md b/docs/llm-workflow-runbooks.md index f3d82e3..09b4d5c 100644 --- a/docs/llm-workflow-runbooks.md +++ b/docs/llm-workflow-runbooks.md @@ -281,6 +281,21 @@ 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. +**Issue-lock recovery (#447):** Do not manually seed, restore, or delete +`/tmp/gitea_issue_lock.json` as a normal recovery path. That file is global +shared state and manual writes can clobber another session's live lease. Use +`sanctioned recovery` instead: + +1. `gitea_lock_issue` on a clean `branches/` worktree (normal path). +2. Own-branch adoption via #442 when the issue's exact branch is already pushed. +3. Operator override only when explicitly authorized — record + `External-state mutations` and `operator override proof` in the final report. + +`gitea_create_pr` rejects lock files that lack sanctioned `lock_provenance` +metadata. Final-report validation blocks handoffs that hide lock read/write/delete +under `External-state mutations: none` or mix author PR creation with reviewer +approval in one run. See also #438 (global lock redesign). + Remote branches matching the issue number are also treated as active work unless the recovery review proves the branch is abandoned or superseded. Never delete or clean up a branch when it has an active lease, dirty worktree, open PR, or is diff --git a/docs/webui-local-dev.md b/docs/webui-local-dev.md new file mode 100644 index 0000000..0a5b935 --- /dev/null +++ b/docs/webui-local-dev.md @@ -0,0 +1,89 @@ +# Internal web UI — local development (#426) + +Read-only MVP skeleton for the MCP Control Plane operator console. Gitea, +MCP capability gates, and `skills/llm-project-workflow/` remain the source of +truth; this UI only provides route stubs and layout. + +## Prerequisites + +- Python 3.11+ with project dependencies installed (`pip install -r requirements.txt`) +- No secrets in repo, config, or client bundle + +## Start the server + +From the repository root (or an issue worktree): + +```bash +./scripts/run-webui +``` + +Or directly: + +```bash +python3 -m webui +``` + +Optional environment variables: + +| Variable | Default | Purpose | +|----------|---------|---------| +| `WEBUI_HOST` | `127.0.0.1` | Bind address (keep local for MVP) | +| `WEBUI_PORT` | `8765` | Listen port | + +## Routes (MVP) + +| Path | Description | +|------|-------------| +| `/` | Home / operator overview | +| `/health` | JSON liveness (`status`, `service`, `mode`, `timestamp`) | +| `/queue` | Live PR and issue queue dashboard (#429) | +| `/api/queue` | JSON queue export with pagination metadata | +| `/projects` | Project registry list (#427) | +| `/projects/{id}` | Project detail + onboarding checklist | +| `/api/projects` | JSON registry export | +| `/prompts` | Prompt library with per-prompt copy buttons (#428) | +| `/api/prompts` | JSON prompt export with workflow hashes | +| `/runtime` | Stub — MCP runtime health (#430) | +| `/audit` | Stub — report audit paste (#431) | +| `/worktrees` | Stub — hygiene dashboard (#432) | +| `/leases` | Stub — lease visibility (#433) | + +All routes are GET-only. POST/PUT/PATCH/DELETE return `405` with +`read-only-mvp`. + +## Project registry (#427) + +Versioned registry file: `webui/data/projects.registry.json` (schema version `1`). + +Override path with `WEBUI_PROJECT_REGISTRY` when operators keep a machine-local +copy outside git. The registry stores repo identity, remotes, profile names, +workflow/schema path references, and onboarding checklist steps — never tokens +or credentials. + +Seed entry: **Gitea-Tools** on `https://gitea.prgs.cc` with `prgs-author`, +`prgs-reviewer`, and `prgs-reconciler` profiles. + +## Prompt library (#428) + +Prompts are generated at load time from canonical workflow files under +`skills/llm-project-workflow/workflows/`. SHA-256 hashes are computed from +`WEBUI_REPO_ROOT` (defaults to the repository root). Prompt bodies are short +copy/paste starters; canonical workflow files remain the only full policy +source. + +## Live queue dashboard (#429) + +`/queue` loads open PRs and issues for the default registry project (seed: +**Gitea-Tools** on `https://gitea.prgs.cc`) using existing `gitea_auth` read +credentials. The UI surfaces pagination proof (returned count, pages fetched, +`has_more`, `inventory_complete`) and classification badges (`claimed`, +`blocked`, `in-review`, `duplicate`) when evidence exists. + +If credentials are missing or the fetch fails, the page shows an explicit error +instead of an empty queue (fail closed). + +## Tests + +```bash +pytest tests/test_webui_skeleton.py tests/test_webui_project_registry.py tests/test_webui_prompt_library.py tests/test_webui_queue_dashboard.py -q +``` \ No newline at end of file diff --git a/final_report_validator.py b/final_report_validator.py index d8bd96f..1af98df 100644 --- a/final_report_validator.py +++ b/final_report_validator.py @@ -11,6 +11,7 @@ import inspect import re from typing import Any, Callable +import issue_lock_provenance from review_proofs import ( HANDOFF_HEADING, assess_controller_handoff, @@ -909,6 +910,54 @@ def _rule_reviewer_mutation_ledger( ) +def _rule_shared_issue_lock_external_state(report_text: str) -> list[dict[str, str]]: + result = issue_lock_provenance.assess_issue_lock_external_state_report(report_text) + if result.get("proven"): + return [] + return _findings_from_reasons( + "shared.issue_lock_external_state", + result.get("reasons") or [], + field="External-state mutations", + severity="block", + safe_next_action=( + "disclose gitea_issue_lock.json read/write/delete under " + "External-state mutations; never claim none after lock seeding" + ), + ) + + +def _rule_shared_manual_lock_pr_override(report_text: str) -> list[dict[str, str]]: + result = issue_lock_provenance.assess_manual_lock_pr_without_override(report_text) + if result.get("proven"): + return [] + return _findings_from_reasons( + "shared.manual_lock_pr_override", + result.get("reasons") or [], + field="External-state mutations", + severity="block", + safe_next_action=( + "use gitea_lock_issue or #442 adoption instead of manual lock seeding; " + "if operator override was authorized, cite override proof" + ), + ) + + +def _rule_shared_author_reviewer_same_run(report_text: str) -> list[dict[str, str]]: + result = issue_lock_provenance.assess_author_reviewer_same_run_report(report_text) + if result.get("proven"): + return [] + return _findings_from_reasons( + "shared.author_reviewer_same_run", + result.get("reasons") or [], + field="Review mutations", + severity="block", + safe_next_action=( + "split author PR creation and reviewer approval across separate " + "sessions and handoffs" + ), + ) + + def _rule_reviewer_review_mutation( report_text: str, *, @@ -928,10 +977,17 @@ def _rule_reviewer_review_mutation( ) +_SHARED_ISSUE_LOCK_RULES = ( + _rule_shared_issue_lock_external_state, + _rule_shared_manual_lock_pr_override, + _rule_shared_author_reviewer_same_run, +) + _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = { "review_pr": [ _rule_shared_controller_handoff, _rule_shared_email_disclosure, + *_SHARED_ISSUE_LOCK_RULES, _rule_reviewer_legacy_workspace_mutations, _rule_reviewer_vague_mutations_none, _rule_reviewer_mutation_categories, @@ -953,6 +1009,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = { "reconcile_already_landed": [ _rule_reconcile_controller_handoff, _rule_shared_email_disclosure, + *_SHARED_ISSUE_LOCK_RULES, _rule_reconcile_stale_author_fields, _rule_reconcile_eligible_reviewed, _rule_reconcile_linked_issue_live, @@ -964,26 +1021,31 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = { "author_issue": [ _rule_shared_controller_handoff, _rule_shared_email_disclosure, + *_SHARED_ISSUE_LOCK_RULES, _rule_reviewer_vague_mutations_none, ], "work_issue": [ _rule_shared_controller_handoff, _rule_shared_email_disclosure, + *_SHARED_ISSUE_LOCK_RULES, _rule_reviewer_vague_mutations_none, _rule_conflict_fix_push_proof, ], "issue_filing": [ _rule_shared_controller_handoff, _rule_shared_email_disclosure, + *_SHARED_ISSUE_LOCK_RULES, ], "inventory": [ _rule_shared_controller_handoff, _rule_shared_email_disclosure, + *_SHARED_ISSUE_LOCK_RULES, _rule_reconcile_pagination_proof, ], "issue_selection": [ _rule_shared_controller_handoff, _rule_shared_email_disclosure, + *_SHARED_ISSUE_LOCK_RULES, ], } diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 543e8f6..11f9ecf 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -440,10 +440,48 @@ def verify_preflight_purity(remote: str | None = None, worktree_path: str | None "Pre-flight order violation: Task capability (gitea_resolve_task_capability) has not been resolved (fail closed)" ) - if worktree_path: - dirty_files = sorted(_parse_porcelain_entries(_get_workspace_porcelain(worktree_path))) + 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), + ) + real_workspace = os.path.realpath(workspace) + real_root = os.path.realpath(PROJECT_ROOT) + + if real_workspace != real_root: + if not _preflight_in_test_mode(): + if not os.path.exists(real_workspace): + raise RuntimeError( + f"Branches-only mutation guard (#274): worktree path '{workspace}' does not exist (fail closed)" + ) + if not os.path.isdir(real_workspace): + raise RuntimeError( + f"Branches-only mutation guard (#274): worktree path '{workspace}' is not a directory (fail closed)" + ) + try: + res = subprocess.run( + ["git", "-C", real_workspace, "rev-parse", "--git-common-dir"], + capture_output=True, + text=True, + check=True, + ) + common_dir = os.path.realpath(res.stdout.strip()) + expected_dir = os.path.realpath(os.path.join(real_root, ".git")) + if common_dir != expected_dir: + raise RuntimeError( + f"Branches-only mutation guard (#274): worktree '{workspace}' does not belong to the target repository '{PROJECT_ROOT}' (fail closed)" + ) + except Exception as e: + if isinstance(e, RuntimeError): + raise e + raise RuntimeError( + f"Branches-only mutation guard (#274): worktree '{workspace}' is not a valid git repository (fail closed)" + ) + + dirty_files = sorted(_parse_porcelain_entries(_get_workspace_porcelain(workspace))) if dirty_files: - details = _preflight_workspace_details(worktree_path, dirty_files) + details = _preflight_workspace_details(workspace, dirty_files) raise RuntimeError( "Pre-flight order violation: Active task workspace has tracked " "file edits before mutation (fail closed). " @@ -500,9 +538,12 @@ import task_capability_map # noqa: E402 import review_proofs # noqa: E402 import agent_temp_artifacts import issue_lock_worktree # noqa: E402 +import issue_lock_provenance # noqa: E402 import already_landed_reconcile # noqa: E402 import author_mutation_worktree # noqa: E402 import issue_claim_heartbeat # noqa: E402 +import issue_work_duplicate_gate # noqa: E402 +import reviewer_pr_lease # noqa: E402 import merged_cleanup_reconcile # noqa: E402 import reconciler_profile # noqa: E402 import reconciliation_workflow # noqa: E402 @@ -548,7 +589,7 @@ 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: + with open(ISSUE_LOCK_FILE, "r", encoding="utf-8") as f: data = json.load(f) return data if isinstance(data, dict) else None except Exception: @@ -642,6 +683,119 @@ def _branch_entry_name(branch: dict | str) -> str: return str(branch.get("name") or branch.get("ref") or "") +def _live_fetch_issue_duplicate_context( + h: str, + o: str, + r: str, + auth: str, + issue_number: int, +) -> tuple[list[dict], list[str], dict]: + """Live open PRs, remote branch names, and claim state for one issue.""" + base = repo_api_url(h, o, r) + open_prs = api_get_all(f"{base}/pulls?state=open", auth) + branches = api_get_all(f"{base}/branches", auth) + branch_names = [_branch_entry_name(b) for b in branches] + issue = api_request("GET", f"{base}/issues/{issue_number}", auth) or {} + comments = api_request( + "GET", f"{base}/issues/{issue_number}/comments", auth + ) or [] + claim_entry = issue_claim_heartbeat.classify_issue_claim( + issue=issue, + comments=comments, + open_prs=open_prs, + branch_names=branch_names, + ) + return open_prs, branch_names, claim_entry + + +# Injectable duplicate-work context fetcher (#400). Production uses the live +# Gitea API path above; unit tests patch this symbol instead of hitting the +# network. +issue_duplicate_context_fetcher = _live_fetch_issue_duplicate_context + + +def _collect_issue_duplicate_context( + h: str, + o: str, + r: str, + auth: str, + issue_number: int, +) -> tuple[list[dict], list[str], dict]: + return issue_duplicate_context_fetcher(h, o, r, auth, issue_number) + + +def _assess_issue_duplicate_gate( + issue_number: int, + *, + h: str, + o: str, + r: str, + auth: str, + locked_branch: str | None = None, + phase: str, +) -> dict: + open_prs, branch_names, claim_entry = _collect_issue_duplicate_context( + h, o, r, auth, issue_number + ) + return issue_work_duplicate_gate.assess_work_issue_duplicate_gate( + issue_number, + open_prs=open_prs, + branch_names=branch_names, + claim_entry=claim_entry, + locked_branch=locked_branch, + phase=phase, + ) + + +def _duplicate_gate_block_response(gate: dict, **extra) -> dict: + out = { + "success": False, + "performed": False, + "reasons": list(gate.get("reasons") or []), + "duplicate_gate": gate, + "safe_next_action": gate.get("safe_next_action"), + } + out.update(extra) + return out + + +def _enforce_locked_issue_duplicate_recheck( + remote: str, + phase: str, + *, + host: str | None = None, + org: str | None = None, + repo: str | None = None, +) -> dict | None: + """Re-check duplicate-work gates for the locked issue (#400).""" + lock_data = _load_existing_issue_lock() + if not lock_data: + return None + issue_number = int(lock_data.get("issue_number") or 0) + locked_branch = lock_data.get("branch_name") + if not issue_number: + return None + h, o, r = _resolve( + remote or lock_data.get("remote") or "dadeschools", + host or lock_data.get("host"), + org or lock_data.get("org"), + repo or lock_data.get("repo"), + ) + auth = _auth(h) + gate = _assess_issue_duplicate_gate( + issue_number, + h=h, + o=o, + r=r, + auth=auth, + locked_branch=locked_branch, + phase=phase, + ) + if gate.get("block"): + return gate + return None + + def _reveal_endpoints() -> bool: """Admin/debug opt-in (#120): include endpoint URLs and token source names in tool output. Off by default so normal LLM-facing responses @@ -973,6 +1127,7 @@ def gitea_create_issue( repo: str | None = None, allow_duplicate_override: bool = False, split_from_issue: int | None = None, + worktree_path: str | None = None, ) -> dict: """Create a new issue on a Gitea repository. @@ -985,6 +1140,7 @@ def gitea_create_issue( repo: Override the repository name. allow_duplicate_override: Operator-approved split after duplicate found. split_from_issue: Existing duplicate issue number when overriding. + worktree_path: Optional path to verify branches-only guard. Returns: dict with 'number' of the created issue ('url' only with the reveal opt-in). @@ -1011,7 +1167,7 @@ def gitea_create_issue( ) if blocked: return blocked - verify_preflight_purity(remote) + verify_preflight_purity(remote, worktree_path=worktree_path) base = repo_api_url(h, o, r) open_issues = api_get_all(f"{base}/issues?state=open&type=issues", auth) closed_issues = api_get_all( @@ -1112,48 +1268,21 @@ def gitea_lock_issue( issue_lock_worktree.format_issue_lock_worktree_error(lock_assessment) ) - # 2. Check if the issue already has an open PR (reuse protection) h, o, r = _resolve(remote, host, org, repo) auth = _auth(h) - url = f"{repo_api_url(h, o, r)}/pulls?state=open" - - try: - prs = api_get_all(url, auth) - except Exception as e: - raise RuntimeError(f"Could not list open PRs to verify issue lock: {e}") - - for pr in prs: - pr_head = pr.get("head", {}).get("ref", "") - pr_title = pr.get("title", "") - pr_body = pr.get("body", "") - - if expected_pattern in pr_head: - raise ValueError( - f"Issue #{issue_number} is already tied to an open PR (PR #{pr.get('number')}, branch '{pr_head}') (fail closed)" - ) - - patterns = [ - f"closes #{issue_number}", - f"fixes #{issue_number}", - ] - text_to_check = f"{pr_title} {pr_body}".lower() - if any(p in text_to_check for p in patterns): - raise ValueError( - f"Issue #{issue_number} is already tied to an open PR (PR #{pr.get('number')}) via Closes/Fixes reference (fail closed)" - ) - - 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)" - ) + duplicate_gate = _assess_issue_duplicate_gate( + issue_number, + h=h, + o=o, + r=r, + auth=auth, + locked_branch=branch_name, + phase=issue_work_duplicate_gate.PHASE_LOCK, + ) + if duplicate_gate.get("block"): + raise ValueError("; ".join(duplicate_gate.get("reasons") or [ + f"duplicate work gate blocked issue #{issue_number} (fail closed)" + ])) work_lease = _build_author_issue_work_lease( issue_number=issue_number, @@ -1169,6 +1298,10 @@ def gitea_lock_issue( "repo": r, "worktree_path": resolved_worktree, "work_lease": work_lease, + "lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance( + tool="gitea_lock_issue", + claimant=work_lease.get("claimant"), + ), } try: @@ -1199,6 +1332,39 @@ def gitea_lock_issue( return result +@mcp.tool() +def gitea_assess_work_issue_duplicate( + issue_number: int, + branch_name: str | None = None, + phase: str = issue_work_duplicate_gate.PHASE_LOCK, + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, +) -> dict: + """Read-only duplicate-work gate for author sessions before mutations (#400).""" + read_block = _profile_operation_gate("gitea.read") + if read_block: + return { + "success": False, + "performed": False, + "reasons": read_block, + "permission_report": _permission_block_report("gitea.read"), + } + h, o, r = _resolve(remote, host, org, repo) + auth = _auth(h) + gate = _assess_issue_duplicate_gate( + issue_number, + h=h, + o=o, + r=r, + auth=auth, + locked_branch=branch_name, + phase=phase, + ) + return {"success": not gate.get("block"), **gate} + + @mcp.tool() def gitea_create_pr( title: str, @@ -1259,6 +1425,14 @@ def gitea_create_pr( except Exception as e: raise RuntimeError(f"Could not read issue lock file: {e} (fail closed)") + lock_provenance_check = issue_lock_provenance.assess_lock_file_for_create_pr( + lock_data + ) + if lock_provenance_check["block"]: + raise RuntimeError( + issue_lock_provenance.format_lock_provenance_error(lock_provenance_check) + ) + locked_issue = lock_data.get("issue_number") locked_branch = lock_data.get("branch_name") locked_worktree = lock_data.get("worktree_path") @@ -1290,6 +1464,21 @@ def gitea_create_pr( f"PR title or body must contain 'Closes #{locked_issue}' or 'Fixes #{locked_issue}' exactly to ensure durable tracking (fail closed)" ) + duplicate_block = _enforce_locked_issue_duplicate_recheck( + remote, + issue_work_duplicate_gate.PHASE_CREATE_PR, + host=host, + org=org, + repo=repo, + ) + if duplicate_block: + return _duplicate_gate_block_response( + duplicate_block, + number=None, + issue_number=locked_issue, + branch_name=locked_branch, + ) + auth = _auth(h) url = f"{repo_api_url(h, o, r)}/pulls" payload = {"title": title, "body": body, "head": head, "base": base} @@ -1829,6 +2018,7 @@ def init_review_decision_lock(remote: str | None, task: str | None): (os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip() or profile_name ) + reviewer_pr_lease.clear_session_lease() _save_review_decision_lock({ "task": task, "remote": remote, @@ -2315,6 +2505,20 @@ def _evaluate_pr_review_submission( result["permission_report"] = elig["permission_report"] return result + if live: + reasons.extend(_reviewer_pr_lease_gate( + pr_number=pr_number, + remote=remote, + host=host, + org=org, + repo=repo, + mutation=action, + live_head_sha=result.get("head_sha"), + pinned_head_sha=expected_head_sha, + )) + if reasons: + return result + auth_user = result["authenticated_user"] pr_author = result["pr_author"] if action == "approve" and auth_user and pr_author and auth_user == pr_author: @@ -2991,6 +3195,20 @@ def gitea_commit_files( if blocked: return blocked + duplicate_block = _enforce_locked_issue_duplicate_recheck( + remote, + issue_work_duplicate_gate.PHASE_COMMIT, + host=host, + org=org, + repo=repo, + ) + if duplicate_block: + return _duplicate_gate_block_response( + duplicate_block, + commit="", + branch="", + ) + verify_preflight_purity(remote) processed_files, source_proofs = _prepare_commit_payload_files(files) @@ -3153,6 +3371,19 @@ def gitea_merge_pr( result["permission_report"] = elig["permission_report"] return result + reasons.extend(_reviewer_pr_lease_gate( + pr_number=pr_number, + remote=remote, + host=host, + org=org, + repo=repo, + mutation="merge", + live_head_sha=result.get("head_sha"), + pinned_head_sha=expected_head_sha, + )) + if reasons: + return result + # Gate 4 — reviewed head SHA is mandatory and must match live PR head (#399). actual_sha = result["head_sha"] if not (expected_head_sha or "").strip(): @@ -4471,6 +4702,261 @@ def _namespace_mutation_block(mutation_task: str, **extra_fields) -> dict | None return blocked +def _fetch_pr_comments( + pr_number: int, + *, + remote: str, + host: str | None, + org: str | None, + repo: str | None, +) -> list[dict]: + h, o, r = _resolve(remote, host, org, repo) + auth = _auth(h) + api = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments" + return api_request("GET", api, auth) or [] + + +def _reviewer_pr_lease_gate( + *, + pr_number: int, + remote: str, + host: str | None, + org: str | None, + repo: str | None, + mutation: str, + live_head_sha: str | None, + pinned_head_sha: str | None, +) -> list[str]: + """Return block reasons when the session lacks an owned PR reviewer lease.""" + session = reviewer_pr_lease.get_session_lease() + session_id = (session or {}).get("session_id") + identity = _authenticated_username(remote) or "" + try: + comments = _fetch_pr_comments( + pr_number, remote=remote, host=host, org=org, repo=repo) + except Exception as exc: + return [f"cannot fetch PR comments for lease gate: {_redact(str(exc))}"] + assessment = reviewer_pr_lease.assess_mutation_lease_gate( + pr_number=pr_number, + comments=comments, + reviewer_identity=identity, + session_id=session_id, + mutation=mutation, + live_head_sha=live_head_sha, + pinned_head_sha=pinned_head_sha, + ) + return list(assessment.get("reasons") or []) if assessment.get("block") else [] + + +@mcp.tool() +def gitea_acquire_reviewer_pr_lease( + pr_number: int, + worktree: str, + candidate_head: str | None = None, + target_branch: str = "master", + target_branch_sha: str | None = None, + issue_number: int | None = None, + session_id: str | None = None, + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, +) -> dict: + """Acquire a per-PR reviewer lease before review/merge mutations (#407).""" + read_block = _profile_operation_gate("gitea.read") + if read_block: + return { + "success": False, + "acquired": False, + "reasons": read_block, + "permission_report": _permission_block_report("gitea.read"), + } + comment_block = _profile_operation_gate("gitea.pr.comment") + if comment_block: + return { + "success": False, + "acquired": False, + "reasons": comment_block, + "permission_report": _permission_block_report("gitea.pr.comment"), + } + + verify_preflight_purity(remote) + h, o, r = _resolve(remote, host, org, repo) + auth = _auth(h) + profile = get_profile() + identity = _authenticated_username(remote) or profile.get("username") or "" + sid = (session_id or "").strip() or reviewer_pr_lease.new_session_id() + repo_label = f"{o}/{r}" + + comments = _fetch_pr_comments( + pr_number, remote=remote, host=host, org=org, repo=repo) + assessment = reviewer_pr_lease.assess_acquire_lease( + comments, + pr_number=pr_number, + reviewer_identity=identity, + profile=profile.get("profile_name") or "unknown", + session_id=sid, + repo=repo_label, + issue_number=issue_number, + worktree=worktree, + candidate_head=candidate_head, + target_branch=target_branch, + target_branch_sha=target_branch_sha, + ) + if not assessment.get("acquire_allowed"): + return { + "success": False, + "acquired": False, + "reasons": assessment.get("reasons") or [], + "existing_lease": assessment.get("existing_lease"), + } + + body = assessment["lease_body"] + comment_url = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments" + with _audited( + "comment_pr", + host=h, + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + request_metadata={"source": "acquire_reviewer_pr_lease"}, + ): + posted = api_request("POST", comment_url, auth, {"body": body}) + + session_lease = reviewer_pr_lease.record_session_lease({ + "pr_number": pr_number, + "issue_number": issue_number, + "session_id": sid, + "reviewer_identity": identity, + "profile": profile.get("profile_name"), + "worktree": worktree, + "phase": "claimed", + "candidate_head": candidate_head, + "target_branch": target_branch, + "target_branch_sha": target_branch_sha, + "repo": repo_label, + "comment_id": posted.get("id"), + }) + return { + "success": True, + "acquired": True, + "pr_number": pr_number, + "session_id": sid, + "comment_id": posted.get("id"), + "session_lease": session_lease, + "reasons": [], + } + + +@mcp.tool() +def gitea_heartbeat_reviewer_pr_lease( + pr_number: int, + phase: str, + worktree: str | None = None, + candidate_head: str | None = None, + target_branch_sha: str | None = None, + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, +) -> dict: + """Post a reviewer lease heartbeat / phase update on the PR thread (#407).""" + comment_block = _profile_operation_gate("gitea.pr.comment") + if comment_block: + return { + "success": False, + "posted": False, + "reasons": comment_block, + "permission_report": _permission_block_report("gitea.pr.comment"), + } + session = reviewer_pr_lease.get_session_lease() + if not session or session.get("pr_number") != pr_number: + return { + "success": False, + "posted": False, + "reasons": [ + f"no in-session lease for PR #{pr_number}; acquire first " + "(fail closed)" + ], + } + + verify_preflight_purity(remote) + h, o, r = _resolve(remote, host, org, repo) + auth = _auth(h) + body = reviewer_pr_lease.format_lease_body( + repo=f"{o}/{r}", + pr_number=pr_number, + issue_number=session.get("issue_number"), + reviewer_identity=session.get("reviewer_identity") or "", + profile=session.get("profile") or "unknown", + session_id=session.get("session_id") or reviewer_pr_lease.new_session_id(), + worktree=worktree or session.get("worktree") or "", + phase=phase, + candidate_head=candidate_head or session.get("candidate_head"), + target_branch=session.get("target_branch") or "master", + target_branch_sha=target_branch_sha or session.get("target_branch_sha"), + ) + comment_url = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments" + with _audited( + "comment_pr", + host=h, + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + request_metadata={"source": "heartbeat_reviewer_pr_lease", "phase": phase}, + ): + posted = api_request("POST", comment_url, auth, {"body": body}) + + updated = reviewer_pr_lease.record_session_lease({ + **session, + "phase": phase, + "worktree": worktree or session.get("worktree"), + "candidate_head": candidate_head or session.get("candidate_head"), + "target_branch_sha": target_branch_sha or session.get("target_branch_sha"), + "last_comment_id": posted.get("id"), + }) + return { + "success": True, + "posted": True, + "pr_number": pr_number, + "phase": phase, + "comment_id": posted.get("id"), + "session_lease": updated, + "reasons": [], + } + + +@mcp.tool() +def gitea_assess_reviewer_pr_lease( + pr_number: int, + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, +) -> dict: + """Read-only: assess active reviewer lease state for a PR (#407).""" + read_block = _profile_operation_gate("gitea.read") + if read_block: + return { + "success": False, + "reasons": read_block, + "permission_report": _permission_block_report("gitea.read"), + } + comments = _fetch_pr_comments( + pr_number, remote=remote, host=host, org=org, repo=repo) + active = reviewer_pr_lease.find_active_reviewer_lease( + comments, pr_number=pr_number) + return { + "success": True, + "pr_number": pr_number, + "active_lease": active, + "session_lease": reviewer_pr_lease.get_session_lease(), + "reasons": [], + } + + @mcp.tool() def gitea_list_issue_comments( issue_number: int, diff --git a/issue_lock_provenance.py b/issue_lock_provenance.py new file mode 100644 index 0000000..87ee38f --- /dev/null +++ b/issue_lock_provenance.py @@ -0,0 +1,269 @@ +"""Issue-lock provenance and external-state disclosure (#447). + +Sanctioned locks are written only by ``gitea_lock_issue`` (or adoption recovery +#442). Manual seeding of ``/tmp/gitea_issue_lock.json`` is unsafe and must be +blocked at PR creation unless explicit operator override proof is recorded. +""" + +from __future__ import annotations + +import os +import re +from datetime import datetime, timezone + +ISSUE_LOCK_FILE = os.environ.get("GITEA_ISSUE_LOCK_FILE", "/tmp/gitea_issue_lock.json") + +SOURCE_LOCK_ISSUE = "gitea_lock_issue" +SOURCE_LOCK_ADOPTION = "gitea_lock_issue_adoption" +SOURCE_OPERATOR_OVERRIDE = "operator_override" + +SANCTIONED_LOCK_SOURCES = frozenset({ + SOURCE_LOCK_ISSUE, + SOURCE_LOCK_ADOPTION, + SOURCE_OPERATOR_OVERRIDE, +}) + +_OPERATOR_OVERRIDE_ENV = "GITEA_ISSUE_LOCK_OPERATOR_OVERRIDE" + +_ISSUE_LOCK_PATH_RE = re.compile( + r"(?:/tmp/)?gitea_issue_lock\.json", + re.IGNORECASE, +) +_LOCK_SEED_RE = re.compile( + r"(?:seed(?:ed|ing)?|restor(?:e|ed|ing)|wrote|written|write|programmatically|" + r"hand[- ]forg|manual(?:ly)?).{0,80}gitea_issue_lock", + re.IGNORECASE | re.DOTALL, +) +_LOCK_REMOVE_RE = re.compile( + r"(?:\brm\b|remove|deleted?|unlink).{0,80}gitea_issue_lock", + re.IGNORECASE | re.DOTALL, +) +_LOCK_READ_RE = re.compile( + r"(?:read|loaded?|parsed?).{0,80}gitea_issue_lock", + re.IGNORECASE | re.DOTALL, +) +_EXTERNAL_NONE_RE = re.compile( + r"external[- ]state mutations\s*:\s*none\b", + re.IGNORECASE, +) +_EXTERNAL_FIELD_RE = re.compile( + r"external[- ]state mutations\s*:\s*(.+)$", + re.IGNORECASE | re.MULTILINE, +) +_CLEANUP_ONLY_RE = re.compile( + r"cleanup mutations\s*:\s*(?:none|lock removed|removed issue lock)", + re.IGNORECASE, +) +_PR_CREATED_RE = re.compile( + r"(?:\bgitea_create_pr\b|PR\s*#\s*\d+\s+created|created\s+PR\s*#|opened\s+PR\s*#|" + r"PR\s+creation\s+(?:succeeded|complete))", + re.IGNORECASE, +) +_REVIEW_APPROVE_RE = re.compile( + r"(?:submitted\s+(?:['\"]approve['\"]|approve\s+review)|" + r"review decision\s*:\s*approve|approved\s+PR\s*#|gitea_review_pr.*approve)", + re.IGNORECASE, +) +_OVERRIDE_PROOF_RE = re.compile( + r"operator[- ]override\s+proof\s*:\s*(.+)$", + re.IGNORECASE | re.MULTILINE, +) + + +def _utc_now_iso() -> str: + return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + + +def build_sanctioned_lock_provenance( + *, + tool: str, + source: str = SOURCE_LOCK_ISSUE, + claimant: dict | None = None, + adoption: dict | None = None, +) -> dict: + """Return provenance metadata stored with a sanctioned lock write.""" + entry = { + "source": source, + "written_at": _utc_now_iso(), + "written_by_tool": tool, + "lock_file_path": ISSUE_LOCK_FILE, + } + if claimant: + entry["claimant"] = claimant + if adoption: + entry["adoption"] = adoption + return entry + + +def operator_override_requested() -> bool: + return os.environ.get(_OPERATOR_OVERRIDE_ENV, "").strip().lower() in { + "1", + "true", + "yes", + } + + +def build_operator_override_provenance(*, reason: str, claimant: dict | None = None) -> dict: + text = (reason or "").strip() + if not text: + raise ValueError( + "operator override requires a non-empty override reason (fail closed)" + ) + entry = build_sanctioned_lock_provenance( + tool="operator_override", + source=SOURCE_OPERATOR_OVERRIDE, + claimant=claimant, + ) + entry["override_reason"] = text + return entry + + +def assess_lock_file_for_create_pr(lock_data: dict | None) -> dict: + """Fail closed when lock file lacks sanctioned provenance (#447).""" + data = lock_data if isinstance(lock_data, dict) else {} + reasons: list[str] = [] + provenance = data.get("lock_provenance") + if not isinstance(provenance, dict): + reasons.append( + "issue lock file lacks sanctioned lock_provenance; manual seeding is " + "not a normal recovery path — call gitea_lock_issue or use #442 adoption" + ) + return _provenance_result(False, reasons, provenance) + + source = str(provenance.get("source") or "").strip() + if source not in SANCTIONED_LOCK_SOURCES: + reasons.append( + f"issue lock provenance source '{source or '(missing)'}' is not sanctioned" + ) + + if source == SOURCE_OPERATOR_OVERRIDE and not str( + provenance.get("override_reason") or "" + ).strip(): + reasons.append( + "operator_override lock provenance requires override_reason proof" + ) + + if not data.get("work_lease"): + reasons.append("issue lock file missing work_lease metadata") + + if not str(provenance.get("written_by_tool") or "").strip(): + reasons.append("issue lock provenance missing written_by_tool") + + proven = not reasons + return _provenance_result(proven, reasons, provenance) + + +def _provenance_result(proven: bool, reasons: list[str], provenance: dict | None) -> dict: + return { + "proven": proven, + "block": not proven, + "reasons": reasons, + "lock_provenance": provenance, + } + + +def format_lock_provenance_error(assessment: dict) -> str: + reasons = "; ".join(assessment.get("reasons") or ["unknown lock provenance violation"]) + return f"Issue lock provenance guard (#447): {reasons} (fail closed)" + + +def _lock_activity_detected(text: str) -> dict[str, bool]: + body = text or "" + return { + "seed_or_restore": bool(_LOCK_SEED_RE.search(body)), + "remove": bool(_LOCK_REMOVE_RE.search(body)), + "read": bool(_LOCK_READ_RE.search(body)), + } + + +def _external_state_discloses_lock(text: str) -> bool: + match = _EXTERNAL_FIELD_RE.search(text or "") + if not match: + return False + value = (match.group(1) or "").strip().lower() + if value in {"", "none", "n/a"}: + return False + return "lock" in value or "gitea_issue_lock" in value or "issue-lock" in value + + +def assess_issue_lock_external_state_report(report_text: str) -> dict: + """Require explicit external-state disclosure for issue-lock mutations (#447).""" + text = report_text or "" + activity = _lock_activity_detected(text) + if not any(activity.values()): + return {"proven": True, "block": False, "reasons": [], "activity": activity} + + reasons: list[str] = [] + disclosed = _external_state_discloses_lock(text) + + if activity["seed_or_restore"] and _EXTERNAL_NONE_RE.search(text): + reasons.append( + "report mentions seeding/restoring gitea_issue_lock.json but claims " + "External-state mutations: none" + ) + elif activity["seed_or_restore"] and not disclosed: + reasons.append( + "report mentions issue-lock file activity but External-state mutations " + "does not disclose read/write of gitea_issue_lock.json" + ) + + if activity["remove"]: + if _EXTERNAL_NONE_RE.search(text): + reasons.append( + "report mentions removing gitea_issue_lock.json but claims " + "External-state mutations: none" + ) + elif not disclosed and _CLEANUP_ONLY_RE.search(text): + reasons.append( + "report removes issue lock but classifies it as cleanup only; " + "record under External-state mutations" + ) + elif not disclosed: + reasons.append( + "report mentions deleting issue lock without External-state " + "mutation disclosure" + ) + + proven = not reasons + return { + "proven": proven, + "block": not proven, + "reasons": reasons, + "activity": activity, + } + + +def assess_manual_lock_pr_without_override(report_text: str) -> dict: + """Block reports that created a PR via manual lock seed without override proof.""" + text = report_text or "" + seeded = bool(_LOCK_SEED_RE.search(text)) + created = bool(_PR_CREATED_RE.search(text)) + if not (seeded and created): + return {"proven": True, "block": False, "reasons": []} + + if _OVERRIDE_PROOF_RE.search(text): + return {"proven": True, "block": False, "reasons": []} + + return { + "proven": False, + "block": True, + "reasons": [ + "report created/opened a PR after manual issue-lock seeding without " + "operator override proof" + ], + } + + +def assess_author_reviewer_same_run_report(report_text: str) -> dict: + """Reviewer handoff must not create and approve the same PR in one run (#447).""" + text = report_text or "" + if not (_PR_CREATED_RE.search(text) and _REVIEW_APPROVE_RE.search(text)): + return {"proven": True, "block": False, "reasons": []} + return { + "proven": False, + "block": True, + "reasons": [ + "report mixes author-side PR creation and reviewer approval in one " + "final handoff; split author and reviewer sessions" + ], + } \ No newline at end of file diff --git a/issue_work_duplicate_gate.py b/issue_work_duplicate_gate.py new file mode 100644 index 0000000..fc70437 --- /dev/null +++ b/issue_work_duplicate_gate.py @@ -0,0 +1,180 @@ +"""Early duplicate-work detection for author work-issue sessions (#400).""" + +from __future__ import annotations + +from typing import Any + +import issue_claim_heartbeat as claim_hb + +PHASE_LOCK = "lock_issue" +PHASE_COMMIT = "commit" +PHASE_PUSH = "push" +PHASE_CREATE_PR = "create_pr" + +OUTCOME_DUPLICATE_PR_PREVENTED = "duplicate_pr_prevented" +OUTCOME_DUPLICATE_BRANCH_PREVENTED = "duplicate_branch_prevented" +OUTCOME_DUPLICATE_COMMIT_PREVENTED = "duplicate_commit_prevented" +OUTCOME_DUPLICATE_WORK_NOT_PREVENTED = "duplicate_work_not_prevented" + +_ACTIVE_CLAIM_STATUSES = frozenset({"active", "awaiting_review"}) + + +def _issue_pattern(issue_number: int) -> str: + return f"issue-{int(issue_number)}" + + +def _linked_open_pr(issue_number: int, open_prs: list[dict]) -> dict | None: + return claim_hb._linked_open_pr(issue_number, open_prs) + + +def _matching_branches( + issue_number: int, + branch_names: list[str], + *, + locked_branch: str | None = None, +) -> list[str]: + pattern = _issue_pattern(issue_number) + matches = [ + name for name in (branch_names or []) + if pattern in (name or "").lower() + ] + if locked_branch: + locked = locked_branch.strip() + matches = [name for name in matches if name != locked] + return matches + + +def assess_work_issue_duplicate_gate( + issue_number: int, + *, + open_prs: list[dict] | None = None, + branch_names: list[str] | None = None, + claim_entry: dict | None = None, + locked_branch: str | None = None, + phase: str = PHASE_LOCK, +) -> dict[str, Any]: + """Fail closed when duplicate work is already in flight for an issue.""" + reasons: list[str] = [] + outcome = OUTCOME_DUPLICATE_WORK_NOT_PREVENTED + prs = list(open_prs or []) + branches = list(branch_names or []) + pattern = _issue_pattern(issue_number) + + linked = _linked_open_pr(issue_number, prs) + if linked: + reasons.append( + f"open PR #{linked.get('number')} already covers issue " + f"#{issue_number} (fail closed)" + ) + outcome = OUTCOME_DUPLICATE_PR_PREVENTED + + conflicting_branches = _matching_branches( + issue_number, branches, locked_branch=locked_branch + ) + if conflicting_branches: + names = ", ".join(conflicting_branches[:5]) + reasons.append( + f"remote branch(es) already match issue pattern '{pattern}': " + f"{names} (fail closed)" + ) + if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED: + outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED + + entry = claim_entry or {} + if entry.get("linked_open_pr") and not linked: + reasons.append( + f"claim inventory reports open PR #{entry['linked_open_pr']} " + f"for issue #{issue_number} (fail closed)" + ) + outcome = OUTCOME_DUPLICATE_PR_PREVENTED + + status = (entry.get("status") or "").strip().lower() + if status in _ACTIVE_CLAIM_STATUSES and not linked: + heartbeat = entry.get("latest_heartbeat") or {} + claim_branch = (heartbeat.get("branch") or "").strip() + if locked_branch and claim_branch and claim_branch != locked_branch: + reasons.append( + f"active claim lease on branch '{claim_branch}' blocks " + f"work on '{locked_branch}' for issue #{issue_number} " + "(fail closed)" + ) + if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED: + outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED + elif not locked_branch and status == "active": + reasons.append( + f"issue #{issue_number} has an active claim lease " + "(fail closed)" + ) + if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED: + outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED + + if phase in {PHASE_COMMIT, PHASE_PUSH} and reasons: + if outcome == OUTCOME_DUPLICATE_PR_PREVENTED: + outcome = OUTCOME_DUPLICATE_COMMIT_PREVENTED + elif outcome == OUTCOME_DUPLICATE_BRANCH_PREVENTED: + outcome = OUTCOME_DUPLICATE_COMMIT_PREVENTED + + block = bool(reasons) + return { + "block": block, + "performed": not block, + "issue_number": issue_number, + "phase": phase, + "outcome": outcome, + "linked_open_pr": linked.get("number") if linked else entry.get("linked_open_pr"), + "conflicting_branches": conflicting_branches, + "claim_status": status or None, + "reasons": reasons, + "safe_next_action": ( + "stop before mutating; preserve local work and produce a " + "reconciliation handoff if a concurrent PR appeared after push" + if block and phase == PHASE_CREATE_PR + else "stop before mutating; do not commit or push duplicate work" + if block + else "proceed" + ), + } + + +def assess_work_issue_duplicate_report(report_text: str) -> dict[str, Any]: + """Require explicit duplicate-work outcome wording in work-issue reports.""" + text = (report_text or "").lower() + markers = { + OUTCOME_DUPLICATE_PR_PREVENTED: ( + "duplicate pr prevented", + "duplicate_pr_prevented", + ), + OUTCOME_DUPLICATE_BRANCH_PREVENTED: ( + "duplicate branch prevented", + "duplicate_branch_prevented", + ), + OUTCOME_DUPLICATE_COMMIT_PREVENTED: ( + "duplicate commit prevented", + "duplicate_commit_prevented", + ), + OUTCOME_DUPLICATE_WORK_NOT_PREVENTED: ( + "duplicate work not prevented", + "duplicate_work_not_prevented", + "no duplicate work", + ), + } + matched = [ + key for key, phrases in markers.items() + if any(phrase in text for phrase in phrases) + ] + if len(matched) != 1: + return { + "complete": False, + "downgraded": True, + "reasons": [ + "work-issue report must state exactly one duplicate-work " + "outcome (duplicate PR/branch/commit prevented, or " + "duplicate work not prevented)" + ], + } + return { + "complete": True, + "downgraded": False, + "outcome": matched[0], + "reasons": [], + } \ No newline at end of file diff --git a/review_proofs.py b/review_proofs.py index 20cf89b..b74ebb8 100644 --- a/review_proofs.py +++ b/review_proofs.py @@ -3624,9 +3624,12 @@ def assess_work_issue_mode_isolation(report_text: str) -> dict: def assess_work_issue_final_report(report_text: str) -> dict: """#139: composite verifier for work-issue final reports.""" + from issue_work_duplicate_gate import assess_work_issue_duplicate_report + checks = { "workflow_source": assess_work_issue_workflow_source(report_text), "mode_isolation": assess_work_issue_mode_isolation(report_text), + "duplicate_work_outcome": assess_work_issue_duplicate_report(report_text), } reasons = [] diff --git a/reviewer_pr_lease.py b/reviewer_pr_lease.py new file mode 100644 index 0000000..6e28be5 --- /dev/null +++ b/reviewer_pr_lease.py @@ -0,0 +1,382 @@ +"""Per-PR reviewer leases for safe parallel review sessions (#407).""" + +from __future__ import annotations + +import os +import re +import uuid +from datetime import datetime, timedelta, timezone +from typing import Any + +MARKER = "" + +_FIELD_RE = re.compile( + r"^\s*([a-z_]+)\s*:\s*(.+?)\s*$", + re.IGNORECASE | re.MULTILINE, +) +_FULL_SHA = re.compile(r"^[0-9a-f]{40}$", re.IGNORECASE) + +_TERMINAL_PHASES = frozenset({"done", "released", "blocked"}) +_ACTIVE_PHASES = frozenset({ + "claimed", + "validating", + "approved", + "request-changes", + "merging", +}) + +DEFAULT_LEASE_TTL_MINUTES = 120 +STALE_WARNING_MINUTES = 30 +RECLAIMABLE_MINUTES = 60 + +_SESSION_LEASE: dict[str, Any] | None = None + + +def _parse_timestamp(value: str | None) -> datetime | None: + if not value: + return None + text = value.strip() + if text.endswith("Z"): + text = text[:-1] + "+00:00" + try: + parsed = datetime.fromisoformat(text) + except ValueError: + return None + if parsed.tzinfo is None: + return parsed.replace(tzinfo=timezone.utc) + return parsed.astimezone(timezone.utc) + + +def _normalize_sha(value: str | None) -> str | None: + text = (value or "").strip().lower() + return text if text and _FULL_SHA.match(text) else None + + +def _parse_pr_ref(value: str | None) -> int | None: + digits = re.sub(r"[^\d]", "", value or "") + return int(digits) if digits.isdigit() else None + + +def new_session_id() -> str: + return f"{os.getpid()}-{uuid.uuid4().hex[:12]}" + + +def format_lease_body( + *, + repo: str, + pr_number: int, + issue_number: int | None, + reviewer_identity: str, + profile: str, + session_id: str, + worktree: str, + phase: str, + candidate_head: str | None, + target_branch: str, + target_branch_sha: str | None, + last_activity: datetime | None = None, + expires_at: datetime | None = None, + blocker: str = "none", +) -> str: + now = last_activity or datetime.now(timezone.utc) + expires = expires_at or (now + timedelta(minutes=DEFAULT_LEASE_TTL_MINUTES)) + last_text = now.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace( + "+00:00", "Z" + ) + expires_text = expires.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace( + "+00:00", "Z" + ) + issue_text = f"#{issue_number}" if issue_number else "none" + lines = [ + MARKER, + f"repo: {repo}", + f"pr: #{pr_number}", + f"issue: {issue_text}", + f"reviewer_identity: {reviewer_identity}", + f"profile: {profile}", + f"session_id: {session_id}", + f"worktree: {worktree}", + f"phase: {phase}", + f"candidate_head: {candidate_head or 'none'}", + f"target_branch: {target_branch}", + f"target_branch_sha: {target_branch_sha or 'none'}", + f"last_activity: {last_text}", + f"expires_at: {expires_text}", + f"blocker: {blocker}", + ] + return "\n".join(lines) + + +def parse_lease_comment(body: str) -> dict[str, Any] | None: + text = body or "" + if MARKER not in text: + return None + fields: dict[str, str] = {} + for match in _FIELD_RE.finditer(text): + fields[match.group(1).strip().lower()] = match.group(2).strip() + if not fields: + return None + return { + "repo": fields.get("repo"), + "pr_number": _parse_pr_ref(fields.get("pr")), + "issue_number": _parse_pr_ref(fields.get("issue")), + "reviewer_identity": fields.get("reviewer_identity"), + "profile": fields.get("profile"), + "session_id": fields.get("session_id"), + "worktree": fields.get("worktree"), + "phase": (fields.get("phase") or "").strip().lower() or None, + "candidate_head": _normalize_sha(fields.get("candidate_head")), + "target_branch": fields.get("target_branch"), + "target_branch_sha": _normalize_sha(fields.get("target_branch_sha")), + "last_activity": fields.get("last_activity"), + "expires_at": fields.get("expires_at"), + "blocker": fields.get("blocker"), + "raw_fields": fields, + } + + +def _lease_entries(comments: list[dict], *, pr_number: int) -> list[dict]: + entries: list[dict] = [] + for comment in comments or []: + parsed = parse_lease_comment(comment.get("body") or "") + if not parsed: + continue + if parsed.get("pr_number") not in (None, pr_number): + continue + entries.append({ + **parsed, + "comment_id": comment.get("id"), + "author": (comment.get("user") or {}).get("login") or comment.get("author"), + "created_at": comment.get("created_at"), + "updated_at": comment.get("updated_at"), + }) + return entries + + +def _lease_expired(lease: dict, *, now: datetime) -> bool: + expires_at = _parse_timestamp(lease.get("expires_at")) + return bool(expires_at and expires_at <= now) + + +def _minutes_since_activity(lease: dict, *, now: datetime) -> float | None: + last = _parse_timestamp(lease.get("last_activity")) + if not last: + return None + return (now - last).total_seconds() / 60.0 + + +def classify_lease_freshness(lease: dict, *, now: datetime | None = None) -> str: + """Return active, stale_warning, reclaimable, expired, or terminal.""" + now = now or datetime.now(timezone.utc) + phase = (lease.get("phase") or "").strip().lower() + if phase in _TERMINAL_PHASES: + return "terminal" + if _lease_expired(lease, now=now): + return "expired" + minutes = _minutes_since_activity(lease, now=now) + if minutes is None: + return "active" + if minutes >= RECLAIMABLE_MINUTES: + return "reclaimable" + if minutes >= STALE_WARNING_MINUTES: + return "stale_warning" + return "active" + + +def find_active_reviewer_lease( + comments: list[dict], + *, + pr_number: int, + now: datetime | None = None, +) -> dict[str, Any] | None: + """Newest non-terminal, unexpired lease for *pr_number*.""" + now = now or datetime.now(timezone.utc) + for lease in reversed(_lease_entries(comments, pr_number=pr_number)): + phase = (lease.get("phase") or "").strip().lower() + if phase in _TERMINAL_PHASES: + continue + if _lease_expired(lease, now=now): + continue + if phase in _ACTIVE_PHASES or phase: + lease = dict(lease) + lease["freshness"] = classify_lease_freshness(lease, now=now) + return lease + return None + + +def assess_acquire_lease( + comments: list[dict], + *, + pr_number: int, + reviewer_identity: str, + profile: str, + session_id: str, + repo: str, + issue_number: int | None, + worktree: str, + candidate_head: str | None, + target_branch: str, + target_branch_sha: str | None, + now: datetime | None = None, +) -> dict[str, Any]: + """Fail closed when another session holds an active lease.""" + now = now or datetime.now(timezone.utc) + reasons: list[str] = [] + existing = find_active_reviewer_lease(comments, pr_number=pr_number, now=now) + if existing: + owner_session = (existing.get("session_id") or "").strip() + freshness = existing.get("freshness") or classify_lease_freshness(existing, now=now) + if owner_session and owner_session != session_id and freshness in { + "active", "stale_warning" + }: + reasons.append( + f"PR #{pr_number} already has active reviewer lease " + f"(session_id={owner_session}, phase={existing.get('phase')})" + ) + elif owner_session and owner_session != session_id and freshness == "reclaimable": + reasons.append( + f"PR #{pr_number} lease is reclaimable but still held by " + f"session_id={owner_session}; explicit reclaim not implemented " + "(fail closed)" + ) + + if not (reviewer_identity or "").strip(): + reasons.append("reviewer identity required for lease acquisition") + if not (session_id or "").strip(): + reasons.append("session_id required for lease acquisition") + if not (worktree or "").strip(): + reasons.append("worktree path required for lease acquisition") + + allowed = not reasons + body = None + if allowed: + body = format_lease_body( + repo=repo, + pr_number=pr_number, + issue_number=issue_number, + reviewer_identity=reviewer_identity, + profile=profile, + session_id=session_id, + worktree=worktree, + phase="claimed", + candidate_head=candidate_head, + target_branch=target_branch, + target_branch_sha=target_branch_sha, + last_activity=now, + ) + return { + "acquire_allowed": allowed, + "reasons": reasons, + "existing_lease": existing, + "lease_body": body, + "session_id": session_id, + } + + +def record_session_lease(lease: dict[str, Any]) -> dict[str, Any]: + global _SESSION_LEASE + _SESSION_LEASE = dict(lease) + return dict(_SESSION_LEASE) + + +def clear_session_lease() -> None: + global _SESSION_LEASE + _SESSION_LEASE = None + + +def get_session_lease() -> dict[str, Any] | None: + return dict(_SESSION_LEASE) if _SESSION_LEASE else None + + +def assess_mutation_lease_gate( + *, + pr_number: int, + comments: list[dict], + reviewer_identity: str, + session_id: str | None, + mutation: str, + live_head_sha: str | None, + pinned_head_sha: str | None, + now: datetime | None = None, +) -> dict[str, Any]: + """Reviewer mutations require an owned, current PR lease.""" + now = now or datetime.now(timezone.utc) + reasons: list[str] = [] + session = get_session_lease() + active = find_active_reviewer_lease(comments, pr_number=pr_number, now=now) + + if not session: + reasons.append( + f"no in-session reviewer lease recorded; acquire via " + f"gitea_acquire_reviewer_pr_lease before {mutation}" + ) + elif session.get("pr_number") != pr_number: + reasons.append( + f"session lease is for PR #{session.get('pr_number')}, not #{pr_number}" + ) + elif (session.get("session_id") or "") != (session_id or session.get("session_id")): + reasons.append("session lease session_id mismatch (fail closed)") + + if active: + owner = (active.get("session_id") or "").strip() + if owner and session_id and owner != session_id: + reasons.append( + f"active PR lease owned by session_id={owner}; current session " + f"cannot {mutation}" + ) + pinned = _normalize_sha(pinned_head_sha) + live = _normalize_sha(live_head_sha) + lease_head = active.get("candidate_head") + if pinned and live and pinned != live: + reasons.append( + "PR head changed during lease; stop and re-validate before " + f"reviewer {mutation}" + ) + if lease_head and live and lease_head != live: + reasons.append( + "live PR head differs from lease candidate_head; refresh lease " + f"before {mutation}" + ) + freshness = active.get("freshness") or classify_lease_freshness(active, now=now) + if freshness in {"expired", "reclaimable"}: + reasons.append(f"reviewer lease freshness is '{freshness}' (fail closed)") + else: + reasons.append(f"no active reviewer lease found on PR #{pr_number}") + + allowed = not reasons + return { + "mutation_allowed": allowed, + "block": not allowed, + "reasons": reasons, + "active_lease": active, + "session_lease": session, + } + + +def assess_lease_inventory( + comments_by_pr: dict[int, list[dict]], + *, + now: datetime | None = None, +) -> dict[str, Any]: + """Summarize lease states across PR comment threads.""" + now = now or datetime.now(timezone.utc) + active: list[dict] = [] + stale: list[dict] = [] + reclaimable: list[dict] = [] + for pr_number, comments in (comments_by_pr or {}).items(): + lease = find_active_reviewer_lease(comments, pr_number=pr_number, now=now) + if not lease: + continue + freshness = lease.get("freshness") or classify_lease_freshness(lease, now=now) + entry = {"pr_number": pr_number, "session_id": lease.get("session_id"), "freshness": freshness} + if freshness == "stale_warning": + stale.append(entry) + elif freshness == "reclaimable": + reclaimable.append(entry) + else: + active.append(entry) + return { + "active_review_leases": active, + "stale_review_leases": stale, + "reclaimable_review_leases": reclaimable, + } \ No newline at end of file diff --git a/scripts/run-webui b/scripts/run-webui new file mode 100755 index 0000000..fe9f58c --- /dev/null +++ b/scripts/run-webui @@ -0,0 +1,6 @@ +#!/usr/bin/env bash +set -euo pipefail +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +repo_root="$(cd "$script_dir/.." && pwd)" +cd "$repo_root" +exec python3 -m webui "$@" \ No newline at end of file diff --git a/skills/llm-project-workflow/workflows/review-merge-pr.md b/skills/llm-project-workflow/workflows/review-merge-pr.md index fe784ca..7be1575 100644 --- a/skills/llm-project-workflow/workflows/review-merge-pr.md +++ b/skills/llm-project-workflow/workflows/review-merge-pr.md @@ -732,7 +732,27 @@ The final report must identify: * whether same-PR merge continuation was allowed * whether the run stopped as required -## 26B. Conflict-fix lease and stale-head protection (#399) +## 26B. Per-PR reviewer lease (#407) + +Parallel reviewer sessions are allowed only when each session holds a distinct, +live PR lease. + +Before validation or review mutation on a selected PR: + +1. Call `gitea_acquire_reviewer_pr_lease` with worktree path, candidate head SHA, + and target branch SHA. +2. Post heartbeats via `gitea_heartbeat_reviewer_pr_lease` before validation, + after validation, before review mutation, and before merge. +3. Do not approve, request changes, or merge unless the in-session lease + matches the selected PR. + +If PR head or target branch advances during the lease, stop and refresh +inventory before continuing. + +Final reports must include lease session id, acquisition proof, heartbeat +status, and release/blocked status. + +## 26C. Conflict-fix lease and stale-head protection (#399) Before validating, approving, or merging a PR: diff --git a/skills/llm-project-workflow/workflows/work-issue.md b/skills/llm-project-workflow/workflows/work-issue.md index 0ee51a1..ceef51c 100644 --- a/skills/llm-project-workflow/workflows/work-issue.md +++ b/skills/llm-project-workflow/workflows/work-issue.md @@ -297,6 +297,36 @@ Report: Do not create another branch/PR for the same issue unless the project explicitly allows taking over or updating existing work and exact capability is proven. +### 10A. Duplicate-work gate phases (#400) + +Before any file edits, prove duplicate-work clearance with +`gitea_assess_work_issue_duplicate` or `gitea_lock_issue` (which runs the same +gate). The gate checks live: + +* open PRs linked to the issue (head branch or Closes/Fixes reference), +* remote branches matching `issue-`, +* active claim leases from structured heartbeats. + +Re-check immediately before: + +* `gitea_commit_files` (commit), +* branch push, +* `gitea_create_pr` (PR creation). + +If a concurrent open PR appears after work begins: + +* before commit/push → stop and preserve local work without pushing, +* after commit but before push → stop without pushing, +* after push but before PR creation → stop and produce a reconciliation + handoff instead of opening a PR. + +Final reports must state exactly one duplicate-work outcome: + +* `duplicate PR prevented` +* `duplicate branch prevented` +* `duplicate commit prevented` +* `duplicate work not prevented` + ## 11. Claim or lock the issue before implementation Claim/lock the issue before implementation if the project provides a claim/lock mechanism. @@ -740,6 +770,12 @@ Use only precise categories: * External-state mutations: * Read-only diagnostics: +Issue-lock file (`/tmp/gitea_issue_lock.json`) read/write/delete is always an +external-state mutation. Never claim `External-state mutations: none` after +seeding, restoring, or removing that file. Manual lock seeding is not a normal +recovery path (#447); use `gitea_lock_issue` or the #442 adoption recovery path +instead. Link broader redesign: #438. + `git fetch`, `git remote update`, and any command that updates refs must be listed under `Git ref mutations`, not read-only diagnostics. If `git reset --hard`, checkout, clean, worktree add/remove, merge simulation, merge abort, or similar commands occurred, report them under `Worktree/index mutations`. diff --git a/tests/test_agent_temp_artifacts.py b/tests/test_agent_temp_artifacts.py index 903eab8..1532143 100644 --- a/tests/test_agent_temp_artifacts.py +++ b/tests/test_agent_temp_artifacts.py @@ -80,7 +80,10 @@ class TestIssueLockArtifactWarning(unittest.TestCase): def tearDown(self): self._env_patcher.stop() - @patch("mcp_server.api_get_all", return_value=[]) + @patch( + "mcp_server.issue_duplicate_context_fetcher", + return_value=([], [], {"status": "not_claimed"}), + ) @patch("mcp_server._auth", return_value="token x") @patch("mcp_server._resolve", return_value=("h", "o", "r")) @patch("mcp_server.ISSUE_LOCK_FILE", new_callable=lambda: tempfile.mktemp()) diff --git a/tests/test_audit.py b/tests/test_audit.py index adb9242..11cecd3 100644 --- a/tests/test_audit.py +++ b/tests/test_audit.py @@ -286,6 +286,21 @@ class TestSimpleToolAudit(_AuditWiringBase): class TestGatedToolAudit(_AuditWiringBase): + def setUp(self): + super().setUp() + from tests.test_mcp_server import _install_owned_reviewer_lease + import reviewer_pr_lease + + self._lease_patch = _install_owned_reviewer_lease(8) + self._lease_patch.start() + self._auth_identity_patch = patch( + "mcp_server._authenticated_username", return_value="reviewer-bot" + ) + self._auth_identity_patch.start() + self.addCleanup(self._auth_identity_patch.stop) + self.addCleanup(self._lease_patch.stop) + self.addCleanup(reviewer_pr_lease.clear_session_lease) + def _pr(self, author, state="open", sha="abc123", mergeable=True): return {"user": {"login": author}, "state": state, "head": {"sha": sha}, "mergeable": mergeable} @@ -344,11 +359,22 @@ class TestGatedToolAudit(_AuditWiringBase): GITEA_ALLOWED_OPERATIONS="read,review,approve") with patch.dict(os.environ, env, clear=True): from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision + from tests.test_mcp_server import _install_owned_reviewer_lease + import reviewer_pr_lease + init_review_decision_lock("prgs", "review_pr") gitea_mark_final_review_decision(8, "approve", remote="prgs") - r = gitea_submit_pr_review(pr_number=8, action="approve", - body="LGTM", remote="prgs", - final_review_decision_ready=True) + lease_patch = _install_owned_reviewer_lease(8) + lease_patch.start() + try: + r = gitea_submit_pr_review( + pr_number=8, action="approve", + body="LGTM", remote="prgs", + final_review_decision_ready=True, + ) + finally: + lease_patch.stop() + reviewer_pr_lease.clear_session_lease() self.assertTrue(r["performed"]) recs = self._records() self.assertEqual(len(recs), 1) diff --git a/tests/test_commit_files_capability.py b/tests/test_commit_files_capability.py index d1203bd..c1474ab 100644 --- a/tests/test_commit_files_capability.py +++ b/tests/test_commit_files_capability.py @@ -76,7 +76,14 @@ class CommitFilesCapabilityBase(unittest.TestCase): with open(self.config_path, "w", encoding="utf-8") as fh: fh.write(json.dumps(CONFIG)) + self._dup_fetcher_patcher = patch( + "mcp_server.issue_duplicate_context_fetcher", + return_value=([], [], {"status": "not_claimed"}), + ) + self._dup_fetcher_patcher.start() + def tearDown(self): + self._dup_fetcher_patcher.stop() self._remotes.stop() mcp_server._IDENTITY_CACHE.clear() mcp_server._preflight_whoami_called, mcp_server._preflight_capability_called = ( diff --git a/tests/test_commit_payloads.py b/tests/test_commit_payloads.py index e8e4123..6307a09 100644 --- a/tests/test_commit_payloads.py +++ b/tests/test_commit_payloads.py @@ -67,6 +67,15 @@ class TestCommitPayloads(unittest.TestCase): self.locked_worktree_path = os.path.realpath(self.locked_worktree_dir.name) self.lock_file_path = "/tmp/gitea_issue_lock.json" + import issue_lock_provenance + + work_lease = { + "operation_type": "author_issue_work", + "issue_number": 263, + "branch": "feat/issue-263-native-commit-payloads", + "claimant": {"username": "test-user", "profile": "test-author"}, + "expires_at": "2999-01-01T00:00:00Z", + } self.lock_data = { "issue_number": 263, "branch_name": "feat/issue-263-native-commit-payloads", @@ -74,6 +83,11 @@ class TestCommitPayloads(unittest.TestCase): "org": "Example-Org", "repo": "Example-Repo", "worktree_path": self.locked_worktree_path, + "work_lease": work_lease, + "lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance( + tool="gitea_lock_issue", + claimant=work_lease.get("claimant"), + ), } with open(self.lock_file_path, "w", encoding="utf-8") as fh: fh.write(json.dumps(self.lock_data)) @@ -84,7 +98,14 @@ class TestCommitPayloads(unittest.TestCase): mcp_server._preflight_whoami_called = True mcp_server._preflight_capability_called = True + self._dup_fetcher_patcher = patch( + "mcp_server.issue_duplicate_context_fetcher", + return_value=([], [], {"status": "not_claimed"}), + ) + self._dup_fetcher_patcher.start() + def tearDown(self): + self._dup_fetcher_patcher.stop() self._remotes.stop() mcp_server._IDENTITY_CACHE.clear() diff --git a/tests/test_create_issue_workspace_guard.py b/tests/test_create_issue_workspace_guard.py new file mode 100644 index 0000000..2cd9f41 --- /dev/null +++ b/tests/test_create_issue_workspace_guard.py @@ -0,0 +1,145 @@ +import os +import sys +import unittest +from pathlib import Path +from unittest.mock import patch, MagicMock + +# Ensure we import from the repo root +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import gitea_mcp_server as srv + +FAKE_AUTH = {"Authorization": "token test-token"} +# Stable control checkout (parent of branches/), not the MCP server worktree root. +CONTROL_CHECKOUT_ROOT = str(Path(__file__).resolve().parents[3]) +PROJECT_ROOT = srv.PROJECT_ROOT + + +class TestCreateIssueWorkspaceGuard(unittest.TestCase): + + def setUp(self): + # Reset preflight flags + srv._preflight_whoami_called = True + srv._preflight_capability_called = True + srv._preflight_resolved_role = "author" + srv._preflight_whoami_violation = False + srv._preflight_capability_violation = False + + # Disable early return in verify_preflight_purity for testing + self._orig_in_test = srv._preflight_in_test_mode + srv._preflight_in_test_mode = lambda: False + + def tearDown(self): + srv._preflight_in_test_mode = self._orig_in_test + + @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) + @patch("gitea_mcp_server._profile_permission_block", return_value=None) + @patch("gitea_mcp_server._namespace_mutation_block", return_value=None) + @patch("gitea_mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", return_value=(True, [])) + @patch("gitea_mcp_server.api_request") + @patch("gitea_mcp_server.api_get_all", return_value=[]) + @patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", return_value={"current_branch": "feat/issue-1"}) + def test_create_issue_stable_checkout_rejected(self, _git, _get_all, mock_api, _role, _ns, _prof, _auth): + # Without worktree_path/env hints, workspace resolves to PROJECT_ROOT. When that + # path is the stable control checkout (not under branches/), mutation must fail. + with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): + with self.assertRaises(RuntimeError) as ctx: + srv.gitea_create_issue(title="Test issue", body="body text") + self.assertIn("stable control checkout", str(ctx.exception)) + + @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) + @patch("gitea_mcp_server._profile_permission_block", return_value=None) + @patch("gitea_mcp_server._namespace_mutation_block", return_value=None) + @patch("gitea_mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", return_value=(True, [])) + @patch("gitea_mcp_server.api_request") + @patch("gitea_mcp_server.api_get_all", return_value=[]) + @patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", return_value={"current_branch": "feat/issue-1"}) + @patch("os.path.exists", return_value=True) + @patch("os.path.isdir", return_value=True) + @patch("subprocess.run") + def test_create_issue_valid_worktree_succeeds(self, mock_run, mock_isdir, mock_exists, _git, _get_all, mock_api, _role, _ns, _prof, _auth): + # Mock subprocess.run for git --git-common-dir to return PROJECT_ROOT/.git + mock_res = MagicMock() + mock_res.stdout = f"{CONTROL_CHECKOUT_ROOT}/.git\n" + mock_run.return_value = mock_res + + mock_api.return_value = {"number": 42, "html_url": "https://gitea.example.com/issues/42"} + + # Provide a valid branches path under the control checkout root + valid_path = os.path.join(CONTROL_CHECKOUT_ROOT, "branches", "feat-issue-1") + + with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): + with patch("gitea_mcp_server._get_workspace_porcelain", return_value=""): + res = srv.gitea_create_issue( + title="Test issue", body="body", worktree_path=valid_path + ) + + self.assertEqual(res["number"], 42) + mock_api.assert_called_once() + + @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) + @patch("gitea_mcp_server._profile_permission_block", return_value=None) + @patch("gitea_mcp_server._namespace_mutation_block", return_value=None) + @patch("gitea_mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", return_value=(True, [])) + @patch("gitea_mcp_server.api_request") + @patch("gitea_mcp_server.api_get_all", return_value=[]) + @patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", return_value={"current_branch": "feat/issue-1"}) + def test_create_issue_missing_worktree_fails_closed(self, _git, _get_all, mock_api, _role, _ns, _prof, _auth): + # Path under branches/ but doesn't exist + missing_path = os.path.join( + CONTROL_CHECKOUT_ROOT, "branches", "nonexistent-worktree-path-999" + ) + + with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): + with self.assertRaises(RuntimeError) as ctx: + srv.gitea_create_issue( + title="Test issue", body="body", worktree_path=missing_path + ) + self.assertIn("does not exist (fail closed)", str(ctx.exception)) + + @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) + @patch("gitea_mcp_server._profile_permission_block", return_value=None) + @patch("gitea_mcp_server._namespace_mutation_block", return_value=None) + @patch("gitea_mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", return_value=(True, [])) + @patch("gitea_mcp_server.api_request") + @patch("gitea_mcp_server.api_get_all", return_value=[]) + @patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", return_value={"current_branch": "feat/issue-1"}) + @patch("os.path.exists", return_value=True) + @patch("os.path.isdir", return_value=True) + @patch("subprocess.run") + def test_create_issue_wrong_repo_fails_closed(self, mock_run, mock_isdir, mock_exists, _git, _get_all, mock_api, _role, _ns, _prof, _auth): + # Mock subprocess.run for git --git-common-dir to return a different path + mock_res = MagicMock() + mock_res.stdout = "/Users/jasonwalker/Development/some-other-repo/.git\n" + mock_run.return_value = mock_res + + wrong_repo_path = os.path.join(CONTROL_CHECKOUT_ROOT, "branches", "feat-issue-1") + + with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): + with self.assertRaises(RuntimeError) as ctx: + srv.gitea_create_issue( + title="Test issue", body="body", worktree_path=wrong_repo_path + ) + self.assertIn("does not belong to the target repository", str(ctx.exception)) + + @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) + @patch("gitea_mcp_server._profile_permission_block", return_value=None) + @patch("gitea_mcp_server._namespace_mutation_block", return_value=None) + def test_create_issue_fails_without_whoami_preflight(self, _ns, _prof, _auth): + srv._preflight_whoami_called = False + with self.assertRaises(RuntimeError) as ctx: + srv.gitea_create_issue(title="Test issue", body="body") + self.assertIn("Identity (gitea_whoami) has not been verified", str(ctx.exception)) + + @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) + @patch("gitea_mcp_server._profile_permission_block", return_value=None) + @patch("gitea_mcp_server._namespace_mutation_block", return_value=None) + def test_create_issue_fails_without_capability_preflight(self, _ns, _prof, _auth): + srv._preflight_capability_called = False + with self.assertRaises(RuntimeError) as ctx: + srv.gitea_create_issue(title="Test issue", body="body") + self.assertIn("Task capability (gitea_resolve_task_capability) has not been resolved", str(ctx.exception)) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_final_report_validator.py b/tests/test_final_report_validator.py index 672d82e..88c5b23 100644 --- a/tests/test_final_report_validator.py +++ b/tests/test_final_report_validator.py @@ -41,6 +41,10 @@ def _review_handoff(**overrides): "- Selected PR: #203", "- Reviewer eligibility: eligible", "- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", + "- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", + "- Final live head SHA before approval: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", + "- Final live head SHA before merge: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", + "- Push occurred during validation: no", "- Worktree path: branches/review-203", "- Worktree dirty: clean", "- Scratch worktree used: yes (branches/review-203)", diff --git a/tests/test_issue_lock_provenance.py b/tests/test_issue_lock_provenance.py new file mode 100644 index 0000000..3e64090 --- /dev/null +++ b/tests/test_issue_lock_provenance.py @@ -0,0 +1,130 @@ +"""Tests for issue-lock provenance and external-state disclosure (#447).""" + +from __future__ import annotations + +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import issue_lock_provenance as ilp # noqa: E402 +from final_report_validator import assess_final_report_validator # noqa: E402 + + +def _sanctioned_lock(**overrides): + work_lease = { + "operation_type": "author_issue_work", + "issue_number": 447, + "branch": "feat/issue-447-lock-provenance", + "claimant": {"username": "jcwalker3", "profile": "prgs-author"}, + "expires_at": "2999-01-01T00:00:00Z", + } + data = { + "issue_number": 447, + "branch_name": "feat/issue-447-lock-provenance", + "work_lease": work_lease, + "lock_provenance": ilp.build_sanctioned_lock_provenance( + tool="gitea_lock_issue", + claimant=work_lease["claimant"], + ), + } + data.update(overrides) + return data + + +class TestLockProvenanceForCreatePr(unittest.TestCase): + def test_sanctioned_lock_passes(self): + result = ilp.assess_lock_file_for_create_pr(_sanctioned_lock()) + self.assertTrue(result["proven"]) + self.assertFalse(result["block"]) + + def test_manual_seed_without_provenance_blocked(self): + result = ilp.assess_lock_file_for_create_pr( + {"issue_number": 420, "branch_name": "feat/x", "work_lease": {}} + ) + self.assertTrue(result["block"]) + self.assertIn("lock_provenance", result["reasons"][0]) + + def test_operator_override_requires_reason(self): + result = ilp.assess_lock_file_for_create_pr( + _sanctioned_lock( + lock_provenance=ilp.build_sanctioned_lock_provenance( + tool="operator_override", + source=ilp.SOURCE_OPERATOR_OVERRIDE, + ) + ) + ) + self.assertTrue(result["block"]) + + +class TestExternalStateReportRules(unittest.TestCase): + def test_seed_with_external_none_blocked(self): + report = ( + "Restored /tmp/gitea_issue_lock.json to unblock PR creation.\n" + "- External-state mutations: none\n" + ) + result = ilp.assess_issue_lock_external_state_report(report) + self.assertTrue(result["block"]) + + def test_seed_with_disclosure_passes(self): + report = ( + "Restored /tmp/gitea_issue_lock.json after MCP restart.\n" + "- External-state mutations: wrote /tmp/gitea_issue_lock.json\n" + ) + result = ilp.assess_issue_lock_external_state_report(report) + self.assertTrue(result["proven"]) + + def test_remove_claimed_as_cleanup_only_blocked(self): + report = ( + "rm /tmp/gitea_issue_lock.json after PR creation.\n" + "- Cleanup mutations: lock removed\n" + "- External-state mutations: none\n" + ) + result = ilp.assess_issue_lock_external_state_report(report) + self.assertTrue(result["block"]) + + def test_manual_lock_pr_without_override_blocked(self): + report = ( + "Programmatically seeded gitea_issue_lock.json then gitea_create_pr.\n" + "PR #444 created.\n" + ) + result = ilp.assess_manual_lock_pr_without_override(report) + self.assertTrue(result["block"]) + + def test_author_reviewer_same_run_blocked(self): + report = ( + "gitea_create_pr opened PR #444.\n" + "Submitted approve review on PR #444.\n" + ) + result = ilp.assess_author_reviewer_same_run_report(report) + self.assertTrue(result["block"]) + + +class TestFinalReportValidatorIntegration(unittest.TestCase): + def test_work_issue_blocks_hidden_lock_mutation(self): + report = ( + "## Controller Handoff\n" + "- Task: work issue #420\n" + "- External-state mutations: none\n" + "Restored /tmp/gitea_issue_lock.json before PR creation.\n" + ) + result = assess_final_report_validator(report, "work_issue") + rule_ids = {f["rule_id"] for f in result["findings"]} + self.assertIn("shared.issue_lock_external_state", rule_ids) + self.assertTrue(result["blocked"]) + + def test_review_pr_blocks_create_and_approve(self): + report = ( + "## Controller Handoff\n" + "- Task: review PR #444\n" + "- Review decision: approve\n" + "Created PR #444 via gitea_create_pr earlier in this run.\n" + ) + result = assess_final_report_validator(report, "review_pr") + rule_ids = {f["rule_id"] for f in result["findings"]} + self.assertIn("shared.author_reviewer_same_run", rule_ids) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_issue_work_duplicate_gate.py b/tests/test_issue_work_duplicate_gate.py new file mode 100644 index 0000000..ac4ccea --- /dev/null +++ b/tests/test_issue_work_duplicate_gate.py @@ -0,0 +1,264 @@ +"""Tests for early duplicate-work detection (#400).""" +import json +import os +import sys +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import issue_lock_provenance +import issue_work_duplicate_gate as dup_gate +import mcp_server +from issue_work_duplicate_gate import ( + OUTCOME_DUPLICATE_BRANCH_PREVENTED, + OUTCOME_DUPLICATE_COMMIT_PREVENTED, + OUTCOME_DUPLICATE_PR_PREVENTED, + OUTCOME_DUPLICATE_WORK_NOT_PREVENTED, + PHASE_COMMIT, + PHASE_CREATE_PR, + PHASE_LOCK, + assess_work_issue_duplicate_gate, + assess_work_issue_duplicate_report, +) + + +class TestDuplicateGateAssessment(unittest.TestCase): + def test_clear_issue_passes(self): + result = assess_work_issue_duplicate_gate( + 400, + open_prs=[], + branch_names=["feat/other-issue-99"], + claim_entry={"status": "not_claimed"}, + locked_branch="feat/issue-400-duplicate-work-preflight", + phase=PHASE_LOCK, + ) + self.assertFalse(result["block"]) + self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_WORK_NOT_PREVENTED) + + def test_open_pr_blocks(self): + prs = [{ + "number": 397, + "title": "feat: handoff", + "body": "Closes #395", + "head": {"ref": "feat/issue-395-proof-backed-review-handoff"}, + }] + result = assess_work_issue_duplicate_gate( + 395, + open_prs=prs, + branch_names=[], + phase=PHASE_LOCK, + ) + self.assertTrue(result["block"]) + self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_PR_PREVENTED) + + def test_conflicting_remote_branch_blocks(self): + result = assess_work_issue_duplicate_gate( + 395, + open_prs=[], + branch_names=["feat/issue-395-proof-backed-handoff-claims"], + locked_branch="feat/issue-395-new-attempt", + phase=PHASE_LOCK, + ) + self.assertTrue(result["block"]) + self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_BRANCH_PREVENTED) + + def test_active_claim_on_other_branch_blocks(self): + result = assess_work_issue_duplicate_gate( + 398, + open_prs=[], + branch_names=[], + claim_entry={ + "status": "active", + "latest_heartbeat": { + "branch": "feat/issue-398-validation-cwd-proof", + }, + }, + locked_branch="feat/issue-398-other-branch", + phase=PHASE_LOCK, + ) + self.assertTrue(result["block"]) + + def test_commit_phase_maps_to_commit_outcome(self): + prs = [{ + "number": 411, + "title": "x", + "body": "Closes #398", + "head": {"ref": "feat/issue-398-validation-cwd-proof"}, + }] + result = assess_work_issue_duplicate_gate( + 398, + open_prs=prs, + branch_names=[], + locked_branch="feat/issue-398-alt", + phase=PHASE_COMMIT, + ) + self.assertTrue(result["block"]) + self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_COMMIT_PREVENTED) + + def test_stale_claim_does_not_block_by_status_alone(self): + result = assess_work_issue_duplicate_gate( + 400, + open_prs=[], + branch_names=[], + claim_entry={"status": "reclaimable", "reasons": ["stale"]}, + locked_branch="feat/issue-400-duplicate-work-preflight", + phase=PHASE_LOCK, + ) + self.assertFalse(result["block"]) + + +class TestDuplicateReportOutcome(unittest.TestCase): + def test_requires_exactly_one_outcome(self): + bad = assess_work_issue_duplicate_report("work finished") + self.assertFalse(bad["complete"]) + + good = assess_work_issue_duplicate_report( + "Duplicate work not prevented for issue #400." + ) + self.assertTrue(good["complete"]) + self.assertEqual(good["outcome"], OUTCOME_DUPLICATE_WORK_NOT_PREVENTED) + + +class TestInjectableDuplicateFetcher(unittest.TestCase): + @patch("mcp_server.get_auth_header", return_value="token x") + def test_lock_issue_uses_injected_fetcher(self, _auth): + seen = {} + + def fetcher(h, o, r, auth, issue_number): + seen["issue_number"] = issue_number + return [], [], {"status": "not_claimed"} + + with patch( + "mcp_server.issue_duplicate_context_fetcher", + side_effect=fetcher, + ), patch( + "mcp_server.issue_lock_worktree.read_worktree_git_state", + return_value={ + "current_branch": "master", + "porcelain_status": "", + "base_equivalent": True, + }, + ), patch.dict(os.environ, { + "GITEA_ALLOWED_OPERATIONS": "gitea.issue.comment", + }, clear=True): + with patch.object(mcp_server, "ISSUE_LOCK_FILE", tempfile.mktemp()): + mcp_server.gitea_lock_issue( + issue_number=400, + branch_name="feat/issue-400-duplicate-work-preflight", + remote="prgs", + ) + self.assertEqual(seen["issue_number"], 400) + + +class TestMcpDuplicateRecheck(unittest.TestCase): + def setUp(self): + self._dir = tempfile.TemporaryDirectory() + self.lock_path = os.path.join(self._dir.name, "gitea_issue_lock.json") + self._lock_patch = patch.object( + mcp_server, "ISSUE_LOCK_FILE", self.lock_path + ) + self._lock_patch.start() + self._remotes = patch.dict(mcp_server.REMOTES, { + "prgs": {"host": "gitea.example.com", "org": "Example-Org", + "repo": "Example-Repo"}, + }) + self._remotes.start() + mcp_server._IDENTITY_CACHE.clear() + + def tearDown(self): + patch.stopall() + self._dir.cleanup() + + def _write_lock(self, issue_number=400, branch="feat/issue-400-x"): + work_lease = { + "operation_type": "author_issue_work", + "issue_number": issue_number, + "branch": branch, + "claimant": {"username": "test-user", "profile": "test-author"}, + "expires_at": "2999-01-01T00:00:00Z", + } + with open(self.lock_path, "w", encoding="utf-8") as fh: + json.dump({ + "issue_number": issue_number, + "branch_name": branch, + "remote": "prgs", + "work_lease": work_lease, + "lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance( + tool="gitea_lock_issue", + claimant=work_lease.get("claimant"), + ), + }, fh) + + @patch("mcp_server._assess_issue_duplicate_gate") + @patch("mcp_server.get_profile", return_value={ + "profile_name": "test-author", + "allowed_operations": ["gitea.read", "gitea.repo.commit"], + "forbidden_operations": [], + "audit_label": "test-author", + }) + @patch("mcp_server.get_auth_header", return_value="token x") + def test_commit_files_blocked_on_recheck(self, _auth, _profile, mock_gate): + self._write_lock() + mock_gate.return_value = { + "block": True, + "reasons": ["open PR #412 already covers issue #400"], + "outcome": OUTCOME_DUPLICATE_COMMIT_PREVENTED, + "safe_next_action": "stop", + } + mcp_server.record_preflight_check("whoami") + mcp_server.record_preflight_check("capability", resolved_role="author") + with patch( + "mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", + return_value=(True, []), + ): + result = mcp_server.gitea_commit_files( + files=[{ + "operation": "create", + "path": "a.txt", + "content_plain": "hi", + }], + message="test", + remote="prgs", + ) + self.assertFalse(result["success"]) + self.assertIn("duplicate_gate", result) + + @patch("mcp_server._assess_issue_duplicate_gate") + @patch("mcp_server.get_profile", return_value={ + "profile_name": "test-author", + "allowed_operations": ["gitea.read", "gitea.pr.create"], + "forbidden_operations": [], + "audit_label": "test-author", + }) + @patch("mcp_server.get_auth_header", return_value="token x") + def test_create_pr_returns_handoff_on_duplicate(self, _auth, _profile, mock_gate): + self._write_lock() + mock_gate.return_value = { + "block": True, + "reasons": ["open PR #412 already covers issue #400"], + "outcome": OUTCOME_DUPLICATE_PR_PREVENTED, + "safe_next_action": "reconciliation handoff", + } + mcp_server.record_preflight_check("whoami") + mcp_server.record_preflight_check("capability", resolved_role="author") + with patch( + "mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", + return_value=(True, []), + ): + result = mcp_server.gitea_create_pr( + title="feat: x (Closes #400)", + head="feat/issue-400-x", + base="master", + body="Closes #400", + remote="prgs", + ) + self.assertFalse(result["success"]) + self.assertIsNone(result.get("number")) + self.assertIn("duplicate_gate", result) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 4002b5e..bdb8eb4 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -60,6 +60,7 @@ def _mark_request_changes_ready(pr_number=8, **kwargs): suppression feedback fetch stubbed to 'no existing blocker'.""" with patch("mcp_server.gitea_get_pr_review_feedback", return_value=dict(_NO_BLOCKER_FEEDBACK)): + kwargs.setdefault("expected_head_sha", "abc123") return gitea_mark_final_review_decision( pr_number, "request_changes", **kwargs) @@ -79,6 +80,65 @@ def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"): return [_formal_review(reviewer, "APPROVED", sha=sha)] +_DEFAULT_LEASE_SESSION = "mcp-test-reviewer-lease" +_NO_PR_WORK_LEASE_BLOCK = {"block": False, "reasons": [], "mutation_allowed": True} + + +def _reviewer_lease_comment( + pr_number, + *, + session_id=_DEFAULT_LEASE_SESSION, + head_sha="abc123", + reviewer="reviewer-bot", +): + from datetime import datetime, timezone + + import reviewer_pr_lease + + body = reviewer_pr_lease.format_lease_body( + repo="Scaled-Tech-Consulting/Gitea-Tools", + pr_number=pr_number, + issue_number=407, + reviewer_identity=reviewer, + profile="gitea-reviewer", + session_id=session_id, + worktree="branches/review-test", + phase="claimed", + candidate_head=head_sha, + target_branch="master", + target_branch_sha="b" * 40, + last_activity=datetime.now(timezone.utc), + ) + return {"id": 9001, "body": body, "user": {"login": reviewer}} + + +def _install_owned_reviewer_lease( + pr_number, + *, + session_id=_DEFAULT_LEASE_SESSION, + head_sha="abc123", +): + import reviewer_pr_lease + + reviewer_pr_lease.clear_session_lease() + reviewer_pr_lease.record_session_lease({ + "pr_number": pr_number, + "session_id": session_id, + "candidate_head": head_sha, + "target_branch": "master", + }) + return patch( + "mcp_server._fetch_pr_comments", + return_value=[ + _reviewer_lease_comment( + pr_number, + session_id=session_id, + head_sha=head_sha, + ) + ], + ) + + # Issue-write tools are profile-gated (#69). ISSUE_WRITE_ENV = { "GITEA_ALLOWED_OPERATIONS": ( @@ -101,17 +161,36 @@ ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json" def _sample_issue_lock(issue_number=123, branch_name="feat/x", **overrides): + import issue_lock_provenance + + work_lease = { + "operation_type": "author_issue_work", + "issue_number": issue_number, + "branch": branch_name, + "claimant": {"username": "test-user", "profile": "test-author"}, + "expires_at": "2999-01-01T00:00:00Z", + } record = { "issue_number": issue_number, "branch_name": branch_name, "remote": "dadeschools", "org": "Scaled-Tech-Consulting", "repo": "Gitea-Tools", + "work_lease": work_lease, + "lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance( + tool="gitea_lock_issue", + claimant=work_lease.get("claimant"), + ), } record.update(overrides) return record +def _clear_duplicate_context_fetcher(*_args, **_kwargs): + """Default injectable duplicate-work context for lock/create_pr tests.""" + return [], [], {"status": "not_claimed"} + + # --------------------------------------------------------------------------- # Create Issue # --------------------------------------------------------------------------- @@ -166,13 +245,17 @@ class TestCreateIssue(unittest.TestCase): # --------------------------------------------------------------------------- class TestCreatePR(unittest.TestCase): + @patch( + "mcp_server.issue_duplicate_context_fetcher", + return_value=([], [], {"status": "not_claimed"}), + ) @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", return_value=(True, [])) @patch("mcp_server.api_request") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("os.path.exists", return_value=True) @patch("builtins.open") - def test_creates_pr(self, mock_open, mock_exists, _auth, mock_api, _role): + def test_creates_pr(self, mock_open, mock_exists, _auth, mock_api, _role, _dup_fetcher): lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x")) mock_open.return_value.__enter__.return_value.read.return_value = lock_json mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"} @@ -187,13 +270,17 @@ class TestCreatePR(unittest.TestCase): self.assertEqual(payload["base"], "main") self.assertIn("Closes #123", payload["title"]) + @patch( + "mcp_server.issue_duplicate_context_fetcher", + return_value=([], [], {"status": "not_claimed"}), + ) @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", return_value=(True, [])) @patch("mcp_server.api_request") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("os.path.exists", return_value=True) @patch("builtins.open") - def test_create_pr_reveal_opt_in_includes_url(self, mock_open, mock_exists, _auth, mock_api, _role): + def test_create_pr_reveal_opt_in_includes_url(self, mock_open, mock_exists, _auth, mock_api, _role, _dup_fetcher): lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x")) mock_open.return_value.__enter__.return_value.read.return_value = lock_json mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"} @@ -512,6 +599,30 @@ class TestViewPR(unittest.TestCase): class TestMergePR(unittest.TestCase): """Gated merge workflow (#16). gitea_merge_pr is the only merge path.""" + def setUp(self): + import reviewer_pr_lease + + self._lease_patch = _install_owned_reviewer_lease(8) + self._lease_patch.start() + self._auth_identity_patch = patch( + "mcp_server._authenticated_username", return_value="reviewer-bot" + ) + self._auth_identity_patch.start() + self.addCleanup(self._auth_identity_patch.stop) + self.addCleanup(self._lease_patch.stop) + self.addCleanup(reviewer_pr_lease.clear_session_lease) + self._pr_lease_comments_patch = patch( + "mcp_server._list_pr_lease_comments", return_value=[] + ) + self._pr_lease_comments_patch.start() + self.addCleanup(self._pr_lease_comments_patch.stop) + self._pr_work_lease_patch = patch( + "mcp_server._pr_work_lease_reviewer_block", + return_value=dict(_NO_PR_WORK_LEASE_BLOCK), + ) + self._pr_work_lease_patch.start() + self.addCleanup(self._pr_work_lease_patch.stop) + def _pr(self, author, state="open", sha="abc123", mergeable=True): return { "user": {"login": author}, @@ -584,7 +695,8 @@ class TestMergePR(unittest.TestCase): with patch.dict(os.environ, env, clear=True): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), - expected_changed_files=["b.py", "a.py"], remote="prgs") + expected_changed_files=["b.py", "a.py"], + expected_head_sha="abc123", remote="prgs") self.assertTrue(r["performed"]) # -- read-back / cleanup surfacing (#98) ----------------------------------- @@ -602,8 +714,10 @@ class TestMergePR(unittest.TestCase): env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} with patch.dict(os.environ, env, clear=True): - r = gitea_merge_pr(pr_number=8, confirmation=self._confirm(8), - remote="prgs") + r = gitea_merge_pr( + pr_number=8, confirmation=self._confirm(8), + expected_head_sha="abc123", remote="prgs", + ) # The merge itself is still reported performed/successful. self.assertTrue(r["performed"]) self.assertEqual(r["merge_result"], "PR #8 merged via 'merge'.") @@ -631,8 +745,10 @@ class TestMergePR(unittest.TestCase): env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} with patch.dict(os.environ, env, clear=True): - r = gitea_merge_pr(pr_number=8, confirmation=self._confirm(8), - remote="prgs") + r = gitea_merge_pr( + pr_number=8, confirmation=self._confirm(8), + expected_head_sha="abc123", remote="prgs", + ) self.assertTrue(r["performed"]) self.assertEqual(r["merge_commit"], "c9") self.assertTrue(r["cleanup_status"].startswith("skipped (cleanup error:")) @@ -646,7 +762,7 @@ class TestMergePR(unittest.TestCase): env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} with patch.dict(os.environ, env, clear=True): - r = gitea_merge_pr(pr_number=8, confirmation="", remote="prgs") + r = gitea_merge_pr(pr_number=8, confirmation="", expected_head_sha="abc123", remote="prgs") self.assertFalse(r["performed"]) self.assertTrue(any("explicit confirmation required" in x for x in r["reasons"])) mock_api.assert_not_called() @@ -657,7 +773,7 @@ class TestMergePR(unittest.TestCase): env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} with patch.dict(os.environ, env, clear=True): - r = gitea_merge_pr(pr_number=8, confirmation="MERGE PR 9", remote="prgs") + r = gitea_merge_pr(pr_number=8, confirmation="MERGE PR 9", expected_head_sha="abc123", remote="prgs") self.assertFalse(r["performed"]) mock_api.assert_not_called() @@ -772,9 +888,11 @@ class TestMergePR(unittest.TestCase): pr_number=8, confirmation=self._confirm(8), expected_head_sha="deadbeef", remote="prgs") self.assertFalse(r["performed"]) - self.assertIn( - "expected head SHA does not match current PR head (fail closed)", - r["reasons"]) + self.assertTrue(any( + "expected head SHA does not match current PR head (fail closed)" in reason + or "PR head changed during lease" in reason + for reason in r["reasons"] + )) self._assert_no_merge_call(mock_api) @patch("mcp_server.api_request") @@ -789,7 +907,8 @@ class TestMergePR(unittest.TestCase): with patch.dict(os.environ, env, clear=True): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), - expected_changed_files=["a.py", "b.py"], remote="prgs") + expected_changed_files=["a.py", "b.py"], + expected_head_sha="abc123", remote="prgs") self.assertFalse(r["performed"]) self.assertIn( "PR changed files do not match expected_changed_files (fail closed)", @@ -802,7 +921,7 @@ class TestMergePR(unittest.TestCase): def test_invalid_merge_method_rejected(self, mock_api): with patch.dict(os.environ, {}, clear=True): r = gitea_merge_pr( - pr_number=8, confirmation="MERGE PR 8", do="octopus", remote="prgs") + pr_number=8, confirmation="MERGE PR 8", do="octopus", expected_head_sha="abc123", remote="prgs") self.assertFalse(r["performed"]) self.assertTrue(any("unknown merge method" in x for x in r["reasons"])) mock_api.assert_not_called() @@ -820,7 +939,9 @@ class TestMergePR(unittest.TestCase): "GITEA_TOKEN": "super-secret-token"} with patch.dict(os.environ, env, clear=True): r = gitea_merge_pr( - pr_number=8, confirmation=self._confirm(8), remote="prgs") + pr_number=8, confirmation=self._confirm(8), + expected_head_sha="abc123", remote="prgs", + ) blob = repr(r).lower() for secret in ("super-secret-token", "authorization", "basic ", FAKE_AUTH.lower()): self.assertNotIn(secret, blob) @@ -837,7 +958,9 @@ class TestMergePR(unittest.TestCase): "GITEA_ALLOWED_OPERATIONS": "read,merge"} with patch.dict(os.environ, env, clear=True): r = gitea_merge_pr( - pr_number=8, confirmation=self._confirm(8), remote="prgs") + pr_number=8, confirmation=self._confirm(8), + expected_head_sha="abc123", remote="prgs", + ) self.assertFalse(r["performed"]) blob = repr(r) self.assertIn("[REDACTED]", blob) @@ -855,7 +978,9 @@ class TestMergePR(unittest.TestCase): "GITEA_ALLOWED_OPERATIONS": "read,merge"} with patch.dict(os.environ, env, clear=True): r = gitea_merge_pr( - pr_number=8, confirmation=self._confirm(8), remote="prgs") + pr_number=8, confirmation=self._confirm(8), + expected_head_sha="abc123", remote="prgs", + ) self.assertFalse(r["performed"]) self.assertFalse(r.get("approval_visible")) self.assertTrue(any("no visible APPROVED review" in x for x in r["reasons"])) @@ -876,7 +1001,9 @@ class TestMergePR(unittest.TestCase): "GITEA_ALLOWED_OPERATIONS": "read,merge"} with patch.dict(os.environ, env, clear=True): r = gitea_merge_pr( - pr_number=8, confirmation=self._confirm(8), remote="prgs") + pr_number=8, confirmation=self._confirm(8), + expected_head_sha="abc123", remote="prgs", + ) self.assertFalse(r["performed"]) self.assertTrue(r.get("has_blocking_change_requests")) self.assertTrue(any("REQUEST_CHANGES" in x for x in r["reasons"])) @@ -986,7 +1113,7 @@ class TestReviewPR(unittest.TestCase): ] from mcp_server import init_review_decision_lock init_review_decision_lock("prgs", "review_pr") - gitea_mark_final_review_decision(1, "approve", remote="prgs") + gitea_mark_final_review_decision(1, "approve", remote="prgs", expected_head_sha="abc123") result = gitea_review_pr( pr_number=1, event="APPROVE", @@ -1653,7 +1780,26 @@ class TestReviewDecisionValidationGate(unittest.TestCase): } def setUp(self): + import reviewer_pr_lease + init_review_decision_lock("prgs", "review_pr") + self._lease_patch = _install_owned_reviewer_lease( + self.PR, head_sha=self.SHA, + ) + self._lease_patch.start() + self._auth_identity_patch = patch( + "mcp_server._authenticated_username", return_value="reviewer-bot" + ) + self._auth_identity_patch.start() + self.addCleanup(self._auth_identity_patch.stop) + self.addCleanup(self._lease_patch.stop) + self.addCleanup(reviewer_pr_lease.clear_session_lease) + self._pr_lease_comments_patch = patch( + "mcp_server._list_pr_lease_comments", return_value=[] + ) + self._pr_lease_comments_patch.start() + self.addCleanup(self._pr_lease_comments_patch.stop) + def _env(self): return patch.dict(os.environ, { @@ -1748,8 +1894,32 @@ class TestSubmitPrReview(unittest.TestCase): """Gated review-mutation tool (#15).""" def setUp(self): + import reviewer_pr_lease + init_review_decision_lock("prgs", "review_pr") - gitea_mark_final_review_decision(8, "approve", remote="prgs") + self._lease_patch = _install_owned_reviewer_lease(8) + self._lease_patch.start() + self._auth_identity_patch = patch( + "mcp_server._authenticated_username", return_value="reviewer-bot" + ) + self._auth_identity_patch.start() + self._pr_lease_comments_patch = patch( + "mcp_server._list_pr_lease_comments", return_value=[] + ) + self._pr_lease_comments_patch.start() + self._pr_work_lease_patch = patch( + "mcp_server._pr_work_lease_reviewer_block", + return_value=dict(_NO_PR_WORK_LEASE_BLOCK), + ) + self._pr_work_lease_patch.start() + gitea_mark_final_review_decision( + 8, "approve", remote="prgs", expected_head_sha="abc123", + ) + self.addCleanup(self._auth_identity_patch.stop) + self.addCleanup(self._lease_patch.stop) + self.addCleanup(self._pr_lease_comments_patch.stop) + self.addCleanup(self._pr_work_lease_patch.stop) + self.addCleanup(reviewer_pr_lease.clear_session_lease) def _pr(self, author, state="open", sha="abc123", mergeable=True): return { @@ -1909,10 +2079,11 @@ class TestSubmitPrReview(unittest.TestCase): @patch("mcp_server.api_request") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_comment_succeeds_when_review_eligible(self, _auth, mock_api): - gitea_mark_final_review_decision(8, "comment", remote="prgs") mock_api.side_effect = [ + {"login": "reviewer-bot"}, self._pr("author-bot"), {"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 3}, ] + gitea_mark_final_review_decision(8, "comment", remote="prgs", expected_head_sha="abc123") env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review"} with patch.dict(os.environ, env, clear=True): @@ -1928,12 +2099,15 @@ class TestSubmitPrReview(unittest.TestCase): with patch("mcp_server.get_auth_header", return_value=FAKE_AUTH), \ patch("mcp_server.api_request") as mock_api: mock_api.side_effect = [ + {"login": "jcwalker3"}, self._pr("jcwalker3"), {"login": "jcwalker3"}, self._pr("jcwalker3"), {"id": 4}, ] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review"} with patch.dict(os.environ, env, clear=True): - gitea_mark_final_review_decision(8, "comment", remote="prgs") + gitea_mark_final_review_decision( + 8, "comment", remote="prgs", expected_head_sha="abc123", + ) r = gitea_submit_pr_review( pr_number=8, action="comment", body="note", remote="prgs", final_review_decision_ready=True, @@ -1976,20 +2150,22 @@ class TestSubmitPrReview(unittest.TestCase): with patch("mcp_server.get_auth_header", return_value=FAKE_AUTH), \ patch("mcp_server.api_request") as mock_api: mock_api.side_effect = [ - {"login": "reviewer-bot"}, self._pr("author-bot", sha="abc123"), + {"login": "reviewer-bot"}, self._pr("author-bot", sha="deadbeef"), ] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} with patch.dict(os.environ, env, clear=True): r = gitea_submit_pr_review( pr_number=8, action="approve", - expected_head_sha="deadbeef", remote="prgs", + expected_head_sha="abc123", remote="prgs", final_review_decision_ready=True, ) self.assertFalse(r["performed"]) - self.assertIn( - "expected head SHA does not match current PR head (fail closed)", - r["reasons"]) + self.assertTrue(any( + "expected head SHA does not match current PR head (fail closed)" in reason + or "PR head changed during lease" in reason + for reason in r["reasons"] + )) self._assert_no_mutation(mock_api) def test_head_sha_match_allows(self): @@ -2032,7 +2208,7 @@ class TestSubmitPrReview(unittest.TestCase): "GITEA_ALLOWED_OPERATIONS": "read,review,approve", "GITEA_TOKEN": "super-secret-token"} with patch.dict(os.environ, env, clear=True): - gitea_mark_final_review_decision(5, "approve", remote="prgs") + gitea_mark_final_review_decision(5, "approve", remote="prgs", expected_head_sha="abc123") r = gitea_submit_pr_review( pr_number=5, action="approve", remote="prgs", final_review_decision_ready=True, @@ -2052,9 +2228,8 @@ class TestSubmitPrReview(unittest.TestCase): env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} with patch.dict(os.environ, env, clear=True): - gitea_mark_final_review_decision(5, "approve", remote="prgs") r = gitea_submit_pr_review( - pr_number=5, action="approve", remote="prgs", + pr_number=8, action="approve", remote="prgs", final_review_decision_ready=True, ) self.assertFalse(r["performed"]) @@ -2142,7 +2317,7 @@ class TestSubmitPrReview(unittest.TestCase): def test_mark_final_decision_rejects_remote_mismatch(self): init_review_decision_lock("prgs", "review_pr") - r = gitea_mark_final_review_decision(8, "approve", remote="dadeschools") + r = gitea_mark_final_review_decision(8, "approve", remote="dadeschools", expected_head_sha="abc123") self.assertFalse(r["marked_ready"]) self.assertTrue(any("does not match locked remote" in x for x in r["reasons"])) @@ -2154,28 +2329,30 @@ class TestSubmitPrReview(unittest.TestCase): {"id": 42, "state": "APPROVED"}, [_formal_review("reviewer-bot", "APPROVED", review_id=42)], {"login": "reviewer-bot"}, self._pr("author-bot"), + {"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 43, "state": "REQUEST_CHANGES"}, [_formal_review("reviewer-bot", "REQUEST_CHANGES", review_id=43)], ] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve,request_changes"} - gitea_mark_final_review_decision(8, "approve", remote="prgs") - with patch.dict(os.environ, env, clear=True): - first = gitea_submit_pr_review( - pr_number=8, action="approve", remote="prgs", - final_review_decision_ready=True, - ) - auth = gitea_authorize_review_correction( - prior_review_id=42, - prior_review_state="approve", - reason="operator approved correcting mistaken approve", - operator_authorized=True, - ) - _mark_request_changes_ready(remote="prgs") - second = gitea_submit_pr_review( - pr_number=8, action="request_changes", remote="prgs", - final_review_decision_ready=True, - ) + with patch("mcp_server.gitea_get_pr_review_feedback", + return_value=dict(_NO_BLOCKER_FEEDBACK)): + with patch.dict(os.environ, env, clear=True): + first = gitea_submit_pr_review( + pr_number=8, action="approve", remote="prgs", + final_review_decision_ready=True, + ) + auth = gitea_authorize_review_correction( + prior_review_id=42, + prior_review_state="approve", + reason="operator approved correcting mistaken approve", + operator_authorized=True, + ) + _mark_request_changes_ready(remote="prgs") + second = gitea_submit_pr_review( + pr_number=8, action="request_changes", remote="prgs", + final_review_decision_ready=True, + ) self.assertTrue(first["performed"]) self.assertTrue(auth["authorized"]) self.assertTrue(second["performed"]) @@ -2187,7 +2364,7 @@ class TestSubmitPrReview(unittest.TestCase): "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} with patch.dict(os.environ, env, clear=True): # Mark decision for specific remote, org, repo - gitea_mark_final_review_decision(8, "approve", remote="prgs", org="MyOrg", repo="MyRepo") + gitea_mark_final_review_decision(8, "approve", remote="prgs", expected_head_sha="abc123", org="MyOrg", repo="MyRepo") # Mismatched remote r = gitea_submit_pr_review( @@ -2273,6 +2450,13 @@ class TestTrackerHygieneCleanup(unittest.TestCase): self.assertEqual(res["cleanup_status"].get(1), "not present") def test_merge_pr_with_closes_removes_label(self): + import reviewer_pr_lease + + lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123") + lease_patch.start() + self.addCleanup(lease_patch.stop) + self.addCleanup(reviewer_pr_lease.clear_session_lease) + def api_side_effect(method, url, auth, payload=None): if method == "GET" and "/user" in url: return {"login": "merger"} @@ -2307,6 +2491,13 @@ class TestTrackerHygieneCleanup(unittest.TestCase): self.assertEqual(res["cleanup_status"].get(123), "released") def test_merge_pr_with_branch_name_removes_label(self): + import reviewer_pr_lease + + lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123") + lease_patch.start() + self.addCleanup(lease_patch.stop) + self.addCleanup(reviewer_pr_lease.clear_session_lease) + def api_side_effect(method, url, auth, payload=None): if method == "GET" and "/user" in url: return {"login": "merger"} @@ -2988,10 +3179,12 @@ class TestVerifyMutationAuthority(unittest.TestCase): # profile; the active profile resolves as reviewer — side-channel # override rejected even with a matching in-process authority. self._authority() - with patch.dict(os.environ, - {"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}): - with self.assertRaises(RuntimeError) as ctx: - mcp_server.verify_mutation_authority("prgs") + with patch("mcp_server.gitea_config.is_runtime_switching_enabled", + return_value=False): + with patch.dict(os.environ, + {"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}): + with self.assertRaises(RuntimeError) as ctx: + mcp_server.verify_mutation_authority("prgs") self.assertIn("side-channel override rejected", str(ctx.exception)) def test_foreign_pid_authority_is_not_trusted(self): @@ -3044,8 +3237,14 @@ class TestIssueLocking(unittest.TestCase): def setUp(self): self._env_patcher = patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True) self._env_patcher.start() + self._dup_fetcher_patcher = patch( + "mcp_server.issue_duplicate_context_fetcher", + return_value=([], [], {"status": "not_claimed"}), + ) + self.mock_dup_fetcher = self._dup_fetcher_patcher.start() def tearDown(self): + self._dup_fetcher_patcher.stop() self._env_patcher.stop() if os.path.exists(ISSUE_LOCK_FILE): os.remove(ISSUE_LOCK_FILE) @@ -3054,10 +3253,8 @@ class TestIssueLocking(unittest.TestCase): "mcp_server.issue_lock_worktree.read_worktree_git_state", return_value=_clean_master_git_state_for_lock(), ) - @patch("mcp_server.api_get_all") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) - def test_lock_issue_success(self, _auth, mock_api, _git_state): - mock_api.return_value = [] # no open PRs + def test_lock_issue_success(self, _auth, _git_state): res = gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") self.assertTrue(res["success"]) self.assertEqual(res["work_lease"]["operation_type"], "author_issue_work") @@ -3082,50 +3279,48 @@ class TestIssueLocking(unittest.TestCase): "mcp_server.issue_lock_worktree.read_worktree_git_state", return_value=_clean_master_git_state_for_lock(), ) - @patch("mcp_server.api_get_all") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) - def test_lock_issue_reused_by_open_pr_branch(self, _auth, mock_api, _git_state): - mock_api.return_value = [{ + def test_lock_issue_reused_by_open_pr_branch(self, _auth, _git_state): + self.mock_dup_fetcher.return_value = ([{ "number": 200, "head": {"ref": "feat/issue-196-boundary"}, "title": "Some PR", - "body": "No closes ref" - }] + "body": "No closes ref", + }], [], {"status": "not_claimed"}) with self.assertRaises(ValueError) as ctx: gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") - self.assertIn("already tied to an open PR", str(ctx.exception)) + self.assertIn("open PR #200 already covers issue", str(ctx.exception)) @patch( "mcp_server.issue_lock_worktree.read_worktree_git_state", return_value=_clean_master_git_state_for_lock(), ) - @patch("mcp_server.api_get_all") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) - def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, mock_api, _git_state): - mock_api.return_value = [{ + def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, _git_state): + self.mock_dup_fetcher.return_value = ([{ "number": 200, "head": {"ref": "feat/other-branch"}, "title": "Some PR", - "body": "fixes #196" - }] + "body": "fixes #196", + }], [], {"status": "not_claimed"}) with self.assertRaises(ValueError) as ctx: gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") - self.assertIn("already tied to an open PR", str(ctx.exception)) + self.assertIn("open PR #200 already covers issue", str(ctx.exception)) @patch( "mcp_server.issue_lock_worktree.read_worktree_git_state", return_value=_clean_master_git_state_for_lock(), ) - @patch("mcp_server.api_get_all") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) - def test_lock_issue_reused_by_remote_branch(self, _auth, mock_api, _git_state): - mock_api.side_effect = [ + def test_lock_issue_reused_by_remote_branch(self, _auth, _git_state): + self.mock_dup_fetcher.return_value = ( [], - [{"name": "feat/issue-196-existing-work"}], - ] + ["feat/issue-196-existing-work"], + {"status": "not_claimed"}, + ) with self.assertRaises(ValueError) as ctx: gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") - self.assertIn("already has matching branch", str(ctx.exception)) + self.assertIn("remote branch(es) already match issue pattern", str(ctx.exception)) def test_lock_issue_blocks_active_same_operation_lease(self): with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: @@ -3284,6 +3479,28 @@ class TestIssueLocking(unittest.TestCase): ) self.assertIn("does not match locked worktree", str(ctx.exception)) + @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", + return_value=(True, [])) + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_create_pr_manual_lock_seed_blocked(self, _auth, _role): + with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f: + json.dump( + _sample_issue_lock( + issue_number=447, + branch_name="feat/issue-447-lock-provenance", + lock_provenance=None, + ), + f, + ) + with patch.dict(os.environ, CREATE_PR_ENV, clear=True): + with self.assertRaises(RuntimeError) as ctx: + gitea_create_pr( + title="feat: lock provenance Closes #447", + head="feat/issue-447-lock-provenance", + remote="prgs", + ) + self.assertIn("lock provenance", str(ctx.exception).lower()) + @patch("mcp_server.api_request") @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", return_value=(True, [])) diff --git a/tests/test_pr_queue_inventory.py b/tests/test_pr_queue_inventory.py index aeb8bc4..1e5a3c5 100644 --- a/tests/test_pr_queue_inventory.py +++ b/tests/test_pr_queue_inventory.py @@ -156,9 +156,22 @@ class TestPRQueueInventory(unittest.TestCase): ] from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision - init_review_decision_lock("prgs", "review_pr") - gitea_mark_final_review_decision(1, "approve", remote="prgs") - result = gitea_review_pr(pr_number=1, event="APPROVE", remote="prgs", final_review_decision_ready=True) + from tests.test_mcp_server import _install_owned_reviewer_lease + import reviewer_pr_lease + + with patch("mcp_server._authenticated_username", return_value="reviewer1"): + init_review_decision_lock("prgs", "review_pr") + gitea_mark_final_review_decision(1, "approve", remote="prgs") + lease_patch = _install_owned_reviewer_lease( + 1, head_sha="abc1", session_id="inventory-review-lease", + ) + lease_patch.start() + self.addCleanup(lease_patch.stop) + self.addCleanup(reviewer_pr_lease.clear_session_lease) + result = gitea_review_pr( + pr_number=1, event="APPROVE", remote="prgs", + final_review_decision_ready=True, + ) self.assertTrue(result["success"]) self.assertIn("=== PR Queue Inventory ===", result["message"]) self.assertIn("Repository:", result["message"]) diff --git a/tests/test_review_proofs.py b/tests/test_review_proofs.py index f132fc2..03d3a6a 100644 --- a/tests/test_review_proofs.py +++ b/tests/test_review_proofs.py @@ -2346,6 +2346,7 @@ class TestWorkIssueFinalReport(unittest.TestCase): "- Safe next action: open PR", "- Next: open PR", "- Safety statement: no review/merge", + "- Duplicate work outcome: duplicate work not prevented", ]) def test_complete_work_issue_report_earns_a(self): diff --git a/tests/test_reviewer_pr_lease.py b/tests/test_reviewer_pr_lease.py new file mode 100644 index 0000000..69ee6d5 --- /dev/null +++ b/tests/test_reviewer_pr_lease.py @@ -0,0 +1,193 @@ +"""Tests for per-PR reviewer leases (#407).""" + +import sys +import unittest +from datetime import datetime, timedelta, timezone +from unittest.mock import patch + +sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) + +import reviewer_pr_lease as leases + + +def _lease_comment( + pr_number: int, + session_id: str, + *, + phase: str = "claimed", + minutes_ago: int = 0, + candidate_head: str = "a" * 40, +) -> dict: + now = datetime.now(timezone.utc) - timedelta(minutes=minutes_ago) + body = leases.format_lease_body( + repo="Scaled-Tech-Consulting/Gitea-Tools", + pr_number=pr_number, + issue_number=295, + reviewer_identity="rev1", + profile="prgs-reviewer", + session_id=session_id, + worktree="branches/review-pr382", + phase=phase, + candidate_head=candidate_head, + target_branch="master", + target_branch_sha="b" * 40, + last_activity=now, + ) + return {"id": 1, "body": body, "user": {"login": "rev1"}} + + +class TestReviewerLeaseAcquire(unittest.TestCase): + def setUp(self): + leases.clear_session_lease() + + def test_two_reviewers_cannot_lease_same_pr(self): + comments = [_lease_comment(382, "session-a")] + result = leases.assess_acquire_lease( + comments, + pr_number=382, + reviewer_identity="rev2", + profile="prgs-reviewer", + session_id="session-b", + repo="Scaled-Tech-Consulting/Gitea-Tools", + issue_number=295, + worktree="branches/review-pr382-b", + candidate_head="c" * 40, + target_branch="master", + target_branch_sha="d" * 40, + ) + self.assertFalse(result["acquire_allowed"]) + self.assertTrue(any("already has active" in r for r in result["reasons"])) + + def test_two_reviewers_can_lease_different_prs(self): + comments = [_lease_comment(382, "session-a")] + result = leases.assess_acquire_lease( + comments, + pr_number=383, + reviewer_identity="rev2", + profile="prgs-reviewer", + session_id="session-b", + repo="Scaled-Tech-Consulting/Gitea-Tools", + issue_number=296, + worktree="branches/review-pr383", + candidate_head="c" * 40, + target_branch="master", + target_branch_sha="d" * 40, + ) + self.assertTrue(result["acquire_allowed"]) + self.assertIsNotNone(result["lease_body"]) + + +class TestReviewerLeaseFreshness(unittest.TestCase): + def test_stale_warning_after_30_minutes(self): + lease = leases.parse_lease_comment( + _lease_comment(382, "session-a", minutes_ago=35)["body"] + ) + self.assertEqual( + leases.classify_lease_freshness(lease), + "stale_warning", + ) + + def test_reclaimable_after_60_minutes(self): + lease = leases.parse_lease_comment( + _lease_comment(382, "session-a", minutes_ago=65)["body"] + ) + self.assertEqual( + leases.classify_lease_freshness(lease), + "reclaimable", + ) + + +class TestReviewerLeaseMutationGate(unittest.TestCase): + def setUp(self): + leases.clear_session_lease() + + def test_reviewer_without_lease_cannot_mutate(self): + head = "f" * 40 + comments = [_lease_comment(382, "other-session", candidate_head=head)] + result = leases.assess_mutation_lease_gate( + pr_number=382, + comments=comments, + reviewer_identity="rev1", + session_id="my-session", + mutation="approve", + live_head_sha=head, + pinned_head_sha=head, + ) + self.assertTrue(result["block"]) + + def test_owned_lease_allows_mutation(self): + head = "f" * 40 + comments = [_lease_comment(382, "my-session", candidate_head=head)] + leases.record_session_lease({ + "pr_number": 382, + "session_id": "my-session", + "candidate_head": head, + "target_branch": "master", + }) + result = leases.assess_mutation_lease_gate( + pr_number=382, + comments=comments, + reviewer_identity="rev1", + session_id="my-session", + mutation="approve", + live_head_sha=head, + pinned_head_sha=head, + ) + self.assertFalse(result["block"]) + + def test_head_change_invalidates_lease(self): + reviewed = "f" * 40 + live = "e" * 40 + comments = [_lease_comment(382, "my-session", candidate_head=reviewed)] + leases.record_session_lease({ + "pr_number": 382, + "session_id": "my-session", + "candidate_head": reviewed, + }) + result = leases.assess_mutation_lease_gate( + pr_number=382, + comments=comments, + reviewer_identity="rev1", + session_id="my-session", + mutation="merge", + live_head_sha=live, + pinned_head_sha=reviewed, + ) + self.assertTrue(result["block"]) + self.assertTrue(any("head" in r.lower() for r in result["reasons"])) + + +class TestReviewerLeaseMcpGate(unittest.TestCase): + def setUp(self): + leases.clear_session_lease() + patch("mcp_server.verify_preflight_purity").start() + patch("gitea_audit.audit_enabled", return_value=False).start() + mcp_server = __import__("mcp_server") + mcp_server._IDENTITY_CACHE.clear() + mcp_server.init_review_decision_lock("prgs", "review_pr") + mcp_server.record_preflight_check("whoami") + mcp_server.record_preflight_check("capability", "reviewer") + + def tearDown(self): + patch.stopall() + leases.clear_session_lease() + + def test_reviewer_pr_lease_gate_helper_blocks_without_session(self): + import mcp_server + head = "a" * 40 + with patch("mcp_server._fetch_pr_comments", return_value=[]): + reasons = mcp_server._reviewer_pr_lease_gate( + pr_number=382, + remote="prgs", + host=None, + org=None, + repo=None, + mutation="approve", + live_head_sha=head, + pinned_head_sha=head, + ) + self.assertTrue(any("lease" in r.lower() for r in reasons)) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_webui_project_registry.py b/tests/test_webui_project_registry.py new file mode 100644 index 0000000..f16774a --- /dev/null +++ b/tests/test_webui_project_registry.py @@ -0,0 +1,111 @@ +"""Tests for web UI project registry (#427).""" +import json +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from starlette.testclient import TestClient + +from webui.app import create_app +from webui.project_registry import ( + default_registry_path, + load_registry, + project_to_dict, +) + + +class TestProjectRegistryLoader(unittest.TestCase): + def test_default_registry_loads_gitea_tools(self): + registry = load_registry() + self.assertEqual(registry.version, 1) + self.assertEqual(len(registry.projects), 1) + project = registry.projects[0] + self.assertEqual(project.id, "gitea-tools") + self.assertEqual(project.repo_name, "Gitea-Tools") + self.assertEqual(project.gitea_owner, "Scaled-Tech-Consulting") + self.assertEqual(project.remote_host, "https://gitea.prgs.cc") + self.assertEqual(project.profiles["author"], "prgs-author") + self.assertEqual(project.profiles["reviewer"], "prgs-reviewer") + self.assertEqual(project.profiles["reconciler"], "prgs-reconciler") + self.assertIn("skill", project.workflow_paths) + self.assertGreaterEqual(len(project.onboarding_checklist), 4) + + def test_registry_rejects_credential_keys(self): + payload = { + "version": 1, + "projects": [ + { + "id": "bad", + "repo_name": "Bad", + "gitea_owner": "Org", + "remote_host": "https://gitea.example.invalid", + "default_branch": "main", + "local_checkout_path": ".", + "profiles": { + "author": "a", + "reviewer": "r", + "reconciler": "c", + }, + "workflow_paths": {"skill": "skills/x.md"}, + "api_token": "secret", + } + ], + } + with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as handle: + json.dump(payload, handle) + path = Path(handle.name) + try: + with self.assertRaises(ValueError): + load_registry(path) + finally: + path.unlink(missing_ok=True) + + def test_default_registry_path_points_at_packaged_data(self): + path = default_registry_path() + self.assertTrue(path.name == "projects.registry.json") + self.assertTrue(path.parent.name == "data") + + +class TestProjectRegistryRoutes(unittest.TestCase): + def setUp(self): + self.client = TestClient(create_app()) + + def test_projects_page_lists_gitea_tools(self): + response = self.client.get("/projects") + self.assertEqual(response.status_code, 200) + self.assertIn("Gitea-Tools", response.text) + self.assertIn("Scaled-Tech-Consulting", response.text) + self.assertIn("prgs-author", response.text) + self.assertNotIn("child issue", response.text.lower()) + + def test_project_detail_renders_checklist(self): + response = self.client.get("/projects/gitea-tools") + self.assertEqual(response.status_code, 200) + self.assertIn("Onboarding checklist", response.text) + self.assertIn("Configure execution profiles", response.text) + self.assertIn("branches/", response.text) + + def test_project_detail_404(self): + response = self.client.get("/projects/unknown-repo") + self.assertEqual(response.status_code, 404) + + def test_api_projects_json(self): + response = self.client.get("/api/projects") + self.assertEqual(response.status_code, 200) + data = response.json() + self.assertEqual(data["version"], 1) + self.assertEqual(len(data["projects"]), 1) + self.assertEqual(data["projects"][0]["id"], "gitea-tools") + self.assertIn("onboarding_checklist", data["projects"][0]) + + def test_project_to_dict_is_json_safe(self): + registry = load_registry() + encoded = json.dumps(project_to_dict(registry.projects[0])) + self.assertIn("gitea-tools", encoded) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_webui_prompt_library.py b/tests/test_webui_prompt_library.py new file mode 100644 index 0000000..3538c2d --- /dev/null +++ b/tests/test_webui_prompt_library.py @@ -0,0 +1,90 @@ +"""Tests for web UI prompt library (#428).""" +import json +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from starlette.testclient import TestClient + +from webui.app import create_app +from webui.prompt_library import find_prompt, library_to_dict, load_prompt_library, prompt_to_dict + +REQUIRED_PROMPT_SLUGS = frozenset({ + "review-pr", + "work-issue", + "create-issue", + "comment-issue", + "cleanup", + "audit", + "onboarding", +}) + + +class TestPromptLibraryLoader(unittest.TestCase): + def test_library_loads_required_prompts(self): + entries = load_prompt_library() + slugs = {entry.slug for entry in entries} + self.assertEqual(slugs, REQUIRED_PROMPT_SLUGS) + + def test_workflow_hashes_present(self): + review = find_prompt("review-pr") + self.assertIsNotNone(review) + assert review is not None + self.assertTrue(review.workflow_hash) + self.assertEqual(len(review.workflow_hash), 64) + + def test_prompt_text_is_short(self): + for entry in load_prompt_library(): + self.assertLess(len(entry.prompt_text), 400) + self.assertNotIn("Do not improvise around the gates", entry.prompt_text) + + +class TestPromptLibraryRoutes(unittest.TestCase): + def setUp(self): + self.client = TestClient(create_app()) + + def test_prompts_page_lists_all_entries(self): + response = self.client.get("/prompts") + self.assertEqual(response.status_code, 200) + for label in ( + "Review PR", + "Work issue", + "Create issue", + "Comment on issue", + "Post-merge cleanup", + "Reconciliation audit", + "Project onboarding", + ): + self.assertIn(label, response.text) + self.assertIn("Copy prompt", response.text) + self.assertIn("sha256:", response.text) + self.assertNotIn("child issue", response.text.lower()) + + def test_prompt_detail_route(self): + response = self.client.get("/prompts/work-issue") + self.assertEqual(response.status_code, 200) + self.assertIn("work-issue.md", response.text) + self.assertIn("Copy prompt", response.text) + + def test_prompt_detail_404(self): + self.assertEqual(self.client.get("/prompts/missing").status_code, 404) + + def test_api_prompts_json(self): + response = self.client.get("/api/prompts") + self.assertEqual(response.status_code, 200) + data = response.json() + self.assertEqual(data["count"], 7) + review = next(item for item in data["prompts"] if item["slug"] == "review-pr") + self.assertIn("workflow_hash", review) + self.assertIn("review-merge-pr.md", review["workflow_path"]) + + def test_prompt_to_dict_roundtrip(self): + entry = load_prompt_library()[0] + encoded = json.dumps(prompt_to_dict(entry)) + self.assertIn("workflow_path", encoded) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_webui_queue_dashboard.py b/tests/test_webui_queue_dashboard.py new file mode 100644 index 0000000..cbdcc16 --- /dev/null +++ b/tests/test_webui_queue_dashboard.py @@ -0,0 +1,288 @@ +"""Tests for web UI live queue dashboard (#429).""" +import sys +import unittest +from datetime import datetime, timedelta, timezone +from pathlib import Path +from unittest import mock + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from starlette.testclient import TestClient + +from webui.app import create_app +from webui.queue_loader import ( + PaginationMeta, + QueueSnapshot, + _classify_issue, + _classify_pr, + _extract_linked_issue, + load_queue_snapshot, + snapshot_to_dict, +) +from webui.queue_views import render_queue_page + +_RECENT = datetime.now(timezone.utc).isoformat() +_STALE = (datetime.now(timezone.utc) - timedelta(days=30)).isoformat() + +_SAMPLE_PRS = [ + { + "number": 100, + "title": "feat: queue dashboard (Closes #429)", + "body": "", + "mergeable": True, + "updated_at": _RECENT, + "head": {"ref": "feat/issue-429", "sha": "abc123def456"}, + "base": {"ref": "master"}, + }, + { + "number": 99, + "title": "fix: conflict", + "body": "Closes #50", + "mergeable": False, + "updated_at": _STALE, + "head": {"ref": "fix/issue-50", "sha": "deadbeef0001"}, + "base": {"ref": "master"}, + }, + { + "number": 98, + "title": "feat: duplicate A (Closes #60)", + "body": "", + "mergeable": True, + "updated_at": _RECENT, + "head": {"ref": "feat/issue-60-a", "sha": "111111111111"}, + "base": {"ref": "master"}, + }, + { + "number": 97, + "title": "feat: duplicate B (Closes #60)", + "body": "", + "mergeable": True, + "updated_at": _RECENT, + "head": {"ref": "feat/issue-60-b", "sha": "222222222222"}, + "base": {"ref": "master"}, + }, +] + +_SAMPLE_ISSUES = [ + { + "number": 429, + "title": "Web UI queue dashboard", + "state": "open", + "labels": [{"name": "status:in-progress"}], + "assignee": None, + "updated_at": _RECENT, + }, + { + "number": 60, + "title": "Duplicate PR target", + "state": "open", + "labels": [], + "assignee": None, + "updated_at": _RECENT, + }, + { + "number": 50, + "title": "Blocked PR target", + "state": "open", + "labels": [], + "assignee": None, + "updated_at": _STALE, + }, +] + + +def _mock_pagination(count: int) -> PaginationMeta: + return PaginationMeta( + page=1, + per_page=50, + returned_count=count, + has_more=False, + is_final_page=True, + inventory_complete=True, + pages_fetched=1, + ) + + +def _mock_fetch_prs(*_args, **_kwargs): + return list(_SAMPLE_PRS), _mock_pagination(len(_SAMPLE_PRS)) + + +def _mock_fetch_issues(*_args, **_kwargs): + return list(_SAMPLE_ISSUES), _mock_pagination(len(_SAMPLE_ISSUES)) + + +class TestQueueClassification(unittest.TestCase): + def test_extract_linked_issue_from_title(self): + self.assertEqual( + _extract_linked_issue("feat: X (Closes #429)", ""), + 429, + ) + + def test_classify_pr_blocked_and_stale(self): + pr = _SAMPLE_PRS[1] + badges = _classify_pr( + pr, + issue_claimed=set(), + issue_to_prs={50: [99]}, + ) + self.assertIn("blocked", badges) + self.assertIn("stale", badges) + + def test_classify_pr_duplicate(self): + badges = _classify_pr( + _SAMPLE_PRS[2], + issue_claimed=set(), + issue_to_prs={60: [98, 97]}, + ) + self.assertIn("duplicate", badges) + self.assertIn("in-review", badges) + + def test_classify_issue_claimed_and_duplicate(self): + claimed = _classify_issue(_SAMPLE_ISSUES[0], linked_prs=[]) + self.assertIn("claimed", claimed) + duplicate = _classify_issue(_SAMPLE_ISSUES[1], linked_prs=[98, 97]) + self.assertIn("duplicate", duplicate) + + +class TestQueueLoader(unittest.TestCase): + def test_snapshot_with_mock_fetch(self): + snapshot = load_queue_snapshot( + fetch_prs=_mock_fetch_prs, + fetch_issues=_mock_fetch_issues, + ) + self.assertEqual(snapshot.project_id, "gitea-tools") + self.assertIsNone(snapshot.fetch_error) + self.assertEqual(len(snapshot.prs), 4) + self.assertEqual(len(snapshot.issues), 3) + self.assertTrue(snapshot.pr_pagination.inventory_complete) + self.assertTrue(snapshot.issue_pagination.inventory_complete) + + pr100 = next(p for p in snapshot.prs if p.number == 100) + self.assertEqual(pr100.extra["linked_issue"], "429") + self.assertIn("claimed", pr100.badges) + + def test_snapshot_dict_export(self): + snapshot = load_queue_snapshot( + fetch_prs=_mock_fetch_prs, + fetch_issues=_mock_fetch_issues, + ) + data = snapshot_to_dict(snapshot) + self.assertEqual(data["project_id"], "gitea-tools") + self.assertIsNone(data["fetch_error"]) + self.assertEqual(len(data["prs"]), 4) + self.assertTrue(data["pagination"]["prs"]["inventory_complete"]) + + +class TestQueueRoutes(unittest.TestCase): + def setUp(self): + self.client = TestClient(create_app()) + self._patch = mock.patch( + "webui.app.load_queue_snapshot", + return_value=load_queue_snapshot( + fetch_prs=_mock_fetch_prs, + fetch_issues=_mock_fetch_issues, + ), + ) + self._patch.start() + + def tearDown(self): + self._patch.stop() + + def test_queue_page_renders_tables_and_pagination(self): + response = self.client.get("/queue") + self.assertEqual(response.status_code, 200) + self.assertIn("Live queue", response.text) + self.assertIn("Open pull requests", response.text) + self.assertIn("Open issues", response.text) + self.assertIn("pages_fetched", response.text) + self.assertIn("Gitea-Tools", response.text) + self.assertNotIn("child issue", response.text.lower()) + + def test_api_queue_json(self): + response = self.client.get("/api/queue") + self.assertEqual(response.status_code, 200) + data = response.json() + self.assertEqual(data["repo"], "Scaled-Tech-Consulting/Gitea-Tools") + self.assertEqual(data["pagination"]["issues"]["returned_count"], 3) + + def test_queue_fail_closed_without_credentials(self): + with mock.patch("webui.queue_loader.get_auth_header", return_value=None): + snapshot = load_queue_snapshot() + self.assertIsNotNone(snapshot.fetch_error) + self.assertEqual(len(snapshot.prs), 0) + + +def _empty_pagination() -> PaginationMeta: + return PaginationMeta( + page=1, + per_page=50, + returned_count=0, + has_more=False, + is_final_page=True, + inventory_complete=True, + pages_fetched=1, + ) + + +def _empty_fetch(*_args, **_kwargs): + return [], _empty_pagination() + + +class TestQueueFailClosedUx(unittest.TestCase): + """Regression tests for #458 fail-closed empty-state copy.""" + + def test_fail_closed_view_suppresses_empty_queue_copy(self): + snapshot = QueueSnapshot( + project_id="gitea-tools", + repo_label="Scaled-Tech-Consulting/Gitea-Tools", + prs=(), + issues=(), + pr_pagination=None, + issue_pagination=None, + fetch_error="Gitea credentials unavailable for gitea.prgs.cc", + ) + html = render_queue_page(snapshot) + self.assertIn("Queue unavailable", html) + self.assertIn("Not loaded", html) + self.assertNotIn("No open items.", html) + self.assertIn("pagination: unavailable", html) + + def test_fail_closed_route_does_not_show_empty_queue_copy(self): + client = TestClient(create_app()) + snapshot = load_queue_snapshot( + fetch_prs=_empty_fetch, + fetch_issues=_empty_fetch, + ) + snapshot = QueueSnapshot( + project_id=snapshot.project_id, + repo_label=snapshot.repo_label, + prs=(), + issues=(), + pr_pagination=None, + issue_pagination=None, + fetch_error="Gitea credentials unavailable for gitea.prgs.cc", + ) + with mock.patch("webui.app.load_queue_snapshot", return_value=snapshot): + response = client.get("/queue") + self.assertEqual(response.status_code, 200) + self.assertIn("Queue unavailable", response.text) + self.assertNotIn("No open items.", response.text) + + def test_successful_empty_inventory_shows_empty_copy(self): + snapshot = load_queue_snapshot( + fetch_prs=_empty_fetch, + fetch_issues=_empty_fetch, + ) + self.assertIsNone(snapshot.fetch_error) + self.assertEqual(len(snapshot.prs), 0) + self.assertEqual(len(snapshot.issues), 0) + self.assertTrue(snapshot.pr_pagination.inventory_complete) + + html = render_queue_page(snapshot) + self.assertNotIn("Queue unavailable", html) + self.assertEqual(html.count("No open items."), 2) + self.assertIn("pagination (complete)", html) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_webui_skeleton.py b/tests/test_webui_skeleton.py new file mode 100644 index 0000000..5dd23a1 --- /dev/null +++ b/tests/test_webui_skeleton.py @@ -0,0 +1,73 @@ +"""Tests for internal web UI skeleton (#426).""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from starlette.testclient import TestClient + +from webui.app import create_app + + +class TestWebuiSkeleton(unittest.TestCase): + def setUp(self): + self.client = TestClient(create_app()) + + def test_health_returns_json(self): + response = self.client.get("/health") + self.assertEqual(response.status_code, 200) + data = response.json() + self.assertEqual(data["status"], "ok") + self.assertEqual(data["service"], "mcp-control-plane-webui") + self.assertEqual(data["mode"], "read-only-mvp") + self.assertIn("timestamp", data) + + def test_home_renders(self): + response = self.client.get("/") + self.assertEqual(response.status_code, 200) + self.assertIn("Operator console", response.text) + self.assertIn("Read-only MVP", response.text) + + def test_route_stubs_render(self): + for path in ("/runtime", "/audit"): + with self.subTest(path=path): + response = self.client.get(path) + self.assertEqual(response.status_code, 200) + self.assertIn("child issue", response.text.lower()) + + def test_prompts_is_implemented(self): + response = self.client.get("/prompts") + self.assertEqual(response.status_code, 200) + self.assertIn("Prompt library", response.text) + + def test_projects_is_implemented(self): + response = self.client.get("/projects") + self.assertEqual(response.status_code, 200) + self.assertIn("Gitea-Tools", response.text) + + def test_extra_stub_routes(self): + for path in ("/worktrees", "/leases"): + with self.subTest(path=path): + self.assertEqual(self.client.get(path).status_code, 200) + + def test_post_is_rejected(self): + response = self.client.post("/health") + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json()["error"], "read-only-mvp") + + def test_queue_route_renders(self): + response = self.client.get("/queue") + self.assertEqual(response.status_code, 200) + self.assertIn("Live queue", response.text) + + def test_nav_links_on_all_pages(self): + for path in ("/", "/queue", "/projects", "/prompts", "/runtime", "/audit"): + with self.subTest(path=path): + text = self.client.get(path).text + for href in ("/queue", "/projects", "/prompts", "/runtime", "/audit"): + self.assertIn(f'href="{href}"', text) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/webui/__init__.py b/webui/__init__.py new file mode 100644 index 0000000..56829f0 --- /dev/null +++ b/webui/__init__.py @@ -0,0 +1,5 @@ +"""Internal MCP Control Plane web UI (read-only MVP skeleton, #426).""" + +from webui.app import create_app + +__all__ = ["create_app"] \ No newline at end of file diff --git a/webui/__main__.py b/webui/__main__.py new file mode 100644 index 0000000..c55b05e --- /dev/null +++ b/webui/__main__.py @@ -0,0 +1,19 @@ +"""Run the internal web UI: ``python -m webui``.""" + +from __future__ import annotations + +import os + +import uvicorn + +from webui.app import create_app + + +def main() -> None: + host = os.environ.get("WEBUI_HOST", "127.0.0.1") + port = int(os.environ.get("WEBUI_PORT", "8765")) + uvicorn.run(create_app(), host=host, port=port, log_level="info") + + +if __name__ == "__main__": + main() \ No newline at end of file diff --git a/webui/app.py b/webui/app.py new file mode 100644 index 0000000..a97d1ad --- /dev/null +++ b/webui/app.py @@ -0,0 +1,180 @@ +"""Starlette application for the internal read-only web UI (#426).""" + +from __future__ import annotations + +from datetime import datetime, timezone + +from starlette.applications import Starlette +from starlette.requests import Request +from starlette.responses import HTMLResponse, JSONResponse, Response +from starlette.routing import Route + +from webui.layout import render_page +from webui.project_registry import find_project, load_registry, registry_to_dict +from webui.project_views import render_project_detail, render_projects_list +from webui.prompt_library import find_prompt, library_to_dict +from webui.prompt_views import render_prompt_detail, render_prompts_page +from webui.queue_loader import load_queue_snapshot, snapshot_to_dict +from webui.queue_views import render_queue_page + +_READ_ONLY_METHODS = frozenset({"GET", "HEAD", "OPTIONS"}) + + +def _stub_page(title: str, description: str) -> HTMLResponse: + body = ( + f"

{title}

" + f'

{description}

' + "

Implementation tracked in a child issue of #425.

" + ) + return HTMLResponse(render_page(title=title, body_html=body)) + + +async def home(_request: Request) -> HTMLResponse: + body = ( + "

Operator console

" + "

Local entry point for MCP Control Plane operational views.

" + "" + ) + return HTMLResponse(render_page(title="Home", body_html=body)) + + +async def health(_request: Request) -> JSONResponse: + return JSONResponse({ + "status": "ok", + "service": "mcp-control-plane-webui", + "mode": "read-only-mvp", + "timestamp": datetime.now(timezone.utc).isoformat(), + }) + + +async def queue(_request: Request) -> HTMLResponse: + snapshot = load_queue_snapshot() + return HTMLResponse(render_page(title="Queue", body_html=render_queue_page(snapshot))) + + +async def api_queue(_request: Request) -> JSONResponse: + return JSONResponse(snapshot_to_dict(load_queue_snapshot())) + + +async def projects(_request: Request) -> HTMLResponse: + registry = load_registry() + return HTMLResponse(render_projects_list(registry)) + + +async def project_detail(request: Request) -> HTMLResponse: + project_id = request.path_params["project_id"] + registry = load_registry() + project = find_project(registry, project_id) + if project is None: + return HTMLResponse( + render_page( + title="Project not found", + body_html=( + "

Project not found

" + f"

No registry entry for {project_id}.

" + '

← All projects

' + ), + ), + status_code=404, + ) + return HTMLResponse(render_project_detail(project)) + + +async def api_projects(_request: Request) -> JSONResponse: + registry = load_registry() + return JSONResponse(registry_to_dict(registry)) + + +async def prompts(_request: Request) -> HTMLResponse: + return HTMLResponse(render_prompts_page()) + + +async def prompt_detail(request: Request) -> HTMLResponse: + prompt_id = request.path_params["prompt_id"] + prompt = find_prompt(prompt_id) + if prompt is None: + return HTMLResponse( + render_page( + title="Prompt not found", + body_html=( + "

Prompt not found

" + f"

No library entry for {prompt_id}.

" + '

← All prompts

' + ), + ), + status_code=404, + ) + return HTMLResponse(render_prompt_detail(prompt)) + + +async def api_prompts(_request: Request) -> JSONResponse: + return JSONResponse(library_to_dict()) + + +async def runtime(_request: Request) -> HTMLResponse: + return _stub_page( + "Runtime", + "Runtime health will report MCP profile, preflight, and stale-server signals.", + ) + + +async def audit(_request: Request) -> HTMLResponse: + return _stub_page( + "Audit", + "Report audit will accept pasted final reports and run validator previews.", + ) + + +async def worktrees(_request: Request) -> HTMLResponse: + return _stub_page( + "Worktrees", + "Worktree hygiene will summarize branches/ session folders and cleanup risk.", + ) + + +async def leases(_request: Request) -> HTMLResponse: + return _stub_page( + "Leases", + "Lease visibility will show active issue and reviewer PR leases.", + ) + + +async def method_not_allowed(request: Request, _exc: Exception) -> Response: + if request.method not in _READ_ONLY_METHODS: + return JSONResponse( + {"error": "read-only-mvp", "detail": f"{request.method} not permitted"}, + status_code=405, + ) + return JSONResponse({"error": "not_found"}, status_code=404) + + +def create_app() -> Starlette: + """Build the read-only MVP Starlette app.""" + return Starlette( + debug=False, + routes=[ + Route("/", home, methods=["GET"]), + Route("/health", health, methods=["GET"]), + Route("/queue", queue, methods=["GET"]), + Route("/api/queue", api_queue, methods=["GET"]), + Route("/projects", projects, methods=["GET"]), + Route("/projects/{project_id}", project_detail, methods=["GET"]), + Route("/api/projects", api_projects, methods=["GET"]), + Route("/prompts", prompts, methods=["GET"]), + Route("/prompts/{prompt_id}", prompt_detail, methods=["GET"]), + Route("/api/prompts", api_prompts, methods=["GET"]), + Route("/runtime", runtime, methods=["GET"]), + Route("/audit", audit, methods=["GET"]), + Route("/worktrees", worktrees, methods=["GET"]), + Route("/leases", leases, methods=["GET"]), + ], + exception_handlers={405: method_not_allowed}, + ) \ No newline at end of file diff --git a/webui/data/projects.registry.json b/webui/data/projects.registry.json new file mode 100644 index 0000000..cd6c852 --- /dev/null +++ b/webui/data/projects.registry.json @@ -0,0 +1,49 @@ +{ + "version": 1, + "projects": [ + { + "id": "gitea-tools", + "repo_name": "Gitea-Tools", + "gitea_owner": "Scaled-Tech-Consulting", + "remote_host": "https://gitea.prgs.cc", + "default_branch": "master", + "local_checkout_path": ".", + "profiles": { + "author": "prgs-author", + "reviewer": "prgs-reviewer", + "reconciler": "prgs-reconciler" + }, + "workflow_paths": { + "skill": "skills/llm-project-workflow/SKILL.md", + "work_issue": "skills/llm-project-workflow/workflows/work-issue.md", + "review_merge": "skills/llm-project-workflow/workflows/review-merge-pr.md" + }, + "schema_paths": { + "mcp_config_v2": "gitea-mcp.v2-contexts.example.json", + "mcp_config_v1": "gitea-mcp.example.json" + }, + "onboarding_checklist": [ + { + "id": "profiles", + "title": "Configure execution profiles", + "description": "Install author, reviewer, and reconciler MCP profiles (prgs-author, prgs-reviewer, prgs-reconciler) in separate namespaces. Tokens stay in keychain — never in this registry." + }, + { + "id": "mcp_config", + "title": "Wire MCP v2 contexts", + "description": "Copy and customize gitea-mcp.v2-contexts.example.json for your machine. Map this repo path under projects with default_owner Scaled-Tech-Consulting and default_repo Gitea-Tools." + }, + { + "id": "wiki_gate", + "title": "Wiki publication readiness", + "description": "For wiki-tracked work, satisfy the live Gitea Wiki proof gate (#224) before closing issues. See docs/wiki/Safety-and-Gates.md." + }, + { + "id": "branches_layout", + "title": "Isolate work under branches/", + "description": "All LLM task edits happen in worktrees under branches/. Main checkout stays clean; use skills/llm-project-workflow templates for start-issue and review flows." + } + ] + } + ] +} \ No newline at end of file diff --git a/webui/layout.py b/webui/layout.py new file mode 100644 index 0000000..a506c2c --- /dev/null +++ b/webui/layout.py @@ -0,0 +1,176 @@ +"""Shared HTML layout for the internal web UI.""" + +from __future__ import annotations + +NAV_ITEMS = ( + ("/", "Home"), + ("/queue", "Queue"), + ("/projects", "Projects"), + ("/prompts", "Prompts"), + ("/runtime", "Runtime"), + ("/audit", "Audit"), + ("/worktrees", "Worktrees"), + ("/leases", "Leases"), +) + +MVP_NOTICE = ( + "Read-only MVP — Gitea, MCP tools, and canonical workflows remain the " + "source of truth. No mutation endpoints." +) + + +def render_page(*, title: str, body_html: str, extra_head: str = "") -> str: + nav_links = "".join( + f'{label}' for href, label in NAV_ITEMS + ) + return f""" + + + + + {title} · MCP Control Plane + + {extra_head} + + +
+

MCP Control Plane

+ +
+
+

{MVP_NOTICE}

+ {body_html} +
+ +""" \ No newline at end of file diff --git a/webui/project_registry.py b/webui/project_registry.py new file mode 100644 index 0000000..2c5e889 --- /dev/null +++ b/webui/project_registry.py @@ -0,0 +1,196 @@ +"""Load and validate the web UI project registry (#427).""" + +from __future__ import annotations + +import json +import os +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +_FORBIDDEN_EXACT_KEYS = frozenset({ + "token", + "password", + "secret", + "credential", + "auth", + "api_key", + "api-key", +}) +_FORBIDDEN_KEY_PREFIXES = ("auth_", "api_key_", "api-key_") +_FORBIDDEN_KEY_SUFFIXES = ("_token", "_secret", "_password", "_credential", "_auth") + + +def _is_forbidden_key(key: str) -> bool: + lowered = key.lower() + if lowered in _FORBIDDEN_EXACT_KEYS: + return True + return ( + lowered.startswith(_FORBIDDEN_KEY_PREFIXES) + or lowered.endswith(_FORBIDDEN_KEY_SUFFIXES) + ) + +_REQUIRED_PROJECT_FIELDS = ( + "id", + "repo_name", + "gitea_owner", + "remote_host", + "default_branch", + "local_checkout_path", + "profiles", + "workflow_paths", +) + +_REQUIRED_PROFILE_ROLES = ("author", "reviewer", "reconciler") + + +@dataclass(frozen=True) +class OnboardingStep: + id: str + title: str + description: str + + +@dataclass(frozen=True) +class ProjectRecord: + id: str + repo_name: str + gitea_owner: str + remote_host: str + default_branch: str + local_checkout_path: str + profiles: dict[str, str] + workflow_paths: dict[str, str] + schema_paths: dict[str, str] + onboarding_checklist: tuple[OnboardingStep, ...] + + +@dataclass(frozen=True) +class ProjectRegistry: + version: int + projects: tuple[ProjectRecord, ...] + source_path: Path + + +def default_registry_path() -> Path: + override = os.environ.get("WEBUI_PROJECT_REGISTRY", "").strip() + if override: + return Path(override).expanduser().resolve() + return (Path(__file__).resolve().parent / "data" / "projects.registry.json").resolve() + + +def _reject_credential_keys(obj: Any, *, path: str = "") -> None: + if isinstance(obj, dict): + for key, value in obj.items(): + key_path = f"{path}.{key}" if path else key + if _is_forbidden_key(key): + raise ValueError(f"registry must not store credentials ({key_path})") + _reject_credential_keys(value, path=key_path) + elif isinstance(obj, list): + for index, item in enumerate(obj): + _reject_credential_keys(item, path=f"{path}[{index}]") + + +def _parse_onboarding(raw: list[dict[str, Any]] | None) -> tuple[OnboardingStep, ...]: + if not raw: + return () + steps: list[OnboardingStep] = [] + for item in raw: + steps.append( + OnboardingStep( + id=str(item["id"]), + title=str(item["title"]), + description=str(item["description"]), + ) + ) + return tuple(steps) + + +def _parse_project(raw: dict[str, Any]) -> ProjectRecord: + missing = [field for field in _REQUIRED_PROJECT_FIELDS if field not in raw] + if missing: + raise ValueError(f"project missing required fields: {', '.join(missing)}") + + profiles = raw["profiles"] + if not isinstance(profiles, dict): + raise ValueError("profiles must be an object") + for role in _REQUIRED_PROFILE_ROLES: + if role not in profiles or not profiles[role]: + raise ValueError(f"profiles.{role} is required") + + workflow_paths = raw["workflow_paths"] + if not isinstance(workflow_paths, dict) or not workflow_paths: + raise ValueError("workflow_paths must be a non-empty object") + + schema_paths = raw.get("schema_paths") or {} + if not isinstance(schema_paths, dict): + raise ValueError("schema_paths must be an object when present") + + return ProjectRecord( + id=str(raw["id"]), + repo_name=str(raw["repo_name"]), + gitea_owner=str(raw["gitea_owner"]), + remote_host=str(raw["remote_host"]), + default_branch=str(raw["default_branch"]), + local_checkout_path=str(raw["local_checkout_path"]), + profiles={role: str(profiles[role]) for role in _REQUIRED_PROFILE_ROLES}, + workflow_paths={key: str(value) for key, value in workflow_paths.items()}, + schema_paths={key: str(value) for key, value in schema_paths.items()}, + onboarding_checklist=_parse_onboarding(raw.get("onboarding_checklist")), + ) + + +def load_registry(path: Path | None = None) -> ProjectRegistry: + """Load the versioned project registry from disk.""" + source = (path or default_registry_path()).resolve() + raw_text = source.read_text(encoding="utf-8") + payload = json.loads(raw_text) + if not isinstance(payload, dict): + raise ValueError("registry root must be an object") + + version = payload.get("version") + if version != 1: + raise ValueError(f"unsupported registry version: {version!r}") + + _reject_credential_keys(payload) + + projects_raw = payload.get("projects") + if not isinstance(projects_raw, list) or not projects_raw: + raise ValueError("projects must be a non-empty array") + + projects = tuple(_parse_project(item) for item in projects_raw) + return ProjectRegistry(version=version, projects=projects, source_path=source) + + +def project_to_dict(project: ProjectRecord) -> dict[str, Any]: + """Serialize a project for JSON API responses.""" + return { + "id": project.id, + "repo_name": project.repo_name, + "gitea_owner": project.gitea_owner, + "remote_host": project.remote_host, + "default_branch": project.default_branch, + "local_checkout_path": project.local_checkout_path, + "profiles": dict(project.profiles), + "workflow_paths": dict(project.workflow_paths), + "schema_paths": dict(project.schema_paths), + "onboarding_checklist": [ + {"id": step.id, "title": step.title, "description": step.description} + for step in project.onboarding_checklist + ], + } + + +def registry_to_dict(registry: ProjectRegistry) -> dict[str, Any]: + return { + "version": registry.version, + "source_path": str(registry.source_path), + "projects": [project_to_dict(project) for project in registry.projects], + } + + +def find_project(registry: ProjectRegistry, project_id: str) -> ProjectRecord | None: + for project in registry.projects: + if project.id == project_id: + return project + return None \ No newline at end of file diff --git a/webui/project_views.py b/webui/project_views.py new file mode 100644 index 0000000..c5076ef --- /dev/null +++ b/webui/project_views.py @@ -0,0 +1,93 @@ +"""HTML views for project registry pages (#427).""" + +from __future__ import annotations + +import html + +from webui.layout import render_page +from webui.project_registry import ProjectRecord, ProjectRegistry + + +def _escape(text: str) -> str: + return html.escape(text, quote=True) + + +def render_projects_list(registry: ProjectRegistry) -> str: + rows = [] + for project in registry.projects: + rows.append( + "" + f"{_escape(project.repo_name)}" + f"{_escape(project.gitea_owner)}" + f"{_escape(project.remote_host)}" + f"{_escape(project.default_branch)}" + f"{_escape(project.profiles['author'])}" + "" + ) + table = ( + "" + "" + "" + "" + "" + f"{''.join(rows)}
RepositoryOwnerRemoteBranchAuthor profile
" + ) + body = ( + "

Projects

" + "

Configured repositories managed by the MCP Control Plane.

" + f"

Registry: {_escape(str(registry.source_path))} " + f"(version {registry.version})

" + f"{table}" + "

JSON API

" + ) + return render_page(title="Projects", body_html=body) + + +def render_project_detail(project: ProjectRecord) -> str: + profile_rows = "".join( + f"{_escape(role)}{_escape(name)}" + for role, name in project.profiles.items() + ) + workflow_rows = "".join( + f"{_escape(key)}{_escape(path)}" + for key, path in project.workflow_paths.items() + ) + schema_rows = "".join( + f"{_escape(key)}{_escape(path)}" + for key, path in project.schema_paths.items() + ) + checklist_items = [] + for index, step in enumerate(project.onboarding_checklist, start=1): + checklist_items.append( + "
  • " + f"{index}. {_escape(step.title)}" + f"

    {_escape(step.description)}

    " + "
    • " + ) + checklist_html = ( + "
      " + "".join(checklist_items) + "
    " + if checklist_items + else "

    No onboarding steps defined.

    " + ) + body = ( + f"

    {_escape(project.repo_name)}

    " + "

    ← All projects

    " + "

    Identity

    " + "" + f"" + f"" + f"" + f"" + f"" + "
    Registry id{_escape(project.id)}
    Gitea owner{_escape(project.gitea_owner)}
    Remote host{_escape(project.remote_host)}
    Default branch{_escape(project.default_branch)}
    Local checkout{_escape(project.local_checkout_path)}
    " + "

    Profiles

    " + f"{profile_rows}
    " + "

    Workflow paths

    " + f"{workflow_rows}
    " + "

    Schema paths

    " + f"{schema_rows}
    " + "

    Onboarding checklist

    " + "

    Read-only MVP — complete these steps outside the UI.

    " + f"{checklist_html}" + ) + return render_page(title=project.repo_name, body_html=body) \ No newline at end of file diff --git a/webui/prompt_library.py b/webui/prompt_library.py new file mode 100644 index 0000000..e5ec1d7 --- /dev/null +++ b/webui/prompt_library.py @@ -0,0 +1,197 @@ +"""Canonical workflow prompt library for the internal web UI (#428).""" + +from __future__ import annotations + +import hashlib +import os +import re +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +_WORKFLOW_ROOT = Path("skills/llm-project-workflow/workflows") + +_DEFAULT_PROMPT_RE = re.compile( + r"\*\*Default task prompt:\*\*\s*\n+>\s*(.+?)(?=\n\n|\nDo not improvise)", + re.DOTALL, +) +_FRONTMATTER_TASK_MODE_RE = re.compile(r"^task_mode:\s*(\S+)", re.MULTILINE) + + +@dataclass(frozen=True) +class PromptEntry: + slug: str + label: str + prompt_text: str + workflow_path: str + task_mode: str | None + workflow_hash: str | None + source_note: str + + +def _repo_root() -> Path: + override = (os.environ.get("WEBUI_REPO_ROOT") or "").strip() + if override: + return Path(override).resolve() + return Path(__file__).resolve().parent.parent + + +def _workflow_file(path: str) -> Path: + return _repo_root() / path + + +def _sha256_hex(content: str) -> str: + return hashlib.sha256(content.encode("utf-8")).hexdigest() + + +def _read_workflow(path: str) -> tuple[str, str]: + file_path = _workflow_file(path) + text = file_path.read_text(encoding="utf-8") + return text, _sha256_hex(text) + + +def _extract_default_prompt(markdown: str) -> str | None: + match = _DEFAULT_PROMPT_RE.search(markdown) + if not match: + return None + lines = [line.strip() for line in match.group(1).splitlines()] + return " ".join(line for line in lines if line) + + +def _extract_task_mode(markdown: str) -> str | None: + match = _FRONTMATTER_TASK_MODE_RE.search(markdown) + return match.group(1) if match else None + + +def _entry_from_workflow( + *, + slug: str, + label: str, + workflow_path: str, + prompt_override: str | None = None, + source_note: str = "", +) -> PromptEntry: + markdown, digest = _read_workflow(workflow_path) + prompt_text = prompt_override or _extract_default_prompt(markdown) + if not prompt_text: + raise ValueError(f"No default task prompt found in {workflow_path}") + return PromptEntry( + slug=slug, + label=label, + prompt_text=prompt_text, + workflow_path=workflow_path, + task_mode=_extract_task_mode(markdown), + workflow_hash=digest, + source_note=source_note, + ) + + +def _static_entry( + *, + slug: str, + label: str, + prompt_text: str, + workflow_path: str, + source_note: str, +) -> PromptEntry: + path = _workflow_file(workflow_path) + digest = _sha256_hex(path.read_text(encoding="utf-8")) if path.is_file() else None + markdown = path.read_text(encoding="utf-8") if path.is_file() else "" + return PromptEntry( + slug=slug, + label=label, + prompt_text=prompt_text, + workflow_path=workflow_path, + task_mode=_extract_task_mode(markdown) if markdown else None, + workflow_hash=digest, + source_note=source_note, + ) + + +def load_prompt_library() -> tuple[PromptEntry, ...]: + """Load operator prompts derived from canonical workflows.""" + entries = ( + _entry_from_workflow( + slug="review-pr", + label="Review PR", + workflow_path=str(_WORKFLOW_ROOT / "review-merge-pr.md"), + ), + _entry_from_workflow( + slug="work-issue", + label="Work issue", + workflow_path=str(_WORKFLOW_ROOT / "work-issue.md"), + ), + _entry_from_workflow( + slug="create-issue", + label="Create issue", + workflow_path=str(_WORKFLOW_ROOT / "create-issue.md"), + ), + _static_entry( + slug="comment-issue", + label="Comment on issue", + workflow_path=str(_WORKFLOW_ROOT / "create-issue.md"), + prompt_text=( + "Comment on the target Gitea issue only if exact comment_issue " + "capability is proven. Load the canonical create-issue workflow " + "first and follow §16 (comment-on-existing issue rule). Include " + "specific evidence; do not duplicate existing comments." + ), + source_note="Derived from create-issue.md §16; full policy remains in the workflow file.", + ), + _static_entry( + slug="cleanup", + label="Post-merge cleanup", + workflow_path="skills/llm-project-workflow/templates/worktree-cleanup.md", + prompt_text=( + "Task: clean up branch/worktree for PR # / issue # after merge. " + "Confirm the merge on remote master before any deletion; never " + "force-remove a dirty worktree." + ), + source_note="Full cleanup steps live in templates/worktree-cleanup.md.", + ), + _entry_from_workflow( + slug="audit", + label="Reconciliation audit", + workflow_path=str(_WORKFLOW_ROOT / "reconcile-landed-pr.md"), + ), + _static_entry( + slug="onboarding", + label="Project onboarding", + workflow_path="skills/llm-project-workflow/SKILL.md", + prompt_text=( + "Onboard this repository into the MCP Control Plane: prove identity " + "and task capability, configure author/reviewer/reconciler profiles " + "in separate namespaces, then complete the checklist at /projects. " + "Canonical router: skills/llm-project-workflow/SKILL.md." + ), + source_note="Checklist details live in webui/data/projects.registry.json and /projects.", + ), + ) + return entries + + +def find_prompt(slug: str) -> PromptEntry | None: + for entry in load_prompt_library(): + if entry.slug == slug: + return entry + return None + + +def prompt_to_dict(entry: PromptEntry) -> dict[str, Any]: + return { + "slug": entry.slug, + "label": entry.label, + "prompt_text": entry.prompt_text, + "workflow_path": entry.workflow_path, + "task_mode": entry.task_mode, + "workflow_hash": entry.workflow_hash, + "source_note": entry.source_note, + } + + +def library_to_dict() -> dict[str, Any]: + entries = load_prompt_library() + return { + "count": len(entries), + "prompts": [prompt_to_dict(entry) for entry in entries], + } \ No newline at end of file diff --git a/webui/prompt_views.py b/webui/prompt_views.py new file mode 100644 index 0000000..fe6d687 --- /dev/null +++ b/webui/prompt_views.py @@ -0,0 +1,87 @@ +"""HTML views for the prompt library (#428).""" + +from __future__ import annotations + +import html + +from webui.layout import render_page +from webui.prompt_library import PromptEntry, load_prompt_library + + +def _escape(text: str) -> str: + return html.escape(text, quote=True) + + +def _render_prompt_card(entry: PromptEntry) -> str: + hash_short = ( + f"{_escape(entry.workflow_hash[:12])}" + if entry.workflow_hash + else "n/a" + ) + task_mode = ( + f"{_escape(entry.task_mode)}" + if entry.task_mode + else "n/a" + ) + note = ( + f'

    {_escape(entry.source_note)}

    ' + if entry.source_note + else "" + ) + prompt_id = f"prompt-{entry.slug}" + return ( + f'
    ' + f"

    {_escape(entry.label)}

    " + f'

    Workflow: {_escape(entry.workflow_path)} · ' + f"task_mode: {task_mode} · sha256: {hash_short}

    " + f'
    {_escape(entry.prompt_text)}
    ' + f'" + f"{note}" + "
    " + ) + + +PROMPT_PAGE_SCRIPT = """ + +""" + + +def render_prompts_page() -> str: + entries = load_prompt_library() + cards = "".join(_render_prompt_card(entry) for entry in entries) + body = ( + "

    Prompt library

    " + "

    Short copy/paste task prompts derived from canonical workflows. " + "Full policy remains in the cited workflow files — not duplicated here.

    " + f"{cards}" + "

    JSON API

    " + f"{PROMPT_PAGE_SCRIPT}" + ) + return render_page(title="Prompts", body_html=body) + + +def render_prompt_detail(entry: PromptEntry) -> str: + body = ( + f"

    ← All prompts

    " + f"{_render_prompt_card(entry)}" + f"{PROMPT_PAGE_SCRIPT}" + ) + return render_page(title=entry.label, body_html=body) \ No newline at end of file diff --git a/webui/queue_loader.py b/webui/queue_loader.py new file mode 100644 index 0000000..a70e58d --- /dev/null +++ b/webui/queue_loader.py @@ -0,0 +1,385 @@ +"""Load live Gitea PR/issue queue state for the web UI dashboard (#429).""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from datetime import datetime, timezone +from typing import Any, Callable +from urllib.parse import urlparse + +from gitea_auth import api_fetch_page, get_auth_header, repo_api_url + +from webui.project_registry import ProjectRecord, load_registry + +_CLOSES_RE = re.compile(r"(?:closes|fixes|resolves)\s+#(\d+)", re.I) +_ISSUE_REF_RE = re.compile(r"#(\d+)") +_STALE_DAYS = 14 + + +@dataclass(frozen=True) +class PaginationMeta: + page: int + per_page: int + returned_count: int + has_more: bool + is_final_page: bool + inventory_complete: bool + pages_fetched: int + + +@dataclass(frozen=True) +class QueueItem: + number: int + title: str + badges: tuple[str, ...] + extra: dict[str, str] + + +@dataclass(frozen=True) +class QueueSnapshot: + project_id: str + repo_label: str + prs: tuple[QueueItem, ...] + issues: tuple[QueueItem, ...] + pr_pagination: PaginationMeta | None + issue_pagination: PaginationMeta | None + fetch_error: str | None = None + + +def _host_from_url(remote_host: str) -> str: + parsed = urlparse(remote_host.strip()) + return parsed.netloc or remote_host.strip().rstrip("/") + + +def _extract_linked_issue(title: str | None, body: str | None = None) -> int | None: + for text in (title, body): + if not text: + continue + match = _CLOSES_RE.search(text) + if match: + return int(match.group(1)) + if body: + refs = _ISSUE_REF_RE.findall(body) + if refs: + return int(refs[0]) + return None + + +def _is_stale(updated_at: str | None) -> bool: + if not updated_at: + return False + try: + normalized = updated_at.replace("Z", "+00:00") + parsed = datetime.fromisoformat(normalized) + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + age = datetime.now(timezone.utc) - parsed.astimezone(timezone.utc) + return age.days >= _STALE_DAYS + except ValueError: + return False + + +def _classify_pr( + pr: dict, + *, + issue_claimed: set[int], + issue_to_prs: dict[int, list[int]], +) -> tuple[str, ...]: + badges: list[str] = [] + mergeable = pr.get("mergeable") + if mergeable is False: + badges.append("blocked") + linked = _extract_linked_issue(pr.get("title"), pr.get("body")) + if linked is not None: + if linked in issue_claimed: + badges.append("claimed") + if len(issue_to_prs.get(linked, [])) > 1: + badges.append("duplicate") + if _is_stale(pr.get("updated_at")): + badges.append("stale") + if mergeable is True and "blocked" not in badges: + badges.append("in-review") + if not badges: + badges.append("open") + return tuple(dict.fromkeys(badges)) + + +def _classify_issue( + issue: dict, + *, + linked_prs: list[int], +) -> tuple[str, ...]: + badges: list[str] = [] + labels = [lb.get("name", "") for lb in issue.get("labels", [])] + if "status:in-progress" in labels: + badges.append("claimed") + if len(linked_prs) > 1: + badges.append("duplicate") + elif linked_prs and "claimed" not in badges: + badges.append("in-review") + if _is_stale(issue.get("updated_at")): + badges.append("stale") + if not badges: + badges.append("open") + return tuple(dict.fromkeys(badges)) + + +def _format_pr_item(pr: dict, badges: tuple[str, ...]) -> QueueItem: + head = pr.get("head") or {} + base = pr.get("base") or {} + mergeable = pr.get("mergeable") + merge_label = ( + "mergeable" if mergeable is True else "conflicted" if mergeable is False else "unknown" + ) + linked = _extract_linked_issue(pr.get("title"), pr.get("body")) + return QueueItem( + number=int(pr["number"]), + title=str(pr.get("title") or ""), + badges=badges, + extra={ + "branch": f"{head.get('ref', '?')} → {base.get('ref', '?')}", + "head_sha": str(head.get("sha") or "")[:12], + "mergeable": merge_label, + "linked_issue": str(linked) if linked is not None else "", + }, + ) + + +def _format_issue_item(issue: dict, badges: tuple[str, ...]) -> QueueItem: + labels = ", ".join(lb.get("name", "") for lb in issue.get("labels", [])) + assignee = (issue.get("assignee") or {}).get("login", "") + return QueueItem( + number=int(issue["number"]), + title=str(issue.get("title") or ""), + badges=badges, + extra={ + "labels": labels or "—", + "assignee": assignee or "unassigned", + "state": str(issue.get("state") or ""), + }, + ) + + +def _pagination_from_pages( + *, + per_page: int, + pages_fetched: int, + returned_count: int, + is_final_page: bool, +) -> PaginationMeta: + return PaginationMeta( + page=1, + per_page=per_page, + returned_count=returned_count, + has_more=not is_final_page, + is_final_page=is_final_page, + inventory_complete=is_final_page, + pages_fetched=pages_fetched, + ) + + +def _fetch_prs( + host: str, + org: str, + repo: str, + auth: str, + *, + per_page: int = 50, +) -> tuple[list[dict], PaginationMeta]: + url = f"{repo_api_url(host, org, repo)}/pulls?state=open" + all_raw: list[dict] = [] + pages_fetched = 0 + is_final = False + page = 1 + while pages_fetched < 20: + raw_page, meta = api_fetch_page(url, auth, page=page, limit=per_page) + pages_fetched += 1 + all_raw.extend(raw_page) + is_final = bool(meta["is_final_page"]) + if is_final: + break + page += 1 + pagination = _pagination_from_pages( + per_page=per_page, + pages_fetched=pages_fetched, + returned_count=len(all_raw), + is_final_page=is_final, + ) + return all_raw, pagination + + +def _fetch_issues( + host: str, + org: str, + repo: str, + auth: str, + *, + per_page: int = 50, +) -> tuple[list[dict], PaginationMeta]: + url = f"{repo_api_url(host, org, repo)}/issues?state=open&type=issues" + all_raw: list[dict] = [] + page = 1 + pages_fetched = 0 + is_final = False + while pages_fetched < 20: + raw_page, meta = api_fetch_page(url, auth, page=page, limit=per_page) + pages_fetched += 1 + all_raw.extend(raw_page) + is_final = bool(meta["is_final_page"]) + if is_final: + break + page += 1 + pagination = _pagination_from_pages( + per_page=per_page, + pages_fetched=pages_fetched, + returned_count=len(all_raw), + is_final_page=is_final, + ) + return all_raw, pagination + + +def load_queue_snapshot( + project_id: str | None = None, + *, + fetch_prs: Callable[..., tuple[list[dict], PaginationMeta]] | None = None, + fetch_issues: Callable[..., tuple[list[dict], PaginationMeta]] | None = None, +) -> QueueSnapshot: + """Load open PR and issue queues for a registry project (default: first entry).""" + registry = load_registry() + project: ProjectRecord | None = None + if project_id: + for entry in registry.projects: + if entry.id == project_id: + project = entry + break + else: + project = registry.projects[0] if registry.projects else None + + if project is None: + return QueueSnapshot( + project_id=project_id or "", + repo_label="", + prs=(), + issues=(), + pr_pagination=None, + issue_pagination=None, + fetch_error="project not found in registry", + ) + + host = _host_from_url(project.remote_host) + pr_fetch = fetch_prs or _fetch_prs + issue_fetch = fetch_issues or _fetch_issues + using_live_fetch = fetch_prs is None or fetch_issues is None + auth = get_auth_header(host) if using_live_fetch else "test-auth" + if using_live_fetch and not auth: + return QueueSnapshot( + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + prs=(), + issues=(), + pr_pagination=None, + issue_pagination=None, + fetch_error=( + f"Gitea credentials unavailable for {host}; " + "queue cannot be loaded (fail closed — not showing empty queue)" + ), + ) + + try: + raw_prs, pr_pagination = pr_fetch( + host, project.gitea_owner, project.repo_name, auth + ) + raw_issues, issue_pagination = issue_fetch( + host, project.gitea_owner, project.repo_name, auth + ) + except Exception as exc: # noqa: BLE001 — surface operator-visible fetch errors + return QueueSnapshot( + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + prs=(), + issues=(), + pr_pagination=None, + issue_pagination=None, + fetch_error=f"Gitea fetch failed: {exc}", + ) + + issue_to_prs: dict[int, list[int]] = {} + for pr in raw_prs: + linked = _extract_linked_issue(pr.get("title"), pr.get("body")) + if linked is not None: + issue_to_prs.setdefault(linked, []).append(int(pr["number"])) + + issue_claimed = { + int(i["number"]) + for i in raw_issues + if any(lb.get("name") == "status:in-progress" for lb in i.get("labels", [])) + } + + pr_items = tuple( + _format_pr_item( + pr, + _classify_pr( + pr, + issue_claimed=issue_claimed, + issue_to_prs=issue_to_prs, + ), + ) + for pr in sorted(raw_prs, key=lambda p: int(p["number"]), reverse=True) + ) + issue_items = tuple( + _format_issue_item( + issue, + _classify_issue( + issue, + linked_prs=issue_to_prs.get(int(issue["number"]), []), + ), + ) + for issue in sorted(raw_issues, key=lambda i: int(i["number"]), reverse=True) + ) + + return QueueSnapshot( + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + prs=pr_items, + issues=issue_items, + pr_pagination=pr_pagination, + issue_pagination=issue_pagination, + ) + + +def snapshot_to_dict(snapshot: QueueSnapshot) -> dict[str, Any]: + """JSON-serializable export for /api/queue.""" + + def _page(meta: PaginationMeta | None) -> dict[str, Any] | None: + if meta is None: + return None + return { + "page": meta.page, + "per_page": meta.per_page, + "returned_count": meta.returned_count, + "has_more": meta.has_more, + "is_final_page": meta.is_final_page, + "inventory_complete": meta.inventory_complete, + "pages_fetched": meta.pages_fetched, + } + + def _item(item: QueueItem) -> dict[str, Any]: + return { + "number": item.number, + "title": item.title, + "badges": list(item.badges), + **item.extra, + } + + return { + "project_id": snapshot.project_id, + "repo": snapshot.repo_label, + "fetch_error": snapshot.fetch_error, + "prs": [_item(p) for p in snapshot.prs], + "issues": [_item(i) for i in snapshot.issues], + "pagination": { + "prs": _page(snapshot.pr_pagination), + "issues": _page(snapshot.issue_pagination), + }, + } \ No newline at end of file diff --git a/webui/queue_views.py b/webui/queue_views.py new file mode 100644 index 0000000..e5e6dd7 --- /dev/null +++ b/webui/queue_views.py @@ -0,0 +1,119 @@ +"""HTML views for the live Gitea queue dashboard (#429).""" + +from __future__ import annotations + +import html + +from webui.queue_loader import PaginationMeta, QueueItem, QueueSnapshot + + +def _badge_html(badges: tuple[str, ...]) -> str: + if not badges: + return "" + chips = "".join( + f'{html.escape(b)}' + for b in badges + ) + return f'{chips}' + + +def _pagination_html(label: str, meta: PaginationMeta | None) -> str: + if meta is None: + return ( + f"

    {html.escape(label)} pagination: " + "unavailable

    " + ) + status = "complete" if meta.inventory_complete else "partial" + more = "yes" if meta.has_more else "no" + return ( + f"

    {html.escape(label)} pagination ({status}): " + f"returned {meta.returned_count} · per_page {meta.per_page} · " + f"pages_fetched {meta.pages_fetched} · has_more {more} · " + f"final_page {'yes' if meta.is_final_page else 'no'}

    " + ) + + +def _queue_table( + *, + title: str, + items: tuple[QueueItem, ...], + columns: tuple[tuple[str, str], ...], + fetch_failed: bool = False, +) -> str: + if fetch_failed: + return ( + f"

    {html.escape(title)}

    " + "

    Not loaded — queue fetch did not complete.

    " + ) + if not items: + return f"

    {html.escape(title)}

    No open items.

    " + + headers = "".join(f"{html.escape(label)}" for _, label in columns) + rows = [] + for item in items: + cells = [] + for key, _ in columns: + if key == "number": + cells.append(f"#{item.number}") + elif key == "title": + cells.append( + f"{html.escape(item.title)}{_badge_html(item.badges)}" + ) + else: + cells.append(f"{html.escape(item.extra.get(key, ''))}") + rows.append("" + "".join(cells) + "") + + body = ( + f"

    {html.escape(title)}

    " + f"{headers}" + f"{''.join(rows)}
    " + ) + return body + + +def render_queue_page(snapshot: QueueSnapshot) -> str: + error_block = "" + if snapshot.fetch_error: + error_block = ( + f'

    Queue unavailable: ' + f"{html.escape(snapshot.fetch_error)}

    " + ) + + fetch_failed = bool(snapshot.fetch_error) + pr_section = _queue_table( + title="Open pull requests", + items=snapshot.prs, + fetch_failed=fetch_failed, + columns=( + ("number", "#"), + ("title", "Title"), + ("branch", "Branch"), + ("head_sha", "Head"), + ("mergeable", "Mergeable"), + ("linked_issue", "Linked issue"), + ), + ) + issue_section = _queue_table( + title="Open issues", + items=snapshot.issues, + fetch_failed=fetch_failed, + columns=( + ("number", "#"), + ("title", "Title"), + ("labels", "Labels"), + ("assignee", "Assignee"), + ("state", "State"), + ), + ) + + return ( + "

    Live queue

    " + f"

    Repository: {html.escape(snapshot.repo_label)} " + f"· project {html.escape(snapshot.project_id)}

    " + f"{error_block}" + f"{_pagination_html('PR', snapshot.pr_pagination)}" + f"{pr_section}" + f"{_pagination_html('Issue', snapshot.issue_pagination)}" + f"{issue_section}" + "

    Read-only MVP — claims, reviews, and merges stay in Gitea/MCP tools.

    " + ) \ No newline at end of file