Merge remote-tracking branch 'prgs/master' into feat/issue-532-merged-cleanup-worktree-aliases
# Conflicts: # gitea_mcp_server.py # merged_cleanup_reconcile.py
This commit is contained in:
@@ -18,6 +18,11 @@ import issue_lock_store
|
||||
PROTECTED_BRANCHES = frozenset({"master", "main", "dev"})
|
||||
CLOSES_FIXES_RE = re.compile(r"\b(?:closes|fixes)\s+#(\d+)\b", re.IGNORECASE)
|
||||
ISSUE_MARKER_RE = re.compile(r"(?:^|[-_/])issue-(\d+)(?:[-_/]|$)", 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:
|
||||
@@ -90,6 +95,44 @@ def list_local_worktrees(project_root: str) -> list[dict[str, Any]]:
|
||||
return entries
|
||||
|
||||
|
||||
def _git_worktree_porcelain(project_root: str) -> list[dict[str, str | None]]:
|
||||
"""Parse ``git worktree list --porcelain`` into structured records."""
|
||||
res = subprocess.run(
|
||||
["git", "-C", project_root, "worktree", "list", "--porcelain"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
if res.returncode != 0:
|
||||
return []
|
||||
|
||||
records: list[dict[str, str | None]] = []
|
||||
current: dict[str, str | None] = {}
|
||||
for line in res.stdout.splitlines():
|
||||
line = line.strip()
|
||||
if not line:
|
||||
if current.get("worktree"):
|
||||
records.append(current)
|
||||
current = {}
|
||||
continue
|
||||
if line.startswith("worktree "):
|
||||
if current.get("worktree"):
|
||||
records.append(current)
|
||||
current = {"worktree": line[9:].strip(), "branch": None, "head": None}
|
||||
elif line.startswith("branch ") and current:
|
||||
ref = line[7:].strip()
|
||||
current["branch"] = (
|
||||
ref[11:] if ref.startswith("refs/heads/") else ref
|
||||
)
|
||||
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] = {}
|
||||
@@ -202,6 +245,99 @@ def resolve_cleanup_worktree_state(
|
||||
return state
|
||||
|
||||
|
||||
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:
|
||||
if path:
|
||||
return issue_lock_store.read_lock_file(path.strip())
|
||||
@@ -454,6 +590,8 @@ def build_reconciliation_report(
|
||||
issue_lock_path: str | None = None,
|
||||
protected_branches: frozenset[str] | None = None,
|
||||
target_ref: str | None = None,
|
||||
active_reviewer_leases: dict[int, bool] | None = None,
|
||||
pr_states: dict[int, dict[str, Any]] | None = None,
|
||||
) -> dict[str, Any]:
|
||||
open_heads = collect_open_pr_heads(open_prs)
|
||||
entries: list[dict[str, Any]] = []
|
||||
@@ -477,10 +615,51 @@ def build_reconciliation_report(
|
||||
target_ref=target_ref,
|
||||
)
|
||||
)
|
||||
|
||||
# #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 {
|
||||
"project_root": os.path.realpath(project_root),
|
||||
"merged_pr_count": len(entries),
|
||||
"entries": entries,
|
||||
"reviewer_scratch_entries": scratch_entries,
|
||||
"reviewer_scratch_count": len(scratch_entries),
|
||||
"dry_run": True,
|
||||
"executed": False,
|
||||
}
|
||||
@@ -516,3 +695,64 @@ def remove_local_worktree(
|
||||
"message": f"removed worktree {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,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user