feat: implement issue #1744 — pr-review sweep reports success while silently omitting an eligible open PR from the candidate pool

.github/workflows/lint.yml

@@ -152,6 +152,7 @@
             tests/test_prefetch_freshness.bats \
             tests/test_engine_unavailable_notice.bats \
             tests/test_batch_skip_reporting.bats \
+            tests/test_list_prs_enumeration.bats \
             tests/test_review_one_pr_ci_pending.bats \
             tests/test_review_one_pr_decline_verdict.bats \
             tests/test_review_one_pr_force_ci_failing.bats \
@@ -259,7 +260,7 @@
     timeout-minutes: 5
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
         # NOTE: persist-credentials is intentionally left at the default (true) —
         # vci_resolve_reusable needs `git fetch origin` to authenticate against this
         # private repo when resolving same-repo channel tags. Setting
         # persist-credentials: false would silently push every such caller into the

scripts/list-prs.sh

@@ -6,18 +6,33 @@
 #   2. All open PRs in repos owned by $TARGET_ORG (organization)
 #   3. All open PRs in additional orgs listed in $DELEGATION_ORGS
 #
-# Filters:
-#   --draft=false       — skip work-in-progress PRs
+# Enumeration source — the List API, not the Search API (issue #1744):
+#   PRs are listed per-repo with `gh pr list`, which reads the strongly-
+#   consistent pull-request endpoint. The prior implementation used
+#   `gh search prs` — the *eventually consistent* Search API — which silently
+#   omitted a PR (#1710) that had just been pushed (its approval dismissed): the
+#   Search index had not yet re-indexed the mutation, so the PR was absent from
+#   results even though it was open, green, and owed a review. The List API has
+#   no such re-index lag, so a just-mutated PR enters the pool immediately (which
+#   is also why targeting the same PR directly always reviewed it fine).
+#
+# Filters (every exclusion is logged — see "Observability" below):
+#   - drafts (isDraft == true)          — work in progress
+#   - self-authored (author == BOT_USER) — GitHub rejects self-approval
+#     unconditionally; queuing such a PR previously aborted the whole session
+#     and starved every later candidate (issue #96).
 #
 # CI filtering is intentionally omitted here — review-one-pr.sh enforces it
-# per-PR as a second layer. Filtering by --checks success would exclude repos
-# with no CI configured (GitHub treats "no checks" as not matching --checks
-# success), causing their PRs to never enter the candidate pool.
+# per-PR as a second layer. Filtering by CI status would exclude repos with no
+# CI configured, causing their PRs to never enter the candidate pool.
 #
-# Self-authored PRs (PRs whose author is $BOT_USER) are excluded here, because
-# GitHub's GraphQL API rejects self-approval unconditionally — including such a
-# PR in the queue previously triggered a fatal session abort that starved every
-# subsequent candidate (see issue #96).
+# Observability (issue #1744, AC #3):
+#   A candidate that is *seen but excluded* is never dropped silently — each
+#   exclusion is logged to stderr as a `::notice::` naming the PR and the reason,
+#   and a per-run summary reports how many PRs were seen / kept / excluded. An
+#   omission must never be indistinguishable from an empty queue. Notices go to
+#   stderr because stdout is the candidate list (the caller redirects it to a
+#   file); GitHub Actions still surfaces `::notice::` annotations from stderr.
 #
 # Output ordering (stable, deterministic):
 #   1. .github and .github-private PRs first (priority 0)
@@ -46,30 +61,71 @@ if ! [[ "$BOT_USER" =~ ^[A-Za-z0-9](-?[A-Za-z0-9]){0,38}$ ]]; then
 fi
 
 all_entries=""
+seen=0
+kept=0
+excluded=0
+
+# JQ filter: classify each PR from a `gh pr list --json url,author,createdAt,isDraft`
+# array into one tab-delimited record per line:
+#   KEEP<TAB><priority>|<createdAt>|<url>   — an eligible candidate
+#   DROP<TAB><reason><TAB><url>             — a seen-but-excluded PR
+# Priority 0 — .github / .github-private repos (infra PRs reviewed first);
+# priority 1 — all other repos. ISO-8601 createdAt sorts lexicographically, so
+# oldest-first within each tier is a plain string sort on the createdAt field.
+JQ_CLASSIFY='.[] |
+  if .isDraft == true then
+    "DROP\tdraft\t" + .url
+  elif .author?.login == $bot then
+    "DROP\tself-authored\t" + .url
+  else
+    "KEEP\t"
+      + (if (.url | test("/[.]github(-private)?/pull/")) then "0" else "1" end)
+      + "|" + .createdAt + "|" + .url
+  end'
 
