fix(noema): re-check for a concurrent submission immediately before posting

CHANGELOG.md

@@ -23,6 +23,16 @@ this file. The format follows Keep a Changelog, and versioned releases follow
 Semantic Versioning where the repository publishes a release.
 
 ## [Unreleased]
+- Narrow `inspect_and_review`'s remaining duplicate-verdict race window: it already re-fetches the
+  PR and re-validates `require_expected_head()` after the LLM call before publishing, but never
+  re-checked `existing_noema_review()` at that same point. `noema-review.yml`'s shared concurrency
+  group is the primary defense against two runs racing to review the same head, but GitHub's
+  `cancel-in-progress` is best-effort and does not preempt a run mid-step: a run already past its
+  own pre-model `existing_noema_review()` check when a newer trigger supersedes it could still
+  finish its diff/context/LLM call and post a duplicate review afterward, since an unchanged head
+  SHA does not prove no other process posted a review for that head in the meantime. Added the same
+  `existing_noema_review()` re-check immediately before `submit_review`, narrowing the window from
+  the full diff/context/LLM-call duration down to one GraphQL round trip.
 - **Fix a stale `test_strix_quick_gate.sh` assertion left broken by the `#1630`
   scheduler-cadence lengthening.** `pr-review-merge-scheduler.yml`'s repository-local
   heartbeat was changed from a quarter-hourly `cron: "*/30 * * * *"` to an hourly

scripts/ci/noema_review_gate.py

@@ -1678,6 +1678,19 @@ def inspect_and_review(repo: str, number: int, expected_head: str) -> int:
     except RuntimeError:
         print("Pull request closed or its head changed during review; stale verdict was not published.")
         return 0
+    # Re-check for a concurrent Noema submission immediately before posting.
+    # noema-review.yml's concurrency group (shared across pull_request_target,
+    # workflow_run, and repository_dispatch triggers) is the primary defense
+    # against two runs racing to review the same head, but GitHub's
+    # cancel-in-progress is best-effort and does not preempt a run mid-step:
+    # a run already past the pre-model existing_noema_review() check when a
+    # newer trigger supersedes it can still finish its own diff/context/LLM
+    # call and post afterward. require_expected_head() alone does not catch
+    # this -- it only proves the head SHA is unchanged, not that no other
+    # process posted a review for that same head in the meantime.
+    if existing_noema_review(current_pr, actor):
+        print("Current head already has a Noema review as of just before submission; not posting a duplicate.")
+        return 0
     submit_review(repo, number, current_pr, actor, verdict)
     return 0
 

tests/test_noema_review_gate.py

@@ -612,7 +612,6 @@ def make_pr(**overrides):
         "headRefOid": "head",
         "reviews": {"nodes": []},
         "reviewThreads": {"nodes": []},
-        "statusCheckRollup": {"contexts": {"nodes": []}},
     }
     value.update(overrides)
     return value
@@ -1633,7 +1632,6 @@ def test_inspect_and_review_does_not_wait_for_other_reviews_or_checks(monkeypatc
         headRefOid=head,
         reviews={"nodes": [review("CHANGES_REQUESTED")]},
         reviewThreads={"nodes": [{"isResolved": False, "isOutdated": False}]},
-        statusCheckRollup={"contexts": {"nodes": [{"__typename": "StatusContext", "context": "ci", "state": "FAILURE"}]}},
     )
     calls = []
     monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: pr)
@@ -1648,6 +1646,53 @@ def test_inspect_and_review_does_not_wait_for_other_reviews_or_checks(monkeypatc
     assert calls
 
 
+def test_inspect_and_review_rechecks_for_a_concurrent_submission_before_posting(monkeypatch):
+    """A second trigger (e.g. workflow_run) that starts while this run is still
+    building context/calling the LLM must not publish a duplicate verdict once
+    the first run has already submitted one for the same head. noema-review.yml's
+    concurrency-group serialization is the primary defense; this re-check
+    narrows the remaining race window (cancel-in-progress is best-effort and
+    does not preempt a run mid-step) down to one GraphQL round trip
+    immediately before the POST -- distinct from require_expected_head()'s own
+    post-model re-check, which only proves the head SHA is unchanged, not that
+    no other process posted a review for that head in the meantime."""
+    head = "a" * 40
+    first_call_pr = make_pr(headRefOid=head)
+    noema_marker = "\n".join(
+        [
+            noema.NOEMA_REVIEW_FOOTER_MARKER,
+            "- Result: APPROVE",
+            f"- Head SHA: `{head}`",
+            "- Reviewer credential: `test`",
+            "- Actor: `noema`",
+            "",
+            f"<!-- noema-review-gate head_sha={head} decision=approve -->",
+        ]
+    )
+    already_reviewed_pr = make_pr(
+        headRefOid=head,
+        reviews={"nodes": [review(commit=head, login="noema", body=noema_marker)]},
+    )
+    fetch_pr_calls = []
+
+    def fake_fetch_pr(repo, number):
+        fetch_pr_calls.append(1)
+        return first_call_pr if len(fetch_pr_calls) == 1 else already_reviewed_pr
+
+    calls = []
+    monkeypatch.setattr(noema, "fetch_pr", fake_fetch_pr)
+    monkeypatch.setattr(noema, "current_actor", lambda: "noema")
+    monkeypatch.setattr(noema, "fetch_diff", lambda repo, number: ("diff", False))
+    monkeypatch.setattr(noema, "fetch_changed_files", lambda repo, number: [])
+    monkeypatch.setattr(noema, "build_review_context", lambda repo, number, pr, changed_files=None: "context")
+    monkeypatch.setattr(noema, "call_llm", lambda *args, **kwargs: {"decision": "approve", "summary": "ok", "findings": []})
+    monkeypatch.setattr(noema, "submit_review", lambda *args, **kwargs: calls.append(args))
+
+    assert noema.inspect_and_review("owner/repo", 7, head) == 0
+    assert calls == []
+    assert len(fetch_pr_calls) == 2
+
+
 def test_stale_trigger_stops_before_identity_or_model_work(monkeypatch):
     monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr(headRefOid="b" * 40))
     monkeypatch.setattr(