From 72b18143f83a060de4b38138cf4da940bc543206 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sat, 11 Jul 2026 19:41:38 -0400 Subject: [PATCH] fix(preflight): unify review worktree regex, resolve Web UI NameError, and fix test isolation --- gitea_mcp_server.py | 3 +-- reviewer_worktree.py | 14 ++++++++++++++ reviewer_worktree_ownership.py | 7 +++---- tests/conftest.py | 11 ++++++++++- tests/test_root_checkout_guard.py | 11 +++++++++-- webui/worktree_scanner.py | 8 ++------ worktree_cleanup_audit.py | 8 +++----- 7 files changed, 42 insertions(+), 20 deletions(-) diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index c62c603..fbbb53a 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -724,8 +724,7 @@ def _verify_role_mutation_workspace( task: str | None = None, ) -> str: """Bind reviewer/merger mutations to the active namespace workspace (#510).""" - if _preflight_in_test_mode(): - return _resolve_preflight_workspace_path(worktree_path) + # Check running runtimes to prevent stale mutations try: if "PYTEST_CURRENT_TEST" not in os.environ or "GITEA_FORCE_MCP_RUNTIME_CHECK" in os.environ: diff --git a/reviewer_worktree.py b/reviewer_worktree.py index 15f2d8b..d67fe08 100644 --- a/reviewer_worktree.py +++ b/reviewer_worktree.py @@ -27,6 +27,20 @@ _READONLY_REVIEWER_GIT = re.compile( _GIT_INVOCATION = re.compile(r"\bgit\b", re.IGNORECASE) +# #673: Canonical pattern for identifying review worktrees under branches/. +REVIEW_WORKTREE_RE = re.compile( + r"branches/(?:review-pr\d+[\w/-]*|merge-simulation-pr\d+|review-[\w-]+)", + re.IGNORECASE, +) + + +def is_review_worktree_path(path: str) -> bool: + """True when the path belongs to a reviewer or simulation worktree.""" + normalized = (path or "").replace("\\", "/") + return bool(REVIEW_WORKTREE_RE.search(normalized)) + + + def parse_dirty_tracked_files(porcelain: str) -> list[str]: """Return tracked paths with local modifications from ``git status --porcelain``. diff --git a/reviewer_worktree_ownership.py b/reviewer_worktree_ownership.py index d30c48d..340796c 100644 --- a/reviewer_worktree_ownership.py +++ b/reviewer_worktree_ownership.py @@ -5,10 +5,9 @@ from __future__ import annotations import re from typing import Any -_SESSION_OWNED_RE = re.compile( - r"branches/(?:review-pr\d+[\w/-]*|review-[\w-]+)", - re.IGNORECASE, -) +from reviewer_worktree import REVIEW_WORKTREE_RE + +_SESSION_OWNED_RE = REVIEW_WORKTREE_RE _WORKTREE_PATH_RE = re.compile( r"(?:review worktree path|worktree path|session-owned worktree)\s*:\s*(\S+)", re.IGNORECASE, diff --git a/tests/conftest.py b/tests/conftest.py index 178380e..40ece4d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -26,7 +26,16 @@ def _reset_mutation_authority(monkeypatch): Pin ``default_state_dir`` / ``DEFAULT_STATE_DIR`` to a per-test temp dir so durable load/save never touches host state even after env clears. """ - monkeypatch.delenv("GITEA_SESSION_PROFILE_LOCK", raising=False) + for env_key in [ + "GITEA_SESSION_PROFILE_LOCK", + "GITEA_ACTIVE_WORKTREE", + "GITEA_AUTHOR_WORKTREE", + "GITEA_REVIEWER_WORKTREE", + "GITEA_MERGER_WORKTREE", + "GITEA_RECONCILER_WORKTREE", + ]: + monkeypatch.delenv(env_key, raising=False) + # Isolate durable session-state files so tests never share host cache (#559). import tempfile diff --git a/tests/test_root_checkout_guard.py b/tests/test_root_checkout_guard.py index 70fe871..8fdf9c0 100644 --- a/tests/test_root_checkout_guard.py +++ b/tests/test_root_checkout_guard.py @@ -141,13 +141,21 @@ class TestVerifyPreflightRootGuardIntegration(unittest.TestCase): self.assertIn("Root checkout guard (#475)", str(ctx.exception)) self.assertIn(rcg.REMEDIATION, str(ctx.exception)) + @patch("os.path.isdir", return_value=True) + @patch("os.path.exists", return_value=True) + @patch("subprocess.run") @patch("gitea_mcp_server._get_workspace_porcelain", return_value="") @patch("gitea_mcp_server.root_checkout_guard.resolve_remote_master_sha", return_value=MASTER_SHA) @patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state") @patch("gitea_mcp_server._resolve_author_mutation_context") def test_reviewer_from_branches_worktree_allowed( - self, mock_ctx, mock_git, _remote_sha, _porcelain, + self, mock_ctx, mock_git, _remote_sha, _porcelain, mock_run, _exists, _isdir, ): + import unittest.mock + mock_run.return_value = unittest.mock.MagicMock( + returncode=0, + stdout=f"{CONTROL_ROOT}/.git\n", + ) srv._preflight_capability_baseline_porcelain = "" mock_ctx.return_value = { "workspace_path": BRANCHES_WORKTREE, @@ -161,6 +169,5 @@ class TestVerifyPreflightRootGuardIntegration(unittest.TestCase): } srv.verify_preflight_purity("prgs", worktree_path=BRANCHES_WORKTREE) - if __name__ == "__main__": unittest.main() \ No newline at end of file diff --git a/webui/worktree_scanner.py b/webui/worktree_scanner.py index 20acb09..61447f0 100644 --- a/webui/worktree_scanner.py +++ b/webui/worktree_scanner.py @@ -14,11 +14,7 @@ from merged_cleanup_reconcile import ( read_issue_lock, read_local_worktree_state, ) - -_REVIEW_WORKTREE_RE = re.compile( - r"branches/(?:review-pr\d+|merge-simulation-pr\d+|review-[\w-]+)", - re.IGNORECASE, -) +from reviewer_worktree import REVIEW_WORKTREE_RE CLASSIFICATIONS = frozenset({ "active-pr", @@ -147,7 +143,7 @@ def classify_entry( if not record: return "orphan", "Directory exists but is not a registered git worktree" - if record.get("detached") and _REVIEW_WORKTREE_RE.search(rel_path): + if record.get("detached") and REVIEW_WORKTREE_RE.search(rel_path): return "detached-review", "Detached HEAD in reviewer/simulation worktree" if state.get("exists") and state.get("clean"): diff --git a/worktree_cleanup_audit.py b/worktree_cleanup_audit.py index 6ede83b..261164a 100644 --- a/worktree_cleanup_audit.py +++ b/worktree_cleanup_audit.py @@ -35,7 +35,7 @@ from datetime import datetime, timezone from typing import Any from merged_cleanup_reconcile import branch_worktree_folder, read_local_worktree_state -from reviewer_worktree import parse_dirty_tracked_files +from reviewer_worktree import parse_dirty_tracked_files, REVIEW_WORKTREE_RE PROTECTED_BRANCHES = frozenset({"master", "main", "dev"}) DEFAULT_TTL_HOURS = float(os.environ.get("GITEA_WORKTREE_TTL_HOURS", "24") or 24) @@ -508,10 +508,8 @@ DISPOSITIONS = frozenset({ "unsafe_unknown", }) -REVIEW_WORKTREE_RE = re.compile( - r"branches/(?:review-pr\d+|merge-simulation-pr\d+|review-[\w-]+)", - re.IGNORECASE, -) + + def normalize_path(path: str) -> str: