feat: diagnose reviewer lease handoff next actions (Closes #599) #608

Merged
sysadmin merged 1 commits from feat/issue-599-reviewer-lease-handoff into master 2026-07-09 16:47:24 -05:00
3 changed files with 402 additions and 1 deletions
+56
View File
@@ -7154,6 +7154,62 @@ def gitea_assess_reviewer_pr_lease(
}
@mcp.tool()
def gitea_diagnose_reviewer_pr_lease_handoff(
pr_number: int,
proposed_worktree: str | None = None,
instructed_session_id: str | None = None,
instructed_comment_id: int | None = None,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
) -> dict:
"""Read-only: diagnose open-PR reviewer lease handoff and emit next action (#599).
Classifies no-lease / own / foreign / instructed-lease-missing-with-replacement
and worktree binding mismatch. Returns a canonical ``next_action`` without
mutating Gitea state. Never steals foreign leases.
"""
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)
h, _o, _r = _resolve(remote, host, org, repo)
try:
username = _authenticated_username(h)
except Exception:
username = None
session_lease = reviewer_pr_lease.get_session_lease() or {}
env_wt = (
(os.environ.get("GITEA_REVIEWER_WORKTREE") or "").strip()
or (os.environ.get("GITEA_ACTIVE_WORKTREE") or "").strip()
or None
)
# Prefer in-session lease id when present; otherwise diagnose as a fresh
# caller without inventing ownership of an on-thread lease.
session_id = (session_lease.get("session_id") or "").strip() or None
diagnosis = reviewer_pr_lease.diagnose_reviewer_pr_lease_handoff(
comments,
pr_number=pr_number,
current_session_id=session_id,
current_reviewer_identity=username,
proposed_worktree=proposed_worktree,
env_bound_worktree=env_wt,
instructed_session_id=instructed_session_id,
instructed_comment_id=instructed_comment_id,
)
diagnosis["success"] = True
diagnosis["remote"] = remote
diagnosis["authenticated_user"] = username
return diagnosis
@mcp.tool()
def gitea_cleanup_post_merge_moot_lease(
pr_number: int,
+238 -1
View File
@@ -522,4 +522,241 @@ def assess_lease_inventory(
"active_review_leases": active,
"stale_review_leases": stale,
"reclaimable_review_leases": reclaimable,
}
}
# Canonical next-action vocabulary for reviewer lease handoff (#599).
NEXT_ACTION_ACQUIRE = "acquire"
NEXT_ACTION_WAIT = "wait"
NEXT_ACTION_RESUME_EXACT_OWNER_SESSION = "resume_exact_owner_session"
NEXT_ACTION_RELEASE_EXPIRED_LEASE = "release_expired_lease"
NEXT_ACTION_OPERATOR_AUTHORIZED_CLEANUP = "operator_authorized_cleanup"
NEXT_ACTION_REPAIR_WORKTREE_BINDING = "repair_worktree_binding"
_HANDOFF_CLASSIFICATIONS = frozenset({
"no_lease",
"own_active",
"own_expired",
"foreign_active",
"foreign_reclaimable",
"foreign_expired",
"instructed_lease_missing_with_replacement",
"worktree_binding_mismatch",
})
def _norm_path(value: str | None) -> str:
text = (value or "").strip()
if not text:
return ""
return os.path.normpath(text.rstrip("/"))
def diagnose_reviewer_pr_lease_handoff(
comments: list[dict],
*,
pr_number: int,
current_session_id: str | None,
current_reviewer_identity: str | None,
proposed_worktree: str | None = None,
env_bound_worktree: str | None = None,
instructed_session_id: str | None = None,
instructed_comment_id: int | None = None,
now: datetime | None = None,
) -> dict[str, Any]:
"""Classify open-PR reviewer lease handoff and emit a canonical next action (#599).
Read-only diagnosis. Never steals, releases, or adopts a foreign lease.
Fail-closed acquisition rules for active foreign leases remain intact.
Returns a structured diagnosis with:
- classification
- next_action (one of wait / resume_exact_owner_session /
release_expired_lease / operator_authorized_cleanup /
repair_worktree_binding / acquire)
- active_lease identity fields when present
- worktree_binding match result
- instructed-lease mismatch flags
"""
now = now or datetime.now(timezone.utc)
session_id = (current_session_id or "").strip()
identity = (current_reviewer_identity or "").strip()
instructed_sid = (instructed_session_id or "").strip()
reasons: list[str] = []
active = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
session_lease = get_session_lease()
# Worktree binding: env-bound vs proposed vs active lease worktree.
env_wt = _norm_path(env_bound_worktree)
prop_wt = _norm_path(proposed_worktree)
lease_wt = _norm_path((active or {}).get("worktree") if active else None)
binding_mismatch = False
binding_details: dict[str, Any] = {
"env_bound_worktree": env_bound_worktree or None,
"proposed_worktree": proposed_worktree or None,
"lease_worktree": (active or {}).get("worktree") if active else None,
"match": True,
}
paths = [p for p in (env_wt, prop_wt, lease_wt) if p]
if len(paths) >= 2 and len(set(paths)) > 1:
binding_mismatch = True
binding_details["match"] = False
reasons.append(
"worktree binding mismatch: env/proposed/lease worktree paths disagree"
)
# Instructed lease gone while a different lease is active (PR #592-style).
instructed_missing_with_replacement = False
if instructed_sid or instructed_comment_id is not None:
if not active:
reasons.append(
"instructed lease is gone and no active replacement lease remains"
)
else:
owner = (active.get("session_id") or "").strip()
cid = active.get("comment_id")
sid_mismatch = bool(instructed_sid and owner and owner != instructed_sid)
cid_mismatch = (
instructed_comment_id is not None
and cid is not None
and int(cid) != int(instructed_comment_id)
)
if sid_mismatch or cid_mismatch:
instructed_missing_with_replacement = True
reasons.append(
"instructed lease is gone; a different active lease replaced it "
f"(active session_id={owner}, comment_id={cid})"
)
# Classification + next_action.
classification = "no_lease"
next_action = NEXT_ACTION_ACQUIRE
if active:
owner = (active.get("session_id") or "").strip()
freshness = active.get("freshness") or classify_lease_freshness(
active, now=now
)
owner_identity = (active.get("reviewer_identity") or "").strip()
is_own = bool(session_id and owner and owner == session_id)
# Same identity alone is NOT ownership for resume; session_id must match.
same_identity = bool(
identity and owner_identity and identity == owner_identity
)
if is_own and freshness in {"active", "stale_warning"}:
classification = "own_active"
next_action = NEXT_ACTION_RESUME_EXACT_OWNER_SESSION
elif is_own and freshness in {"reclaimable", "expired"}:
classification = "own_expired"
next_action = NEXT_ACTION_RELEASE_EXPIRED_LEASE
reasons.append(
f"own lease freshness is '{freshness}'; release via "
"gitea_release_reviewer_pr_lease then re-acquire"
)
elif not is_own and freshness in {"active", "stale_warning"}:
classification = "foreign_active"
next_action = NEXT_ACTION_WAIT
reasons.append(
f"foreign active reviewer lease (session_id={owner}, "
f"phase={active.get('phase')}, freshness={freshness}); "
"do not submit; do not steal"
)
if same_identity:
reasons.append(
"lease identity matches current reviewer but session_id differs; "
"resume only from the exact owner session_id or wait"
)
elif not is_own and freshness == "reclaimable":
classification = "foreign_reclaimable"
next_action = NEXT_ACTION_RELEASE_EXPIRED_LEASE
reasons.append(
f"foreign reclaimable lease (session_id={owner}); clear only via "
"sanctioned gitea_release_reviewer_pr_lease when reclaimable"
)
elif not is_own and freshness == "expired":
classification = "foreign_expired"
next_action = NEXT_ACTION_RELEASE_EXPIRED_LEASE
reasons.append(
f"foreign expired lease (session_id={owner}); use sanctioned release"
)
else:
classification = "foreign_active"
next_action = NEXT_ACTION_WAIT
reasons.append(
f"unclassified active lease state (session_id={owner}, "
f"freshness={freshness}); wait fail-closed"
)
if instructed_missing_with_replacement and classification.startswith(
"foreign"
):
classification = "instructed_lease_missing_with_replacement"
# Foreign active still means wait; reclaimable still release.
if next_action == NEXT_ACTION_WAIT:
reasons.append(
"replacement foreign lease is active — wait; "
"operator_authorized_cleanup only with explicit operator authority"
)
else:
classification = "no_lease"
next_action = NEXT_ACTION_ACQUIRE
reasons.append("no active reviewer lease; acquire via gitea_acquire_reviewer_pr_lease")
# Binding mismatch is a first-class blocker before submit, but does not
# erase foreign-lease wait/release guidance. Override next_action only when
# the session would otherwise be free to acquire or resume (submit path).
if binding_mismatch:
if next_action in {
NEXT_ACTION_ACQUIRE,
NEXT_ACTION_RESUME_EXACT_OWNER_SESSION,
}:
classification = "worktree_binding_mismatch"
next_action = NEXT_ACTION_REPAIR_WORKTREE_BINDING
else:
reasons.append(
"also repair worktree binding before submit "
f"(next_action remains {next_action})"
)
lease_summary = None
if active:
lease_summary = {
"comment_id": active.get("comment_id"),
"session_id": active.get("session_id"),
"phase": active.get("phase"),
"candidate_head": active.get("candidate_head"),
"expires_at": active.get("expires_at"),
"last_activity": active.get("last_activity"),
"freshness": active.get("freshness")
or classify_lease_freshness(active, now=now),
"reviewer_identity": active.get("reviewer_identity"),
"profile": active.get("profile"),
"worktree": active.get("worktree"),
"blocker": active.get("blocker"),
}
return {
"pr_number": pr_number,
"classification": classification,
"next_action": next_action,
"active_lease": lease_summary,
"session_lease": session_lease,
"worktree_binding": binding_details,
"instructed_session_id": instructed_session_id,
"instructed_comment_id": instructed_comment_id,
"instructed_lease_missing_with_replacement": instructed_missing_with_replacement,
"mutation_allowed": (
next_action == NEXT_ACTION_RESUME_EXACT_OWNER_SESSION
and not binding_mismatch
and bool(session_lease)
),
"reasons": reasons,
"forbidden": [
"manual lock deletion",
"raw API bypass",
"mtime manipulation",
"direct _SESSION_LEASE seeding",
"silent foreign lease steal",
],
}
+108
View File
@@ -237,5 +237,113 @@ class TestReviewerLeaseMcpGate(unittest.TestCase):
self.assertTrue(any("lease" in r.lower() for r in reasons))
class TestReviewerLeaseHandoffDiagnose(unittest.TestCase):
"""#599: canonical next-action diagnosis for open-PR lease handoff."""
def setUp(self):
leases.clear_session_lease()
def test_no_lease_next_action_acquire(self):
result = leases.diagnose_reviewer_pr_lease_handoff(
[],
pr_number=592,
current_session_id="me-1",
current_reviewer_identity="sysadmin",
proposed_worktree="branches/review-pr-592",
)
self.assertEqual(result["classification"], "no_lease")
self.assertEqual(result["next_action"], leases.NEXT_ACTION_ACQUIRE)
self.assertIsNone(result["active_lease"])
def test_foreign_active_wait_pr592_style(self):
# Instructed lease gone; newer foreign claim is active.
old = _lease_comment(592, "23139-da018dab43ba", phase="released")
old["id"] = 8640
active = _lease_comment(592, "51515-2bfb70f0685f", phase="claimed")
active["id"] = 8647
result = leases.diagnose_reviewer_pr_lease_handoff(
[old, active],
pr_number=592,
current_session_id="fresh-session",
current_reviewer_identity="sysadmin",
proposed_worktree="branches/review-pr-592-issue-590",
instructed_session_id="23139-da018dab43ba",
instructed_comment_id=8640,
)
self.assertEqual(
result["classification"],
"instructed_lease_missing_with_replacement",
)
self.assertEqual(result["next_action"], leases.NEXT_ACTION_WAIT)
self.assertTrue(result["instructed_lease_missing_with_replacement"])
self.assertEqual(result["active_lease"]["session_id"], "51515-2bfb70f0685f")
self.assertEqual(result["active_lease"]["comment_id"], 8647)
self.assertFalse(result["mutation_allowed"])
def test_foreign_reclaimable_release_expired(self):
reclaim = _lease_comment(
592, "foreign-old", phase="claimed", minutes_ago=65
)
reclaim["id"] = 99
result = leases.diagnose_reviewer_pr_lease_handoff(
[reclaim],
pr_number=592,
current_session_id="me-1",
current_reviewer_identity="sysadmin",
proposed_worktree="branches/review-pr-592",
)
self.assertEqual(result["classification"], "foreign_reclaimable")
self.assertEqual(
result["next_action"], leases.NEXT_ACTION_RELEASE_EXPIRED_LEASE
)
def test_own_active_resume(self):
head = "a" * 40
claim = _lease_comment(592, "me-1", phase="claimed", candidate_head=head)
claim["id"] = 10
leases.record_session_lease({
"pr_number": 592,
"session_id": "me-1",
"candidate_head": head,
"comment_id": 10,
}, lease_provenance=mla.build_lease_provenance(
source=mla.SOURCE_ACQUIRE,
comment_id=10,
))
result = leases.diagnose_reviewer_pr_lease_handoff(
[claim],
pr_number=592,
current_session_id="me-1",
current_reviewer_identity="rev1",
proposed_worktree="branches/review-pr382",
)
self.assertEqual(result["classification"], "own_active")
self.assertEqual(
result["next_action"], leases.NEXT_ACTION_RESUME_EXACT_OWNER_SESSION
)
self.assertTrue(result["mutation_allowed"])
def test_worktree_binding_mismatch(self):
claim = _lease_comment(592, "me-1", phase="claimed")
claim["id"] = 11
# _lease_comment uses branches/review-pr382
result = leases.diagnose_reviewer_pr_lease_handoff(
[claim],
pr_number=592,
current_session_id="me-1",
current_reviewer_identity="rev1",
proposed_worktree="branches/review-pr-592-issue-590",
env_bound_worktree="branches/review-pr-538",
)
self.assertEqual(result["classification"], "worktree_binding_mismatch")
self.assertEqual(
result["next_action"], leases.NEXT_ACTION_REPAIR_WORKTREE_BINDING
)
self.assertFalse(result["worktree_binding"]["match"])
self.assertFalse(result["mutation_allowed"])
if __name__ == "__main__":
unittest.main()