fix(pr-review): retry the merge-queue state read and surface a failed read

AGENTS.md

@@ -458,6 +458,11 @@ stops the merge: PR 2084 was approved, enqueued, had that approval dismissed, an
   call is a no-op. A failed dequeue logs a `::warning::` and posts one comment asking a maintainer to
   remove the PR from the queue by hand. An unreadable live state logs a `::warning::` and takes no
   action. The marker is posted at most once per head SHA.
+- **A failed state read is retried, then surfaced.** The live read is tried `MQ_READ_ATTEMPTS` times
+  (default 3, `MQ_READ_RETRY_DELAY_SEC` apart, default 2). If every attempt fails the helper cannot tell
+  whether a queued PR lost its last approval: it logs a `::warning::`, writes the unknown state to the job
+  step summary so a maintainer can check the queue, makes no dequeue call and posts no PR comment (most PRs
+  that reach this path are not queued), and never fails the run (#2186).
 - **Auto-merge after a dequeue.** A dequeue consumes the PR's auto-merge request. dev-lead's idempotent
   `try_enable_auto_merge` re-enables it on its next pass; once an approval stands again, the PR
   rejoins the queue. This is intended.

scripts/lib/merge-queue-dequeue.sh

@@ -56,6 +56,30 @@ _mq_read_state() {
   ' 2>/dev/null
 }
 
+# _mq_read_state_retry <owner> <name> <number>
+#   _mq_read_state with a bounded retry (#2186). The helper runs right after an
+#   approval was dismissed, and the window that matters is seconds long, so one
+#   transient GraphQL error must not end the check. Tunables (also for tests):
+#   MQ_READ_ATTEMPTS (default 3) and MQ_READ_RETRY_DELAY_SEC (default 2).
+#   Non-zero when every attempt failed.
+_mq_read_state_retry() {
+  local owner="$1" name="$2" number="$3" st i
+  local attempts="${MQ_READ_ATTEMPTS:-3}" delay="${MQ_READ_RETRY_DELAY_SEC:-2}"
+  [[ "$attempts" =~ ^[1-9][0-9]*$ ]] || attempts=3
+  [[ "$delay" =~ ^[0-9]+$ ]] || delay=2
+  for ((i = 1; i <= attempts; i++)); do
+    if st=$(_mq_read_state "$owner" "$name" "$number"); then
+      printf '%s' "$st"
+      return 0
+    fi
+    if [ "$i" -lt "$attempts" ]; then
+      echo "  merge-queue: live-state read failed (attempt $i of $attempts) — retrying" >&2
+      [ "$delay" -gt 0 ] && sleep "$delay"
+    fi
+  done
+  return 1
+}
+
 # _mq_post_notice <owner> <name> <number> <pr_url> <head> <body> [marker_prefix]
 #   Post <body> unless a comment carrying this head's marker already exists, so
 #   repeat runs at one head post one notice. If the comments cannot be listed,
@@ -98,9 +122,18 @@ mq_dequeue_if_unapproved() {
   fi
 
   local st status_st=0
-  st=$(_mq_read_state "$owner" "$name" "$number") || status_st=$?
+  st=$(_mq_read_state_retry "$owner" "$name" "$number") || status_st=$?
   if [ "$status_st" -ne 0 ]; then
     echo "::warning::merge-queue: could not read the merge-queue and review state of $pr_url — cannot tell whether a queued PR lost its last approval; check the queue by hand (#2174)"
+    # Surface it where a maintainer looks, not only in the run log (#2186). No PR
+    # comment on this path: most PRs that reach it are not in the queue at all.
+    if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
+      {
+        printf '### Merge queue: state unknown after an approval was withdrawn\n\n'
+        printf 'The live merge-queue and review state of %s could not be read after every retry. ' "$pr_url"
+        printf 'If that pull request is in the merge queue it may merge without an approval: check the queue and remove it by hand if needed.\n\n'
+      } >> "$GITHUB_STEP_SUMMARY" 2>/dev/null || true
+    fi
     return 0
   fi
 

tests/test_merge_queue_dequeue.bats

@@ -40,11 +40,15 @@ setup() {
   export PR_STATE_FILE="$TEST_DIR/pr_state";      echo OPEN > "$PR_STATE_FILE"
   export CLOCK_FILE="$TEST_DIR/clock";            echo 10 > "$CLOCK_FILE"
   export CALLS="$TEST_DIR/gh_calls.log";          : > "$CALLS"
+  # #2186: the live-state read retries; no real sleeps in tests.
+  export MQ_READ_RETRY_DELAY_SEC=0
+  export READ_FAILS_FILE="$TEST_DIR/read_fails";  echo 0 > "$READ_FAILS_FILE"
 
   cat > "$TEST_DIR/bin/gh" <<'GHEOF'
 #!/bin/bash
 # Stateful gh mock for #2174. Env knobs:
 #   GRAPHQL_READ_RC        — non-zero ⇒ the live-state GraphQL read fails
+#   GRAPHQL_READ_FAIL_TIMES — N ⇒ only the first N live-state reads fail (#2186)
 #   DEQUEUE_RC             — non-zero ⇒ dequeuePullRequest fails (state unchanged)
 #   LEAVE_QUEUE_ON_DEQUEUE — 1 ⇒ the PR leaves the queue just before the mutation,
 #                            which then fails (the check/call race)
@@ -106,6 +110,12 @@ if [ "$1" = "api" ] && [ "$2" = "graphql" ]; then
     exit 0
   fi
   [ "${GRAPHQL_READ_RC:-0}" = "0" ] || { echo 'HTTP 502' >&2; exit "$GRAPHQL_READ_RC"; }
+  if [ "${GRAPHQL_READ_FAIL_TIMES:-0}" != "0" ]; then
+    n=$(cat "$READ_FAILS_FILE")
+    if [ "$n" -lt "$GRAPHQL_READ_FAIL_TIMES" ]; then
+      echo $((n + 1)) > "$READ_FAILS_FILE"; echo 'HTTP 502' >&2; exit 1
+    fi
+  fi
   # Live state: latest review per author (GitHub's latestOpinionatedReviews).
   jq -n --slurpfile r "$REVIEWS_FILE" --arg st "$(cat "$PR_STATE_FILE")" \
         --arg q "$(cat "$QUEUED_FILE")" --arg h "$PR_HEAD_SHA" '
@@ -331,6 +341,40 @@ fix_request_verdict() {
   [ "$(jq length "$COMMENTS_FILE")" -eq 0 ]
 }
 
+@test "live-state read fails once then succeeds → the queued, unapproved PR is still dequeued (#2186)" {
+  export GRAPHQL_READ_FAIL_TIMES=1
+  run_lib
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"attempt 1 of 3"* ]]
+  [ "$(dequeue_calls)" -eq 1 ]
+  [ "$(cat "$QUEUED_FILE")" = "false" ]
+  [ "$(marker_count)" -eq 1 ]
+}
+
+@test "every live-state read fails → bounded retries, no dequeue, no comment, step-summary entry, exit 0 (#2186)" {
+  export GRAPHQL_READ_RC=1 MQ_READ_ATTEMPTS=4
+  export GITHUB_STEP_SUMMARY="$TEST_DIR/summary.md"; : > "$GITHUB_STEP_SUMMARY"
+  run_lib
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"::warning::"* ]]
+  # Exactly MQ_READ_ATTEMPTS reads, then it stops.
+  [ "$(grep -c 'api graphql' "$CALLS")" -eq 4 ]
+  [ "$(dequeue_calls)" -eq 0 ]
+  [ "$(jq length "$COMMENTS_FILE")" -eq 0 ]
+  grep -q 'state unknown after an approval was withdrawn' "$GITHUB_STEP_SUMMARY"
+  grep -qF "$PR_URL" "$GITHUB_STEP_SUMMARY"
+}
+
+@test "a malformed retry setting falls back to the default instead of looping or aborting (#2186)" {
+  export GRAPHQL_READ_RC=1 MQ_READ_ATTEMPTS=abc MQ_READ_RETRY_DELAY_SEC=-1
+  # -1 is not a valid delay, so the default (2s) would apply; keep the test fast
+  # by proving only the attempt count here with a valid zero delay.
+  export MQ_READ_RETRY_DELAY_SEC=0
+  run_lib
+  [ "$status" -eq 0 ]
+  [ "$(grep -c 'api graphql' "$CALLS")" -eq 3 ]
+}
+
 @test "under set -e the helper never aborts its caller" {
   export DEQUEUE_RC=1 COMMENT_LIST_RC=1
   run bash -c 'set -euo pipefail; source "$LIB"; mq_dequeue_if_unapproved "$1"; echo CONTINUED' _ "$PR_URL"