feat: request bot as PR reviewer via GraphQL botIds

[aliasunder]

Summary

• Replace the dead REST API POST /pulls/{n}/requested_reviewers step in self_review.yml — GitHub silently ignores bot accounts in the reviewers field (returns 200 but requested_reviewers: [])
• Build the reviewer request into the action itself via the GraphQL requestReviews mutation's botIds field — the only API that supports requesting bots as PR reviewers (community discussion)
• The action auto-discovers its bot identity (viewer { login } → REST /users/{slug}[bot] → node ID) and requests itself before posting the review — best-effort, never blocks the pipeline

Root cause

The REST API and GraphQL userIds both reject bot accounts. GraphQL requestReviews has a separate botIds parameter that the REST API doesn't surface. Without it, the bot appears under "+1 more reviewer" with "Reviewers whose approvals may not affect merge requirements."

Changes

• src/github/event.ts — add nodeId to PrContext (parsed from event payload and REST response)
• src/github/client.ts — add graphql + users.getByUsername to OctokitLike, new requestBotReview method
• src/orchestrate.ts — call requestBotReview after PR context resolution (step 3.5), wrapped in try/catch
• .github/workflows/self_review.yml — remove dead REST reviewer-request step
• Fixtures and tests updated across all affected modules

Test plan

• 3 new tests in client.test.ts — happy path (viewer → REST lookup → GraphQL mutation), malformed bot response, viewer query error
• 2 new tests in orchestrate.test.ts — requestBotReview called with PR node ID, pipeline continues when requestBotReview throws
• npm test — 263 passing (21 new)
• npm run lint — clean
• npm run build — clean

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

@aliasunder, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2f2a7a89-4d63-4571-a0bf-efdddc7743f4

📥 Commits

Reviewing files that changed from the base of the PR and between af70f65 and ec7a172.

📒 Files selected for processing (11)

• .github/workflows/release.yml
• .github/workflows/self_review.yml
• fixtures/pull.get.json
• fixtures/pull_request.opened.json
• src/__tests__/orchestrate.test.ts
• src/github/__tests__/client.test.ts
• src/github/__tests__/event.test.ts
• src/github/client.ts
• src/github/event.ts
• src/orchestrate.ts
• src/review/__tests__/prompt.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch worktree-bot-reviewer-request

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In requestBotReview, consider extending botNodeIdSchema (e.g., to include and validate type === "Bot") so that you fail fast if the username resolves to a non-bot account rather than just relying on the login naming convention.
• To reduce repeated REST calls when the action runs multiple times on the same bot, consider caching the resolved bot node_id in memory within createGithubClient so requestBotReview can skip the users.getByUsername lookup after the first success.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `requestBotReview`, consider extending `botNodeIdSchema` (e.g., to include and validate `type === "Bot"`) so that you fail fast if the username resolves to a non-bot account rather than just relying on the login naming convention.
- To reduce repeated REST calls when the action runs multiple times on the same bot, consider caching the resolved bot `node_id` in memory within `createGithubClient` so `requestBotReview` can skip the `users.getByUsername` lookup after the first success.

## Individual Comments

