From 8a63476787d7616030c43152476ab95fc09988d6 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Thu, 23 Jul 2026 03:21:07 -0400 Subject: [PATCH] fix: conflict-fix lease lifecycle chain termination and TTL handling (Closes #847, Refs #842) --- pr_work_lease.py | 53 ++++++++++++- tests/test_pr_work_lease.py | 153 ++++++++++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+), 2 deletions(-) diff --git a/pr_work_lease.py b/pr_work_lease.py index 2fb6876..b877f10 100644 --- a/pr_work_lease.py +++ b/pr_work_lease.py @@ -228,25 +228,74 @@ def find_active_reviewer_lease( return None +def _conflict_fix_chain_key(lease: dict) -> tuple | None: + """Identity of the lease chain a conflict-fix marker belongs to (#842). + + Keyed by PR number, profile, head_before, and branch. Returns None when any + required component (pr_number, profile, head_before) is missing or malformed. + """ + raw = lease.get("raw_fields") or {} + pr_number = lease.get("pr_number") + profile = (lease.get("profile") or "").strip().lower() + head_before = lease.get("head_before") + branch = (lease.get("branch") or raw.get("branch") or "").strip() + if not (pr_number and profile and head_before): + return None + return (pr_number, profile, head_before, branch) + + +def _conflict_fix_chain_matches(key1: tuple, key2: tuple) -> bool: + """True when two conflict-fix chain keys refer to the same lease chain.""" + pr1, profile1, head1, branch1 = key1 + pr2, profile2, head2, branch2 = key2 + if pr1 != pr2 or profile1 != profile2 or head1 != head2: + return False + if branch1 and branch2 and branch1 != branch2: + return False + return True + + +def _conflict_fix_chain_terminated_after(entries: list[dict], index: int) -> bool: + """True when a later marker terminates the conflict-fix chain of ``entries[index]``. + + Append-only newest-wins: a terminal marker (phase=released/blocked/done) + ends only its matching claim chain (#842). + """ + key = _conflict_fix_chain_key(entries[index]) + if key is None: + return False + for later in entries[index + 1:]: + phase = (later.get("phase") or "").strip().lower() + if phase not in _TERMINAL_CONFLICT_FIX_PHASES: + continue + later_key = _conflict_fix_chain_key(later) + if later_key and _conflict_fix_chain_matches(key, later_key): + return True + return False + + def find_active_conflict_fix_lease( comments: list[dict], *, pr_number: int, now: datetime | None = None, ) -> dict[str, Any] | None: - """Return the newest unexpired conflict-fix lease for *pr_number*, if any.""" + """Return the newest unexpired, non-terminated conflict-fix lease for *pr_number*, if any.""" now = now or datetime.now(timezone.utc) candidates = [ entry for entry in _comment_entries(comments, pr_number=pr_number) if entry.get("lease_kind") == "conflict_fix" ] - for lease in reversed(candidates): + for index in range(len(candidates) - 1, -1, -1): + lease = candidates[index] if _lease_expired(lease, now=now): continue phase = (lease.get("phase") or "").strip().lower() if phase in _TERMINAL_CONFLICT_FIX_PHASES: continue if phase in _ACTIVE_CONFLICT_FIX_PHASES or phase: + if _conflict_fix_chain_terminated_after(candidates, index): + continue return lease return None diff --git a/tests/test_pr_work_lease.py b/tests/test_pr_work_lease.py index 8dbcb33..ddbf2cc 100644 --- a/tests/test_pr_work_lease.py +++ b/tests/test_pr_work_lease.py @@ -19,6 +19,7 @@ from pr_work_lease import ( # noqa: E402 assess_reviewer_mutation_blocked, assess_reviewer_stale_head_final_report, format_conflict_fix_lease_body, + find_active_conflict_fix_lease, parse_conflict_fix_lease_comment, parse_reviewer_lease_comment, ) @@ -203,5 +204,157 @@ class TestFormatLease(unittest.TestCase): self.assertEqual(parsed["pr_number"], 376) +class TestConflictFixLeaseLifecycle(unittest.TestCase): + def test_claim_followed_by_matching_release(self): + claim_body = _conflict_fix_body(phase="claimed", worktree="branches/fix-376") + expires = (NOW + timedelta(minutes=60)).isoformat().replace("+00:00", "Z") + release_body = "\n".join([ + CONFLICT_FIX_LEASE_MARKER, + "pr: #376", + "branch: feat/fix-376", + "worktree: branches/fix-376", + "profile: prgs-author", + "phase: released", + f"head_before: {HEAD_A}", + f"head_after: {HEAD_B}", + f"expires_at: {expires}", + ]) + comments = [{"body": claim_body}, {"body": release_body}] + lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW) + self.assertIsNone(lease) + + def test_expired_claim_without_release(self): + past_expires = (NOW - timedelta(minutes=10)).isoformat().replace("+00:00", "Z") + claim_body = "\n".join([ + CONFLICT_FIX_LEASE_MARKER, + "pr: #376", + "phase: claimed", + f"head_before: {HEAD_A}", + f"expires_at: {past_expires}", + "profile: prgs-author", + ]) + comments = [{"body": claim_body}] + lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW) + self.assertIsNone(lease) + + def test_mismatched_release_different_head(self): + claim_body = _conflict_fix_body(phase="claimed", worktree="branches/fix-376") + expires = (NOW + timedelta(minutes=60)).isoformat().replace("+00:00", "Z") + release_body = "\n".join([ + CONFLICT_FIX_LEASE_MARKER, + "pr: #376", + "profile: prgs-author", + "phase: released", + f"head_before: {HEAD_B}", + f"expires_at: {expires}", + ]) + comments = [{"body": claim_body}, {"body": release_body}] + lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW) + self.assertIsNotNone(lease) + self.assertEqual(lease["phase"], "claimed") + + def test_mismatched_release_different_branch(self): + claim_body = "\n".join([ + CONFLICT_FIX_LEASE_MARKER, + "pr: #376", + "branch: feat/branch-A", + "phase: claimed", + f"head_before: {HEAD_A}", + f"expires_at: {(NOW + timedelta(minutes=60)).isoformat().replace('+00:00', 'Z')}", + "profile: prgs-author", + ]) + release_body = "\n".join([ + CONFLICT_FIX_LEASE_MARKER, + "pr: #376", + "branch: feat/branch-B", + "phase: released", + f"head_before: {HEAD_A}", + f"expires_at: {(NOW + timedelta(minutes=60)).isoformat().replace('+00:00', 'Z')}", + "profile: prgs-author", + ]) + comments = [{"body": claim_body}, {"body": release_body}] + lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW) + self.assertIsNotNone(lease) + self.assertEqual(lease["phase"], "claimed") + + def test_release_followed_by_newer_claim(self): + claim_1 = _conflict_fix_body(phase="claimed", worktree="branches/fix-376") + expires = (NOW + timedelta(minutes=60)).isoformat().replace("+00:00", "Z") + release_1 = "\n".join([ + CONFLICT_FIX_LEASE_MARKER, + "pr: #376", + "profile: prgs-author", + "phase: released", + f"head_before: {HEAD_A}", + f"head_after: {HEAD_B}", + f"expires_at: {expires}", + ]) + claim_2 = "\n".join([ + CONFLICT_FIX_LEASE_MARKER, + "pr: #376", + "profile: prgs-author", + "phase: claimed", + f"head_before: {HEAD_B}", + f"expires_at: {expires}", + ]) + comments = [{"body": claim_1}, {"body": release_1}, {"body": claim_2}] + lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW) + self.assertIsNotNone(lease) + self.assertEqual(lease["head_before"], HEAD_B) + + def test_malformed_or_ambiguous_markers(self): + malformed_release = "\n".join([ + CONFLICT_FIX_LEASE_MARKER, + "pr: #376", + "phase: released", + # missing head_before and profile + ]) + claim_body = _conflict_fix_body(phase="claimed") + comments = [{"body": claim_body}, {"body": malformed_release}] + lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW) + self.assertIsNotNone(lease) + + def test_pr818_historical_sequence(self): + comment_14696 = "\n".join([ + "", + "pr: #818", + "branch: feat/issue-638-webui-app-shell-phase1", + "worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-638-webui-app-shell-phase1", + "profile: prgs-author", + "session_id: unknown", + "phase: claimed", + "head_before: 08061b7b8aebdd099a37d1abf5dafcf38e4fd3fb", + "expires_at: 2026-07-23T07:12:13Z", + "reviewer_active: no", + ]) + comment_14730 = "\n".join([ + "", + "pr: #818", + "branch: feat/issue-638-webui-app-shell-phase1", + "worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-638-webui-app-shell-phase1", + "profile: prgs-author", + "session_id: prgs-author-61241-e5129c60", + "phase: released", + "head_before: 08061b7b8aebdd099a37d1abf5dafcf38e4fd3fb", + "head_after: 64b6eb5d5402663098de5ded3b0617cc3b3df98f", + "expires_at: 2026-07-23T06:05:00Z", + "reviewer_active: no", + ]) + comments = [{"body": comment_14696}, {"body": comment_14730}] + check_now = datetime(2026, 7, 23, 6, 30, tzinfo=timezone.utc) + lease = find_active_conflict_fix_lease(comments, pr_number=818, now=check_now) + self.assertIsNone(lease) + + reviewer_gate = assess_reviewer_mutation_blocked( + pr_number=818, + comments=comments, + reviewed_head_sha="64b6eb5d5402663098de5ded3b0617cc3b3df98f", + live_head_sha="64b6eb5d5402663098de5ded3b0617cc3b3df98f", + mutation="approve", + now=check_now, + ) + self.assertTrue(reviewer_gate["mutation_allowed"]) + + if __name__ == "__main__": unittest.main() \ No newline at end of file -- 2.43.7