-# JQ filter: emit  <priority>|<createdAt>|<url>  for non-bot-authored PRs.
-#   Priority 0 — .github / .github-private repos (infra PRs reviewed first)
-#   Priority 1 — all other repos
-# ISO-8601 createdAt sorts lexicographically, so oldest-first within each
-# tier is achieved with a plain string sort on field 2.
-JQ_WITH_SORT=".[] | select(.author.login != \"$BOT_USER\") |
-  (if (.url | test(\"/[.]github(-private)?/pull/\")) then \"0\" else \"1\" end)
-    + \"|\" + .createdAt + \"|\" + .url"
+# Route one repo's classified PRs into the candidate buffer, logging every
+# exclusion. Runs in the current shell (no subshell) so the counters and
+# all_entries accumulate across repos.
+process_repo_prs() {
+  local raw="$1" line tag rest reason url
+  while IFS= read -r line || [ -n "$line" ]; do
+    [ -z "$line" ] && continue
+    tag="${line%%$'\t'*}"
+    rest="${line#*$'\t'}"
+    case "$tag" in
+      KEEP)
+        all_entries="${all_entries}${rest}"$'\n'
+        kept=$((kept + 1))
+        seen=$((seen + 1))
+        ;;
+      DROP)
+        reason="${rest%%$'\t'*}"
+        url="${rest#*$'\t'}"
+        echo "::notice::list-prs: excluded $url from candidate pool (reason: $reason)" >&2
+        excluded=$((excluded + 1))
+        seen=$((seen + 1))
+        ;;
+    esac
+  done < <(jq -r --arg bot "$BOT_USER" "$JQ_CLASSIFY" <<< "$raw" 2>/dev/null)
+}
 
-# Search all open, non-draft PRs across every repo owned by $owner.
-# --limit 200 on repo list handles orgs that grow beyond gh's default 30.
+# List all open PRs across every repo owned by $owner via the List API.
+# Both --limit values are set high enough that neither repos nor per-repo PRs
+# are silently truncated: a hard cap would omit eligible open PRs beyond it,
+# reintroducing the very silent-omission defect this script exists to prevent
+# (issue #1744). gh paginates internally up to the requested limit.
 search_namespace() {
-  local owner="$1"
-  while IFS= read -r repo; do
-    entries=$(gh search prs \
-      --state open \
+  local owner="$1" raw
+  while IFS= read -r repo || [ -n "$repo" ]; do
+    [ -z "$repo" ] && continue
+    raw=$(gh pr list \
       --repo "$repo" \
-      --draft=false \
-      --limit 100 \
-      --json url,author,createdAt \
-      --jq "$JQ_WITH_SORT" 2>/dev/null || true)
-    all_entries="${all_entries}${entries}"$'\n'
-  done < <(gh repo list "$owner" --limit 200 --json nameWithOwner --jq '.[].nameWithOwner' 2>/dev/null || true)
+      --state open \
+      --limit 1000 \
+      --json url,author,createdAt,isDraft 2>/dev/null || echo '[]')
+    [ -z "$raw" ] && raw='[]'
+    process_repo_prs "$raw"
+  done < <(gh repo list "$owner" --limit 1000 --json nameWithOwner --jq '.[].nameWithOwner' 2>/dev/null || true)
 }
 
 # Bot's personal account
@@ -88,6 +144,10 @@ if [ -n "$DELEGATION_ORGS" ]; then
   done
 fi
 
+# Per-run enumeration summary — makes the pool composition observable so an
+# excluded candidate can never be mistaken for an empty queue (issue #1744).
+echo "::notice::list-prs: enumeration complete — ${seen} open PR(s) seen, ${kept} kept as candidates, ${excluded} excluded (drafts + self-authored, logged above)" >&2
+
 # 1. Drop blank lines
 # 2. Deduplicate by URL (field 3) keeping first occurrence
 # 3. Sort: priority (field 1) ascending, then createdAt (field 2) ascending
@@ -96,4 +156,4 @@ grep -v '^$' <<< "$all_entries" \
   | sort -t'|' -k3 -u \
   | sort -t'|' -k1,1n -k2,2 \
   | cut -d'|' -f3- \
-  || true
+  || true

scripts/review-batch.sh

@@ -102,6 +102,10 @@ validate_engines
 # absent or invalid on a scheduled run with zero candidate PRs.
 if [ ! -s "$PRS_FILE" ]; then
   echo "::notice::No candidate PRs to review."
+  # Emit a summary line even for an empty queue so "0 reviews posted" can never
+  # be confused with a run that found candidates and no-op'd every one of them
+  # (issue #1744 AC #4). "empty queue" is the distinguishing marker.
+  echo "Summary: no candidate PRs found (empty queue) — 0 reviews posted, 0 no-ops skipped, 0 failures"
   exit 0
 fi
 

tests/test_batch_skip_reporting.bats

@@ -125,3 +125,37 @@ EOF
   # An already-reviewed no-op is genuinely done — it is NOT a CI deferral.
   [[ "$output" != *"deferred"* ]]
 }
