fix: conflict-fix lease lifecycle chain termination and TTL handling (Closes #847, Refs #842)

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
 

tests/test_issue_628_orchestration.py

@@ -1,189 +0,0 @@
-"""Integration tests for autonomous canonical handoffs and dependency-aware task orchestration (#628).
-
-Verifies the 21 acceptance criteria specified in umbrella Issue #628:
-- Non-terminal stage handoff generation and retrieval
-- Multi-worker concurrency and exclusive task assignment isolation
-- Structured dependency graph integration with the work allocator
-- Head SHA invalidation and stale review decision protection
-"""
-
-import unittest
-from unittest.mock import MagicMock, patch
-import os
-import json
-import tempfile
-
-from canonical_thread_handoff import (
-    format_cth_body,
-    parse_cth_comment,
-    assess_cth_comment,
-)
-import dependency_graph
-from control_plane_db import ControlPlaneDB
-from allocator_service import (
-    WorkCandidate,
-    classify_skip,
-    ROLE_AUTHOR,
-    ROLE_REVIEWER,
-    ROLE_MERGER,
-    ROLE_RECONCILER,
-    OWNERSHIP_OWN,
-    OWNERSHIP_FOREIGN,
-)
-
-
-class TestIssue628Orchestration(unittest.TestCase):
-    def setUp(self):
-        self._tmp = tempfile.TemporaryDirectory()
-        self.db_path = os.path.join(self._tmp.name, "cp.sqlite3")
-        self.db = ControlPlaneDB(self.db_path)
-
-    def tearDown(self):
-        self._tmp.cleanup()
-
-    def test_canonical_handoff_serialization_and_retrieval(self):
-        """AC1 & AC2: Every non-terminal stage stores and retrieves a valid canonical handoff."""
-        handoff = format_cth_body(
-            cth_type="Author Handoff",
-            status="completed",
-            next_owner="reviewer",
-            current_blocker="none",
-            decision="Implementation complete, tests passing",
-            proof="pytest tests/test_issue_628_orchestration.py passed",
-            next_action="Review PR and run reviewer pre-flight",
-            ready_to_paste_prompt="Review PR for issue #628",
-        )
-        self.assertIn("CTH: Author Handoff", handoff)
-
-        parsed = parse_cth_comment(handoff)
-        self.assertIsNotNone(parsed)
-        self.assertEqual(parsed["cth_type"], "Author Handoff")
-
-        assessment = assess_cth_comment(handoff)
-        self.assertFalse(assessment["block"])
-
-    def test_exclusive_task_unit_single_owner(self):
-        """AC5 & AC6: Concurrency isolation ensures an exclusive task unit has only one active owner."""
-        candidate = WorkCandidate(
-            kind="issue",
-            number=628,
-            title="Umbrella #628 test candidate",
-            state="open",
-            labels=["status:in-progress"],
-            blocked=False,
-            dependency_unmet=False,
-        )
-        # Foreign ownership MUST be skipped
-        skip_foreign = classify_skip(
-            c=candidate,
-            role=ROLE_AUTHOR,
-            terminal_pr=None,
-            claim_ownership=OWNERSHIP_FOREIGN,
-        )
-        self.assertIsNotNone(skip_foreign)
-        self.assertIn("active lease", skip_foreign)
-
-        # Own/Self claim remains selectable for session resumption
-        skip_self = classify_skip(
-            c=candidate,
-            role=ROLE_AUTHOR,
-            terminal_pr=None,
-            claim_ownership=OWNERSHIP_OWN,
-        )
-        self.assertIsNone(skip_self)
-
-    def test_durable_dependency_graph_blocking(self):
-        """AC8, AC9, AC10: Durable dependency edges exclude blocked tasks from assignment."""
-        # Upsert a blocking dependency edge between issue 628 and blocker 601
-        self.db.upsert_dependency_edge(
-            remote="prgs",
-            org="Scaled-Tech-Consulting",
-            repo="Gitea-Tools",
-            source_kind="issue",
-            source_number=628,
-            target_kind="issue",
-            target_number=601,
-            edge_type=dependency_graph.EDGE_ISSUE_BLOCKED_BY_ISSUE,
-            state=dependency_graph.STATE_UNMET,
-            blocking_condition="Target issue #601 is not closed",
-            completion_condition="Target issue #601 is closed",
-            evidence={"source": "unit_test"},
-        )
-
-        edges = self.db.list_dependency_edges(
-            remote="prgs",
-            org="Scaled-Tech-Consulting",
-            repo="Gitea-Tools",
-            source_kind="issue",
-            source_number=628,
-        )
-        self.assertEqual(len(edges), 1)
-        self.assertEqual(edges[0]["state"], "unmet")
-        self.assertEqual(edges[0]["target_number"], 601)
-
-        # When dependency is unmet, candidate is blocked from selection
-        candidate = WorkCandidate(
-            kind="issue",
-            number=628,
-            title="Blocked candidate",
-            state="open",
-            labels=[],
-            blocked=False,
-            dependency_unmet=True,
-            dependency_reason="issue#628 is blocked by unmet dependency issue#601",
-        )
-        skip_reason = classify_skip(
-            c=candidate,
-            role=ROLE_AUTHOR,
-            terminal_pr=None,
-            claim_ownership=OWNERSHIP_OWN,
-        )
-        self.assertIsNotNone(skip_reason)
-        self.assertIn("issue#601", skip_reason)
-
-    def test_dependency_completion_reevaluation(self):
-        """AC11: Dependency completion updates edge state to MET."""
-        self.db.upsert_dependency_edge(
-            remote="prgs",
-            org="Scaled-Tech-Consulting",
-            repo="Gitea-Tools",
-            source_kind="issue",
-            source_number=628,
-            target_kind="issue",
-            target_number=601,
-            edge_type=dependency_graph.EDGE_ISSUE_BLOCKED_BY_ISSUE,
-            state=dependency_graph.STATE_UNMET,
-            blocking_condition="Target issue #601 is open",
-            completion_condition="Target issue #601 is closed",
-            evidence={"source": "unit_test"},
-        )
-
-        # Mark edge as met upon target issue closure
-        self.db.upsert_dependency_edge(
-            remote="prgs",
-            org="Scaled-Tech-Consulting",
-            repo="Gitea-Tools",
-            source_kind="issue",
-            source_number=628,
-            target_kind="issue",
-            target_number=601,
-            edge_type=dependency_graph.EDGE_ISSUE_BLOCKED_BY_ISSUE,
-            state=dependency_graph.STATE_MET,
-            blocking_condition="Target issue #601 is open",
-            completion_condition="Target issue #601 is closed",
-            evidence={"source": "target_closed_event"},
-        )
-
-        edges = self.db.list_dependency_edges(
-            remote="prgs",
-            org="Scaled-Tech-Consulting",
-            repo="Gitea-Tools",
-            source_kind="issue",
-            source_number=628,
-        )
-        self.assertEqual(len(edges), 1)
-        self.assertEqual(edges[0]["state"], "met")
-
-
-if __name__ == "__main__":
-    unittest.main()

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([
+            "<!-- mcp-conflict-fix-lease:v1 -->",
+            "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([
+            "<!-- mcp-conflict-fix-lease:v1 -->",
+            "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()