### Comment 1
<location path="src/github/__tests__/client.test.ts" line_range="291-300" />
<code_context>
+  it("discovers the bot identity via REST and requests review via GraphQL", async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test case for when the `requestReviews` GraphQL mutation itself fails

The new tests cover discovery well, but they’re missing the case where the `requestReviews` mutation (second GraphQL call using `botIds`) rejects. Please add a test where:

- the first `graphql` returns a valid `viewer.login`,
- `getByUsername` returns a valid bot matching `botNodeIdSchema`, and
- the second `graphql` result contains `{ error: new Error("requestReviews failed") }`.

Then assert that `requestBotReview` rejects with that error, so the current behavior (error propagation to `orchestrate`) is documented and protected against future changes.

Suggested implementation:

```typescript
describe("requestBotReview", () => {
  it("discovers the bot identity via REST and requests review via GraphQL", async () => {
    const stub = makeOctokitStub({
      graphqlResponses: [
        { data: { viewer: { login: "umm-actually" } } },
        { data: { requestReviews: { pullRequest: { id: "PR_kwDOMock7" } } } },
      ],
      getByUsernameResponses: [
        {
          data: {
            node_id: "BOT_kgDOEewBdQ",
            login: "umm-actually[bot]",
          },
        },
      ],
    });

    // Existing expectations/assertions for the happy path go here
    await requestBotReview(stub, {
      owner: "acme",
      repo: "widgets",
      pullNumber: 7,
    });
  });

  it("propagates errors from the requestReviews GraphQL mutation", async () => {
    const error = new Error("requestReviews failed");

    const stub = makeOctokitStub({
      graphqlResponses: [
        // First GraphQL call: discover viewer login
        { data: { viewer: { login: "umm-actually" } } },
        // Second GraphQL call: requestReviews fails
        { error },
      ],
      getByUsernameResponses: [
        {
          data: {
            node_id: "BOT_kgDOEewBdQ",
            login: "umm-actually[bot]",
          },
        },
      ],
    });

    await expect(
      requestBotReview(stub, {
        owner: "acme",
        repo: "widgets",
        pullNumber: 7,
      }),
    ).rejects.toBe(error);
  });

```

- Ensure the arguments passed to `requestBotReview` (`owner`, `repo`, `pullNumber`) match its actual signature and any existing tests; adjust them if your current test suite uses different names (e.g. `pullRequestNumber`).
- If your `makeOctokitStub` uses a different shape for error responses (e.g. `{ error: { message: string } }` or throws directly), adapt the `graphqlResponses` entry for the second call to match that convention and adjust the expectation (e.g. `rejects.toThrow("requestReviews failed")` instead of `rejects.toBe(error)`).
- If there are existing expectations in the original “discovers the bot identity…” test after the snippet you shared, keep them intact below the `await requestBotReview(...)` call, or move the `await` to wherever it currently is so you don’t lose any assertions.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[umm-actually]

Reviewed — 1 finding(s) posted as inline comments.

---

umm-actually · deepseek/deepseek-v4-pro

[aliasunder]

Phase 1: PR Review

5 findings across 2 files + 1 doc.

Summary

#	Dim	Severity	File	Description	Disposition
1	D1	HIGH	src/github/client.ts:225	botIds is not a valid field on RequestReviewsInput — should be userIds	Would fix (trivial)
2	D1	HIGH	src/github/__tests__/client.test.ts:314	Test assertion checks botIds — should be userIds	Would fix (trivial)
3	D1	MEDIUM	src/github/client.ts:216	getByUsername REST error path is untested	Would fix (trivial)
4	D4	LOW	src/github/client.ts:204	botNodeIdSchema could validate type: "Bot"	Would fix (trivial)
5	D6	HIGH	AGENTS.md:22	Structure tree github/ description incomplete — missing "bot review request"	Would fix (trivial)

Finding 5 (body-only): AGENTS.md structure tree

The structure tree comment for github/ currently says "diff fetch, review posting" but the module now also handles bot review requesting via the new requestBotReview method. Update the comment to reflect the full scope:

src/github/  # GitHub I/O: event payload → PrContext, octokit wrappers (diff fetch, review posting, bot review request)

Finding 1: botIds is the critical finding

The requestReviews GraphQL mutation uses botIds as the input field name, but GitHub's GraphQL schema defines RequestReviewsInput with userIds and teamIds fields — not botIds. Bots are users in GitHub's model and must be requested via userIds. The mutation will fail at runtime with a GraphQL validation error.

Verdict: needs-changes — the botIds → userIds fix is required before this works at runtime.

---

🔍 ship-check · pr-review · deepseek/deepseek-v4-pro

[aliasunder]

Phase 2: Code Quality

3 findings across 2 files (all on pre-existing code in changed files — not diff-anchorable).

Findings

1. [comments] src/github/event.ts:56 — Redundant comment. The variable name prNumberOverride and the logger.info on the next line already say what's happening. Remove per AGENTS.md: "Comments explain non-obvious domain context; never restate what a self-documenting name already says."

      if (prNumberOverride !== undefined) {

2. [structure] src/github/event.ts:85 — Use truthy check over explicit === undefined comparison. After Zod parsing, pull_request is Record<string, unknown> | undefined — an empty object {} is truthy, only undefined is falsy. AGENTS.md: "Truthy/falsy checks over explicit !== undefined comparisons — use if (value) not if (value !== undefined) unless distinguishing undefined from other falsy values actually matters for correctness."

    if (!parsed.data.issue.pull_request) {

3. [structure] src/orchestrate.ts:249 — Use truthy check over explicit === undefined comparison. phase is ReviewPhase | undefined where ReviewPhase is an object type (always truthy). !phase is equivalent and preferred per AGENTS.md conventions.

  if (!phase) {

By category

• Comments: 1 (redundant comment restating self-documenting code)
• Structure: 2 (explicit === undefined over truthy/falsy checks)

No findings in CI/CD, config, or fixture files.

---

🔍 ship-check · code-quality · deepseek/deepseek-v4-pro

[aliasunder]

Phase 3: Test Audit

5 test quality findings, 1 soft coverage gap across 2 test files and 1 production file.

Findings breakdown

File	Category	Issue
client.test.ts:312	assertion	Substring match on deterministic query string
orchestrate.test.ts:704	assertion	Loose matchers (toBeGreaterThanOrEqual, toBeTruthy) on known values
orchestrate.test.ts:321	assertion	Substring toThrow("severity") on deterministic error message
orchestrate.test.ts:333	assertion	Substring toThrow("phases") on deterministic error message
orchestrate.test.ts:583	assertion	Decomposed toContain/not.toContain on changedPaths
client.ts:216	soft gap	getByUsername REST call error propagation not tested

Pre-existing issues (outside diff — noted in body only)

Three findings are on pre-existing lines outside the PR diff context. They cannot be posted as inline comments:

orchestrate.test.ts:321 — toThrow("severity") is a substring match on a deterministic error message. The exact message is unknown severity_threshold "invalid" — valid: critical | high | medium | low. Use exact match:

await expect(orchestrate(stubs.deps, logger)).rejects.toThrow(
  'unknown severity_threshold "invalid" — valid: critical | high | medium | low',
)

orchestrate.test.ts:333 — toThrow("phases") is a substring match on a deterministic error message. The exact message is unknown phases value "invalid" — V1 supports only "combined". Use exact match:

await expect(orchestrate(stubs.deps, logger)).rejects.toThrow(
  'unknown phases value "invalid" — V1 supports only "combined"',
)

orchestrate.test.ts:583 — Decomposed toContain/not.toContain checks on changedPaths elements don't catch extra or missing paths. Assert the full array shape with toEqual(expect.arrayContaining([...])) instead of individual toContain on each element.

Coverage gaps

One soft gap: the requestBotReview function in client.ts has three external calls that can throw — the viewer GraphQL query, the bot user REST lookup, and the requestReviews mutation. Only the viewer query error case is tested. The REST call error case (rate limit, network error) should be covered by a dedicated test.

Tests verified

All new tests for the requestBotReview feature are present and well-structured — the orchestrate.test.ts gains "calls requestBotReview with the PR node ID" and "continues when requestBotReview throws", and client.test.ts gains "discovers the bot identity", "logs a warning when the bot user response is malformed", and "propagates errors from the viewer query". The nodeId field was correctly added to all fixture contexts and expected assertions across all test files.

---

🔍 ship-check · test-audit · deepseek/deepseek-v4-pro

[aliasunder]

ship-check pipeline complete (comment mode)

Pipeline ran on commit 59828a0 — 4 phases, comment-only mode.

Results

Phase	Findings	Notes
PR Review	5	2 confirmed false positives (see below)
Code Quality	3	All pre-existing style issues in changed files
Test Audit	6	5 assertion quality + 1 soft coverage gap
Bug Check	0	No bugs found
PR Monitor	skipped	Comment mode

False positive corrections

Findings #1 and #2 from Phase 1 are incorrect — botIds is a valid field on RequestReviewsInput in the GitHub GraphQL schema (confirmed via schema introspection):

RequestReviewsInput.inputFields: pullRequestId, userIds, botIds, teamIds, union

The GraphQL mutation at src/github/client.ts:225 is correct as written. Please disregard those two inline comments.

Deferred: none

Verdict: ship

No bugs, no correctness issues, no blocking findings. All remaining findings are minor (pre-existing style conventions, assertion precision, and one soft coverage gap).

---

🤖 ship-check pipeline · deepseek/deepseek-v4-pro

[umm-actually]

umm-actually re-reviewed at ec7a172

1 new finding(s) posted as inline comments (2 total across all reviews).

---

umm-actually · deepseek/deepseek-v4-pro

[aliasunder]

Sourcery review body finding (review #4689737018): validate type === "Bot" on the REST response

Fixed — botNodeIdSchema now includes type: z.literal("Bot") with a test that verifies a non-Bot user type is rejected. Also added min(1) to the string validators per user feedback.

---

🔍 ship-check · pr-monitor · Opus 4.6 (1M context)

[umm-actually]

Reviewed — 1 finding(s) posted as inline comments.

---

umm-actually · deepseek/deepseek-v4-pro

[umm-actually]

[low/correctness] Verify node_id is present in all event payload shapes (confidence: high)

The pull_request_event_schema in event.ts now requires node_id in the event payload, but issue_comment events do not contain pull_request.node_id. When an issue_comment event resolves to needs_fetch and the caller fetches the PR via fetchPullRequest, the node_id field is populated from the REST response — this is correct. However, the Zod schema in event.ts for pull_request_event does not validate that node_id is present, but the fetchPullRequest Zod schema does. If the event payload's node_id is missing for pull_request/pull_request_target events, the Zod parse will fail and the event will be rejected as malformed — this is the intended behavior. No bug found.

Failure scenario: If a pull_request event payload were missing the node_id field, pullRequestEventSchema.safeParse would fail, causing the event to be rejected as kind: 'not_a_pr' with reason malformed pull_request payload. This is intentional and matches the existing behavior for other missing fields.

Suggested fix

The current handling is correct — no change needed.