fix(scripts): count a codex approving reaction on the PR body as sign-off (#1568)

* fix(scripts): count a codex approving reaction on the PR body as sign-off

pre-merge-check.sh detected reviewer activity via reviews, PR comments,
SHA-pinned codex issue-comment verdicts, and reviewer check runs — but not
reactions. Codex (chatgpt-codex-connector[bot]) often signs off on a clean PR
by leaving a thumbs-up (+1) reaction on the PR description instead of posting a
review or comment, so such PRs blocked forever even though the required
reviewer had approved (observed on #1565).

Add PR-body reaction detection: a required reviewer counts as having weighed in
if they left an explicitly POSITIVE reaction (+1/heart/hooray/rocket) on the PR
body. Negative/ambiguous reactions (-1, confused, eyes) never count, and a
positive reaction from a non-reviewer is not credited to the reviewer. The
reactions read is non-fatal — a failure falls back to the other detection paths
rather than crashing the gate.

Extend tests/pre-merge-check.test.mjs: codex +1 signs off; negative reactions
and other users' reactions still block; a reactions-endpoint failure degrades
to a missing-reviewer verdict rather than an API-read crash.

* fix(scripts): bind reaction sign-off to the current head commit

Codex (P1 on #1568): a PR-body reaction carries no commit SHA and GitHub does
not dismiss it on new pushes, so a +1 left on an earlier revision would keep
satisfying the required reviewer for a newer diff it never saw — bypassing the
current-head protection the issue-comment verdict path enforces via SHA pinning.

Reactions can't be SHA-pinned, so bind by time: only count a reaction created
at/after the head commit's committer date. A commit pushed after the +1 has a
newer committer date and invalidates the stale reaction; a rebase rewrites
committer dates and does the same — both fail closed and force a fresh sign-off.
If the head commit date can't be read, no reaction counts. Timestamp compare is
byte-wise under LC_ALL=C on GitHub's fixed-format ISO-8601 UTC values.

Residual (documented): cherry-picking a commit with an OLD committer date as the
new head could re-admit a reaction in that window; the SHA-pinned issue-comment
path stays the strong signal.

Tests: reject a reaction predating the head commit; fail closed when the head
commit date is unknown.

* fix(scripts): bind reaction sign-off to head push time, not committer date

Codex (2nd P1 on #1568): committer date is not push-visibility time. A commit
amended/created locally at 10:00, a +1 left at 10:05 on the still-old head, then
the commit pushed at 10:10 — the reaction is not before the committer date
(10:00) so the previous fix still credited a reaction placed before the new SHA
was ever visible for review.

Bind instead to when the head became visible on the PR: HEAD_VISIBLE_AT =
earliest check-suite created_at for the head SHA (a push-time signal, stable
across job re-runs, and — unlike author/committer date — not backdatable by a
local amend or cherry-pick). A reaction counts only if created at/after that.
GitHub's Commit.pushedDate was the obvious field but is null in practice, so the
check-suite creation time is the reliable push marker. If it can't be read (no
suite yet / API failure), no reaction is credited — fail closed.

Tests updated: stale reaction now means "placed before the head's check-suite
(push) time"; the fail-closed scenario keys off missing push time.

* docs(scripts): document the accepted reaction-freshness residual

Repo-owner decision on codex's 3rd P1: check-suite created_at is keyed to the
SHA, not to when it became this PR's head, and GitHub exposes no clean per-PR
"head advanced at" timestamp (Commit.pushedDate is null in practice). The
remaining bypass needs the identical SHA pushed to two branches with an
adversarially-timed +1 — impossible in this repo's one-branch-per-PR flow. The
SHA-pinned issue-comment verdict stays the strong signal; reactions are a
convenience sign-off, not the security boundary. Document the residual explicitly
rather than drop reaction support (no reliable PR-scoped push signal exists).

Author: joshuaswarren

scripts/pre-merge-check.sh

@@ -9,8 +9,12 @@ set -euo pipefail
 # AI reviewers had time to post reviews. This script blocks merging until
 # reviewers have weighed in and all threads are resolved.
 #
-# Reviewer activity is detected via PR reviews, PR comments, and completed
-# check runs (Cursor Bugbot and Kilo Code Review run as GitHub App checks).
+# Reviewer activity is detected via PR reviews, PR comments, completed check
+# runs (Cursor Bugbot and Kilo Code Review run as GitHub App checks), and an
+# approving reaction on the PR body. Codex (chatgpt-codex-connector[bot]) often
+# signs off on a clean PR by leaving a thumbs-up (+1) reaction on the PR
+# description rather than posting a review or comment — without reaction
+# detection those PRs block forever even though the reviewer approved.
 
 PR_NUMBER="${1:?Usage: scripts/pre-merge-check.sh <PR_NUMBER>}"
 REPO="${REMNIC_REPO:-joshuaswarren/remnic}"
@@ -127,13 +131,80 @@ if ! HEAD_SHA=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json headRefOid --jq .h
   exit 1
 fi
 CHECK_RUNS=""
+HEAD_VISIBLE_AT=""
 if [[ -n "$HEAD_SHA" ]]; then
   if ! CHECK_RUNS=$(gh api "repos/${REPO}/commits/${HEAD_SHA}/check-runs" \
     --jq '.check_runs[] | select(.conclusion == "success" or .conclusion == "failure" or .conclusion == "neutral") | [.app.slug, .name] | @tsv' \
     2>/dev/null); then
     echo "[pre-merge] BLOCKED: Failed to read PR check runs from GitHub."
     exit 1
   fi
+  # When did this exact SHA become visible on the PR? The earliest check-suite
+  # created_at for the head commit marks when the push landed and CI kicked off
+  # — a push-time signal (unlike the commit's committer/author date, which a
+  # local amend or cherry-pick can backdate). It is also stable across job
+  # re-runs (the suite is not recreated). Used to reject reaction sign-offs that
+  # predate the current head being pushed (see the reactions block). Empty when
+  # no check suite exists yet or the read fails → reactions fail closed.
+  HEAD_VISIBLE_AT=$(gh api "repos/${REPO}/commits/${HEAD_SHA}/check-suites" \
+    --jq '[.check_suites[].created_at] | min // empty' 2>/dev/null || true)
+fi
+
+# Reviewer approval via a reaction on the PR body. Codex signs off on clean PRs
+# with a thumbs-up (+1) reaction on the PR description instead of a review or
+# comment. Only count explicitly POSITIVE reactions as a sign-off — a "-1" or
+# "confused" reaction is the opposite of approval, and "eyes" means "still
+# looking", so none of those count.
+#
+# CURRENT-HEAD BINDING: a reaction carries no commit SHA and GitHub does NOT
+# dismiss it when new commits are pushed, so a bare reaction would let a +1 left
+# on an earlier revision satisfy the reviewer for a newer diff it never saw.
+# Reactions cannot be SHA-pinned like issue-comment verdicts, so we bind to when
+# the current head became visible on the PR (HEAD_VISIBLE_AT = earliest
+# check-suite created_at for the head SHA — a push-time signal, not the commit's
+# author/committer date, which a local amend or cherry-pick can backdate). A
+# reaction only counts when it was created at/after the head was pushed; a +1
+# left before the current SHA landed no longer counts, forcing a fresh sign-off.
+# If head visibility cannot be proven (no check suite yet, or the read failed),
+# no reaction is credited — fail closed. A reactions read failure is likewise
+# non-fatal: we fall back to the other detection paths rather than block the gate.
+#
+# ACCEPTED RESIDUAL (deliberate, repo-owner decision): check-suite created_at is
+# keyed to the SHA, not to the moment that SHA became THIS PR's head, and GitHub
+# exposes no clean per-PR "head advanced at" timestamp (Commit.pushedDate is null
+# in practice). So if the identical SHA was already pushed to another branch —
+# creating its check suite earlier — a +1 placed on the old PR revision after
+# that suite but before the PR fast-forwards to the SHA could still be credited.
+# This requires an adversarial same-SHA-on-two-branches race that does not occur
+# in this repo's one-branch-per-PR flow; the SHA-pinned issue-comment verdict
+# remains the strong signal. Reactions are a convenience sign-off, not the
+# security boundary. Tightening this further would require dropping reaction
+# support entirely (no reliable PR-scoped push signal exists).
+#
+# Byte-wise "is $1 strictly before $2" for two GitHub ISO-8601 UTC (…Z)
+# timestamps. Runs in a subshell so LC_ALL=C stays scoped; the fixed-width
+# format makes a byte comparison chronological.
+iso_before() ( LC_ALL=C; [[ "$1" < "$2" ]] )
+
+POSITIVE_REACTIONS_RE='^(\+1|heart|hooray|rocket)$'
+POSITIVE_REACTORS=""
+if [[ -z "$HEAD_VISIBLE_AT" ]]; then
+  echo "[pre-merge] NOTE: Head push time unknown; PR-body reactions will not count as sign-off."
+elif BODY_REACTIONS=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/reactions" --paginate \
+  -H "Accept: application/vnd.github+json" \
+  --jq '.[] | [.user.login, .content, .created_at] | @tsv' 2>/dev/null); then
+  while IFS=$'\t' read -r rx_login rx_content rx_created; do
+    [[ -n "$rx_login" ]] || continue
+    [[ "$rx_content" =~ $POSITIVE_REACTIONS_RE ]] || continue
+    # Reject reactions placed before the current head was pushed (stale sign-off).
+    [[ -n "$rx_created" ]] || continue
+    if iso_before "$rx_created" "$HEAD_VISIBLE_AT"; then
+      continue
+    fi
+    POSITIVE_REACTORS+="${rx_login}"$'\n'
+  done <<< "$BODY_REACTIONS"
+else
+  echo "[pre-merge] NOTE: Could not read PR body reactions; relying on reviews/comments/checks."
 fi
 
 # Issue comments are PR-wide, not head-scoped, and this gate reads them for
@@ -188,11 +259,20 @@ has_reviewer_check_run() {
   return 1
 }
 
+# A required reviewer counts as having signed off if they left a positive
+# reaction on the PR body (exact, case-insensitive login match).
+has_reviewer_positive_reaction() {
+  local reviewer="$1"
+  [[ -n "$POSITIVE_REACTORS" ]] || return 1
+  grep -qxiF "$reviewer" <<< "$POSITIVE_REACTORS"
+}
+
 MISSING_REVIEWERS=()
 for reviewer in "${REQUIRED_REVIEWERS[@]}"; do
   # Use exact line match (-x) to avoid substring false positives.
   if ! echo "$ALL_REVIEWERS" | grep -qxiF "$reviewer" && \
-     ! has_reviewer_check_run "$reviewer"; then
+     ! has_reviewer_check_run "$reviewer" && \
+     ! has_reviewer_positive_reaction "$reviewer"; then
     MISSING_REVIEWERS+=("$reviewer")
   fi
 done

tests/pre-merge-check.test.mjs

@@ -42,7 +42,7 @@ if [[ "$1" == "api" && "$2" == "graphql" ]]; then
     malformed_page:1)
       printf '5\\t0\\n'
       ;;
-    cursor_check_ok:1|unrelated_cursor_check:1|all_required_check_runs_ok:1|codex_issue_comment_ok:1|codex_issue_comment_short_sha:1|codex_issue_comment_stale:1|codex_issue_comment_not_verdict:1|codex_verdict_sha_in_prose:1|codex_verdict_unpinned:1|generic_issue_comment_ignored:1|issue_comments_fail:1)
+    cursor_check_ok:1|unrelated_cursor_check:1|all_required_check_runs_ok:1|codex_issue_comment_ok:1|codex_issue_comment_short_sha:1|codex_issue_comment_stale:1|codex_issue_comment_not_verdict:1|codex_verdict_sha_in_prose:1|codex_verdict_unpinned:1|generic_issue_comment_ignored:1|issue_comments_fail:1|codex_reaction_ok:1|codex_reaction_stale:1|codex_reaction_negative:1|reaction_wrong_user:1|reactions_read_fail:1|reaction_head_date_missing:1)
       printf '3\\t0\\tfalse\\t\\n'
       ;;
     repeated_cursor:1)
