feat: harden PR queue reconciliation against stale/conflicting state (#166)

- Add updated_at (list_prs), + merged_at/merge_commit_sha/closed_at/updated_at (view_pr) for staleness detection in tool output and inventory. - Surface updated_at in gitea_review_pr inventory report. - Strengthen gitea-pr-review skill steps, _COMMON_WORKFLOWS, _GUIDE_RULES with explicit live reconciliation checklist, 'do not trust prior handoffs', stop on conflict. - Add dedicated live queue reconciliation runbook section in docs/llm-workflow-runbooks.md. - Add passthrough tests and assertions for new fields. - All gates, redaction, author/reviewer separation untouched. - Tests pass, py_compile clean, diff clean, secret sweep clean. Closes #166
2026-07-05 12:45:05 -04:00
parent a02d990bc5
commit 194301359e
4 changed files with 77 additions and 12 deletions
@@ -345,6 +345,25 @@ touching anything.
 - **Prompt:** `Use any eligible reviewer profile to review PR #N. Approve only
  if scope matches issue #M and checks pass; otherwise request changes.`

+**Live queue reconciliation (mandatory before any review/merge decision):**
+
+- Reconcile live state first. Do **not** assume prior handoffs, cached tool
+  output, or chat summaries are current.
+- Steps (in order):
+  1. Call `gitea_list_prs` (open state) with explicit remote/org/repo.
+  2. Immediately `gitea_view_pr <number>` for the candidate; capture head SHA,
+     state, mergeable, updated_at, merged_at/merge_commit_sha if present.
+  3. Verify against any prior report: state, head SHA, updated timestamp,
+     linked issue state (use `gitea_view_issue` + `gitea_list_issues`).
+  4. `git fetch <remote> --prune && git checkout master && git pull <remote> master --ff-only`
+  5. If conflict/staleness detected (prior said "merged" but live open; head or
+     updated_at differs from claimed; merge commit missing on master), report
+     the inconsistency explicitly and **STOP** before review or merge.
+- After a successful merge: re-run list_prs + view_pr on the PR, confirm
+  master advanced, and include the live post-merge verification in the handoff.
+- Treat any ambiguous queue state as a blocker until a fresh, consistent live
+  picture is obtained.
+
 ### Merge a PR

 - **Profile:** merger (allowed to merge; must **not** be the PR author).
@@ -402,8 +402,9 @@ def gitea_list_prs(
        repo: Override the repository name.

    Returns:
-        List of dicts with 'number', 'title', 'state', 'head', 'base', and
-        'mergeable' ('url' only with the reveal opt-in).
+        List of dicts with 'number', 'title', 'state', 'head', 'base',
+        'mergeable', 'updated_at' ('url' only with the reveal opt-in).
+        The additional 'updated_at' aids stale/conflicting queue detection.
    """
    h, o, r = _resolve(remote, host, org, repo)
    auth = _auth(h)
@@ -418,6 +419,7 @@ def gitea_list_prs(
            "head": pr["head"]["ref"],
            "base": pr["base"]["ref"],
            "mergeable": pr.get("mergeable"),
+            "updated_at": pr.get("updated_at"),
        }
        results.append(_with_optional_url(entry, pr.get("html_url")))
    return results
@@ -441,7 +443,10 @@ def gitea_view_pr(
        repo: Override the repository name.

    Returns:
-        dict with PR details.
+        dict with PR details including 'updated_at', 'merged_at',
+        'merge_commit_sha', 'closed_at' (when present) to support
+        live-state reconciliation and staleness detection against
+        prior reports or handoffs.
    """
    h, o, r = _resolve(remote, host, org, repo)
    auth = _auth(h)
@@ -456,6 +461,10 @@ def gitea_view_pr(
        "base": pr["base"]["ref"],
        "mergeable": pr.get("mergeable"),
        "user": pr.get("user", {}).get("login", ""),
+        "updated_at": pr.get("updated_at"),
+        "merged_at": pr.get("merged_at"),
+        "merge_commit_sha": pr.get("merge_commit_sha"),
+        "closed_at": pr.get("closed_at"),
    }
    return _with_optional_url(result, pr.get("html_url"))

@@ -1404,6 +1413,7 @@ def gitea_review_pr(
                    "linked_issues": list(set(linked)),
                    "labels": labels,
                    "head_sha": pr_head_sha,
+                    "updated_at": pr.get("updated_at"),
                    "requires_non_author_reviewer": req_non_author
                })
        except Exception as e:
@@ -1429,6 +1439,7 @@ def gitea_review_pr(
                    f"  Author: {item['author']}\n"
                    f"  Base/Head: {item['base']} / {item['head']}\n"
                    f"  Head SHA: {item['head_sha']}\n"
+                    f"  Updated: {item.get('updated_at') or 'unknown'}\n"
                    f"  Mergeability: {item['mergeable']}\n"
                    f"  Linked Issues: {item['linked_issues']}\n"
                    f"  Labels: {item['labels']}\n"
@@ -1968,6 +1979,12 @@ _GUIDE_RULES = {
        "Do not hardcode Gitea identities; always dynamically verify your "
        "identity and resolve task capability via gitea_resolve_task_capability.",
    ],
+    "live_state_reconciliation": [
+        "Never rely on prior handoffs, chat history, or cached reports for PR queue state.",
+        "Before review or merge decisions: always call gitea_list_prs + gitea_view_pr (for the specific PR), re-fetch remotes, pull master --ff-only, and compare live fields (state, mergeable, head SHA, updated_at, merged_at, merge_commit_sha) against any prior claims.",
+        "Conflicting or stale PR state (e.g. prior said merged but live open, or head/updated mismatch) is a hard blocker: report it explicitly and stop until live state is unambiguous.",
+        "After merge, re-list PRs and re-view the PR (plus verify master contains the merge) before claiming completion.",
+    ],
 }

 _COMMON_WORKFLOWS = [
@@ -1978,10 +1995,15 @@ _COMMON_WORKFLOWS = [
    "implementation: claim issue, branch from fresh master, implement only "
    "the issue scope, test, open a PR referencing the issue, stop.",
    "PR review: verify reviewer identity (must be a reviewer profile/namespace), "
-    "pin the head SHA, validate independently, post a verdict; never review your own work.",
+    "reconcile live state first (list_prs + view_pr selected PR immediately; "
+    "verify state/mergeable/head SHA/updated_at/merged info vs any prior reports; "
+    "git fetch --prune and pull --ff-only master; check linked issue state); "
+    "pin the head SHA; treat conflicting/stale reports as blocker and stop; "
+    "validate independently, post a verdict; never review your own work. "
+    "After merge, re-list and re-view.",
    "PR merge: reviewer identity (must be a reviewer profile/namespace) + "
    "eligibility check + pinned head + explicit 'MERGE PR <n>' confirmation, "
-    "only with operator authorization.",
+    "only with operator authorization. Reconcile live state before deciding.",
 ]

 # Skill registry (#128). status: 'available' = backed by tools in this
@@ -2042,11 +2064,18 @@ _PROJECT_SKILLS = {
            "to confirm reviewer namespace and avoid author-profile blocks.",
            "Verify reviewer identity with gitea_whoami; the PR author "
            "must be a different user.",
-            "Pin the PR head SHA (gitea_view_pr) before validating.",
+            "Reconcile live queue state FIRST (do not trust prior handoffs): "
+            "call gitea_list_prs, then immediately gitea_view_pr on the target; "
+            "verify state, mergeable, head vs current, updated_at/merged info, "
+            "linked issue; git fetch --prune; git checkout master; git pull <remote> master --ff-only. "
+            "If prior report conflicts with live (e.g. 'merged' but live open, or head/updated mismatch), "
+            "report inconsistency and STOP before review/merge.",
+            "Pin the reviewed head SHA (gitea_view_pr) before validating.",
            "Validate independently: scope vs the linked issue, tests, "
            "diff check, secret sweep.",
            "Post the verdict with gitea_review_pr / "
            "gitea_submit_pr_review.",
+            "After any merge, re-list open PRs and re-view the PR to confirm landed state.",
            "Do not merge unless the operator explicitly authorizes it.",
        ],
    },
@@ -303,7 +303,8 @@ class TestListPRs(unittest.TestCase):
            {
                "number": 1, "title": "PR 1", "state": "open",
                "head": {"ref": "branch1"}, "base": {"ref": "main"},
-                "html_url": "http://url1", "mergeable": True
+                "html_url": "http://url1", "mergeable": True,
+                "updated_at": "2026-07-05T12:00:00Z"
            }
        ]
        with patch.dict(os.environ, {}, clear=True):