+
+# ── AC #4 (issue #1744): "0 reviews posted" must not mean two different things ──
+# An empty queue (no candidates found) and a full queue where every candidate was
+# a no-op both yield "0 reviews posted". The summary line must distinguish them so
+# a silent-success omission (an eligible PR missing from the pool) can't hide as an
+# ordinary all-no-ops run.
+
+@test "batch: empty queue reports a distinct 'no candidates found' summary" {
+  : > "$PRS_FILE"   # empty candidate list — nothing enumerated
+
+  run bash scripts/review-batch.sh
+  echo "$output" >&2
+
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"Summary:"* ]]
+  # Names the empty-queue state explicitly...
+  [[ "$output" == *"empty queue"* ]]
+  # ...and is NOT phrased like a run that processed candidates.
+  [[ "$output" != *"processed"* ]]
+}
+
+@test "batch: candidates-all-no-ops summary is distinct from an empty queue" {
+  _stub_already_reviewed   # one candidate, reviewed as a no-op
+
+  run bash scripts/review-batch.sh
+  echo "$output" >&2
+
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"Summary:"* ]]
+  # This run DID find candidates — it must say so, and must not read as empty.
+  [[ "$output" == *"candidate"* ]]
+  [[ "$output" != *"empty queue"* ]]
+  [[ "$output" == *"no-op"* ]]
+}

tests/test_list_prs_enumeration.bats