@@ -68,7 +68,7 @@ if [[ "$1" == "api" && "$2" == "repos/example/repo/pulls/7/reviews" ]]; then
     printf 'chatgpt-codex-connector[bot]\\n'
     exit 0
   fi
-  if [[ "$GH_STUB_SCENARIO" == codex_issue_comment_* || "$GH_STUB_SCENARIO" == "codex_verdict_sha_in_prose" || "$GH_STUB_SCENARIO" == "generic_issue_comment_ignored" ]]; then
+  if [[ "$GH_STUB_SCENARIO" == codex_issue_comment_* || "$GH_STUB_SCENARIO" == "codex_verdict_sha_in_prose" || "$GH_STUB_SCENARIO" == "generic_issue_comment_ignored" || "$GH_STUB_SCENARIO" == codex_reaction_* || "$GH_STUB_SCENARIO" == "reaction_wrong_user" || "$GH_STUB_SCENARIO" == "reactions_read_fail" || "$GH_STUB_SCENARIO" == "reaction_head_date_missing" ]]; then
     printf 'cursor[bot]\\n'
     exit 0
   fi
@@ -119,6 +119,42 @@ if [[ "$1" == "api" && "$2" == "repos/example/repo/issues/7/comments" ]]; then
   exit 0
 fi
 