@@ -312,6 +313,8 @@ class TestListPRs(unittest.TestCase):
        self.assertEqual(result[0]["number"], 1)
        self.assertEqual(result[0]["head"], "branch1")
        self.assertNotIn("url", result[0])
+        # Staleness fields for queue reconciliation (#166)
+        self.assertEqual(result[0]["updated_at"], "2026-07-05T12:00:00Z")

    @patch("mcp_server.api_get_all")
    @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
@@ -320,7 +323,8 @@ class TestListPRs(unittest.TestCase):
            {
                "number": 1, "title": "PR 1", "state": "open",
                "head": {"ref": "branch1"}, "base": {"ref": "main"},
-                "html_url": "http://url1", "mergeable": True
+                "html_url": "http://url1", "mergeable": True,
+                "updated_at": "2026-07-05T12:00:00Z"
            }
        ]
        with patch.dict(os.environ, {"GITEA_MCP_REVEAL_ENDPOINTS": "1"}, clear=True):
@@ -340,7 +344,11 @@ class TestViewPR(unittest.TestCase):
            "number": 1, "title": "PR 1", "state": "open",
            "head": {"ref": "branch1"}, "base": {"ref": "main"},
            "html_url": "http://url1", "mergeable": True, "body": "description",
-            "user": {"login": "user1"}
+            "user": {"login": "user1"},
+            "updated_at": "2026-07-05T12:00:00Z",
+            "merged_at": None,
+            "merge_commit_sha": None,
+            "closed_at": None
        }
        with patch.dict(os.environ, {}, clear=True):
            result = gitea_view_pr(pr_number=1)
