fix: scope the review-run check to its own PR, and survive a transient 503

[johanzander]

Problem

Two failures in scripts/request-pr-review.sh, both observed live tonight, both of which cost a paid review round.

1. Wrong run — it watched whatever ran most recently, not its own PR

review_run_state() took the newest PR Review run created after the trigger, with no filter for which PR it belonged to.

pr-review.yml triggers on issue_comment — which fires for comments on issues, not only PRs. Those runs are gated out and complete as skipped in about ten seconds. So any comment posted anywhere in the repo during a review produces a newer run that is completed and not success — which this function read as failed.

Measured on #636: a routine Product Owner comment on issue #441 at 21:13:07 made the script abandon the review. That review went on to APPROVE at 21:15:45.

completed  skipped   Question: Is it always counting with 15 minutes?   ← #441, mistaken for ours
in_progress          feat: put PRs on the board ...                     ← #636, still thinking

The caller was told the run was broken while it was still working, and the natural response — re-request — would have spent a second review round on a review already in flight.

Fix: displayTitle carries the PR title for a run triggered on that PR, so it is the discriminator. The title is read once before the loop and matched through --arg rather than string interpolation — a title containing a quote would otherwise break the filter silently.

2. A single 503 killed the poll after the trigger had already posted

set -e turned one transient failure on the verdict read into a fatal exit 1. Because the @claude-bot review comment had already posted, re-running spent a second round on a live review. GitHub returned 503s for roughly ninety minutes on 2026-08-17 and this fired twice.

Fix: both review reads tolerate a failed call and retry on the next poll — which is exactly what review_run_state already did via its unknown state. A failed read is not a result.

Evidence the tests discriminate

Reverted the script to HEAD and re-ran the three new tests:

FAILED  test_a_newer_run_for_a_different_pr_is_not_this_review
FAILED  test_a_transient_api_failure_does_not_kill_the_poll
PASSED  test_this_prs_own_failed_run_is_still_reported
2 failed, 1 passed

• The first two fail pre-fix and pass post-fix. The second reproduces the exact production shape — returncode=1, stderr='HTTP 503: no server is currently available'.
• The third passes both before and after, deliberately. It is the guard that the scoping does not swallow a genuine failure of this PR's own run — the failure mode the run-state check was added for in the first place (fix: make backlog grooming reflect what actually blocks an issue #623 burned 16 minutes waiting on a run that had already died).

Restored, all 12 tests green.

Test plan

• ./scripts/quality-check.sh green
• 12/12 in backend/tests/test_request_pr_review.py
• Mutation verified above, not merely asserted

Outcome-level coverage

Not applicable — no optimizer path touched. The pins above are the coverage, and the harness exercises the real jq filters rather than echoing fixtures back.

Scope

scripts/request-pr-review.sh and its test file. No CHANGELOG entry: agent tooling, no user-visible effect.

🤖 Generated with Claude Code

[bess-agent]

@claude-bot review

[bess-manager-claude-bot]

Verification performed

I didn't just read the diff — I cloned fix/review-run-detection into a scratch checkout and ran the actual suite:

• pytest backend/tests/test_request_pr_review.py: 12/12 pass on the branch.
• Reverted scripts/request-pr-review.sh to main's version and re-ran: 2 of the 3 new tests go RED exactly as the PR body claims —
  test_a_newer_run_for_a_different_pr_is_not_this_review and test_a_transient_api_failure_does_not_kill_the_poll fail pre-fix / pass post-fix; test_this_prs_own_failed_run_is_still_reported passes both ways (the correct shape for a guard test). This is genuine regression coverage, not asserted-but-unproven.
• black --check, ruff check, shellcheck scripts/request-pr-review.sh: all clean.
• Checked the fix's core premise against reality rather than trusting the PR body: gh run list --workflow "PR Review" --json displayTitle,headBranch on this actual repo confirms displayTitle is the PR title for issue_comment-triggered runs, and headBranch is always main (the checkout doesn't track the PR ref) — so title is genuinely the best available discriminator gh run list's JSON exposes, not a guess. Also reproduced the exact scoping story: title "fix: guard git push by shape so feature-branch pushes run unattended" appears twice in real run history (a retrigger), confirming createdAt + title together correctly select "this PR's latest run," not just "a run with this title."

Checklist

• Root-cause match: yes. Both fixes map 1:1 to the two measured incidents in the PR body (#636 title-scoping miss; the 2026-08-17 503 window) — not speculative.
• Regression coverage: yes, verified as RED-before/GREEN-after above, not just present.
• Minimal scope: yes — only scripts/request-pr-review.sh and its test file, no unrelated changes.
• Rule compliance: no Optional[x] (extra_runs: list | None), no hasattr/getattr-with-default, no new classes, no exception-string matching, comments are WHY-only matching the file's existing style, --arg-based jq (not string interpolation) closes a real quote-injection risk in the old code.
• Scope-assessment statement (rules.md step 9): not present in the PR body, but this is unambiguously a local fix — review_run_state() stays within its existing contract (which run is this?), and the two 2>/dev/null || verdict="" additions are the same pattern the function already used for its own gh-read. Not structural; the omission isn't a problem here.

One non-blocking nit

backend/tests/test_request_pr_review.py:100 (runs = (extra_runs or []) + runs) introduces a new mypy error:

error: No overload variant of "__add__" of "list" matches argument type "object"  [operator]

Root cause is pre-existing weak typing in _gh()'s runs variable (the {...}[run_state] dict-indexing already produces object and already errors at line 93 on main today — "object" has no attribute "__iter__"). This PR adds a second instance of the same underlying gap rather than introducing a new one. Not blocking because: scripts/quality-check.sh doesn't invoke mypy at all (verified — no mypy reference in it), and main is not currently mypy-clean for this file either, so this isn't a regression from a green baseline. Worth a one-line type annotation on runs in a follow-up.

APPROVE — the fix is correct, evidenced (not just asserted), minimally scoped, and the test coverage genuinely discriminates. The mypy nit above is the only thing I'd want fixed, and it doesn't block.