+# Head-visibility time = earliest check-suite created_at for the head SHA. Used
+# to reject reaction sign-offs placed before the current head was pushed.
+if [[ "$1" == "api" && "$2" == "repos/example/repo/commits/deadbeef1234567890abcdef1234567890abcdef/check-suites" ]]; then
+  if [[ "$GH_STUB_SCENARIO" == "reaction_head_date_missing" ]]; then
+    exit 0  # empty output -> HEAD_VISIBLE_AT unknown -> fail closed
+  fi
+  printf '2026-06-01T00:00:00Z\\n'
+  exit 0
+fi
+
+# PR-body reactions: login <TAB> content <TAB> created_at. Head-visibility time
+# in the stub is 2026-06-01T00:00:00Z, so "…06-02…" is fresh and "…05-01…" is
+# stale (placed before the head was pushed).
+if [[ "$1" == "api" && "$2" == "repos/example/repo/issues/7/reactions" ]]; then
+  case "$GH_STUB_SCENARIO" in
+    reactions_read_fail)
+      echo "reactions unavailable" >&2
+      exit 3
+      ;;
+    codex_reaction_ok|reaction_head_date_missing)
+      printf 'chatgpt-codex-connector[bot]\\t+1\\t2026-06-02T00:00:00Z\\n'
+      ;;
+    codex_reaction_stale)
+      printf 'chatgpt-codex-connector[bot]\\t+1\\t2026-05-01T00:00:00Z\\n'
+      ;;
+    codex_reaction_negative)
+      printf 'chatgpt-codex-connector[bot]\\tconfused\\t2026-06-02T00:00:00Z\\n'
+      printf 'chatgpt-codex-connector[bot]\\t-1\\t2026-06-02T00:00:00Z\\n'
+      ;;
+    reaction_wrong_user)
+      printf 'someuser\\t+1\\t2026-06-02T00:00:00Z\\n'
+      ;;
+  esac
+  exit 0
+fi
+
 if [[ "$1" == "pr" && "$2" == "view" ]]; then
   if [[ "$*" != *"--repo example/repo"* ]]; then
     echo "gh pr view must pass --repo" >&2
