From 194301359e1cac56addb8691b1043a98daa14220 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sun, 5 Jul 2026 12:45:05 -0400 Subject: [PATCH] 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 --- docs/llm-workflow-runbooks.md | 19 +++++++++++++++ mcp_server.py | 41 +++++++++++++++++++++++++++----- tests/test_mcp_server.py | 21 ++++++++++++---- tests/test_pr_queue_inventory.py | 8 +++++-- 4 files changed, 77 insertions(+), 12 deletions(-) diff --git a/docs/llm-workflow-runbooks.md b/docs/llm-workflow-runbooks.md index 1ab0b14..f8afd5d 100644 --- a/docs/llm-workflow-runbooks.md +++ b/docs/llm-workflow-runbooks.md @@ -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 ` 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 --prune && git checkout master && git pull 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). diff --git a/mcp_server.py b/mcp_server.py index cb2f9c4..57251d4 100644 --- a/mcp_server.py +++ b/mcp_server.py @@ -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 ' 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 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.", ], }, diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index abff0d0..d39ff87 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -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) diff --git a/tests/test_pr_queue_inventory.py b/tests/test_pr_queue_inventory.py index 9b88b80..3e282e2 100644 --- a/tests/test_pr_queue_inventory.py +++ b/tests/test_pr_queue_inventory.py @@ -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")