From c9305c8d48787f728b3daeee68492eea860848b6 Mon Sep 17 00:00:00 2001
From: Jakob Heuser <jakob@codedrift.com>
Date: Sat, 29 Aug 2026 09:57:08 -0700
Subject: [PATCH] fix(ci): scope the posted-review check to bot comments
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two gaps in the guard that landed in #210, both found by review on #211.

The comment scan had no author filter, so ANY comment carrying this run's
id and the `Review mode:` marker satisfied it: a quote of the job URL, a
paste of this workflow, an acknowledgement citing the review it answers.
That is exactly the case the step exists to catch — the action posted
nothing — being masked by someone talking about it. It now filters on
`.user.type == "Bot"` before grepping. Filtering on type rather than on a
login means an app rename does not fail every review closed, while the
actual threat, a human comment, is excluded.

Verified against the record: the dead run on #210 still reports not-posted,
the real review on #211 still reports posted, and four human comments on
#210 are now excluded from consideration.

An execution file that exists but holds no result record was also no longer
diagnosed. It used to be a hard error; once `posted` became the verdict it
passed through as `turns=None is_error=None cost=None` with nothing naming
the corruption, so a partial write or an action version skew would vanish
rather than be noticed. It now warns, and deliberately does not fail, since
it says nothing about whether a review was posted.
---
 .../claude-code-review-on-demand.yml          | 29 ++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/claude-code-review-on-demand.yml b/.github/workflows/claude-code-review-on-demand.yml
index 13fc459f..6cf0cedb 100644
--- a/.github/workflows/claude-code-review-on-demand.yml
+++ b/.github/workflows/claude-code-review-on-demand.yml
@@ -431,9 +431,21 @@ jobs:
           # but the entire job of this check is to identify THIS run's comment
           # rather than an older review's, and "exact except for numeric-prefix
           # collisions" is not that.
+          # Only BOT comments count. Without the filter, any comment on the
+          # PR carrying both substrings satisfies the guard — a quote of the
+          # job URL, a paste of this workflow file, an acknowledgement that
+          # cites the review it is answering. That is the precise case this
+          # step exists to catch (the action posted nothing) being masked by
+          # someone talking about it, so the check has to be scoped to who
+          # wrote the comment and not only to what it says.
+          #
+          # Filtering on `.user.type` rather than on a login: an app rename
+          # would otherwise fail every review closed, and the threat here is a
+          # human comment, which this excludes. A DIFFERENT bot would still
+          # need to reproduce this run's id and the marker to matter.
           posted=false
           if gh api "repos/${GITHUB_REPOSITORY}/issues/${PR}/comments" \
-               --paginate --jq '.[].body | @json' 2>/dev/null \
+               --paginate --jq '.[] | select(.user.type == "Bot") | .body | @json' 2>/dev/null \
              | grep -E "runs/${GITHUB_RUN_ID}([^0-9]|$)" \
              | grep -qF "Review mode:"; then
             posted=true
@@ -477,6 +489,21 @@ jobs:
               if isinstance(message, dict) and message.get("type") == "result":
                   result = message
 
+          # A file that exists but yields no result record is a DIFFERENT
+          # failure from one that was never written, and it used to be a hard
+          # error. Since `posted` became the verdict, an unparseable file would
+          # otherwise pass through as `turns=None is_error=None cost=None` with
+          # nothing pointing at the corruption, so a metrics-pipeline
+          # regression (a partial write, an action version skew) would vanish
+          # rather than being noticed. Not a failure, because it says nothing
+          # about whether a review was posted.
+          if path and os.path.exists(path) and result is None:
+              print(
+                  "::warning::The execution file exists but holds no result"
+                  " record, so run metrics are unavailable. This does not"
+                  " affect whether a review was posted; see review_posted."
+              )
+
           turns = result.get("num_turns") if result else None
           is_error = result.get("is_error") if result else None
           cost = result.get("total_cost_usd") if result else None