Merge pull request 'feat: diagnose reviewer lease handoff next actions (Closes #599)' (#608) from feat/issue-599-reviewer-lease-handoff into master

gitea_mcp_server.py

@@ -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,

reviewer_pr_lease.py

@@ -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",
+        ],
+    }

tests/test_reviewer_pr_lease.py

@@ -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()