From cf057d78297ed185c478ace3dbbde3ea5898fde4 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 11:35:43 -0400 Subject: [PATCH 01/15] feat: move duplicate-work detection before author mutations (Closes #400) Extract issue_work_duplicate_gate for open PR, remote branch, and active claim checks; wire it into lock_issue, commit_files recheck, and create_pr recheck. Add gitea_assess_work_issue_duplicate read-only preflight, work-issue report outcome validation, and workflow section 10A. Co-Authored-By: Claude Opus 4.8 (1M context) --- gitea_mcp_server.py | 213 +++++++++++++---- issue_work_duplicate_gate.py | 180 ++++++++++++++ review_proofs.py | 3 + .../workflows/work-issue.md | 30 +++ tests/test_issue_work_duplicate_gate.py | 219 ++++++++++++++++++ tests/test_review_proofs.py | 1 + 6 files changed, 606 insertions(+), 40 deletions(-) create mode 100644 issue_work_duplicate_gate.py create mode 100644 tests/test_issue_work_duplicate_gate.py diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 0bc82e4..c4e4fdc 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -503,6 +503,7 @@ import issue_lock_worktree # noqa: E402 import already_landed_reconcile # noqa: E402 import author_mutation_worktree # noqa: E402 import issue_claim_heartbeat # noqa: E402 +import issue_work_duplicate_gate # noqa: E402 import merged_cleanup_reconcile # noqa: E402 import reconciler_profile # noqa: E402 import reconciliation_workflow # noqa: E402 @@ -641,6 +642,103 @@ def _branch_entry_name(branch: dict | str) -> str: return str(branch.get("name") or branch.get("ref") or "") +def _collect_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 + + +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 @@ -1111,48 +1209,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, @@ -1198,6 +1269,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, @@ -1289,6 +1393,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} @@ -2893,6 +3012,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) 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/skills/llm-project-workflow/workflows/work-issue.md b/skills/llm-project-workflow/workflows/work-issue.md index f450cd7..c7bc8a7 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. diff --git a/tests/test_issue_work_duplicate_gate.py b/tests/test_issue_work_duplicate_gate.py new file mode 100644 index 0000000..02816ce --- /dev/null +++ b/tests/test_issue_work_duplicate_gate.py @@ -0,0 +1,219 @@ +"""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_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 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"): + with open(self.lock_path, "w", encoding="utf-8") as fh: + json.dump({ + "issue_number": issue_number, + "branch_name": branch, + "remote": "prgs", + }, 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", + }) + def test_create_pr_returns_handoff_on_duplicate(self, _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_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): From 0bad26230b1066fd35cec8c318ddc5638a0d3c14 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 13:08:53 -0400 Subject: [PATCH 02/15] fix: resolve conflicts for PR #413 Rebase onto current prgs/master and patch create_pr duplicate-gate test with auth header mock to match post-rebase recheck path. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_issue_work_duplicate_gate.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_issue_work_duplicate_gate.py b/tests/test_issue_work_duplicate_gate.py index 02816ce..a041ef3 100644 --- a/tests/test_issue_work_duplicate_gate.py +++ b/tests/test_issue_work_duplicate_gate.py @@ -189,7 +189,8 @@ class TestMcpDuplicateRecheck(unittest.TestCase): "forbidden_operations": [], "audit_label": "test-author", }) - def test_create_pr_returns_handoff_on_duplicate(self, _profile, mock_gate): + @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, From a8fcf0e01cede2b1f0d3013797f2f05a05b6b002 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 13:23:09 -0400 Subject: [PATCH 03/15] feat: add internal web UI server skeleton (Closes #426) Starlette read-only MVP with shared layout, /health JSON liveness, and route stubs for projects, prompts, runtime, audit, worktrees, and leases. Includes scripts/run-webui, docs/webui-local-dev.md, and tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/webui-local-dev.md | 53 ++++++++++++++++ scripts/run-webui | 6 ++ tests/test_webui_skeleton.py | 58 +++++++++++++++++ webui/__init__.py | 5 ++ webui/__main__.py | 19 ++++++ webui/app.py | 117 +++++++++++++++++++++++++++++++++++ webui/layout.py | 102 ++++++++++++++++++++++++++++++ 7 files changed, 360 insertions(+) create mode 100644 docs/webui-local-dev.md create mode 100755 scripts/run-webui create mode 100644 tests/test_webui_skeleton.py create mode 100644 webui/__init__.py create mode 100644 webui/__main__.py create mode 100644 webui/app.py create mode 100644 webui/layout.py diff --git a/docs/webui-local-dev.md b/docs/webui-local-dev.md new file mode 100644 index 0000000..48a540c --- /dev/null +++ b/docs/webui-local-dev.md @@ -0,0 +1,53 @@ +# 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`) | +| `/projects` | Stub — registry (#427) | +| `/prompts` | Stub — prompt library (#428) | +| `/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`. + +## Tests + +```bash +pytest tests/test_webui_skeleton.py -q +``` \ 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/tests/test_webui_skeleton.py b/tests/test_webui_skeleton.py new file mode 100644 index 0000000..89471af --- /dev/null +++ b/tests/test_webui_skeleton.py @@ -0,0 +1,58 @@ +"""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 ("/projects", "/prompts", "/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_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_nav_links_on_all_pages(self): + for path in ("/", "/projects", "/prompts", "/runtime", "/audit"): + with self.subTest(path=path): + text = self.client.get(path).text + for href in ("/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..aa0c10f --- /dev/null +++ b/webui/app.py @@ -0,0 +1,117 @@ +"""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 + +_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.

" + "
    " + "
  • Projects — registry and onboarding (#427)
    • " + "
  • Prompts — canonical workflow prompt library (#428)
    • " + "
  • Runtime — MCP health and stale-runtime detection (#430)
    • " + "
  • Audit — final-report paste and validator preview (#431)
    • " + "
  • Worktrees — branch hygiene dashboard (#432)
    • " + "
  • Leases — collision and lease visibility (#433)
    • " + "
" + ) + 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 projects(_request: Request) -> HTMLResponse: + return _stub_page( + "Projects", + "Project registry and onboarding model will list configured repos and profiles.", + ) + + +async def prompts(_request: Request) -> HTMLResponse: + return _stub_page( + "Prompts", + "Prompt library will surface canonical workflows from skills/llm-project-workflow/.", + ) + + +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("/projects", projects, methods=["GET"]), + Route("/prompts", 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/layout.py b/webui/layout.py new file mode 100644 index 0000000..0aa2a87 --- /dev/null +++ b/webui/layout.py @@ -0,0 +1,102 @@ +"""Shared HTML layout for the internal web UI.""" + +from __future__ import annotations + +NAV_ITEMS = ( + ("/", "Home"), + ("/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) -> str: + nav_links = "".join( + f'{label}' for href, label in NAV_ITEMS + ) + return f""" + + + + + {title} · MCP Control Plane + + + +
+

MCP Control Plane

+ +
+
+

{MVP_NOTICE}

+ {body_html} +
+ +""" \ No newline at end of file From 81fcdb09fd88fdeda702d176fbfb0b74e59dc6e5 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 14:38:10 -0400 Subject: [PATCH 04/15] feat: thread worktree_path through gitea_create_issue (#450) Hardens #274 branches-only guard for gitea_create_issue: - verify_preflight_purity: validate resolved worktree exists, is a directory, and belongs to the target repository before author mutations. - gitea_create_issue: add worktree_path threaded into preflight guard. - tests/test_create_issue_workspace_guard.py: stable-control-checkout rejection and invalid-path fail-closed cases (PROJECT_ROOT simulated as control checkout). Preserves WIP commit 8530109 implementation; test fix completes stable-checkout path. Closes #450 --- gitea_mcp_server.py | 48 ++++++- tests/test_create_issue_workspace_guard.py | 145 +++++++++++++++++++++ 2 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 tests/test_create_issue_workspace_guard.py diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 0bc82e4..beb6084 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). " @@ -972,6 +1010,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. @@ -984,6 +1023,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). @@ -1010,7 +1050,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( 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() From 16dcf65825b560b7044c97dd7e95705aeebcf3f3 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 13:27:55 -0400 Subject: [PATCH 05/15] feat: add web UI project registry and onboarding (#427) Load projects from versioned webui/data/projects.registry.json with profile mappings, workflow/schema paths, and read-only onboarding checklist UI. Seeds Gitea-Tools; exposes /projects, /projects/{id}, and /api/projects. Closes #427 --- docs/webui-local-dev.md | 18 ++- tests/test_webui_project_registry.py | 111 +++++++++++++++ tests/test_webui_skeleton.py | 7 +- webui/app.py | 34 ++++- webui/data/projects.registry.json | 49 +++++++ webui/layout.py | 28 ++++ webui/project_registry.py | 196 +++++++++++++++++++++++++++ webui/project_views.py | 93 +++++++++++++ 8 files changed, 529 insertions(+), 7 deletions(-) create mode 100644 tests/test_webui_project_registry.py create mode 100644 webui/data/projects.registry.json create mode 100644 webui/project_registry.py create mode 100644 webui/project_views.py diff --git a/docs/webui-local-dev.md b/docs/webui-local-dev.md index 48a540c..011b71a 100644 --- a/docs/webui-local-dev.md +++ b/docs/webui-local-dev.md @@ -36,7 +36,9 @@ Optional environment variables: |------|-------------| | `/` | Home / operator overview | | `/health` | JSON liveness (`status`, `service`, `mode`, `timestamp`) | -| `/projects` | Stub — registry (#427) | +| `/projects` | Project registry list (#427) | +| `/projects/{id}` | Project detail + onboarding checklist | +| `/api/projects` | JSON registry export | | `/prompts` | Stub — prompt library (#428) | | `/runtime` | Stub — MCP runtime health (#430) | | `/audit` | Stub — report audit paste (#431) | @@ -46,8 +48,20 @@ Optional environment variables: 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. + ## Tests ```bash -pytest tests/test_webui_skeleton.py -q +pytest tests/test_webui_skeleton.py tests/test_webui_project_registry.py -q ``` \ 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_skeleton.py b/tests/test_webui_skeleton.py index 89471af..7e139c6 100644 --- a/tests/test_webui_skeleton.py +++ b/tests/test_webui_skeleton.py @@ -30,12 +30,17 @@ class TestWebuiSkeleton(unittest.TestCase): self.assertIn("Read-only MVP", response.text) def test_route_stubs_render(self): - for path in ("/projects", "/prompts", "/runtime", "/audit"): + for path in ("/prompts", "/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_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): diff --git a/webui/app.py b/webui/app.py index aa0c10f..4f1cba9 100644 --- a/webui/app.py +++ b/webui/app.py @@ -10,6 +10,8 @@ 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 _READ_ONLY_METHODS = frozenset({"GET", "HEAD", "OPTIONS"}) @@ -49,10 +51,32 @@ async def health(_request: Request) -> JSONResponse: async def projects(_request: Request) -> HTMLResponse: - return _stub_page( - "Projects", - "Project registry and onboarding model will list configured repos and profiles.", - ) + 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: @@ -107,6 +131,8 @@ def create_app() -> Starlette: Route("/", home, methods=["GET"]), Route("/health", health, 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("/runtime", runtime, methods=["GET"]), Route("/audit", audit, methods=["GET"]), 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 index 0aa2a87..fc75efb 100644 --- a/webui/layout.py +++ b/webui/layout.py @@ -87,6 +87,34 @@ def render_page(*, title: str, body_html: str) -> str: padding-left: 0.85rem; margin: 1rem 0; }} + .meta {{ font-size: 0.85rem; }} + table.registry, table.detail {{ + width: 100%; + border-collapse: collapse; + margin: 1rem 0; + font-size: 0.9rem; + }} + table.registry th, table.registry td, + table.detail th, table.detail td {{ + text-align: left; + padding: 0.45rem 0.6rem; + border-bottom: 1px solid var(--border); + }} + table.registry th, table.detail th {{ + color: var(--muted); + font-weight: 500; + }} + code {{ + font-family: ui-monospace, monospace; + font-size: 0.85em; + color: var(--text); + }} + ol.checklist {{ + padding-left: 1.25rem; + margin: 0.5rem 0 1.5rem; + }} + ol.checklist li {{ margin-bottom: 0.85rem; }} + ol.checklist p {{ margin: 0.25rem 0 0; font-size: 0.9rem; }} 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 From 6670c72e65f89606d7ca86e3cd613a0a78797b73 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 13:31:40 -0400 Subject: [PATCH 06/15] feat: add canonical workflow prompt library to web UI (Closes #428) Expose short copy/paste operator prompts on /prompts derived from canonical workflow files with workflow path and SHA-256 hash. Adds JSON export at /api/prompts, copy buttons, and tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/webui-local-dev.md | 13 +- tests/test_webui_prompt_library.py | 81 ++++++++++++ tests/test_webui_skeleton.py | 7 +- webui/app.py | 12 +- webui/layout.py | 33 ++++- webui/prompt_library.py | 192 +++++++++++++++++++++++++++++ webui/prompt_views.py | 80 ++++++++++++ 7 files changed, 410 insertions(+), 8 deletions(-) create mode 100644 tests/test_webui_prompt_library.py create mode 100644 webui/prompt_library.py create mode 100644 webui/prompt_views.py diff --git a/docs/webui-local-dev.md b/docs/webui-local-dev.md index 011b71a..4b94aed 100644 --- a/docs/webui-local-dev.md +++ b/docs/webui-local-dev.md @@ -39,7 +39,8 @@ Optional environment variables: | `/projects` | Project registry list (#427) | | `/projects/{id}` | Project detail + onboarding checklist | | `/api/projects` | JSON registry export | -| `/prompts` | Stub — prompt library (#428) | +| `/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) | @@ -60,8 +61,16 @@ 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. + ## Tests ```bash -pytest tests/test_webui_skeleton.py tests/test_webui_project_registry.py -q +pytest tests/test_webui_skeleton.py tests/test_webui_project_registry.py tests/test_webui_prompt_library.py -q ``` \ 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..442800d --- /dev/null +++ b/tests/test_webui_prompt_library.py @@ -0,0 +1,81 @@ +"""Tests for web UI prompt library (#428).""" +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 load_prompt_library, prompt_to_dict + + +class TestWebuiPromptLibrary(unittest.TestCase): + def setUp(self): + self.client = TestClient(create_app()) + + def test_library_has_required_prompts(self): + entries = load_prompt_library() + slugs = {entry.slug for entry in entries} + self.assertEqual( + slugs, + { + "review-pr", + "work-issue", + "create-issue", + "comment-issue", + "cleanup", + "audit", + "onboarding", + }, + ) + + def test_workflow_entries_include_hash_and_task_mode(self): + review = next(e for e in load_prompt_library() if e.slug == "review-pr") + self.assertEqual( + review.workflow_path, + "skills/llm-project-workflow/workflows/review-merge-pr.md", + ) + self.assertEqual(review.task_mode, "review-merge-pr") + self.assertIsNotNone(review.workflow_hash) + self.assertGreater(len(review.workflow_hash), 12) + self.assertIn("eligible open PR", review.prompt_text) + + def test_comment_issue_defers_to_workflow(self): + comment = next(e for e in load_prompt_library() if e.slug == "comment-issue") + self.assertIn("comment_issue", comment.prompt_text) + self.assertIn("create-issue", comment.workflow_path) + self.assertIn("§16", comment.source_note) + + def test_prompts_page_renders_cards_and_copy_buttons(self): + response = self.client.get("/prompts") + self.assertEqual(response.status_code, 200) + text = response.text + self.assertIn("Prompt library", text) + self.assertIn("review-merge-pr.md", text) + self.assertIn("Copy prompt", text) + self.assertIn('data-copy-target="prompt-review-pr"', text) + self.assertNotIn("child issue", text.lower()) + + 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) + self.assertEqual(len(data["prompts"]), 7) + first = data["prompts"][0] + self.assertIn("slug", first) + self.assertIn("prompt_text", first) + self.assertIn("workflow_path", first) + self.assertIn("workflow_hash", first) + + def test_prompt_to_dict_round_trip(self): + entry = load_prompt_library()[0] + payload = prompt_to_dict(entry) + self.assertEqual(payload["slug"], entry.slug) + self.assertEqual(payload["prompt_text"], entry.prompt_text) + + +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 index 7e139c6..937b095 100644 --- a/tests/test_webui_skeleton.py +++ b/tests/test_webui_skeleton.py @@ -30,12 +30,17 @@ class TestWebuiSkeleton(unittest.TestCase): self.assertIn("Read-only MVP", response.text) def test_route_stubs_render(self): - for path in ("/prompts", "/runtime", "/audit"): + 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) diff --git a/webui/app.py b/webui/app.py index 4f1cba9..dc99cdb 100644 --- a/webui/app.py +++ b/webui/app.py @@ -12,6 +12,8 @@ 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 library_to_dict +from webui.prompt_views import render_prompts_page _READ_ONLY_METHODS = frozenset({"GET", "HEAD", "OPTIONS"}) @@ -80,10 +82,11 @@ async def api_projects(_request: Request) -> JSONResponse: async def prompts(_request: Request) -> HTMLResponse: - return _stub_page( - "Prompts", - "Prompt library will surface canonical workflows from skills/llm-project-workflow/.", - ) + return HTMLResponse(render_prompts_page()) + + +async def api_prompts(_request: Request) -> JSONResponse: + return JSONResponse(library_to_dict()) async def runtime(_request: Request) -> HTMLResponse: @@ -134,6 +137,7 @@ def create_app() -> Starlette: Route("/projects/{project_id}", project_detail, methods=["GET"]), Route("/api/projects", api_projects, methods=["GET"]), Route("/prompts", prompts, methods=["GET"]), + Route("/api/prompts", api_prompts, methods=["GET"]), Route("/runtime", runtime, methods=["GET"]), Route("/audit", audit, methods=["GET"]), Route("/worktrees", worktrees, methods=["GET"]), diff --git a/webui/layout.py b/webui/layout.py index fc75efb..d948204 100644 --- a/webui/layout.py +++ b/webui/layout.py @@ -18,7 +18,7 @@ MVP_NOTICE = ( ) -def render_page(*, title: str, body_html: str) -> str: +def render_page(*, title: str, body_html: str, extra_head: str = "") -> str: nav_links = "".join( f'{label}' for href, label in NAV_ITEMS ) @@ -115,7 +115,38 @@ def render_page(*, title: str, body_html: str) -> str: }} ol.checklist li {{ margin-bottom: 0.85rem; }} ol.checklist p {{ margin: 0.25rem 0 0; font-size: 0.9rem; }} + .prompt-card {{ + margin: 1.25rem 0 1.75rem; + padding: 1rem 1.1rem; + border: 1px solid var(--border); + border-radius: 8px; + background: var(--surface); + }} + .prompt-card h3 {{ margin: 0 0 0.5rem; font-size: 1.05rem; }} + pre.prompt-text {{ + white-space: pre-wrap; + word-break: break-word; + margin: 0.75rem 0; + padding: 0.75rem 0.85rem; + border-radius: 6px; + background: var(--bg); + border: 1px solid var(--border); + font-size: 0.9rem; + color: var(--text); + }} + .copy-btn {{ + background: var(--accent); + color: #0b1219; + border: none; + border-radius: 6px; + padding: 0.4rem 0.85rem; + font-size: 0.85rem; + cursor: pointer; + }} + .copy-btn:hover {{ filter: brightness(1.08); }} + .muted {{ color: var(--muted); }} + {extra_head}
    diff --git a/webui/prompt_library.py b/webui/prompt_library.py new file mode 100644 index 0000000..5d2e768 --- /dev/null +++ b/webui/prompt_library.py @@ -0,0 +1,192 @@ +"""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 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="PR queue cleanup", + workflow_path=str(_WORKFLOW_ROOT / "pr-queue-cleanup.md"), + prompt_text=( + "Run PR-only queue cleanup: build a complete open-PR inventory " + "with pagination proof, select exactly one eligible PR, dispatch " + "one canonical review for that PR, then stop after any terminal " + "review mutation. Load pr-queue-cleanup.md and review-merge-pr.md " + "before any mutation." + ), + source_note="Derived from pr-queue-cleanup.md; composes with review-merge-pr.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="docs/webui-local-dev.md", + prompt_text=( + "Start a new MCP Control Plane session: call mcp_get_control_plane_guide, " + "prove identity and task capability, then complete the onboarding checklist " + "for this project at /projects. Keep Gitea, MCP tools, and canonical " + "workflows as the source of truth." + ), + source_note="Operator onboarding; checklist details live in the project registry UI.", + ), + ) + return entries + + +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..1a6b22c --- /dev/null +++ b/webui/prompt_views.py @@ -0,0 +1,80 @@ +"""HTML views for the prompt library (#428).""" + +from __future__ import annotations + +import html + +from webui.prompt_library import PromptEntry, load_prompt_library + + +def _escape(text: str) -> str: + return html.escape(text, quote=True) + + +def render_prompts_page() -> str: + entries = load_prompt_library() + cards: list[str] = [] + for entry in entries: + cards.append(_render_prompt_card(entry)) + body = ( + "

    Prompt library

    " + "

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

    " + f"{''.join(cards)}" + f"{PROMPT_PAGE_SCRIPT}" + ) + from webui.layout import render_page + + return render_page(title="Prompts", body_html=body) + + +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 = """ + +""" \ No newline at end of file From f845864889a2b9d9f236abde873c3b90dff9c041 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 13:32:05 -0400 Subject: [PATCH 07/15] feat: add web UI prompt library from canonical workflows (#428) Surface short copy/paste operator prompts derived from canonical workflow files with workflow path and SHA-256 citations. Adds /prompts, /prompts/{id}, and /api/prompts with one-click copy. Closes #428 --- tests/test_webui_prompt_library.py | 113 ++++++++++++++++------------- webui/app.py | 23 +++++- webui/prompt_library.py | 35 +++++---- webui/prompt_views.py | 43 ++++++----- 4 files changed, 127 insertions(+), 87 deletions(-) diff --git a/tests/test_webui_prompt_library.py b/tests/test_webui_prompt_library.py index 442800d..3538c2d 100644 --- a/tests/test_webui_prompt_library.py +++ b/tests/test_webui_prompt_library.py @@ -1,4 +1,5 @@ """Tests for web UI prompt library (#428).""" +import json import sys import unittest from pathlib import Path @@ -8,73 +9,81 @@ 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 load_prompt_library, prompt_to_dict +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 TestWebuiPromptLibrary(unittest.TestCase): +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_library_has_required_prompts(self): - entries = load_prompt_library() - slugs = {entry.slug for entry in entries} - self.assertEqual( - slugs, - { - "review-pr", - "work-issue", - "create-issue", - "comment-issue", - "cleanup", - "audit", - "onboarding", - }, - ) - - def test_workflow_entries_include_hash_and_task_mode(self): - review = next(e for e in load_prompt_library() if e.slug == "review-pr") - self.assertEqual( - review.workflow_path, - "skills/llm-project-workflow/workflows/review-merge-pr.md", - ) - self.assertEqual(review.task_mode, "review-merge-pr") - self.assertIsNotNone(review.workflow_hash) - self.assertGreater(len(review.workflow_hash), 12) - self.assertIn("eligible open PR", review.prompt_text) - - def test_comment_issue_defers_to_workflow(self): - comment = next(e for e in load_prompt_library() if e.slug == "comment-issue") - self.assertIn("comment_issue", comment.prompt_text) - self.assertIn("create-issue", comment.workflow_path) - self.assertIn("§16", comment.source_note) - - def test_prompts_page_renders_cards_and_copy_buttons(self): + def test_prompts_page_lists_all_entries(self): response = self.client.get("/prompts") self.assertEqual(response.status_code, 200) - text = response.text - self.assertIn("Prompt library", text) - self.assertIn("review-merge-pr.md", text) - self.assertIn("Copy prompt", text) - self.assertIn('data-copy-target="prompt-review-pr"', text) - self.assertNotIn("child issue", text.lower()) + 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) - self.assertEqual(len(data["prompts"]), 7) - first = data["prompts"][0] - self.assertIn("slug", first) - self.assertIn("prompt_text", first) - self.assertIn("workflow_path", first) - self.assertIn("workflow_hash", first) + 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_round_trip(self): + def test_prompt_to_dict_roundtrip(self): entry = load_prompt_library()[0] - payload = prompt_to_dict(entry) - self.assertEqual(payload["slug"], entry.slug) - self.assertEqual(payload["prompt_text"], entry.prompt_text) + encoded = json.dumps(prompt_to_dict(entry)) + self.assertIn("workflow_path", encoded) if __name__ == "__main__": diff --git a/webui/app.py b/webui/app.py index dc99cdb..d1e1c38 100644 --- a/webui/app.py +++ b/webui/app.py @@ -12,8 +12,8 @@ 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 library_to_dict -from webui.prompt_views import render_prompts_page +from webui.prompt_library import find_prompt, library_to_dict +from webui.prompt_views import render_prompt_detail, render_prompts_page _READ_ONLY_METHODS = frozenset({"GET", "HEAD", "OPTIONS"}) @@ -85,6 +85,24 @@ 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()) @@ -137,6 +155,7 @@ def create_app() -> Starlette: 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"]), diff --git a/webui/prompt_library.py b/webui/prompt_library.py index 5d2e768..e5ec1d7 100644 --- a/webui/prompt_library.py +++ b/webui/prompt_library.py @@ -128,7 +128,7 @@ def load_prompt_library() -> tuple[PromptEntry, ...]: ), _static_entry( slug="comment-issue", - label="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 " @@ -140,16 +140,14 @@ def load_prompt_library() -> tuple[PromptEntry, ...]: ), _static_entry( slug="cleanup", - label="PR queue cleanup", - workflow_path=str(_WORKFLOW_ROOT / "pr-queue-cleanup.md"), + label="Post-merge cleanup", + workflow_path="skills/llm-project-workflow/templates/worktree-cleanup.md", prompt_text=( - "Run PR-only queue cleanup: build a complete open-PR inventory " - "with pagination proof, select exactly one eligible PR, dispatch " - "one canonical review for that PR, then stop after any terminal " - "review mutation. Load pr-queue-cleanup.md and review-merge-pr.md " - "before any mutation." + "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="Derived from pr-queue-cleanup.md; composes with review-merge-pr.md.", + source_note="Full cleanup steps live in templates/worktree-cleanup.md.", ), _entry_from_workflow( slug="audit", @@ -159,19 +157,26 @@ def load_prompt_library() -> tuple[PromptEntry, ...]: _static_entry( slug="onboarding", label="Project onboarding", - workflow_path="docs/webui-local-dev.md", + workflow_path="skills/llm-project-workflow/SKILL.md", prompt_text=( - "Start a new MCP Control Plane session: call mcp_get_control_plane_guide, " - "prove identity and task capability, then complete the onboarding checklist " - "for this project at /projects. Keep Gitea, MCP tools, and canonical " - "workflows as the source of truth." + "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="Operator onboarding; checklist details live in the project registry UI.", + 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, diff --git a/webui/prompt_views.py b/webui/prompt_views.py index 1a6b22c..fe6d687 100644 --- a/webui/prompt_views.py +++ b/webui/prompt_views.py @@ -4,6 +4,7 @@ from __future__ import annotations import html +from webui.layout import render_page from webui.prompt_library import PromptEntry, load_prompt_library @@ -11,23 +12,6 @@ def _escape(text: str) -> str: return html.escape(text, quote=True) -def render_prompts_page() -> str: - entries = load_prompt_library() - cards: list[str] = [] - for entry in entries: - cards.append(_render_prompt_card(entry)) - body = ( - "

    Prompt library

    " - "

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

    " - f"{''.join(cards)}" - f"{PROMPT_PAGE_SCRIPT}" - ) - from webui.layout import render_page - - return render_page(title="Prompts", body_html=body) - - def _render_prompt_card(entry: PromptEntry) -> str: hash_short = ( f"{_escape(entry.workflow_hash[:12])}" @@ -77,4 +61,27 @@ document.querySelectorAll('.copy-btn').forEach((btn) => { }); }); -""" \ No newline at end of file +""" + + +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 From c91e3642de07ad374961d9277fcd492e8f6c775f Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 13:55:21 -0400 Subject: [PATCH 08/15] feat: add live PR/issue queue dashboard to web UI (Closes #429) Adds read-only /queue and /api/queue routes that load open PRs and issues from the default registry project via gitea_auth, surface pagination proof, and classify items with claimed/blocked/in-review/duplicate badges. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/webui-local-dev.md | 15 +- tests/test_webui_queue_dashboard.py | 138 ++++++++++ webui/app.py | 14 + webui/layout.py | 15 ++ webui/queue_loader.py | 385 ++++++++++++++++++++++++++++ webui/queue_views.py | 110 ++++++++ 6 files changed, 676 insertions(+), 1 deletion(-) create mode 100644 tests/test_webui_queue_dashboard.py create mode 100644 webui/queue_loader.py create mode 100644 webui/queue_views.py diff --git a/docs/webui-local-dev.md b/docs/webui-local-dev.md index 4b94aed..0a5b935 100644 --- a/docs/webui-local-dev.md +++ b/docs/webui-local-dev.md @@ -36,6 +36,8 @@ Optional environment variables: |------|-------------| | `/` | 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 | @@ -69,8 +71,19 @@ Prompts are generated at load time from canonical workflow files under 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 -q +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/tests/test_webui_queue_dashboard.py b/tests/test_webui_queue_dashboard.py new file mode 100644 index 0000000..f4671ac --- /dev/null +++ b/tests/test_webui_queue_dashboard.py @@ -0,0 +1,138 @@ +"""Tests for web UI live queue dashboard (#429).""" +import json +import sys +import unittest +from pathlib import Path +from unittest.mock import patch + +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, + load_queue_snapshot, + snapshot_to_dict, +) + + +def _page(items: list[dict], *, final: bool = True) -> tuple[list[dict], PaginationMeta]: + return items, PaginationMeta( + page=1, + per_page=50, + returned_count=len(items), + has_more=not final, + is_final_page=final, + inventory_complete=final, + pages_fetched=1, + ) + + +SAMPLE_PRS = [ + { + "number": 10, + "title": "feat: example", + "mergeable": True, + "body": "Closes #5", + "head": {"ref": "feat/x", "sha": "abc123def456"}, + "base": {"ref": "master"}, + }, + { + "number": 9, + "title": "fix: conflict", + "mergeable": False, + "body": "Closes #5", + "head": {"ref": "feat/y", "sha": "deadbeef0000"}, + "base": {"ref": "master"}, + }, +] + +SAMPLE_ISSUES = [ + { + "number": 5, + "title": "Tracked issue", + "state": "open", + "labels": [{"name": "status:in-progress"}], + "assignee": {"login": "jcwalker3"}, + }, + { + "number": 4, + "title": "Idle issue", + "state": "open", + "labels": [], + "assignee": None, + }, +] + + +class TestQueueLoader(unittest.TestCase): + def test_classifies_claimed_duplicate_and_blocked(self): + snapshot = load_queue_snapshot( + fetch_prs=lambda *_a, **_k: _page(SAMPLE_PRS), + fetch_issues=lambda *_a, **_k: _page(SAMPLE_ISSUES), + ) + self.assertIsNone(snapshot.fetch_error) + pr_badges = {p.number: p.badges for p in snapshot.prs} + self.assertIn("blocked", pr_badges[9]) + self.assertIn("duplicate", pr_badges[9]) + issue_badges = {i.number: i.badges for i in snapshot.issues} + self.assertIn("claimed", issue_badges[5]) + self.assertIn("duplicate", issue_badges[5]) + + def test_fail_closed_without_credentials(self): + with patch("webui.queue_loader.get_auth_header", return_value=None): + snapshot = load_queue_snapshot() + self.assertIsNotNone(snapshot.fetch_error) + self.assertIn("credentials unavailable", snapshot.fetch_error.lower()) + self.assertEqual(snapshot.prs, ()) + self.assertEqual(snapshot.issues, ()) + + def test_snapshot_json_includes_pagination(self): + snapshot = load_queue_snapshot( + fetch_prs=lambda *_a, **_k: _page(SAMPLE_PRS, final=False), + fetch_issues=lambda *_a, **_k: _page(SAMPLE_ISSUES), + ) + payload = snapshot_to_dict(snapshot) + self.assertFalse(payload["pagination"]["prs"]["inventory_complete"]) + self.assertTrue(payload["pagination"]["prs"]["has_more"]) + self.assertTrue(payload["pagination"]["issues"]["inventory_complete"]) + + +class TestQueueRoutes(unittest.TestCase): + def setUp(self): + self.client = TestClient(create_app()) + + def test_queue_page_renders_with_mocked_loader(self): + snapshot = load_queue_snapshot( + fetch_prs=lambda *_a, **_k: _page(SAMPLE_PRS), + fetch_issues=lambda *_a, **_k: _page(SAMPLE_ISSUES), + ) + with patch("webui.app.load_queue_snapshot", return_value=snapshot): + response = self.client.get("/queue") + self.assertEqual(response.status_code, 200) + self.assertIn("Live queue", response.text) + self.assertIn("feat: example", response.text) + self.assertIn("Tracked issue", response.text) + self.assertIn("pagination", response.text.lower()) + + def test_api_queue_json(self): + snapshot = load_queue_snapshot( + fetch_prs=lambda *_a, **_k: _page(SAMPLE_PRS), + fetch_issues=lambda *_a, **_k: _page(SAMPLE_ISSUES), + ) + with patch("webui.app.load_queue_snapshot", return_value=snapshot): + response = self.client.get("/api/queue") + self.assertEqual(response.status_code, 200) + payload = json.loads(response.text) + self.assertEqual(len(payload["prs"]), 2) + self.assertEqual(len(payload["issues"]), 2) + + def test_nav_includes_queue_link(self): + response = self.client.get("/") + self.assertEqual(response.status_code, 200) + self.assertIn('href="/queue"', response.text) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/webui/app.py b/webui/app.py index d1e1c38..a97d1ad 100644 --- a/webui/app.py +++ b/webui/app.py @@ -14,6 +14,8 @@ 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"}) @@ -32,6 +34,7 @@ async def home(_request: Request) -> HTMLResponse: "

    Operator console

    " "

    Local entry point for MCP Control Plane operational views.

    " "
      " + "
    • Queue — live PR and issue dashboard (#429)
      • " "
    • Projects — registry and onboarding (#427)
      • " "
    • Prompts — canonical workflow prompt library (#428)
      • " "
    • Runtime — MCP health and stale-runtime detection (#430)
      • " @@ -52,6 +55,15 @@ async def health(_request: Request) -> JSONResponse: }) +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)) @@ -151,6 +163,8 @@ def create_app() -> Starlette: 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"]), diff --git a/webui/layout.py b/webui/layout.py index d948204..a506c2c 100644 --- a/webui/layout.py +++ b/webui/layout.py @@ -4,6 +4,7 @@ from __future__ import annotations NAV_ITEMS = ( ("/", "Home"), + ("/queue", "Queue"), ("/projects", "Projects"), ("/prompts", "Prompts"), ("/runtime", "Runtime"), @@ -145,6 +146,20 @@ def render_page(*, title: str, body_html: str, extra_head: str = "") -> str: }} .copy-btn:hover {{ filter: brightness(1.08); }} .muted {{ color: var(--muted); }} + .badges {{ display: inline-flex; flex-wrap: wrap; gap: 0.35rem; margin-left: 0.5rem; }} + .badge {{ + font-size: 0.72rem; + padding: 0.1rem 0.45rem; + border-radius: 999px; + border: 1px solid var(--border); + color: var(--muted); + text-transform: lowercase; + }} + .badge-claimed {{ color: #8fd19e; border-color: #3d6b4a; }} + .badge-blocked {{ color: #f0a8a8; border-color: #7a3b3b; }} + .badge-in-review {{ color: #9ec8f0; border-color: #3d5f7a; }} + .badge-duplicate {{ color: #e0c27a; border-color: #6b5730; }} + .badge-stale {{ color: #c9b8e8; border-color: #5a4a78; }} {extra_head} diff --git a/webui/queue_loader.py b/webui/queue_loader.py new file mode 100644 index 0000000..57ebeeb --- /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, api_get_all, 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..8294b8e --- /dev/null +++ b/webui/queue_views.py @@ -0,0 +1,110 @@ +"""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], ...], +) -> str: + 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)}

      " + ) + + pr_section = _queue_table( + title="Open pull requests", + items=snapshot.prs, + 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, + 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 From f5370a94d3739908dbb6acbd50996568ccb8dafd Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 13:55:58 -0400 Subject: [PATCH 09/15] test: harden queue dashboard classification and route coverage (#429) Expand queue dashboard tests for stale/duplicate badges, title-linked issues, and nav coverage; drop unused import in queue_loader. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_webui_queue_dashboard.py | 252 ++++++++++++++++++---------- tests/test_webui_skeleton.py | 9 +- webui/queue_loader.py | 2 +- 3 files changed, 172 insertions(+), 91 deletions(-) diff --git a/tests/test_webui_queue_dashboard.py b/tests/test_webui_queue_dashboard.py index f4671ac..cc9ea97 100644 --- a/tests/test_webui_queue_dashboard.py +++ b/tests/test_webui_queue_dashboard.py @@ -1,9 +1,9 @@ """Tests for web UI live queue dashboard (#429).""" -import json import sys import unittest +from datetime import datetime, timedelta, timezone from pathlib import Path -from unittest.mock import patch +from unittest import mock sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) @@ -12,126 +12,202 @@ from starlette.testclient import TestClient from webui.app import create_app from webui.queue_loader import ( PaginationMeta, + _classify_issue, + _classify_pr, + _extract_linked_issue, load_queue_snapshot, snapshot_to_dict, ) +_RECENT = datetime.now(timezone.utc).isoformat() +_STALE = (datetime.now(timezone.utc) - timedelta(days=30)).isoformat() -def _page(items: list[dict], *, final: bool = True) -> tuple[list[dict], PaginationMeta]: - return items, PaginationMeta( +_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=len(items), - has_more=not final, - is_final_page=final, - inventory_complete=final, + returned_count=count, + has_more=False, + is_final_page=True, + inventory_complete=True, pages_fetched=1, ) -SAMPLE_PRS = [ - { - "number": 10, - "title": "feat: example", - "mergeable": True, - "body": "Closes #5", - "head": {"ref": "feat/x", "sha": "abc123def456"}, - "base": {"ref": "master"}, - }, - { - "number": 9, - "title": "fix: conflict", - "mergeable": False, - "body": "Closes #5", - "head": {"ref": "feat/y", "sha": "deadbeef0000"}, - "base": {"ref": "master"}, - }, -] +def _mock_fetch_prs(*_args, **_kwargs): + return list(_SAMPLE_PRS), _mock_pagination(len(_SAMPLE_PRS)) -SAMPLE_ISSUES = [ - { - "number": 5, - "title": "Tracked issue", - "state": "open", - "labels": [{"name": "status:in-progress"}], - "assignee": {"login": "jcwalker3"}, - }, - { - "number": 4, - "title": "Idle issue", - "state": "open", - "labels": [], - "assignee": None, - }, -] + +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_classifies_claimed_duplicate_and_blocked(self): + def test_snapshot_with_mock_fetch(self): snapshot = load_queue_snapshot( - fetch_prs=lambda *_a, **_k: _page(SAMPLE_PRS), - fetch_issues=lambda *_a, **_k: _page(SAMPLE_ISSUES), + fetch_prs=_mock_fetch_prs, + fetch_issues=_mock_fetch_issues, ) + self.assertEqual(snapshot.project_id, "gitea-tools") self.assertIsNone(snapshot.fetch_error) - pr_badges = {p.number: p.badges for p in snapshot.prs} - self.assertIn("blocked", pr_badges[9]) - self.assertIn("duplicate", pr_badges[9]) - issue_badges = {i.number: i.badges for i in snapshot.issues} - self.assertIn("claimed", issue_badges[5]) - self.assertIn("duplicate", issue_badges[5]) + 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) - def test_fail_closed_without_credentials(self): - with patch("webui.queue_loader.get_auth_header", return_value=None): - snapshot = load_queue_snapshot() - self.assertIsNotNone(snapshot.fetch_error) - self.assertIn("credentials unavailable", snapshot.fetch_error.lower()) - self.assertEqual(snapshot.prs, ()) - self.assertEqual(snapshot.issues, ()) + 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_json_includes_pagination(self): + def test_snapshot_dict_export(self): snapshot = load_queue_snapshot( - fetch_prs=lambda *_a, **_k: _page(SAMPLE_PRS, final=False), - fetch_issues=lambda *_a, **_k: _page(SAMPLE_ISSUES), + fetch_prs=_mock_fetch_prs, + fetch_issues=_mock_fetch_issues, ) - payload = snapshot_to_dict(snapshot) - self.assertFalse(payload["pagination"]["prs"]["inventory_complete"]) - self.assertTrue(payload["pagination"]["prs"]["has_more"]) - self.assertTrue(payload["pagination"]["issues"]["inventory_complete"]) + 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()) - - def test_queue_page_renders_with_mocked_loader(self): - snapshot = load_queue_snapshot( - fetch_prs=lambda *_a, **_k: _page(SAMPLE_PRS), - fetch_issues=lambda *_a, **_k: _page(SAMPLE_ISSUES), + self._patch = mock.patch( + "webui.app.load_queue_snapshot", + return_value=load_queue_snapshot( + fetch_prs=_mock_fetch_prs, + fetch_issues=_mock_fetch_issues, + ), ) - with patch("webui.app.load_queue_snapshot", return_value=snapshot): - response = self.client.get("/queue") + 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("feat: example", response.text) - self.assertIn("Tracked issue", response.text) - self.assertIn("pagination", response.text.lower()) + 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): - snapshot = load_queue_snapshot( - fetch_prs=lambda *_a, **_k: _page(SAMPLE_PRS), - fetch_issues=lambda *_a, **_k: _page(SAMPLE_ISSUES), - ) - with patch("webui.app.load_queue_snapshot", return_value=snapshot): - response = self.client.get("/api/queue") + response = self.client.get("/api/queue") self.assertEqual(response.status_code, 200) - payload = json.loads(response.text) - self.assertEqual(len(payload["prs"]), 2) - self.assertEqual(len(payload["issues"]), 2) + data = response.json() + self.assertEqual(data["repo"], "Scaled-Tech-Consulting/Gitea-Tools") + self.assertEqual(data["pagination"]["issues"]["returned_count"], 3) - def test_nav_includes_queue_link(self): - response = self.client.get("/") - self.assertEqual(response.status_code, 200) - self.assertIn('href="/queue"', response.text) + 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) if __name__ == "__main__": diff --git a/tests/test_webui_skeleton.py b/tests/test_webui_skeleton.py index 937b095..5dd23a1 100644 --- a/tests/test_webui_skeleton.py +++ b/tests/test_webui_skeleton.py @@ -56,11 +56,16 @@ class TestWebuiSkeleton(unittest.TestCase): 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 ("/", "/projects", "/prompts", "/runtime", "/audit"): + for path in ("/", "/queue", "/projects", "/prompts", "/runtime", "/audit"): with self.subTest(path=path): text = self.client.get(path).text - for href in ("/projects", "/prompts", "/runtime", "/audit"): + for href in ("/queue", "/projects", "/prompts", "/runtime", "/audit"): self.assertIn(f'href="{href}"', text) diff --git a/webui/queue_loader.py b/webui/queue_loader.py index 57ebeeb..a70e58d 100644 --- a/webui/queue_loader.py +++ b/webui/queue_loader.py @@ -8,7 +8,7 @@ from datetime import datetime, timezone from typing import Any, Callable from urllib.parse import urlparse -from gitea_auth import api_fetch_page, api_get_all, get_auth_header, repo_api_url +from gitea_auth import api_fetch_page, get_auth_header, repo_api_url from webui.project_registry import ProjectRecord, load_registry From 22a1c4c2df5efe5e6286c8ee8251d03ee946e6fd Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 15:53:10 -0400 Subject: [PATCH 10/15] fix(webui): suppress empty queue copy on fetch failure (#458) When Gitea credentials are missing or the queue fetch fails closed, show "Not loaded" instead of "No open items." and keep pagination marked unavailable. Successful empty inventories still show the empty state with complete pagination proof. Closes #458 --- tests/test_webui_queue_dashboard.py | 74 +++++++++++++++++++++++++++++ webui/queue_views.py | 9 ++++ 2 files changed, 83 insertions(+) diff --git a/tests/test_webui_queue_dashboard.py b/tests/test_webui_queue_dashboard.py index cc9ea97..cbdcc16 100644 --- a/tests/test_webui_queue_dashboard.py +++ b/tests/test_webui_queue_dashboard.py @@ -12,12 +12,14 @@ 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() @@ -210,5 +212,77 @@ class TestQueueRoutes(unittest.TestCase): 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/webui/queue_views.py b/webui/queue_views.py index 8294b8e..e5e6dd7 100644 --- a/webui/queue_views.py +++ b/webui/queue_views.py @@ -38,7 +38,13 @@ 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.

      " @@ -73,9 +79,11 @@ def render_queue_page(snapshot: QueueSnapshot) -> str: 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"), @@ -88,6 +96,7 @@ def render_queue_page(snapshot: QueueSnapshot) -> str: issue_section = _queue_table( title="Open issues", items=snapshot.issues, + fetch_failed=fetch_failed, columns=( ("number", "#"), ("title", "Title"), From 795f54404798a1eb70be74419a6260153e423652 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 16:18:42 -0400 Subject: [PATCH 11/15] feat: block manual issue-lock seeding and require lock disclosure (#447) Add lock_provenance metadata on gitea_lock_issue writes and fail closed at gitea_create_pr when provenance is missing. Final-report validation now requires explicit External-state mutations disclosure for issue-lock read/write/delete and blocks mixed author PR creation with reviewer approval. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/llm-workflow-runbooks.md | 15 + final_report_validator.py | 62 ++++ gitea_mcp_server.py | 13 + issue_lock_provenance.py | 269 ++++++++++++++++++ .../workflows/work-issue.md | 6 + tests/test_commit_payloads.py | 14 + tests/test_issue_lock_provenance.py | 130 +++++++++ tests/test_mcp_server.py | 36 +++ 8 files changed, 545 insertions(+) create mode 100644 issue_lock_provenance.py create mode 100644 tests/test_issue_lock_provenance.py 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/final_report_validator.py b/final_report_validator.py index 3fbac0f..bbc4b6c 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, @@ -870,6 +871,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, *, @@ -889,10 +938,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, @@ -913,6 +969,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, @@ -924,25 +981,30 @@ _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, ], "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 beb6084..25eb9f3 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -538,6 +538,7 @@ 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 @@ -1208,6 +1209,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: @@ -1298,6 +1303,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") 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/skills/llm-project-workflow/workflows/work-issue.md b/skills/llm-project-workflow/workflows/work-issue.md index f450cd7..8f7a227 100644 --- a/skills/llm-project-workflow/workflows/work-issue.md +++ b/skills/llm-project-workflow/workflows/work-issue.md @@ -717,6 +717,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_commit_payloads.py b/tests/test_commit_payloads.py index e8e4123..eb712a0 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)) 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_mcp_server.py b/tests/test_mcp_server.py index 4002b5e..9f39ae1 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -101,12 +101,26 @@ 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 @@ -3284,6 +3298,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, [])) From e4adccd82a86c9f417c20fe8339ca4ef2d5edfe0 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 16:33:31 -0400 Subject: [PATCH 12/15] fix: inject duplicate-work context fetcher for testable lock/create_pr paths (#400) Expose issue_duplicate_context_fetcher on the MCP server so lock_issue, commit_files, and create_pr duplicate rechecks avoid live Gitea calls in unit tests. Update affected test suites to patch the fetcher without weakening duplicate-work gate assertions. Co-Authored-By: Claude Opus 4.8 (1M context) --- gitea_mcp_server.py | 20 +++++++- tests/test_agent_temp_artifacts.py | 5 +- tests/test_commit_files_capability.py | 7 +++ tests/test_commit_payloads.py | 7 +++ tests/test_issue_work_duplicate_gate.py | 31 +++++++++++++ tests/test_mcp_server.py | 61 +++++++++++++++---------- 6 files changed, 105 insertions(+), 26 deletions(-) diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index c4e4fdc..f1b1849 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -548,7 +548,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,7 +642,7 @@ def _branch_entry_name(branch: dict | str) -> str: return str(branch.get("name") or branch.get("ref") or "") -def _collect_issue_duplicate_context( +def _live_fetch_issue_duplicate_context( h: str, o: str, r: str, @@ -667,6 +667,22 @@ def _collect_issue_duplicate_context( 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, *, 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_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..1e684e7 100644 --- a/tests/test_commit_payloads.py +++ b/tests/test_commit_payloads.py @@ -84,7 +84,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_issue_work_duplicate_gate.py b/tests/test_issue_work_duplicate_gate.py index a041ef3..6e9ea5a 100644 --- a/tests/test_issue_work_duplicate_gate.py +++ b/tests/test_issue_work_duplicate_gate.py @@ -121,6 +121,37 @@ class TestDuplicateReportOutcome(unittest.TestCase): 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() diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 4002b5e..588e6e8 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -112,6 +112,11 @@ def _sample_issue_lock(issue_number=123, branch_name="feat/x", **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 +171,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 +196,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"} @@ -3044,8 +3057,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 +3073,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 +3099,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: From 06c20692348214c8fc2cf4bccbcd0a5dc500ae6f Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 13:07:22 -0400 Subject: [PATCH 13/15] feat: add per-PR reviewer leases for parallel review (Closes #407) Structured PR-thread leases with acquire/heartbeat MCP tools and mutation-time enforcement so reviewers cannot race the same PR queue. Co-Authored-By: Claude Opus 4.8 (1M context) --- gitea_mcp_server.py | 284 +++++++++++++ reviewer_pr_lease.py | 382 ++++++++++++++++++ .../workflows/review-merge-pr.md | 20 + tests/test_reviewer_pr_lease.py | 193 +++++++++ 4 files changed, 879 insertions(+) create mode 100644 reviewer_pr_lease.py create mode 100644 tests/test_reviewer_pr_lease.py diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 17938e3..cd24d8d 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -543,6 +543,7 @@ 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 @@ -2016,6 +2017,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, @@ -2458,6 +2460,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: @@ -3257,6 +3273,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 — head SHA must match if the caller pinned a reviewed SHA. actual_sha = result["head_sha"] if expected_head_sha and actual_sha and expected_head_sha != actual_sha: @@ -4556,6 +4585,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/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/skills/llm-project-workflow/workflows/review-merge-pr.md b/skills/llm-project-workflow/workflows/review-merge-pr.md index 5b0e10a..4f9f6eb 100644 --- a/skills/llm-project-workflow/workflows/review-merge-pr.md +++ b/skills/llm-project-workflow/workflows/review-merge-pr.md @@ -732,6 +732,26 @@ The final report must identify: * whether same-PR merge continuation was allowed * whether the run stopped as required +## 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. + ## 27. Merge rules Before merge, rerun fresh live checks: 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 From 1830360850520e49a537bd4d9ec3bf730c20ce1b Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 16:32:23 -0400 Subject: [PATCH 14/15] test: fix reviewer lease gate regressions in MCP integration tests (#407) Align merge/review/audit mocks with the per-PR reviewer lease gate introduced in #407: install owned session leases after init_review_decision_lock, stub _fetch_pr_comments and _authenticated_username to preserve mock ordering, and update head-SHA mismatch expectations for lease-first fail-closed paths. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_audit.py | 32 ++++++- tests/test_mcp_server.py | 139 ++++++++++++++++++++++++++++--- tests/test_pr_queue_inventory.py | 19 ++++- 3 files changed, 172 insertions(+), 18 deletions(-) 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_mcp_server.py b/tests/test_mcp_server.py index aabb7e7..d04cf38 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -79,6 +79,64 @@ def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"): return [_formal_review(reviewer, "APPROVED", sha=sha)] +_DEFAULT_LEASE_SESSION = "mcp-test-reviewer-lease" + + +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": ( @@ -539,6 +597,19 @@ 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) + def _pr(self, author, state="open", sha="abc123", mergeable=True): return { "user": {"login": author}, @@ -799,9 +870,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") @@ -1680,7 +1753,20 @@ 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) def _env(self): return patch.dict(os.environ, { @@ -1775,8 +1861,19 @@ 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.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 { @@ -2014,9 +2111,11 @@ class TestSubmitPrReview(unittest.TestCase): 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): @@ -2079,9 +2178,9 @@ 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") + gitea_mark_final_review_decision(8, "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"]) @@ -2300,6 +2399,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"} @@ -2334,6 +2440,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"} @@ -3015,10 +3128,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): 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"]) From 3e4b721d60e97147ba0704773cf57cd0d42cbe31 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 17:18:00 -0400 Subject: [PATCH 15/15] test: align duplicate gate lock fixtures with #447 provenance (#407) Rebase onto master (#413/#463) requires sanctioned lock_provenance in create_pr duplicate-recheck tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_issue_work_duplicate_gate.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_issue_work_duplicate_gate.py b/tests/test_issue_work_duplicate_gate.py index 6e9ea5a..ac4ccea 100644 --- a/tests/test_issue_work_duplicate_gate.py +++ b/tests/test_issue_work_duplicate_gate.py @@ -9,6 +9,7 @@ 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 ( @@ -172,11 +173,23 @@ class TestMcpDuplicateRecheck(unittest.TestCase): 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")