Merge remote-tracking branch 'origin/main' into github-api-error-message

src/main/github/client.ts

@@ -485,16 +485,32 @@ function mapIssueWorkItem(item: Record<string, unknown>): MainWorkItem {
           .filter(Boolean)
       : [],
     updatedAt: String(item.updated_at ?? item.updatedAt ?? ''),
-    author:
-      typeof item.user === 'object' && item.user !== null && 'login' in item.user
-        ? String((item.user as { login?: unknown }).login ?? '')
-        : typeof item.author === 'object' && item.author !== null && 'login' in item.author
-          ? String((item.author as { login?: unknown }).login ?? '')
-          : null,
+    ...authorFieldsFromUnknown(item),
     ...(item.assignees !== undefined ? { assignees: usersFromUnknown(item.assignees) } : {})
   }
 }
 
+/**
+ * Derive both the author login and its API avatar_url in one place so GHE
+ * avatars render (the login-only `github.com/{login}.png` URL 404s on GHE).
+ *
+ * REST exposes `user.avatar_url`; gh/GraphQL expose `author.avatarUrl`. `gh pr
+ * view` omits the avatar entirely, so authorAvatarUrl is left undefined and the
+ * UI falls back to the login URL then a placeholder. See #8784.
+ */
+function authorFieldsFromUnknown(
+  item: Record<string, unknown>
+): Pick<MainWorkItem, 'author' | 'authorAvatarUrl'> {
+  const user = userFromUnknown(item.user ?? item.author)
+  if (!user) {
+    return { author: null }
+  }
+  return {
+    author: user.login,
+    ...(user.avatarUrl ? { authorAvatarUrl: user.avatarUrl } : {})
+  }
+}
+
 function extractHeadOwnerLogin(item: Record<string, unknown>): string | null {
   // gh CLI `pr list --json headRepositoryOwner` shape: { login }
   if (typeof item.headRepositoryOwner === 'object' && item.headRepositoryOwner !== null) {
@@ -746,12 +762,7 @@ function mapPullRequestWorkItem(
           .filter(Boolean)
       : [],
     updatedAt: String(item.updated_at ?? item.updatedAt ?? ''),
-    author:
-      typeof item.user === 'object' && item.user !== null && 'login' in item.user
-        ? String((item.user as { login?: unknown }).login ?? '')
-        : typeof item.author === 'object' && item.author !== null && 'login' in item.author
-          ? String((item.author as { login?: unknown }).login ?? '')
-          : null,
+    ...authorFieldsFromUnknown(item),
     branchName:
       typeof item.head === 'object' && item.head !== null && 'ref' in item.head
         ? String((item.head as { ref?: unknown }).ref ?? '')

src/main/github/work-item-details.test.ts

@@ -216,6 +216,56 @@ describe('getWorkItemDetails', () => {
     expect(details?.participants?.[0]?.login).toBe('issue-author')
   })
 
+  it('enriches a non-participating assignee avatar from the GraphQL assignees connection', async () => {
+    // Why: a GHE assignee who never commented is absent from `participants`, so
+    // the enrichment must also draw avatars from the `assignees` connection or
+    // item.assignees keeps the blank avatar `gh` returns.
+    getWorkItemMock.mockResolvedValueOnce({
+      id: 'issue:924',
+      type: 'issue',
+      number: 924,
+      title: 'Assignee avatar',
+      state: 'open',
+      url: 'https://github.com/acme/widgets/issues/924',
+      labels: [],
+      updatedAt: '2026-04-01T00:00:00Z',
+      author: 'issue-author',
+      assignees: [{ login: 'ghe-assignee', name: 'GHE Assignee', avatarUrl: '' }]
+    })
+    getIssueOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
+    ghExecFileAsyncMock
+      .mockResolvedValueOnce({
+        stdout: JSON.stringify({
+          data: {
+            repository: {
+              issue: {
+                body: 'Issue body',
+                assignees: {
+                  nodes: [
+                    {
+                      login: 'ghe-assignee',
+                      name: 'GHE Assignee',
+                      avatarUrl: 'https://ghe.example.com/avatars/ghe-assignee'
+                    }
+                  ]
+                },
+                participants: { nodes: [{ login: 'issue-author', avatarUrl: 'https://x/y' }] },
+                comments: { nodes: [] }
+              }
+            }
+          }
+        })
+      })
+      .mockResolvedValueOnce({ stdout: '' })
+
+    const details = await getWorkItemDetails('/repo-root', 924, 'issue')
+
+    expect(details?.item.assignees?.[0]).toMatchObject({
+      login: 'ghe-assignee',
+      avatarUrl: 'https://ghe.example.com/avatars/ghe-assignee'
+    })
+  })
+
   it('caps issue timeline pagination by supported activity items', async () => {
     getWorkItemMock.mockResolvedValueOnce({
       id: 'issue:923',
@@ -568,4 +618,66 @@ describe('getWorkItemDetails', () => {
     expect(details?.filesUnavailable).toBe(false)
     expect(details?.files).toEqual([])
   })
+
+  // Why: `gh pr view` omits avatar_url, so the login-based github.com URL 404s on
+  // GHE. getWorkItemDetails must resolve author/reviewer/assignee avatars via the
+  // GraphQL user(login:) batch and stamp them onto the returned item. See #8784.
+  it('enriches PR author, reviewer, and assignee avatars from the GraphQL user lookup', async () => {
+    getWorkItemMock.mockResolvedValueOnce({
+      id: 'pr:1102',
+      type: 'pr',
+      number: 1102,
+      title: 'Enterprise PR',
+      state: 'open',
+      url: 'https://ghe.example.com/acme/widgets/pull/1102',
+      labels: [],
+      updatedAt: '2026-07-11T00:00:00Z',
+      author: 'seah',
+      reviewRequests: [{ login: 'ludi', name: null, avatarUrl: '' }],
+      latestReviews: [{ login: 'inho', state: 'APPROVED', avatarUrl: '' }],
+      // A default `u/0` placeholder must be replaced by the resolved avatar.
+      assignees: [{ login: 'seah', name: 'Seah', avatarUrl: 'https://avatars.example.com/u/0?v=4' }]
+    })
+    getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
+    getPRCommentsMock.mockResolvedValue([])
+    getPRChecksMock.mockResolvedValue([])
+    const avatars: Record<string, string> = {
+      seah: 'https://avatars.example.com/u/1?v=4',
+      ludi: 'https://avatars.example.com/u/2?v=4',
+      inho: 'https://avatars.example.com/u/3?v=4'
+    }
+    ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
+      const target = args.at(-1)
+      if (target === 'repos/acme/widgets/pulls/1102') {
+        return {
+          stdout: JSON.stringify({ body: 'PR body', head: { sha: 'h' }, base: { sha: 'b' } })
+        }
+      }
+      if (target === 'repos/acme/widgets/pulls/1102/files?per_page=100') {
+        return { stdout: '[]' }
+      }
+      const query = args.find((arg) => arg.startsWith('query=')) ?? ''
+      if (query.includes('user(login:')) {
+        // Return the aliased users the batch asked for, keyed by their login.
+        const data: Record<string, { login: string; name: null; avatarUrl: string }> = {}
+        let index = 0
+        for (const login of Object.keys(avatars)) {
+          if (query.includes(`user(login: "${login}")`)) {
+            data[`u${index}`] = { login, name: null, avatarUrl: avatars[login] }
+            index += 1
+          }
+        }
+        return { stdout: JSON.stringify({ data }) }
+      }
+      return { stdout: JSON.stringify({ data: {} }) }
+    })
+
+    const details = await getWorkItemDetails('/repo-root', 1102, 'pr')
+
+    expect(details?.item.authorAvatarUrl).toBe(avatars.seah)
+    expect(details?.item.reviewRequests?.[0]?.avatarUrl).toBe(avatars.ludi)
+    expect(details?.item.latestReviews?.[0]?.avatarUrl).toBe(avatars.inho)
+    // The default u/0 placeholder is overridden by the resolved avatar.
+    expect(details?.item.assignees?.[0]?.avatarUrl).toBe(avatars.seah)
+  })
 })

src/main/github/work-item-details.ts

@@ -85,7 +85,7 @@ const ISSUE_DETAILS_QUERY = `query($owner: String!, $repo: String!, $number: Int
   repository(owner: $owner, name: $repo) {
     issue(number: $number) {
       body
-      assignees(first: 50) { nodes { login } }
+      assignees(first: 50) { nodes { login avatarUrl(size: 48) ... on User { name } } }
       participants(first: 100) {
         nodes { login avatarUrl(size: 48) ... on User { name } }
       }
@@ -111,7 +111,7 @@ type GraphQLIssueDetailsResponse = {
     repository?: {
       issue?: {
         body?: string | null
-        assignees?: { nodes?: { login?: string }[] }
+        assignees?: { nodes?: { login?: string; avatarUrl?: string; name?: string | null }[] }
         participants?: { nodes?: GitHubAssignableUser[] }
         comments?: {
           nodes?: {
@@ -323,6 +323,12 @@ async function getIssueTimelineItems(
   }
 }
 
+/**
+ * Fetch an issue's body, comments, assignees, participants, and timeline in a
+ * single GraphQL round-trip. Returns null on any partial error so the caller
+ * falls back to the strict REST path. Assignee/participant avatars are resolved
+ * here so GitHub Enterprise users don't render blank.
+ */
 async function getIssueDetailsViaGraphQL(
   repoPath: string,
   issueNumber: number,
@@ -332,6 +338,10 @@ async function getIssueDetailsViaGraphQL(
   body: string
   comments: PRComment[]
   assignees: string[]
+  // Avatar-bearing assignees, kept separate from the `assignees` login list so
+  // callers that only need logins are unaffected; used to enrich assignee
+  // avatars that `gh` leaves blank (notably GitHub Enterprise users).
+  assigneeUsers: GitHubAssignableUser[]
   participants: GitHubAssignableUser[]
   timelineItems: GitHubIssueTimelineItem[]
 } | null> {
@@ -386,9 +396,16 @@ async function getIssueDetailsViaGraphQL(
         url: c.url ?? '',
         isBot: c.author?.__typename === 'Bot'
       }))
-    const assignees = (issue.assignees?.nodes ?? [])
-      .map((a) => a.login)
-      .filter((login): login is string => Boolean(login))
+    const assigneeUsers: GitHubAssignableUser[] = (issue.assignees?.nodes ?? [])
+      .filter((a): a is { login: string; avatarUrl?: string; name?: string | null } =>
+        Boolean(a.login)
+      )
+      .map((a) => ({
+        login: a.login,
+        name: a.name ?? null,
+        avatarUrl: a.avatarUrl ?? ''
+      }))
+    const assignees = assigneeUsers.map((a) => a.login)
     const participants: GitHubAssignableUser[] = (issue.participants?.nodes ?? [])
       .filter((u) => Boolean(u.login))
       .map((u) => ({
@@ -401,6 +418,7 @@ async function getIssueDetailsViaGraphQL(
       body: issue.body ?? '',
       comments,
       assignees,
+      assigneeUsers,
       participants,
       timelineItems
     }
@@ -877,6 +895,11 @@ async function getGitHubUsersByLogin(
     return []
   }
   if (rateLimitGuard('graphql').blocked) {
+    // Why: surface the skip. Callers degrade silently to login-based avatar URLs
+    // (blank on GHE), so without this log "why are avatars missing" is untraceable.
+    console.warn(
+      `getGitHubUsersByLogin skipped: GraphQL rate-limit budget exhausted (${uniqueLogins.length} logins unresolved)`
+    )
     return []
   }
   const fields = uniqueLogins
@@ -921,19 +944,100 @@ async function getGitHubUsersByLogin(
   }
 }
 
+/**
+ * Stamp GraphQL-resolved avatars onto a work item's author, reviewers, review
+ * requests, latest reviews, and assignees.
+ *
+ * The work item's users come from `gh pr view`, which omits avatar_url. On
+ * GitHub Enterprise the login-based `github.com/{login}.png` URL 404s, so those
+ * avatars render blank while comment avatars (fetched via GraphQL) work.
+ * `knownUsers` (participants + the mention-author batch, which already includes
+ * the item's reviewers/assignees) carries the resolved avatars, so we apply
+ * them without any extra round-trip. See #8784.
+ */
+function enrichItemDisplayAvatars(
+  item: Omit<GitHubWorkItem, 'repoId'>,
+  knownUsers: GitHubAssignableUser[]
+): Omit<GitHubWorkItem, 'repoId'> {
+  const avatarByLogin = new Map<string, string>()
+  for (const user of knownUsers) {
+    if (user.login && user.avatarUrl) {
+      avatarByLogin.set(user.login.toLowerCase(), user.avatarUrl)
+    }
+  }
+  if (avatarByLogin.size === 0) {
+    return item
+  }
+  // Why: prefer the GraphQL-resolved avatar over whatever `gh` returned. `gh pr
+  // view` often yields an empty avatar or the default `u/0` placeholder for
+  // enterprise users; the resolved avatar is the authoritative one for that
+  // login. Fall back to the original only when the lookup found nothing.
+  const avatarFor = (login: string): string | undefined => avatarByLogin.get(login.toLowerCase())
+  const resolvedAvatar = (login: string, existing?: string | null): string | undefined =>
+    avatarFor(login) || existing || undefined
+  // Callers coalesce a missing result to the field's "no avatar" sentinel (''
+  // for GitHubAssignableUser, null for GitHubPRReviewSummary); GitHubUserAvatar
+  // treats both as falsy and falls back to the login URL then initials.
+  const authorAvatarUrl = (item.author ? avatarFor(item.author) : undefined) || item.authorAvatarUrl
+  return {
+    ...item,
+    ...(authorAvatarUrl ? { authorAvatarUrl } : {}),
+    ...(item.reviewRequests
+      ? {
+          reviewRequests: item.reviewRequests.map((user) => ({
+            ...user,
+            avatarUrl: resolvedAvatar(user.login, user.avatarUrl) ?? ''
+          }))
+        }
+      : {}),
+    ...(item.latestReviews
+      ? {
+          latestReviews: item.latestReviews.map((review) => ({
+            ...review,
+            avatarUrl: resolvedAvatar(review.login, review.avatarUrl) ?? null
+          }))
+        }
+      : {}),
+    ...(item.assignees
+      ? {
+          assignees: item.assignees.map((user) => ({
+            ...user,
+            avatarUrl: resolvedAvatar(user.login, user.avatarUrl) ?? ''
+          }))
+        }
+      : {})
+  }
+}
+
 async function getMentionParticipants(
   repoPath: string,
-  item: Pick<GitHubWorkItem, 'author' | 'number' | 'type'>,
+  item: Pick<
+    GitHubWorkItem,
+    'author' | 'number' | 'type' | 'reviewRequests' | 'latestReviews' | 'assignees'
+  >,
   comments: PRComment[],
   participants: GitHubAssignableUser[],
   connectionId?: string | null,
   localGitOptions: LocalGitExecOptions = {}
 ): Promise<GitHubAssignableUser[]> {
-  const visibleLogins = [item.author ?? '', ...comments.map((comment) => comment.author)]
-  // Why: one aliased GraphQL query returns login/name/avatarUrl for every
-  // mentioned author in a single round-trip. The previous REST fan-out
-  // (/users/<login>) returned the same fields but cost one rate-limit point
-  // per user.
+  // Why: resolve mention authors AND the item's reviewers/assignees in ONE
+  // aliased GraphQL round-trip. Folding the display users in here means the
+  // caller can reuse this result to stamp avatars (see enrichItemDisplayAvatars)
+  // without a second, separately rate-limited lookup — which is what made GHE
+  // reviewer/assignee avatars resolve intermittently. See #8784.
+  // Why: order the always-visible display users (author, reviewers, assignees)
+  // before comment authors so the 40-login cap in getGitHubUsersByLogin never
+  // drops a reviewer/assignee avatar on a PR with many commenters. On a PR with
+  // 40+ distinct participants the trailing comment authors are dropped from this
+  // batch and lose only their @mention-suggestion avatar; rendered comment
+  // avatars are unaffected (they carry their own avatarUrl from the comment).
+  const visibleLogins = [
+    item.author ?? '',
+    ...(item.reviewRequests ?? []).map((user) => user.login),
+    ...(item.latestReviews ?? []).map((review) => review.login),
+    ...(item.assignees ?? []).map((user) => user.login),
+    ...comments.map((comment) => comment.author)
+  ]
   const graphQlUsers = await getGitHubUsersByLogin(
     repoPath,
     visibleLogins,
@@ -1007,7 +1111,13 @@ export async function getWorkItemDetails(
       )
       if (collapsed) {
         return {
-          item,
+          // Include assigneeUsers: a non-participating assignee is absent from
+          // the participants connection, so without them item.assignees keeps a
+          // blank/placeholder avatar (common for GitHub Enterprise users).
+          item: enrichItemDisplayAvatars(item, [
+            ...collapsed.participants,
+            ...collapsed.assigneeUsers
+          ]),
           body: collapsed.body,
           comments: collapsed.comments,
           assignees: collapsed.assignees,
@@ -1030,7 +1140,7 @@ export async function getWorkItemDetails(
         localGitOptions
       )
       return {
-        item,
+        item: enrichItemDisplayAvatars(item, mentionParticipants),
         body,
         comments,
         assignees,
@@ -1064,7 +1174,7 @@ export async function getWorkItemDetails(
     ])
 
     return {
-      item,
+      item: enrichItemDisplayAvatars(item, mentionParticipants),
       body,
       comments,
       headSha: shas?.headSha,