feat: authorized cleanup for reviewer scratch worktrees (Closes #534) #575
@@ -4737,6 +4737,34 @@ def gitea_reconcile_merged_cleanups(
|
|||||||
)
|
)
|
||||||
|
|
||||||
delete_capability_allowed = not _profile_operation_gate("gitea.branch.delete")
|
delete_capability_allowed = not _profile_operation_gate("gitea.branch.delete")
|
||||||
|
|
||||||
|
# #534: discover reviewer scratch trees and active leases for those PRs.
|
||||||
|
scratch_candidates = merged_cleanup_reconcile.discover_reviewer_scratch_worktrees(
|
||||||
|
PROJECT_ROOT
|
||||||
|
)
|
||||||
|
active_reviewer_leases: dict[int, bool] = {}
|
||||||
|
pr_states: dict[int, dict] = {}
|
||||||
|
for scratch in scratch_candidates:
|
||||||
|
pr_num = int(scratch["pr_number"])
|
||||||
|
if pr_num in active_reviewer_leases:
|
||||||
|
continue
|
||||||
|
try:
|
||||||
|
comments = api_get_all(f"{base}/issues/{pr_num}/comments", auth)
|
||||||
|
except Exception:
|
||||||
|
comments = []
|
||||||
|
lease = reviewer_pr_lease.find_active_reviewer_lease(
|
||||||
|
comments, pr_number=pr_num
|
||||||
|
)
|
||||||
|
active_reviewer_leases[pr_num] = bool(lease)
|
||||||
|
try:
|
||||||
|
pr_live = api_request("GET", f"{base}/pulls/{pr_num}", auth)
|
||||||
|
except Exception:
|
||||||
|
pr_live = {}
|
||||||
|
pr_states[pr_num] = {
|
||||||
|
"merged": bool((pr_live or {}).get("merged") or (pr_live or {}).get("merged_at")),
|
||||||
|
"closed": (pr_live or {}).get("state") == "closed",
|
||||||
|
}
|
||||||
|
|
||||||
report = merged_cleanup_reconcile.build_reconciliation_report(
|
report = merged_cleanup_reconcile.build_reconciliation_report(
|
||||||
project_root=PROJECT_ROOT,
|
project_root=PROJECT_ROOT,
|
||||||
closed_prs=merged_closed,
|
closed_prs=merged_closed,
|
||||||
@@ -4744,6 +4772,8 @@ def gitea_reconcile_merged_cleanups(
|
|||||||
remote_branch_exists=remote_branch_exists,
|
remote_branch_exists=remote_branch_exists,
|
||||||
head_on_master=head_on_master,
|
head_on_master=head_on_master,
|
||||||
delete_capability_allowed=delete_capability_allowed,
|
delete_capability_allowed=delete_capability_allowed,
|
||||||
|
active_reviewer_leases=active_reviewer_leases,
|
||||||
|
pr_states=pr_states,
|
||||||
)
|
)
|
||||||
|
|
||||||
if dry_run:
|
if dry_run:
|
||||||
@@ -4787,6 +4817,14 @@ def gitea_reconcile_merged_cleanups(
|
|||||||
)
|
)
|
||||||
actions.append({"action": "remove_local_worktree", **result})
|
actions.append({"action": "remove_local_worktree", **result})
|
||||||
|
|
||||||
|
for scratch in report.get("reviewer_scratch_entries") or []:
|
||||||
|
if not scratch.get("safe_to_remove_worktree"):
|
||||||
|
continue
|
||||||
|
result = merged_cleanup_reconcile.remove_reviewer_scratch_worktree(
|
||||||
|
PROJECT_ROOT, scratch.get("worktree_path") or ""
|
||||||
|
)
|
||||||
|
actions.append({"action": "remove_reviewer_scratch_worktree", **result})
|
||||||
|
|
||||||
report["dry_run"] = False
|
report["dry_run"] = False
|
||||||
report["executed"] = True
|
report["executed"] = True
|
||||||
report["actions"] = actions
|
report["actions"] = actions
|
||||||
|
|||||||
+234
-11
@@ -17,6 +17,11 @@ import issue_lock_store
|
|||||||
|
|
||||||
PROTECTED_BRANCHES = frozenset({"master", "main", "dev"})
|
PROTECTED_BRANCHES = frozenset({"master", "main", "dev"})
|
||||||
CLOSES_FIXES_RE = re.compile(r"\b(?:closes|fixes)\s+#(\d+)\b", re.IGNORECASE)
|
CLOSES_FIXES_RE = re.compile(r"\b(?:closes|fixes)\s+#(\d+)\b", re.IGNORECASE)
|
||||||
|
# Reviewer scratch folders: review-pr<N> or review-pr<N>-submit (#534).
|
||||||
|
REVIEWER_SCRATCH_NAME_RE = re.compile(
|
||||||
|
r"^review-pr(?P<pr>\d+)(?:-submit)?$",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def extract_linked_issue(title: str, body: str) -> int | None:
|
def extract_linked_issue(title: str, body: str) -> int | None:
|
||||||
@@ -36,8 +41,8 @@ def resolve_worktree_path(project_root: str, branch: str) -> str:
|
|||||||
return os.path.join(project_root, "branches", branch_worktree_folder(branch))
|
return os.path.join(project_root, "branches", branch_worktree_folder(branch))
|
||||||
|
|
||||||
|
|
||||||
def discover_local_worktrees(project_root: str) -> dict[str, str]:
|
def _git_worktree_porcelain(project_root: str) -> list[dict[str, str | None]]:
|
||||||
"""Return a mapping from branch name to absolute worktree path (#528)."""
|
"""Parse ``git worktree list --porcelain`` into structured records."""
|
||||||
res = subprocess.run(
|
res = subprocess.run(
|
||||||
["git", "-C", project_root, "worktree", "list", "--porcelain"],
|
["git", "-C", project_root, "worktree", "list", "--porcelain"],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
@@ -45,25 +50,139 @@ def discover_local_worktrees(project_root: str) -> dict[str, str]:
|
|||||||
check=False,
|
check=False,
|
||||||
)
|
)
|
||||||
if res.returncode != 0:
|
if res.returncode != 0:
|
||||||
return {}
|
return []
|
||||||
|
|
||||||
mapping: dict[str, str] = {}
|
records: list[dict[str, str | None]] = []
|
||||||
current_worktree = None
|
current: dict[str, str | None] = {}
|
||||||
for line in res.stdout.splitlines():
|
for line in res.stdout.splitlines():
|
||||||
line = line.strip()
|
line = line.strip()
|
||||||
if not line:
|
if not line:
|
||||||
current_worktree = None
|
if current.get("worktree"):
|
||||||
|
records.append(current)
|
||||||
|
current = {}
|
||||||
continue
|
continue
|
||||||
if line.startswith("worktree "):
|
if line.startswith("worktree "):
|
||||||
current_worktree = line[9:].strip()
|
if current.get("worktree"):
|
||||||
elif line.startswith("branch ") and current_worktree:
|
records.append(current)
|
||||||
|
current = {"worktree": line[9:].strip(), "branch": None, "head": None}
|
||||||
|
elif line.startswith("branch ") and current:
|
||||||
ref = line[7:].strip()
|
ref = line[7:].strip()
|
||||||
if ref.startswith("refs/heads/"):
|
current["branch"] = (
|
||||||
branch_name = ref[11:]
|
ref[11:] if ref.startswith("refs/heads/") else ref
|
||||||
mapping[branch_name] = current_worktree
|
)
|
||||||
|
elif line.startswith("HEAD ") and current:
|
||||||
|
current["head"] = line[5:].strip()
|
||||||
|
elif line == "detached" and current:
|
||||||
|
current["branch"] = None
|
||||||
|
if current.get("worktree"):
|
||||||
|
records.append(current)
|
||||||
|
return records
|
||||||
|
|
||||||
|
|
||||||
|
def discover_local_worktrees(project_root: str) -> dict[str, str]:
|
||||||
|
"""Return a mapping from branch name to absolute worktree path (#528)."""
|
||||||
|
mapping: dict[str, str] = {}
|
||||||
|
for record in _git_worktree_porcelain(project_root):
|
||||||
|
branch_name = record.get("branch")
|
||||||
|
path = record.get("worktree")
|
||||||
|
if branch_name and path:
|
||||||
|
mapping[str(branch_name)] = str(path)
|
||||||
return mapping
|
return mapping
|
||||||
|
|
||||||
|
|
||||||
|
def parse_reviewer_scratch_folder(folder_name: str) -> int | None:
|
||||||
|
"""Return PR number for a reviewer scratch folder name, else None (#534)."""
|
||||||
|
match = REVIEWER_SCRATCH_NAME_RE.match((folder_name or "").strip())
|
||||||
|
if not match:
|
||||||
|
return None
|
||||||
|
return int(match.group("pr"))
|
||||||
|
|
||||||
|
|
||||||
|
def discover_reviewer_scratch_worktrees(project_root: str) -> list[dict[str, Any]]:
|
||||||
|
"""Discover detached reviewer scratch worktrees under ``branches/`` (#534).
|
||||||
|
|
||||||
|
Matches folder names ``review-pr<N>`` and ``review-pr<N>-submit`` only.
|
||||||
|
These are never treated as author worktree paths.
|
||||||
|
"""
|
||||||
|
root = os.path.realpath(project_root)
|
||||||
|
branches_root = os.path.realpath(os.path.join(root, "branches"))
|
||||||
|
found: list[dict[str, Any]] = []
|
||||||
|
for record in _git_worktree_porcelain(project_root):
|
||||||
|
path = record.get("worktree")
|
||||||
|
if not path:
|
||||||
|
continue
|
||||||
|
real = os.path.realpath(path)
|
||||||
|
parent = os.path.dirname(real)
|
||||||
|
if parent != branches_root:
|
||||||
|
continue
|
||||||
|
folder = os.path.basename(real)
|
||||||
|
pr_number = parse_reviewer_scratch_folder(folder)
|
||||||
|
if pr_number is None:
|
||||||
|
continue
|
||||||
|
found.append(
|
||||||
|
{
|
||||||
|
"pr_number": pr_number,
|
||||||
|
"worktree_path": real,
|
||||||
|
"folder_name": folder,
|
||||||
|
"current_branch": record.get("branch"),
|
||||||
|
"head_sha": record.get("head"),
|
||||||
|
"worktree_kind": "reviewer_scratch",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
return found
|
||||||
|
|
||||||
|
|
||||||
|
def assess_reviewer_scratch_cleanup(
|
||||||
|
*,
|
||||||
|
pr_number: int,
|
||||||
|
worktree_path: str,
|
||||||
|
folder_name: str,
|
||||||
|
pr_merged: bool,
|
||||||
|
pr_closed: bool,
|
||||||
|
worktree_state: dict[str, Any],
|
||||||
|
active_reviewer_lease: bool,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Assess whether a reviewer scratch worktree is safe to remove (#534)."""
|
||||||
|
reasons: list[str] = []
|
||||||
|
exists = bool(worktree_state.get("exists"))
|
||||||
|
if not (pr_merged or pr_closed):
|
||||||
|
reasons.append("associated PR is still open")
|
||||||
|
if not exists:
|
||||||
|
reasons.append("reviewer scratch worktree not present")
|
||||||
|
if exists and worktree_state.get("clean") is False:
|
||||||
|
dirty = worktree_state.get("dirty_files") or []
|
||||||
|
reasons.append(
|
||||||
|
"reviewer scratch worktree has tracked edits"
|
||||||
|
+ (f" ({', '.join(dirty)})" if dirty else "")
|
||||||
|
)
|
||||||
|
if active_reviewer_lease:
|
||||||
|
reasons.append("active reviewer lease still requires this worktree")
|
||||||
|
current_branch = worktree_state.get("current_branch")
|
||||||
|
if current_branch:
|
||||||
|
# Scratch trees are expected detached; a named branch is ambiguous ownership.
|
||||||
|
reasons.append(
|
||||||
|
f"reviewer scratch worktree is on branch '{current_branch}' "
|
||||||
|
"(expected detached HEAD for review-pr scratch trees)"
|
||||||
|
)
|
||||||
|
|
||||||
|
safe = exists and not reasons
|
||||||
|
return {
|
||||||
|
"pr_number": pr_number,
|
||||||
|
"worktree_path": worktree_path,
|
||||||
|
"folder_name": folder_name,
|
||||||
|
"worktree_kind": "reviewer_scratch",
|
||||||
|
"worktree_exists": exists,
|
||||||
|
"worktree_clean": worktree_state.get("clean"),
|
||||||
|
"safe_to_remove_worktree": safe,
|
||||||
|
"block_reasons": reasons,
|
||||||
|
"recommended_action": (
|
||||||
|
"remove_reviewer_scratch_worktree" if safe else "keep_reviewer_scratch_worktree"
|
||||||
|
),
|
||||||
|
# Never treat as author worktree cleanup.
|
||||||
|
"author_worktree_cleanup": False,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def read_issue_lock(path: str | None = None) -> dict[str, Any] | None:
|
def read_issue_lock(path: str | None = None) -> dict[str, Any] | None:
|
||||||
if path:
|
if path:
|
||||||
return issue_lock_store.read_lock_file(path.strip())
|
return issue_lock_store.read_lock_file(path.strip())
|
||||||
@@ -310,6 +429,8 @@ def build_reconciliation_report(
|
|||||||
delete_capability_allowed: bool,
|
delete_capability_allowed: bool,
|
||||||
issue_lock_path: str | None = None,
|
issue_lock_path: str | None = None,
|
||||||
protected_branches: frozenset[str] | None = None,
|
protected_branches: frozenset[str] | None = None,
|
||||||
|
active_reviewer_leases: dict[int, bool] | None = None,
|
||||||
|
pr_states: dict[int, dict[str, Any]] | None = None,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
open_heads = collect_open_pr_heads(open_prs)
|
open_heads = collect_open_pr_heads(open_prs)
|
||||||
worktree_map = discover_local_worktrees(project_root)
|
worktree_map = discover_local_worktrees(project_root)
|
||||||
@@ -334,10 +455,51 @@ def build_reconciliation_report(
|
|||||||
worktree_map=worktree_map,
|
worktree_map=worktree_map,
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# #534: reviewer scratch worktrees are reported separately from author cleanup.
|
||||||
|
lease_map = active_reviewer_leases or {}
|
||||||
|
state_map = pr_states or {}
|
||||||
|
open_pr_numbers = {
|
||||||
|
int(pr["number"]) for pr in (open_prs or []) if pr.get("number") is not None
|
||||||
|
}
|
||||||
|
closed_by_number = {
|
||||||
|
int(pr["number"]): pr for pr in (closed_prs or []) if pr.get("number") is not None
|
||||||
|
}
|
||||||
|
scratch_entries: list[dict[str, Any]] = []
|
||||||
|
for scratch in discover_reviewer_scratch_worktrees(project_root):
|
||||||
|
pr_number = int(scratch["pr_number"])
|
||||||
|
state_info = state_map.get(pr_number) or {}
|
||||||
|
closed_pr = closed_by_number.get(pr_number)
|
||||||
|
if closed_pr is not None:
|
||||||
|
pr_merged = bool(closed_pr.get("merged_at") or closed_pr.get("merged"))
|
||||||
|
pr_closed = True
|
||||||
|
elif pr_number in open_pr_numbers:
|
||||||
|
pr_merged = False
|
||||||
|
pr_closed = False
|
||||||
|
else:
|
||||||
|
pr_merged = bool(state_info.get("merged"))
|
||||||
|
pr_closed = bool(state_info.get("closed") or state_info.get("merged"))
|
||||||
|
worktree_path = scratch["worktree_path"]
|
||||||
|
worktree_state = read_local_worktree_state(worktree_path)
|
||||||
|
worktree_state["worktree_path"] = worktree_path
|
||||||
|
# Prefer live branch from state (git) over porcelain branch field.
|
||||||
|
assessment = assess_reviewer_scratch_cleanup(
|
||||||
|
pr_number=pr_number,
|
||||||
|
worktree_path=worktree_path,
|
||||||
|
folder_name=scratch["folder_name"],
|
||||||
|
pr_merged=pr_merged,
|
||||||
|
pr_closed=pr_closed,
|
||||||
|
worktree_state=worktree_state,
|
||||||
|
active_reviewer_lease=bool(lease_map.get(pr_number)),
|
||||||
|
)
|
||||||
|
scratch_entries.append(assessment)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"project_root": os.path.realpath(project_root),
|
"project_root": os.path.realpath(project_root),
|
||||||
"merged_pr_count": len(entries),
|
"merged_pr_count": len(entries),
|
||||||
"entries": entries,
|
"entries": entries,
|
||||||
|
"reviewer_scratch_entries": scratch_entries,
|
||||||
|
"reviewer_scratch_count": len(scratch_entries),
|
||||||
"dry_run": True,
|
"dry_run": True,
|
||||||
"executed": False,
|
"executed": False,
|
||||||
}
|
}
|
||||||
@@ -371,3 +533,64 @@ def remove_local_worktree(project_root: str, branch: str) -> dict[str, Any]:
|
|||||||
"message": f"removed worktree {worktree_path}",
|
"message": f"removed worktree {worktree_path}",
|
||||||
"worktree_path": worktree_path,
|
"worktree_path": worktree_path,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def remove_reviewer_scratch_worktree(
|
||||||
|
project_root: str,
|
||||||
|
worktree_path: str,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Remove a reviewer scratch worktree by absolute path (#534).
|
||||||
|
|
||||||
|
Fail closed unless the path is a direct ``branches/review-pr*`` child of
|
||||||
|
*project_root*. Never routes through author branch-name derivation.
|
||||||
|
"""
|
||||||
|
root = os.path.realpath(project_root)
|
||||||
|
branches_root = os.path.realpath(os.path.join(root, "branches"))
|
||||||
|
real = os.path.realpath((worktree_path or "").strip())
|
||||||
|
folder = os.path.basename(real)
|
||||||
|
if os.path.dirname(real) != branches_root:
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"performed": False,
|
||||||
|
"worktree_kind": "reviewer_scratch",
|
||||||
|
"message": (
|
||||||
|
"reviewer scratch path must be a direct child of "
|
||||||
|
f"{branches_root}; got {real}"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
if parse_reviewer_scratch_folder(folder) is None:
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"performed": False,
|
||||||
|
"worktree_kind": "reviewer_scratch",
|
||||||
|
"message": f"path is not a reviewer scratch folder: {folder}",
|
||||||
|
}
|
||||||
|
if not os.path.isdir(real):
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"performed": False,
|
||||||
|
"worktree_kind": "reviewer_scratch",
|
||||||
|
"message": f"worktree not found: {real}",
|
||||||
|
}
|
||||||
|
res = subprocess.run(
|
||||||
|
["git", "-C", project_root, "worktree", "remove", real],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=False,
|
||||||
|
)
|
||||||
|
if res.returncode != 0:
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"performed": False,
|
||||||
|
"worktree_kind": "reviewer_scratch",
|
||||||
|
"message": (res.stderr or res.stdout or "worktree remove failed").strip(),
|
||||||
|
"worktree_path": real,
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
"success": True,
|
||||||
|
"performed": True,
|
||||||
|
"worktree_kind": "reviewer_scratch",
|
||||||
|
"message": f"removed reviewer scratch worktree {real}",
|
||||||
|
"worktree_path": real,
|
||||||
|
"author_worktree_cleanup": False,
|
||||||
|
}
|
||||||
@@ -238,5 +238,206 @@ class TestMergedCleanupReport(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestReviewerScratchCleanup(unittest.TestCase):
|
||||||
|
"""#534: authorized cleanup path for reviewer scratch worktrees."""
|
||||||
|
|
||||||
|
def test_parse_reviewer_scratch_folder(self):
|
||||||
|
self.assertEqual(mcr.parse_reviewer_scratch_folder("review-pr512-submit"), 512)
|
||||||
|
self.assertEqual(mcr.parse_reviewer_scratch_folder("review-pr99"), 99)
|
||||||
|
self.assertIsNone(mcr.parse_reviewer_scratch_folder("feat-issue-11-x"))
|
||||||
|
self.assertIsNone(mcr.parse_reviewer_scratch_folder("review-feat-issue-11"))
|
||||||
|
self.assertIsNone(mcr.parse_reviewer_scratch_folder("review-pr512-extra-tail"))
|
||||||
|
|
||||||
|
def test_assess_safe_clean_detached_after_merge(self):
|
||||||
|
assessment = mcr.assess_reviewer_scratch_cleanup(
|
||||||
|
pr_number=512,
|
||||||
|
worktree_path="/repo/Gitea-Tools/branches/review-pr512-submit",
|
||||||
|
folder_name="review-pr512-submit",
|
||||||
|
pr_merged=True,
|
||||||
|
pr_closed=True,
|
||||||
|
worktree_state={
|
||||||
|
"exists": True,
|
||||||
|
"clean": True,
|
||||||
|
"current_branch": None,
|
||||||
|
"head_sha": "abc",
|
||||||
|
"dirty_files": [],
|
||||||
|
},
|
||||||
|
active_reviewer_lease=False,
|
||||||
|
)
|
||||||
|
self.assertTrue(assessment["safe_to_remove_worktree"])
|
||||||
|
self.assertFalse(assessment["author_worktree_cleanup"])
|
||||||
|
self.assertEqual(
|
||||||
|
assessment["recommended_action"], "remove_reviewer_scratch_worktree"
|
||||||
|
)
|
||||||
|
self.assertEqual(assessment["worktree_kind"], "reviewer_scratch")
|
||||||
|
|
||||||
|
def test_assess_blocks_dirty_scratch(self):
|
||||||
|
assessment = mcr.assess_reviewer_scratch_cleanup(
|
||||||
|
pr_number=512,
|
||||||
|
worktree_path="/repo/Gitea-Tools/branches/review-pr512-submit",
|
||||||
|
folder_name="review-pr512-submit",
|
||||||
|
pr_merged=True,
|
||||||
|
pr_closed=True,
|
||||||
|
worktree_state={
|
||||||
|
"exists": True,
|
||||||
|
"clean": False,
|
||||||
|
"current_branch": None,
|
||||||
|
"head_sha": "abc",
|
||||||
|
"dirty_files": ["foo.py"],
|
||||||
|
},
|
||||||
|
active_reviewer_lease=False,
|
||||||
|
)
|
||||||
|
self.assertFalse(assessment["safe_to_remove_worktree"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("tracked edits" in r for r in assessment["block_reasons"])
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_assess_blocks_active_lease(self):
|
||||||
|
assessment = mcr.assess_reviewer_scratch_cleanup(
|
||||||
|
pr_number=512,
|
||||||
|
worktree_path="/repo/Gitea-Tools/branches/review-pr512-submit",
|
||||||
|
folder_name="review-pr512-submit",
|
||||||
|
pr_merged=True,
|
||||||
|
pr_closed=True,
|
||||||
|
worktree_state={
|
||||||
|
"exists": True,
|
||||||
|
"clean": True,
|
||||||
|
"current_branch": None,
|
||||||
|
"head_sha": "abc",
|
||||||
|
"dirty_files": [],
|
||||||
|
},
|
||||||
|
active_reviewer_lease=True,
|
||||||
|
)
|
||||||
|
self.assertFalse(assessment["safe_to_remove_worktree"])
|
||||||
|
self.assertTrue(
|
||||||
|
any("active reviewer lease" in r for r in assessment["block_reasons"])
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_assess_blocks_open_pr(self):
|
||||||
|
assessment = mcr.assess_reviewer_scratch_cleanup(
|
||||||
|
pr_number=512,
|
||||||
|
worktree_path="/repo/Gitea-Tools/branches/review-pr512-submit",
|
||||||
|
folder_name="review-pr512-submit",
|
||||||
|
pr_merged=False,
|
||||||
|
pr_closed=False,
|
||||||
|
worktree_state={
|
||||||
|
"exists": True,
|
||||||
|
"clean": True,
|
||||||
|
"current_branch": None,
|
||||||
|
"head_sha": "abc",
|
||||||
|
"dirty_files": [],
|
||||||
|
},
|
||||||
|
active_reviewer_lease=False,
|
||||||
|
)
|
||||||
|
self.assertFalse(assessment["safe_to_remove_worktree"])
|
||||||
|
self.assertTrue(any("still open" in r for r in assessment["block_reasons"]))
|
||||||
|
|
||||||
|
@patch("subprocess.run")
|
||||||
|
@patch("merged_cleanup_reconcile.read_local_worktree_state")
|
||||||
|
def test_report_lists_scratch_separately_from_author(self, mock_state, mock_run):
|
||||||
|
mock_output = (
|
||||||
|
"worktree /repo/Gitea-Tools\n"
|
||||||
|
"bare\n\n"
|
||||||
|
"worktree /repo/Gitea-Tools/branches/feat-issue-11-x\n"
|
||||||
|
"branch refs/heads/feat/issue-11-x\n"
|
||||||
|
"HEAD deadbeef\n\n"
|
||||||
|
"worktree /repo/Gitea-Tools/branches/review-pr512-submit\n"
|
||||||
|
"HEAD abcdef0\n"
|
||||||
|
"detached\n\n"
|
||||||
|
)
|
||||||
|
mock_run.return_value = MagicMock(returncode=0, stdout=mock_output)
|
||||||
|
|
||||||
|
def _state(path):
|
||||||
|
if "review-pr512" in path:
|
||||||
|
return {
|
||||||
|
"exists": True,
|
||||||
|
"clean": True,
|
||||||
|
"current_branch": None,
|
||||||
|
"head_sha": "abcdef0",
|
||||||
|
"dirty_files": [],
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
"exists": True,
|
||||||
|
"clean": True,
|
||||||
|
"current_branch": "feat/issue-11-x",
|
||||||
|
"head_sha": "deadbeef",
|
||||||
|
"dirty_files": [],
|
||||||
|
}
|
||||||
|
|
||||||
|
mock_state.side_effect = _state
|
||||||
|
report = mcr.build_reconciliation_report(
|
||||||
|
project_root="/repo/Gitea-Tools",
|
||||||
|
closed_prs=[
|
||||||
|
{
|
||||||
|
"number": 11,
|
||||||
|
"title": "merged author",
|
||||||
|
"body": "Closes #11",
|
||||||
|
"head": {"ref": "feat/issue-11-x"},
|
||||||
|
"merged_at": "2026-07-06T12:00:00Z",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"number": 512,
|
||||||
|
"title": "merged reviewed",
|
||||||
|
"body": "Closes #512",
|
||||||
|
"head": {"ref": "feat/issue-512-x"},
|
||||||
|
"merged_at": "2026-07-06T12:00:00Z",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
open_prs=[],
|
||||||
|
remote_branch_exists={
|
||||||
|
"feat/issue-11-x": False,
|
||||||
|
"feat/issue-512-x": False,
|
||||||
|
},
|
||||||
|
head_on_master={11: True, 512: True},
|
||||||
|
delete_capability_allowed=True,
|
||||||
|
active_reviewer_leases={512: False},
|
||||||
|
)
|
||||||
|
self.assertEqual(report["merged_pr_count"], 2)
|
||||||
|
self.assertEqual(report["reviewer_scratch_count"], 1)
|
||||||
|
scratch = report["reviewer_scratch_entries"][0]
|
||||||
|
self.assertEqual(scratch["pr_number"], 512)
|
||||||
|
self.assertEqual(scratch["worktree_kind"], "reviewer_scratch")
|
||||||
|
self.assertFalse(scratch["author_worktree_cleanup"])
|
||||||
|
self.assertTrue(scratch["safe_to_remove_worktree"])
|
||||||
|
# Author entries must not swallow the scratch path as local_worktree.
|
||||||
|
author_paths = {
|
||||||
|
(e.get("local_worktree") or {}).get("worktree_path")
|
||||||
|
for e in report["entries"]
|
||||||
|
}
|
||||||
|
self.assertNotIn(
|
||||||
|
"/repo/Gitea-Tools/branches/review-pr512-submit", author_paths
|
||||||
|
)
|
||||||
|
|
||||||
|
@patch("subprocess.run")
|
||||||
|
def test_remove_reviewer_scratch_worktree_path_guard(self, mock_run):
|
||||||
|
mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="")
|
||||||
|
with patch("os.path.isdir", return_value=True):
|
||||||
|
bad = mcr.remove_reviewer_scratch_worktree(
|
||||||
|
"/repo/Gitea-Tools",
|
||||||
|
"/repo/Gitea-Tools/branches/feat-issue-11-x",
|
||||||
|
)
|
||||||
|
self.assertFalse(bad["performed"])
|
||||||
|
good = mcr.remove_reviewer_scratch_worktree(
|
||||||
|
"/repo/Gitea-Tools",
|
||||||
|
"/repo/Gitea-Tools/branches/review-pr512-submit",
|
||||||
|
)
|
||||||
|
self.assertTrue(good["success"])
|
||||||
|
self.assertTrue(good["performed"])
|
||||||
|
self.assertFalse(good["author_worktree_cleanup"])
|
||||||
|
mock_run.assert_any_call(
|
||||||
|
[
|
||||||
|
"git",
|
||||||
|
"-C",
|
||||||
|
"/repo/Gitea-Tools",
|
||||||
|
"worktree",
|
||||||
|
"remove",
|
||||||
|
"/repo/Gitea-Tools/branches/review-pr512-submit",
|
||||||
|
],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
Reference in New Issue
Block a user