@@ -348,3 +384,70 @@ test("pre-merge check rejects unrelated checks from a matching app slug", async
     assert.equal((await readFile(countPath, "utf8")).trim(), "1");
   });
 });
+
+test("pre-merge check accepts a codex thumbs-up reaction on the PR body as sign-off", async () => {
+  // Codex often signs off on a clean PR with a +1 reaction on the PR
+  // description rather than a review/comment/check run — the gap this fix
+  // closes. cursor[bot] is satisfied via its review; codex only via the
+  // reaction.
+  await withGhStub("codex_reaction_ok", async (env) => {
+    const result = runPreMergeCheck(env);
+
+    assert.equal(result.status, 0, result.stderr);
+    assert.match(result.stdout, /OK: All reviewers posted/);
+  });
+});
+
+test("pre-merge check rejects a codex reaction placed before the head was pushed", async () => {
+  // A +1 on an earlier revision must not satisfy the reviewer once a newer
+  // commit is pushed — the reaction predates the head's check-suite (push) time.
+  await withGhStub("codex_reaction_stale", async (env) => {
+    const result = runPreMergeCheck(env);
+
+    assert.equal(result.status, 1);
+    assert.match(result.stdout, /Missing reviews from: chatgpt-codex-connector\[bot\]/);
+  });
+});
+
+test("pre-merge check does not count reactions when the head push time is unknown", async () => {
+  // Fail closed: without a push-visibility timestamp there is no way to prove a
+  // reaction is fresh, so even a positive codex reaction must not sign off.
+  await withGhStub("reaction_head_date_missing", async (env) => {
+    const result = runPreMergeCheck(env);
+
+    assert.equal(result.status, 1);
+    assert.match(result.stdout, /Head push time unknown/);
+    assert.match(result.stdout, /Missing reviews from: chatgpt-codex-connector\[bot\]/);
+  });
+});
+
+test("pre-merge check ignores negative codex reactions (confused/-1) as sign-off", async () => {
+  await withGhStub("codex_reaction_negative", async (env) => {
+    const result = runPreMergeCheck(env);
+
+    assert.equal(result.status, 1);
+    assert.match(result.stdout, /Missing reviews from: chatgpt-codex-connector\[bot\]/);
+  });
+});
+
+test("pre-merge check never credits a reviewer for another user's positive reaction", async () => {
+  await withGhStub("reaction_wrong_user", async (env) => {
+    const result = runPreMergeCheck(env);
+
+    assert.equal(result.status, 1);
+    assert.match(result.stdout, /Missing reviews from: chatgpt-codex-connector\[bot\]/);
+  });
+});
+
+test("pre-merge check degrades gracefully when reactions cannot be read", async () => {
+  // A reactions-endpoint failure is non-fatal: the gate falls back to the
+  // other detection paths (here codex is otherwise absent, so it still blocks
+  // — but on a missing-reviewer verdict, not an API-read crash).
+  await withGhStub("reactions_read_fail", async (env) => {
+    const result = runPreMergeCheck(env);
+
+    assert.equal(result.status, 1);
+    assert.match(result.stdout, /Could not read PR body reactions/);
+    assert.match(result.stdout, /Missing reviews from: chatgpt-codex-connector\[bot\]/);
+  });
+});