feat: request bot as PR reviewer via GraphQL botIds

.github/workflows/release.yml

@@ -54,7 +54,7 @@ jobs:
     permissions:
       contents: write
     steps:
-      - uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2
+      - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
         id: app-token
         with:
           app-id: ${{ secrets.RELEASE_APP_ID }}

.github/workflows/self_review.yml

@@ -31,22 +31,14 @@ jobs:
         with:
           persist-credentials: false
 
-      - uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2
+      - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
         id: app-token
         with:
           app-id: ${{ secrets.UMM_APP_ID }}
           private-key: ${{ secrets.UMM_PRIVATE_KEY }}
           permission-contents: read
           permission-pull-requests: write
 
-      - name: Request review from umm-actually bot
-        env:
-          GH_TOKEN: ${{ steps.app-token.outputs.token }}
-          PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }}
-        run: |
-          gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}/requested_reviewers" \
-            --method POST -f 'reviewers[]=umm-actually[bot]' || true
-
       - uses: ./
         with:
           github_token: ${{ steps.app-token.outputs.token }}

fixtures/pull.get.json

@@ -1,5 +1,6 @@
 {
   "number": 7,
+  "node_id": "PR_kwDOMock7",
   "state": "open",
   "title": "feat: trim names before greeting",
   "body": "Trims whitespace from names and validates registry keys.",

fixtures/pull_request.opened.json

@@ -3,6 +3,7 @@
   "number": 7,
   "pull_request": {
     "number": 7,
+    "node_id": "PR_kwDOMock7",
     "title": "feat: trim names before greeting",
     "body": "Trims whitespace from names and validates registry keys.",
     "head": {

src/__tests__/orchestrate.test.ts

@@ -59,6 +59,7 @@ const fixtureReviewResponse: ReviewResponse = JSON.parse(
 
 const fixturePrContext: PrContext = {
   prNumber: 7,
+  nodeId: "PR_kwDOMock7",
   title: "feat: trim names before greeting",
   body: "Trims whitespace from names and validates registry keys.",
   headSha: "abc123def456abc123def456abc123def456abc1",
@@ -191,6 +192,7 @@ type RecordingStubs = {
   deps: OrchestrateDeps
   fetchPullRequestCalls: { prNumber: number }[]
   fetchDiffCalls: { prNumber: number }[]
+  requestBotReviewCalls: { prNodeId: string }[]
   submitReviewCalls: SubmitReviewParams[]
   fetchReviewCommentsCalls: { prNumber: number }[]
   upsertSummaryCommentCalls: UpsertSummaryCommentParams[]
@@ -228,6 +230,8 @@ const makeOrchestrateDeps = (
     ...overrides.fixtureResult,
   }
 
+  const requestBotReviewCalls: { prNodeId: string }[] = []
+
   const githubClient: GithubClient = {
     fetchPullRequest: async (params) => {
       fetchPullRequestCalls.push(params)
@@ -237,6 +241,9 @@ const makeOrchestrateDeps = (
       fetchDiffCalls.push(params)
       return { kind: "ok" as const, diff: sampleDiff }
     },
+    requestBotReview: async (params) => {
+      requestBotReviewCalls.push(params)
+    },
     submitReview: async (params) => {
       submitReviewCalls.push(params)
       return {
@@ -292,6 +299,7 @@ const makeOrchestrateDeps = (
     deps,
     fetchPullRequestCalls,
     fetchDiffCalls,
+    requestBotReviewCalls,
     submitReviewCalls,
     fetchReviewCommentsCalls,
     upsertSummaryCommentCalls,
@@ -681,6 +689,38 @@ describe("orchestrate", () => {
       expect(stubs.fetchPullRequestCalls).toHaveLength(0)
     })
 
+    it("continues when requestBotReview throws", async () => {
+      const stubs = makeOrchestrateDeps({
+        githubClient: {
+          requestBotReview: async () => {
+            throw new Error("GraphQL viewer query failed")
+          },
+        },
+      })
+      const logger = createTestLogger()
+
+      const result = await orchestrate(stubs.deps, logger)
+
+      expect(result.findingsCount).toBe(expectedSelection.selected.length)
+      expect(result.reviewUrl).toBe("https://github.com/test/review/1")
+      expect(logger.messages).toContainEqual({
+        level: "warn",
+        message: "failed to request bot review",
+        data: { error: "[Error]: GraphQL viewer query failed" },
+      })
+    })
+
+    it("calls requestBotReview with the PR node ID", async () => {
+      const stubs = makeOrchestrateDeps()
+      const logger = createTestLogger()
+
+      await orchestrate(stubs.deps, logger)
+
+      expect(stubs.requestBotReviewCalls).toEqual([
+        { prNodeId: "PR_kwDOMock7" },
+      ])
+    })
+
     it("filters non-findings from LLM output", async () => {
       const nonFinding = makeFinding({
         line: 3,

src/github/__tests__/client.test.ts

@@ -15,27 +15,38 @@ const pullGetResponse: Record<string, unknown> = JSON.parse(
 type StubResponse = { data: unknown } | { error: unknown }
 
 /** Records every call; replays queued responses in order, throwing queued errors. */
+type GraphqlResponse = { data: unknown } | { error: unknown }
+
 const makeOctokitStub = ({
   getResponses = [],
   createReviewResponses = [],
   listReviewCommentsResponses = [],
   listCommentsResponses = [],
   createCommentResponses = [],
   updateCommentResponses = [],
+  getByUsernameResponses = [],
+  graphqlResponses = [],
 }: {
   getResponses?: StubResponse[]
   createReviewResponses?: StubResponse[]
   listReviewCommentsResponses?: StubResponse[]
   listCommentsResponses?: StubResponse[]
   createCommentResponses?: StubResponse[]
   updateCommentResponses?: StubResponse[]
+  getByUsernameResponses?: StubResponse[]
+  graphqlResponses?: GraphqlResponse[]
 } = {}) => {
   const getCalls: Record<string, unknown>[] = []
   const createReviewCalls: Record<string, unknown>[] = []
   const listReviewCommentsCalls: Record<string, unknown>[] = []
   const listCommentsCalls: Record<string, unknown>[] = []
   const createCommentCalls: Record<string, unknown>[] = []
   const updateCommentCalls: Record<string, unknown>[] = []
+  const getByUsernameCalls: Record<string, unknown>[] = []
+  const graphqlCalls: {
+    query: string
+    parameters?: Record<string, unknown>
+  }[] = []
 
   const takeNext = (
     queue: StubResponse[],
@@ -51,7 +62,30 @@ const makeOctokitStub = ({
   }
 
   const octokit: OctokitLike = {
+    graphql: <T = unknown>(
+      query: string,
+      parameters?: Record<string, unknown>,
+    ): Promise<T> => {
+      graphqlCalls.push({ query, ...(parameters ? { parameters } : {}) })
+      const next = graphqlResponses[graphqlCalls.length - 1]
+      if (next === undefined) {
+        throw new Error(`stub: unexpected graphql call #${graphqlCalls.length}`)
+      }
+      if ("error" in next) return Promise.reject(next.error)
+      // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test stub must satisfy the generic signature
+      return Promise.resolve(next.data as T)
+    },
     rest: {
+      users: {
+        getByUsername: async (params: Record<string, unknown>) => {
+          getByUsernameCalls.push(params)
+          return takeNext(
+            getByUsernameResponses,
+            getByUsernameCalls.length,
+            "users.getByUsername",
+          )
+        },
+      },
       pulls: {
         get: async (params) => {
           getCalls.push(params)
@@ -111,6 +145,8 @@ const makeOctokitStub = ({
     listCommentsCalls,
     createCommentCalls,
     updateCommentCalls,
+    getByUsernameCalls,
+    graphqlCalls,
   }
 }
 
@@ -154,6 +190,7 @@ describe("fetchPullRequest", () => {
     ])
     expect(prContext).toEqual({
       prNumber: 7,
+      nodeId: "PR_kwDOMock7",
       title: "feat: trim names before greeting",
       body: "Trims whitespace from names and validates registry keys.",
       headSha: "abc123def456abc123def456abc123def456abc1",
@@ -250,6 +287,130 @@ describe("fetchDiff", () => {
   })
 })
 
+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]",
+            type: "Bot",
+          },
+        },
+      ],
+    })
+    const { client, logger } = makeClient(stub)
+
+    await client.requestBotReview({ prNodeId: "PR_kwDOMock7" })
+
+    expect(stub.graphqlCalls).toHaveLength(2)
+    expect(stub.graphqlCalls[0]?.query).toBe("query { viewer { login } }")
+    expect(stub.getByUsernameCalls).toEqual([{ username: "umm-actually[bot]" }])
+    expect(stub.graphqlCalls[1]?.parameters).toEqual({
+      prId: "PR_kwDOMock7",
+      botIds: ["BOT_kgDOEewBdQ"],
+    })
+    expect(logger.messages).toContainEqual({
+      level: "info",
+      message: "requested bot review",
+      data: { login: "umm-actually[bot]" },
+    })
+  })
+
+  it("logs a warning when the bot user response is malformed", async () => {
+    const stub = makeOctokitStub({
+      graphqlResponses: [{ data: { viewer: { login: "umm-actually" } } }],
+      getByUsernameResponses: [{ data: { message: "Not Found" } }],
+    })
+    const { client, logger } = makeClient(stub)
+
+    await client.requestBotReview({ prNodeId: "PR_kwDOMock7" })
+
+    expect(stub.graphqlCalls).toHaveLength(1)
+    expect(logger.messages).toContainEqual({
+      level: "warn",
+      message: "could not resolve bot user for review request",
+      data: { botLogin: "umm-actually[bot]" },
+    })
+  })
+
+  it("propagates errors from the viewer query", async () => {
+    const stub = makeOctokitStub({
+      graphqlResponses: [{ error: new Error("token expired") }],
+    })
+    const { client } = makeClient(stub)
+
+    await expect(
+      client.requestBotReview({ prNodeId: "PR_kwDOMock7" }),
+    ).rejects.toThrow("token expired")
+  })
+
+  it("propagates errors from the getByUsername REST call", async () => {
+    const stub = makeOctokitStub({
+      graphqlResponses: [{ data: { viewer: { login: "umm-actually" } } }],
+      getByUsernameResponses: [{ error: makeStatusError(404) }],
+    })
+    const { client } = makeClient(stub)
+
+    await expect(
+      client.requestBotReview({ prNodeId: "PR_kwDOMock7" }),
+    ).rejects.toThrow("HTTP 404")
+  })
+
+  it("propagates errors from the requestReviews mutation", async () => {
+    const stub = makeOctokitStub({
+      graphqlResponses: [
+        { data: { viewer: { login: "umm-actually" } } },
+        { error: new Error("insufficient permissions") },
+      ],
+      getByUsernameResponses: [
+        {
+          data: {
+            node_id: "BOT_kgDOEewBdQ",
+            login: "umm-actually[bot]",
+            type: "Bot",
+          },
+        },
+      ],
+    })
+    const { client } = makeClient(stub)
+
+    await expect(
+      client.requestBotReview({ prNodeId: "PR_kwDOMock7" }),
+    ).rejects.toThrow("insufficient permissions")
+  })
+
+  it("rejects a non-Bot user type", async () => {
+    const stub = makeOctokitStub({
+      graphqlResponses: [{ data: { viewer: { login: "some-app" } } }],
+      getByUsernameResponses: [
+        {
+          data: {
+            node_id: "MDQ6VXNlcjEyMzQ=",
+            login: "some-app[bot]",
+            type: "User",
+          },
+        },
+      ],
+    })
+    const { client, logger } = makeClient(stub)
+
+    await client.requestBotReview({ prNodeId: "PR_kwDOMock7" })
+
+    expect(stub.graphqlCalls).toHaveLength(1)
+    expect(logger.messages).toContainEqual({
+      level: "warn",
+      message: "could not resolve bot user for review request",
+      data: { botLogin: "some-app[bot]" },
+    })
+  })
+})
+
 describe("submitReview", () => {
   const reviewUrl =
     "https://github.com/aliasunder/fixture/pull/7#pullrequestreview-1"

src/github/__tests__/event.test.ts

@@ -31,6 +31,7 @@ describe("resolvePullRequestEvent", () => {
       kind: "complete",
       context: {
         prNumber: 7,
+        nodeId: "PR_kwDOMock7",
         title: "feat: trim names before greeting",
         body: "Trims whitespace from names and validates registry keys.",
         headSha: "abc123def456abc123def456abc123def456abc1",
@@ -54,6 +55,7 @@ describe("resolvePullRequestEvent", () => {
       kind: "complete",
       context: {
         prNumber: 7,
+        nodeId: "PR_kwDOMock7",
         title: "feat: trim names before greeting",
         body: "Trims whitespace from names and validates registry keys.",
         headSha: "abc123def456abc123def456abc123def456abc1",