@@ -0,0 +1,151 @@
+#!/usr/bin/env bats
+# Regression tests for scripts/list-prs.sh candidate enumeration (issue #1744).
+#
+# The sweep reported success while silently omitting an open, non-draft,
+# non-self-authored PR (#1710) from the candidate pool. Root cause: enumeration
+# used `gh search prs` — the eventually-consistent Search API. #1710 had just
+# been pushed (its approval dismissed), so the Search index had not yet
+# re-indexed it and it was absent from results, while the strongly-consistent
+# List API (`gh pr list`) returns it immediately. The omission was invisible
+# because filter/scope exclusions were never logged.
+#
+# These tests pin the fix:
+#   - enumeration sources candidates from the List API (`gh pr list`), so an
+#     eligible PR is present even when the Search API returns nothing;
+#   - every PR excluded by a filter is logged at notice level with its reason,
+#     so an omission can never be mistaken for an empty queue;
+#   - a per-run enumeration summary reports seen/kept/excluded counts.
+#
+# gh is fully mocked: `gh repo list` emits repo names per owner, `gh pr list`
+# emits a JSON fixture per repo, and `gh search prs` returns nothing — so a test
+# passes only if list-prs.sh reads from the List API, not the Search index.
+#
+# stdout (the candidate URL list) and stderr (the `::notice::` observability
+# stream) are captured to separate files so the two channels never bleed
+# together — the whole point of the fix is that they are distinct.
+#
+# Run with: bats tests/test_list_prs_enumeration.bats
+
+SCRIPT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)/scripts/list-prs.sh"
+
+setup() {
+  MOCK_BIN="$BATS_TEST_TMPDIR/mock_bin"
+  REPO_LIST_DIR="$BATS_TEST_TMPDIR/repo_list"
+  PR_LIST_DIR="$BATS_TEST_TMPDIR/pr_list"
+  OUT="$BATS_TEST_TMPDIR/out"
+  ERR="$BATS_TEST_TMPDIR/err"
+  mkdir -p "$MOCK_BIN" "$REPO_LIST_DIR" "$PR_LIST_DIR"
+  export MOCK_BIN REPO_LIST_DIR PR_LIST_DIR
+  export PATH="$MOCK_BIN:$PATH"
+
+  export BOT_USER="donpetry-bot"
+  export TARGET_ORG="petry-projects"
+  unset DELEGATION_ORGS || true
+
+  cat > "$MOCK_BIN/gh" <<'EOF'
+#!/usr/bin/env bash
+sub="$1 $2"
+case "$sub" in
+  "repo list")
+    owner="$3"
+    cat "$REPO_LIST_DIR/${owner}.txt" 2>/dev/null || true
+    exit 0
+    ;;
+  "pr list")
+    repo=""
+    while [ "$#" -gt 0 ]; do
+      if [ "$1" = "--repo" ]; then repo="$2"; break; fi
+      shift
+    done
+    f="$PR_LIST_DIR/${repo//\//__}.json"
+    if [ -f "$f" ]; then cat "$f"; else echo '[]'; fi
+    exit 0
+    ;;
+  "search prs")
+    # The Search API is eventually consistent — the source of the #1744 bug.
+    # Returning nothing here proves the candidate pool no longer depends on it.
+    exit 0
+    ;;
+esac
+exit 0
+EOF
+  chmod +x "$MOCK_BIN/gh"
+
+  # Default: only the org owns repos; the bot account owns none.
+  printf 'petry-projects/.github-private\n' > "$REPO_LIST_DIR/petry-projects.txt"
+}
+
+# write_prs <repo> <json-array>
+write_prs() {
+  printf '%s' "$2" > "$PR_LIST_DIR/${1//\//__}.json"
+}
+
+# run_list_prs — invoke the script capturing stdout/stderr to $OUT/$ERR.
+run_list_prs() {
+  bash "$SCRIPT" >"$OUT" 2>"$ERR"
+}
+
+@test "eligible open non-draft non-self PR is present in the candidate pool" {
+  write_prs "petry-projects/.github-private" \
+    '[{"url":"https://github.com/petry-projects/.github-private/pull/1710","author":{"login":"don-petry"},"createdAt":"2026-09-13T00:00:00Z","isDraft":false}]'
+
+  run_list_prs
+  grep -qx 'https://github.com/petry-projects/.github-private/pull/1710' "$OUT"
+}
+
+@test "eligible PR is found via the List API even when the Search API is empty" {
+  # This is the #1710 case: a just-pushed PR the Search index has not re-indexed.
+  # The mocked `gh search prs` returns nothing; the PR must still appear because
+  # enumeration reads from the strongly-consistent List API.
+  write_prs "petry-projects/.github-private" \
+    '[{"url":"https://github.com/petry-projects/.github-private/pull/1710","author":{"login":"don-petry"},"createdAt":"2026-09-13T00:00:00Z","isDraft":false}]'
+
+  run_list_prs
+  [ "$(grep -c '/pull/1710' "$OUT")" -eq 1 ]
+}
+
+@test "draft PR is excluded from the pool AND logged with a reason at notice level" {
+  write_prs "petry-projects/.github-private" \
+    '[{"url":"https://github.com/petry-projects/.github-private/pull/2000","author":{"login":"don-petry"},"createdAt":"2026-09-13T00:00:00Z","isDraft":true}]'
+
+  run_list_prs
+  # Not in the candidate list (stdout)...
+  ! grep -q '/pull/2000' "$OUT"
+  # ...but observably excluded with its reason (stderr notice).
+  grep -q '::notice::' "$ERR"
+  grep -q '/pull/2000' "$ERR"
+  grep -qi 'draft' "$ERR"
+}
+
+@test "self-authored PR is excluded from the pool AND logged with a reason" {
+  write_prs "petry-projects/.github-private" \
+    '[{"url":"https://github.com/petry-projects/.github-private/pull/2001","author":{"login":"donpetry-bot"},"createdAt":"2026-09-13T00:00:00Z","isDraft":false}]'
+
+  run_list_prs
+  ! grep -q '/pull/2001' "$OUT"
+  grep -q '::notice::' "$ERR"
+  grep -q '/pull/2001' "$ERR"
+  grep -qi 'self-authored' "$ERR"
+}
+
+@test "an eligible PR is kept while a draft alongside it is dropped and logged" {
+  write_prs "petry-projects/.github-private" \
+    '[{"url":"https://github.com/petry-projects/.github-private/pull/1710","author":{"login":"don-petry"},"createdAt":"2026-09-13T00:00:00Z","isDraft":false},{"url":"https://github.com/petry-projects/.github-private/pull/2000","author":{"login":"don-petry"},"createdAt":"2026-09-13T00:00:00Z","isDraft":true}]'
+
+  run_list_prs
+  grep -qx 'https://github.com/petry-projects/.github-private/pull/1710' "$OUT"
+  ! grep -q '/pull/2000' "$OUT"
+  grep -q '/pull/2000' "$ERR"
+  grep -qi 'draft' "$ERR"
+}
+
+@test "enumeration emits a seen/kept/excluded summary at notice level" {
+  write_prs "petry-projects/.github-private" \
+    '[{"url":"https://github.com/petry-projects/.github-private/pull/1710","author":{"login":"don-petry"},"createdAt":"2026-09-13T00:00:00Z","isDraft":false},{"url":"https://github.com/petry-projects/.github-private/pull/2000","author":{"login":"don-petry"},"createdAt":"2026-09-13T00:00:00Z","isDraft":true}]'
+
+  run_list_prs
+  grep -q '::notice::' "$ERR"
+  grep -qi 'enumeration' "$ERR"
+  grep -qi 'seen' "$ERR"
+  grep -qi 'excluded' "$ERR"
+}