github source: collect the review requester on reason=review_requested notifications

[polyglotAI-bot]

Summary

Follow-up to #285, which named this gap and left it out: a review_requested notification has the same blind spot as an assignment. /notifications carries no actor, a review request has no comment body, and neither the notification nor the PR payload names who requested the review. So the requester is never in actors.

With a non-empty allow_actors, this drops a maintainer's review request on any PR whose author, reviewers and commenters are all outside the allowlist. A dependency-update bot's PR is the typical case: the author is a bot, the automatic reviewer is a bot, and every comment is from CI. The guardrail drops the record fail-closed, the cursor advances past it, and the request is lost. The only trace is one INFO line:

[INFO] nerve.sources.runner: Source github: dropped <id> on actors=['some-ci[bot]', 'review-helper[bot]']
  — [owner/repo] Bump a dependency (review_requested)

As with assignments, no configuration recovers it: the login that would pass the allowlist is not in the record.

What changed

• _enrich_review_request() resolves the requester for reason=review_requested on a PR. It reads the PR's /issues/{n}/events and takes the newest review_requested event whose requested_reviewer is the authenticated account. The requester is read from review_requester, with actor as the fallback.
• The authenticated login comes from gh api user, read once and cached on the source. A failure is not cached.
• The requester joins actors and is rendered as Review requested by: <login>.
• _gh_api_get_all() reads every page with gh api --paginate --slurp (gh 2.48 or later). It logs failures at warning level and kills the process on timeout.
• docs/sources.md documents the review-request case under the actor guardrail.

Deliberate choices worth a reviewer's eye:

• Only the request to this account counts. The other review requests on the PR are skipped. For example, a bot that opens a PR requests its default reviewers, and that requester did not ask the inbox account for anything.

• The requester is not assumed to be trusted. Unlike an assigner, a PR author can re-request a review from a past reviewer without write access. Verified on live PRs: the newest request naming the account was the PR author's own re-request. It is credited, and then the allowlist rejects it. The code never falls back to an older request by a maintainer.

• Fail closed, never fall back. The scan stops at the newest event about the account's own review. It credits nobody in these cases:

  • the newest event is a review_request_removed (the request was withdrawn);
  • the requester cannot be identified, or is the account itself;
  • a different login made a newer request to a team, or to a target that cannot be identified. The account may be in that team, and then the team request is the trigger.

  A team request by the same login does not block. One action that asks the account and a team writes two events, in either order.

• All pages, unlike _enrich_assignment. A re-review request on a long-lived PR is the newest event on a busy history. That makes it exactly the entry that a single first page of 100 misses.

Open question: how long the credit lasts

GitHub keeps review_requested as the thread's reason after later activity. Verified on a live thread: a bot review two hours after the request updated the thread, and the reason stayed review_requested. So, exactly as with the assigner from #285, the requester stays among the actors for later updates of that thread, until someone makes a newer request naming the account. Later comments by non-allowlisted users on that PR then pass. This matches the guardrail's existing semantics ("the thread involves a trusted login"): the recent-comments enrichment and the assigner behave the same way. I documented it in docs/sources.md and did not add a time bound.

A bound is possible, but each obvious anchor has a cost:

• last_read_at: this is null for an inbox account that never marks notifications read.
• The fetch since: this does not bound anything when the request's own notification was the last one fetched.
• A window from the notification's updated_at: this drops a real request when the first fetch after the request is late (for example after downtime), which is the failure this PR fixes.

If you want it bounded, I suggest one shared rule for the assigner and the requester, in a separate PR.

Test plan

• tests/test_github_source_actors.py: new tests for _collect_actors, newest-request selection, case-insensitive match, actor fallback, other-reviewer skip, no fallback on an unidentifiable requester, withdrawn request, re-request after removal, PR-author re-request, self-request, newer team request by another or unknown login, team request by the same login before and after, older team request, malformed events, a non-PR URL, an owner named pulls, missing login and missing history, login caching and retry, _gh_api_get_all page flattening, failure paths and timeout kill, and dispatch only for review_requested.
• Regression test for the exact failure: test_maintainer_review_request_on_bot_pr_passes_guardrail drives the real fetch + enrichment + the production guardrail from build_source_runners. With only nerve/sources/github.py reverted, it fails at assert actors == [...]: 'maintainer' is missing.
• Live check, read-only: the new enrichment ran against the inbox account's three real review_requested threads.
  • The thread whose request was dropped in production now resolves the maintainer and passes the allowlist.
  • The two threads whose newest request is the PR author's re-request resolve the author and still do not pass.
• Full suite: 5769 passed, 38 skipped.
• Backward compatible: review_requester defaults to "", and every other reason is unchanged. It costs one gh api user call per process and one event-history read per review_requested notification.
• Pre-PR review: two read-only subagent reviews, no blocking findings. The second confirmed the fixes from the first (team-request fall-through, withdrawn requests, self-requests, malformed events, --slurp failure visibility, timeout kill, anchored /pulls/ rewrite). The remaining point is the open question above.

🤖 Generated with Claude Code

[polyglotAI-bot]

Moved to draft in favour of #513.

This PR guesses the requester from the PR's event history, because a notification carries no actor and no event id. Two limits stay, whatever the guessing rule:

• GitHub keeps review_requested as the thread's reason after later activity, so the requester stays among the actors for later updates of that PR.
• Events between two polls merge into one notification, so a maintainer's request and a stranger's comment can become one record that no single actor describes.

#513 adds a github_repo_events source instead. It reads the repo's issue-event log and emits one record per event, with the login that acted, so allow_actors judges who made the request. If #513 lands, this PR can be closed. It is kept as a draft in case notification-only users still want the best-effort enrichment.