fix(reviewer): harden verdict path against security-review findings

Adversarial security review (detector fan-out + proof-or-kill verifier)
of the verdict feature surfaced several verdict-integrity gaps; resolve
the confirmed ones:

- head-drift (high): a mid-run push moves the resolved head, so an APPROVE
  could anchor to an unreviewed commit. Downgrade any verdict to a comment
  when the resolved head differs from the reviewed head (verdict_ignored
  reason head_moved); the push's own re-review submits a fresh verdict.
- self-review fail-open: downgrade to comment when the PR author cannot be
  confirmed (author_unknown), and compare bot logins case-insensitively.
- verdict_submitted now reflects GitHub's returned review state, not just
  the event we asked for, so a coerced APPROVE isn't reported as submitted.
- an authorized verdict whose findings all anchor outside the diff now
  posts as a bodied review with zero inline comments instead of failing.
- add finding_reply to the shared data-block escape tag superset.

agent/tools/publish_review.py

@@ -63,6 +63,9 @@ from ..utils.tracing import REVIEW_TRACING_PROJECT
 logger = logging.getLogger(__name__)
 
 _VERDICT_EVENTS = {"approve": "APPROVE", "request_changes": "REQUEST_CHANGES"}
+# Map the GitHub review event we POST to the review "state" GitHub reports back
+# on the created review object, so we can confirm the verdict actually landed.
+_EVENT_TO_STATE = {"APPROVE": "APPROVED", "REQUEST_CHANGES": "CHANGES_REQUESTED"}
 
 
 async def publish_review(
@@ -113,9 +116,11 @@ async def publish_review(
         GitHub Review was created.
 
         When ``verdict`` was passed, the result also carries
-        ``verdict_submitted`` (a non-COMMENT review state was actually posted)
-        or ``verdict_ignored`` + ``verdict_ignored_reason``
-        (``"verdict_not_requested"`` or ``"self_review"``).
+        ``verdict_submitted`` (GitHub confirmed the requested APPROVE/
+        REQUEST_CHANGES state) or ``verdict_ignored`` +
+        ``verdict_ignored_reason`` (``"verdict_not_requested"``,
+        ``"self_review"``, ``"head_moved"`` — the reviewed commit is no longer
+        the PR head — or ``"author_unknown"``).
     """
     if severity_threshold not in {"low", "medium", "high", "critical"}:
         return {"success": False, "error": f"Invalid severity_threshold: {severity_threshold}"}
@@ -308,26 +313,64 @@ async def _publish_review_async(
 ) -> dict[str, Any]:
     thread_id = get_thread_id_from_runtime()
     verdict_ignored_reason: str | None = None
-    if verdict is not None:
-        pr_author = _pr_author_from_thread(await get_thread_metadata(thread_id))
-        if pr_author in INTERNAL_BOT_LOGINS:
-            logger.info(
-                "publish_review verdict %r downgraded to comment: PR %s/%s#%s was "
-                "authored by internal bot %r (self-review)",
-                verdict,
-                owner,
-                repo,
-                pr_number,
-                pr_author,
-            )
-            verdict = None
-            verdict_ignored_reason = "self_review"
-    event = _VERDICT_EVENTS.get(verdict or "", "COMMENT")
+    reviewed_head_sha = head_sha
     # The run config's head_sha is frozen at run creation; a push that arrived
     # mid-run updated the live head in thread metadata. Prefer that so the
     # review anchors to (and last_reviewed_sha advances to) the commit actually
     # reviewed, not the stale one this run was created for.
     head_sha = await resolve_review_head_sha(thread_id, {"head_sha": head_sha})
+
+    if verdict is not None:
+        metadata = await get_thread_metadata(thread_id)
+        # A verdict is merge-affecting (a real APPROVE/REQUEST_CHANGES), so it
+        # must reflect exactly the commit the agent reviewed. If a push landed
+        # mid-run and moved the head, the resolved head no longer matches what
+        # this run examined — downgrade to a comment rather than stamp an
+        # approval onto unreviewed code (and let the push's own re-review submit
+        # a fresh verdict). A plain comment publish still safely retargets.
+        if reviewed_head_sha and head_sha and head_sha != reviewed_head_sha:
+            logger.info(
+                "publish_review verdict %r downgraded to comment: head moved %s -> %s "
+                "mid-run for %s/%s#%s",
+                verdict,
+                reviewed_head_sha,
+                head_sha,
+                owner,
+                repo,
+                pr_number,
+            )
+            verdict = None
+            verdict_ignored_reason = "head_moved"
+        else:
+            pr_author = _pr_author_from_thread(metadata)
+            bot_logins = {login.casefold() for login in INTERNAL_BOT_LOGINS}
+            if not pr_author:
+                # Fail closed: a verdict on a PR whose author we cannot confirm
+                # might be a self-review on an Open SWE PR. Downgrade rather than
+                # risk approving our own work.
+                logger.info(
+                    "publish_review verdict %r downgraded to comment: PR author "
+                    "unknown for %s/%s#%s",
+                    verdict,
+                    owner,
+                    repo,
+                    pr_number,
+                )
+                verdict = None
+                verdict_ignored_reason = "author_unknown"
+            elif pr_author.casefold() in bot_logins:
+                logger.info(
+                    "publish_review verdict %r downgraded to comment: PR %s/%s#%s was "
+                    "authored by internal bot %r (self-review)",
+                    verdict,
+                    owner,
+                    repo,
+                    pr_number,
+                    pr_author,
+                )
+                verdict = None
+                verdict_ignored_reason = "self_review"
+    event = _VERDICT_EVENTS.get(verdict or "", "COMMENT")
     review_trace_url = await _resolve_review_trace_url(thread_id, trace_link_config_override)
     review_ui_url = dashboard_review_url(owner, repo, pr_number)
     findings = await _backfill_findings_from_pr_threads(
@@ -518,6 +561,49 @@ async def _publish_review_async(
                         "or fix their file/line before retrying."
                     ),
                 }
+        elif event != "COMMENT":
+            # A verdict is pending but every inline comment anchors outside the
+            # diff. GitHub accepts a bodied review with zero inline comments, so
+            # post the authorized verdict rather than dropping it — the verdict
+            # must land even when the findings can't be anchored.
+            verdict_only_body = _decorate_review_body(
+                render_review_body(
+                    pr_number=pr_number,
+                    surfaced_count=0,
+                    trace_url=review_trace_url,
+                    ui_url=review_ui_url,
+                    additional_findings_count=additional_findings_count,
+                ),
+                verdict=verdict,
+                verdict_requester=verdict_requester,
+                verdict_ignored_reason=verdict_ignored_reason,
+            )
+            verdict_only_response = await post_pull_request_review(
+                owner=owner,
+                repo=repo,
+                pr_number=pr_number,
+                head_sha=head_sha,
+                body=verdict_only_body,
+                inline_comments=[],
+                token=token,
+                event=event,
+            )
+            if isinstance(verdict_only_response, dict) and "_error" not in verdict_only_response:
+                review_response = verdict_only_response
+                inline_comments = []
+                eligible_with_payload = []
+                unresolvable_findings = dropped_ids
+            else:
+                verdict_error = (
+                    verdict_only_response.get("_error", "unknown error")
+                    if isinstance(verdict_only_response, dict)
+                    else "no response"
+                )
+                return {
+                    "success": False,
+                    "error": f"Failed to POST PR review: {verdict_error}",
+                    "unresolvable_findings": dropped_ids,
+                }
         else:
             # Either nothing to drop (no diff_line_set available, so we can't
             # tell which findings are bad) or everything would be dropped.
@@ -546,7 +632,19 @@ async def _publish_review_async(
         }
     review_id = review_response.get("id") if isinstance(review_response, dict) else None
 
-    verdict_submitted = event != "COMMENT" and review_id is not None
+    # Trust GitHub's recorded review state, not the event we asked for: GitHub
+    # can accept the POST (returning an id) yet land the review as COMMENTED
+    # (e.g. a same-identity re-approval). Only claim a verdict when the returned
+    # state actually matches. When the response omits state, fall back to the
+    # requested event so a valid submission isn't under-reported.
+    returned_state = review_response.get("state") if isinstance(review_response, dict) else None
+    expected_state = _EVENT_TO_STATE.get(event)
+    if expected_state is None:
+        verdict_submitted = False
+    elif isinstance(returned_state, str) and returned_state:
+        verdict_submitted = returned_state.upper() == expected_state and review_id is not None
+    else:
+        verdict_submitted = review_id is not None
     await _reconcile_last_verdict(
         thread_id=thread_id,
         owner=owner,

agent/utils/prompt_data.py

@@ -18,6 +18,7 @@ DATA_BLOCK_WRAPPER_TAGS = (
     "body",
     "pr_overview",
     "title",
+    "finding_reply",
     "requester_instructions",
 )
 _CLOSING_TAG_RE = re.compile(

tests/reviewer/test_reviewer_publish.py

@@ -2469,13 +2469,21 @@ async def test_publish_review_forwards_authorized_verdict() -> None:
     assert "verdict_ignored" not in result
 
 
+# Sentinel so a test can pass an explicit empty-metadata dict (to exercise the
+# author-unknown fail-closed path) distinctly from "not specified" (which
+# defaults to a normal non-bot author so verdicts are honored).
+_UNSET_METADATA: dict[str, Any] = {"__unset__": True}
+
+
 def _verdict_publish_patches(
     *,
     findings: list[Finding],
     post_review: AsyncMock,
-    thread_metadata: dict[str, Any] | None = None,
+    thread_metadata: dict[str, Any] | None = _UNSET_METADATA,
     dismiss: AsyncMock | None = None,
 ) -> list[Any]:
+    if thread_metadata is _UNSET_METADATA:
+        thread_metadata = {"pr": {"author": "external-contributor"}}
     return [
         patch("agent.tools.publish_review.get_thread_id_from_runtime", return_value="tid"),
         patch("agent.tools.publish_review.list_findings_async", AsyncMock(return_value=findings)),
@@ -2656,3 +2664,171 @@ async def test_publish_async_comment_publish_does_not_dismiss_without_findings()
         )
 
     dismiss.assert_not_awaited()
+
+
+async def test_publish_async_verdict_downgraded_when_head_moves_mid_run() -> None:
+    """A mid-run push that moves the head must downgrade a verdict to a comment
+    rather than stamp an approval on an unreviewed commit."""
+    from contextlib import ExitStack
+
+    from agent.tools.publish_review import _publish_review_async
+
+    post_review = AsyncMock(return_value={"id": 2001, "state": "COMMENTED"})
+    with ExitStack() as stack:
+        for p in _verdict_publish_patches(findings=[], post_review=post_review):
+            stack.enter_context(p)
+        # resolved head differs from the reviewed (config) head → drift.
+        stack.enter_context(
+            patch(
+                "agent.tools.publish_review.resolve_review_head_sha",
+                AsyncMock(return_value="newhead"),
+            )
+        )
+        result = await _publish_review_async(
+            owner="o",
+            repo="r",
+            pr_number=7,
+            head_sha="reviewedhead",
+            token="t",
+            severity_threshold="medium",
+            cap=15,
+            is_re_review=False,
+            verdict="approve",
+            verdict_requester="amoussa1229",
+        )
+
+    assert result["verdict_ignored"] is True
+    assert result["verdict_ignored_reason"] == "head_moved"
+    assert post_review.await_args.kwargs["event"] == "COMMENT"
+
+
+async def test_publish_async_verdict_fails_closed_on_unknown_author() -> None:
+    from contextlib import ExitStack
+
+    from agent.tools.publish_review import _publish_review_async
+
+    post_review = AsyncMock(return_value={"id": 2002, "state": "COMMENTED"})
+    with ExitStack() as stack:
+        # No pr author in metadata → cannot confirm it isn't a bot self-review.
+        for p in _verdict_publish_patches(findings=[], post_review=post_review, thread_metadata={}):
+            stack.enter_context(p)
+        result = await _publish_review_async(
+            owner="o",
+            repo="r",
+            pr_number=7,
+            head_sha="sha",
+            token="t",
+            severity_threshold="medium",
+            cap=15,
+            is_re_review=False,
+            verdict="approve",
+            verdict_requester="amoussa1229",
+        )
+
+    assert result["verdict_ignored"] is True
+    assert result["verdict_ignored_reason"] == "author_unknown"
+    assert post_review.await_args.kwargs["event"] == "COMMENT"
+
+
+async def test_publish_async_self_review_guard_is_case_insensitive() -> None:
+    from contextlib import ExitStack
+
+    from agent.tools.publish_review import _publish_review_async
+
+    post_review = AsyncMock(return_value={"id": 2003, "state": "COMMENTED"})
+    with ExitStack() as stack:
+        for p in _verdict_publish_patches(
+            findings=[],
+            post_review=post_review,
+            thread_metadata={"pr": {"author": "Seahaven-OpenSWE[bot]"}},
+        ):
+            stack.enter_context(p)
+        result = await _publish_review_async(
+            owner="o",
+            repo="r",
+            pr_number=7,
+            head_sha="sha",
+            token="t",
+            severity_threshold="medium",
+            cap=15,
+            is_re_review=False,
+            verdict="approve",
+            verdict_requester="amoussa1229",
+        )
+
+    assert result["verdict_ignored_reason"] == "self_review"
+    assert post_review.await_args.kwargs["event"] == "COMMENT"
+
+
+async def test_publish_async_verdict_lands_when_all_findings_out_of_diff() -> None:
+    """When every finding anchors outside the diff, an authorized verdict must
+    still post as a bodied review with zero inline comments."""
+    from contextlib import ExitStack
+
+    from agent.tools.publish_review import _publish_review_async
+
+    findings = [_f(id="f_1", severity="high", file="a.py", start_line=1, end_line=1)]
+    # First POST 422s on the anchor; the verdict-only retry succeeds.
+    post_review = AsyncMock(
+        side_effect=[
+            {"_error": "HTTP 422", "_error_kind": "unresolved_anchor", "_status": 422},
+            {"id": 2004, "state": "CHANGES_REQUESTED"},
+        ]
+    )
+    with ExitStack() as stack:
+        for p in _verdict_publish_patches(findings=findings, post_review=post_review):
+            stack.enter_context(p)
+        # Force _filter_against_pr_diff to drop everything (no valid comments).
+        stack.enter_context(
+            patch(
+                "agent.tools.publish_review._filter_against_pr_diff",
+                AsyncMock(return_value=([], ["f_1"])),
+            )
+        )
+        result = await _publish_review_async(
+            owner="o",
+            repo="r",
+            pr_number=7,
+            head_sha="sha",
+            token="t",
+            severity_threshold="medium",
+            cap=15,
+            is_re_review=False,
+            verdict="request_changes",
+            verdict_requester="amoussa1229",
+        )
+
+    assert result["success"] is True
+    assert result["verdict_submitted"] is True
+    assert result["verdict_event"] == "REQUEST_CHANGES"
+    # Second (retry) call posts the verdict with no inline comments.
+    assert post_review.await_args.kwargs["event"] == "REQUEST_CHANGES"
+    assert post_review.await_args.kwargs["inline_comments"] == []
+
+
+async def test_publish_async_verdict_not_submitted_when_github_coerces_state() -> None:
+    """GitHub can accept the POST but land the review as COMMENTED; the result
+    must not claim a verdict in that case."""
+    from contextlib import ExitStack
+
+    from agent.tools.publish_review import _publish_review_async
+
+    post_review = AsyncMock(return_value={"id": 2005, "state": "COMMENTED"})
+    with ExitStack() as stack:
+        for p in _verdict_publish_patches(findings=[], post_review=post_review):
+            stack.enter_context(p)
+        result = await _publish_review_async(
+            owner="o",
+            repo="r",
+            pr_number=7,
+            head_sha="sha",
+            token="t",
+            severity_threshold="medium",
+            cap=15,
+            is_re_review=False,
+            verdict="approve",
+            verdict_requester="amoussa1229",
+        )
+
+    assert result["success"] is True
+    assert result.get("verdict_submitted") is not True