Files
Gitea-Tools/issue_lock_worktree.py
T
sysadminandClaude Opus 4.8 3edeba4d7f fix(mcp): allow author-lock recovery after the owning session exits (Closes #753)
A durable author issue lock records the PID of the MCP session that took it.
When that process exits, assess_lock_freshness marks the lock stale (live=False)
even while its lease is still within TTL, so every ownership check that needs a
live lock fails closed -- including gitea_update_pr_branch_by_merge.

Re-taking the lock was unreachable for real work. assess_issue_lock_worktree
requires the worktree to be base-equivalent to master/main/dev, and a branch
that already carries commits is ahead of its base by construction. The existing
assess_expired_lock_reclaim affordance does not apply either: it is only
consulted once the lease has expired, so a dead PID under an unexpired lease
never reaches it. assess_own_branch_adoption already speaks of "lock recovery",
but it runs after the base-equivalence gate and so was never reached.

This adds issue_lock_recovery, a pure assessor that grants a narrow waiver only
when every element of durable ownership still matches exactly and the recorded
process is demonstrably dead: same remote/org/repo/issue, same branch (and the
worktree actually on it), same registered worktree, clean worktree, local head
== remote head == open PR head, same claimant identity/profile, no competing
live lock or lease, and no ambiguous branch claims. A malformed or incomplete
lock record can never prove ownership.

The waiver suppresses base-equivalence and nothing else. Cleanliness and every
other precondition still apply, and brand-new issue claims keep the full
requirement. A refused assessment never raises: it withholds the waiver and
lets the pre-existing guard fail closed exactly as before, so recovery can only
ever add permission, never remove a guard. Refusal reasons are appended to the
block message so a caller sees the exact missing evidence.

A completed recovery is recorded on the lock as dead_session_recovery with the
prior and replacement session PIDs, heads, and claimant, so the takeover is
auditable and never looks like an original claim. Rebinding sets the live
session PID, so the recovered lock satisfies verify_lock_for_mutation and the
downstream PR update paths.

Validation:
* new tests/test_issue_753_dead_pid_lock_recovery.py -- 33 passed
* issue-lock, adoption, store, provenance, registration, duplicate-gate,
  worktree, create-issue-guard suites -- 120 passed
* MCP server, commit payloads, handoff ledger, PR ownership, branch cleanup
  suites -- 286 passed
* full suite -- 3498 passed, 6 skipped, 2 failed
* the same 2 failures reproduce identically on pristine master 0425bf9a
  (test_issue_702_review_findings_f1_f6 F1 recovery-before-probe and
  test_reconciler_supersession_close org/repo forwarding), so they are
  pre-existing and unrelated
* git diff --check clean

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01L9jMhtvTjm5EajofqqaR3F
2026-07-18 19:31:55 -04:00

264 lines
8.9 KiB
Python

"""Issue-lock worktree validation (#249).
Author issue locks must validate the caller's own scratch clone (or declared
worktree path), not the shared MCP server working directory. A clean scratch at
``master``/``main`` must remain lockable while an unrelated session leaves the
shared dev worktree dirty or on a feature branch.
"""
from __future__ import annotations
import os
import subprocess
from reviewer_worktree import parse_dirty_tracked_files
AUTHOR_WORKTREE_ENV = "GITEA_AUTHOR_WORKTREE"
BASE_BRANCHES = frozenset({"master", "main", "dev"})
def resolve_author_worktree_path(
explicit: str | None,
project_root: str,
) -> str:
"""Resolve the author worktree path for lock/PR gates."""
path = (explicit or "").strip()
if not path:
path = (os.environ.get(AUTHOR_WORKTREE_ENV) or "").strip()
if not path:
path = project_root
return os.path.realpath(os.path.abspath(path))
def read_worktree_git_state(
worktree_path: str,
extra_bases: tuple[str, ...] | list[str] = (),
) -> dict:
"""Read branch name and porcelain status from a git worktree.
``extra_bases`` names additional branches (e.g. an approved stacked base)
that may anchor base-equivalence in addition to master/main/dev. When empty
(the default), only the normal base branches are considered.
"""
path = (worktree_path or "").strip()
if not path:
return {"current_branch": None, "porcelain_status": ""}
branch_res = subprocess.run(
["git", "-C", path, "branch", "--show-current"],
capture_output=True,
text=True,
check=False,
)
current_branch = (branch_res.stdout or "").strip() or None
status_res = subprocess.run(
["git", "-C", path, "status", "--porcelain"],
capture_output=True,
text=True,
check=False,
)
root_res = subprocess.run(
["git", "-C", path, "rev-parse", "--show-toplevel"],
capture_output=True,
text=True,
check=False,
)
head_res = subprocess.run(
["git", "-C", path, "rev-parse", "HEAD"],
capture_output=True,
text=True,
check=False,
)
head_sha = (head_res.stdout or "").strip() if head_res.returncode == 0 else None
base_branch, base_sha = _find_matching_base_ref(path, head_sha, extra_bases)
return {
"current_branch": current_branch,
"porcelain_status": status_res.stdout or "",
"inspected_git_root": (root_res.stdout or "").strip() if root_res.returncode == 0 else None,
"head_sha": head_sha,
"base_branch": base_branch,
"base_sha": base_sha,
"base_equivalent": bool(head_sha and base_sha and head_sha == base_sha),
}
def assess_issue_lock_worktree(
*,
worktree_path: str,
current_branch: str | None,
porcelain_status: str,
base_equivalent: bool | None = None,
inspected_git_root: str | None = None,
base_branch: str | None = None,
base_branches: frozenset[str] | None = None,
recovery_sanctioned: bool = False,
) -> dict:
"""Fail closed when lock preconditions are not met on the declared worktree.
``recovery_sanctioned`` is set only when ``issue_lock_recovery`` has already
proven, from the durable lock itself, that this is a dead-session recovery of
an existing claim (#753): same issue, branch, worktree, author, and head, with
the recording process dead. In that one case the base-equivalence requirement
is waived, because a branch that already carries the work is ahead of its base
by construction and could never satisfy it. Every other precondition —
notably worktree cleanliness — still applies unchanged, and brand-new issue
claims keep the full base-equivalence requirement.
"""
bases = base_branches or BASE_BRANCHES
reasons: list[str] = []
path = (worktree_path or "").strip()
if not path:
reasons.append("worktree path not declared for issue lock; fail closed")
return _assessment(False, reasons, path, None, [])
branch = (current_branch or "").strip()
dirty_files = parse_dirty_tracked_files(porcelain_status)
if dirty_files:
reasons.append(
"tracked file edits exist before issue lock; "
f"lock must precede implementation work in '{path}' "
f"(dirty files: {', '.join(dirty_files)})"
)
if recovery_sanctioned:
# Base-equivalence intentionally not evaluated: ownership was proven
# against the durable lock record instead (#753).
pass
elif base_equivalent is False:
reasons.append(
"issue lock worktree must be base-equivalent to one of "
f"{_base_list(bases)} before implementation work; inspected "
f"branch '{branch or '(detached)'}' at '{path}'"
)
elif base_equivalent is None:
if not branch:
reasons.append(
"current branch unknown (detached HEAD?); issue lock base-equivalence "
f"to {_base_list(bases)} could not be proven"
)
elif branch not in bases:
reasons.append(
"issue lock worktree base-equivalence could not be proven; "
f"branch '{branch}' is not {_base_list(bases)}"
)
proven = not reasons
return _assessment(
proven,
reasons,
path,
branch or None,
dirty_files,
inspected_git_root=inspected_git_root,
base_branch=base_branch,
base_equivalent=base_equivalent,
recovery_sanctioned=recovery_sanctioned,
)
def format_issue_lock_worktree_error(assessment: dict) -> str:
"""Format a single fail-closed error for ``gitea_lock_issue``."""
reasons = list(assessment.get("reasons") or [])
if not reasons:
reasons = ["issue lock worktree validation failed"]
return "; ".join(reasons) + " (fail closed)"
def verify_pr_worktree_matches_lock(
locked_worktree_path: str | None,
declared_worktree_path: str | None,
project_root: str,
) -> dict:
"""PR creation must use the same worktree the lock was validated against."""
locked = (locked_worktree_path or "").strip()
if not locked:
return {"proven": True, "block": False, "reasons": []}
declared = resolve_author_worktree_path(declared_worktree_path, project_root)
locked_real = os.path.realpath(locked)
declared_real = os.path.realpath(declared)
if locked_real != declared_real:
return {
"proven": False,
"block": True,
"reasons": [
f"PR worktree '{declared_real}' does not match locked worktree "
f"'{locked_real}' (fail closed)"
],
"locked_worktree_path": locked_real,
"declared_worktree_path": declared_real,
}
return {
"proven": True,
"block": False,
"reasons": [],
"locked_worktree_path": locked_real,
"declared_worktree_path": declared_real,
}
def _base_list(bases: frozenset[str]) -> str:
return "/".join(sorted(bases))
def _assessment(
proven: bool,
reasons: list[str],
worktree_path: str,
current_branch: str | None,
dirty_files: list[str],
*,
inspected_git_root: str | None = None,
base_branch: str | None = None,
base_equivalent: bool | None = None,
recovery_sanctioned: bool = False,
) -> dict:
return {
"proven": proven,
"block": not proven,
"reasons": reasons,
"worktree_path": worktree_path or None,
"inspected_git_root": inspected_git_root,
"current_branch": current_branch,
"dirty_files": dirty_files,
"base_branch": base_branch,
"base_equivalent": base_equivalent,
"recovery_sanctioned": recovery_sanctioned,
"base_equivalence_waived": bool(recovery_sanctioned),
}
def _find_matching_base_ref(
path: str,
head_sha: str | None,
extra_bases: tuple[str, ...] | list[str] = (),
) -> tuple[str | None, str | None]:
"""Return the stable branch ref whose commit matches HEAD, if any.
Normal base branches (master/main/dev) are always considered. ``extra_bases``
adds explicitly-approved stacked bases; each is checked as a local ref and via
the ``prgs``/``origin`` remotes.
"""
if not head_sha:
return None, None
candidates: list[str] = []
for branch in sorted(BASE_BRANCHES):
candidates.extend((f"origin/{branch}", branch))
for branch in extra_bases:
name = (branch or "").strip()
if name:
candidates.extend((f"prgs/{name}", f"origin/{name}", name))
for ref in candidates:
res = subprocess.run(
["git", "-C", path, "rev-parse", "--verify", ref],
capture_output=True,
text=True,
check=False,
)
sha = (res.stdout or "").strip()
if res.returncode == 0 and sha == head_sha:
return ref, sha
return None, None