@@ -348,6 +356,10 @@ class TestViewPR(unittest.TestCase):
        self.assertEqual(result["body"], "description")
        self.assertEqual(result["user"], "user1")
        self.assertNotIn("url", result)
+        # Staleness / merge fields for live reconciliation (#166) - passthrough
+        self.assertEqual(result["updated_at"], "2026-07-05T12:00:00Z")
+        self.assertIsNone(result["merged_at"])
+        self.assertIsNone(result["merge_commit_sha"])

    @patch("mcp_server.api_request")
    @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
@@ -356,7 +368,8 @@ class TestViewPR(unittest.TestCase):
            "number": 1, "title": "PR 1", "state": "open",
            "head": {"ref": "branch1"}, "base": {"ref": "main"},
            "html_url": "http://url1", "mergeable": True, "body": "description",
-            "user": {"login": "user1"}
+            "user": {"login": "user1"},
+            "updated_at": "2026-07-05T12:00:00Z", "merged_at": None, "merge_commit_sha": None, "closed_at": None
        }
        with patch.dict(os.environ, {"GITEA_MCP_REVEAL_ENDPOINTS": "1"}, clear=True):
            result = gitea_view_pr(pr_number=1)
@@ -56,7 +56,8 @@ class TestPRQueueInventory(unittest.TestCase):
                "mergeable": True,
                "user": {"login": "jcwalker3"},
                "labels": [{"name": "bug"}],
-                "body": "Fixes #10"
+                "body": "Fixes #10",
+                "updated_at": "2026-07-05T10:00:00Z"
            },
            {
                "number": 2,
@@ -67,7 +68,8 @@ class TestPRQueueInventory(unittest.TestCase):
                "mergeable": True,
                "user": {"login": "other_user"},
                "labels": [],
-                "body": ""
+                "body": "",
+                "updated_at": "2026-07-05T11:00:00Z"
            }
        ]
        mock_api.return_value = {"login": "jcwalker3"} # whoami
@@ -80,6 +82,8 @@ class TestPRQueueInventory(unittest.TestCase):
        self.assertIn("Review/merge blocked (Self-author). Requires a genuine non-author reviewer", result["message"])
        self.assertIn("Review/merge blocked (Current profile is Author)", result["message"])
        self.assertIn("gitea_activate_profile", result["message"])
+        # updated_at surfaced for reconciliation (#166)
+        self.assertIn("Updated: 2026-07-05T10:00:00Z", result["message"])

    @patch("mcp_server.api_get_all")
    @patch("mcp_server.api_request")