Reuse issue metadata from existing detail conversation queries (#35546)

* Reuse GraphQL detail reads for work-item metadata * Keep PR metadata aligned with cached file listings * Recover incomplete issue assignee metadata without reloading conversation --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
2026-10-30 16:47:34 +00:00 · 2026-10-28 21:20:17 -07:00
parent 14fdb57750
commit 6d342a0039
8 changed files with 347 additions and 48 deletions
@@ -8,6 +8,10 @@ import {
 } from '../../github-api-repository'
 import { githubRepoIdentityKey } from '../../../../shared/github/repository-identity-key'
 import type { MainWorkItem } from './../map/work-item-field-coercion'
+import {
+  getIssueDetailsViaGraphQL,
+  type CollapsedIssueDetails
+} from '../../issue-work-item-details'
 import {
  fetchIssueWorkItem,
  fetchPullRequestWorkItem,
@@ -35,6 +39,7 @@ export async function getWorkItem(
 export type WorkItemRepositoryLookup = {
  item: MainWorkItem | null
  repository: GitHubApiRepository | null
+  issueDetails?: CollapsedIssueDetails | null
 }

 export async function getWorkItemWithRepository(
@@ -43,7 +48,8 @@ export async function getWorkItemWithRepository(
  type?: 'issue' | 'pr',
  connectionId?: string | null,
  localGitOptions: LocalGitExecOptions = {},
-  preference?: IssueSourcePreference
+  preference?: IssueSourcePreference,
+  includeIssueDetails = false
 ): Promise<WorkItemRepositoryLookup> {
  await acquire()
  try {
@@ -59,9 +65,23 @@ export async function getWorkItemWithRepository(
      if (!source && preference === 'origin') {
        return { item: null, repository: null }
      }
+      const issueDetails =
+        includeIssueDetails && source
+          ? await getIssueDetailsViaGraphQL(
+              repoPath,
+              number,
+              source,
+              connectionId,
+              localGitOptions,
+              true
+            )
+          : undefined
      return {
-        item: await fetchIssueWorkItem(repoPath, source, number, connectionId, localGitOptions),
-        repository: source
+        item:
+          issueDetails?.item ??
+          (await fetchIssueWorkItem(repoPath, source, number, connectionId, localGitOptions)),
+        repository: source,
+        ...(includeIssueDetails ? { issueDetails } : {})
      }
    }
    if (type === 'pr') {
@@ -5,6 +5,7 @@ import { noteRepositoryRateLimitSpend } from '../../rate-limit'
 import type { GitHubRepoExecOptions } from '../../github-api-repository'
 import { fetchIssueWorkItem } from '../fetch/work-item-fetch'
 import { mapIssueWorkItem } from '../map/work-item'
+import { restIssueActor } from '../map/issue-actor'
 import type { MainWorkItem } from '../map/work-item-field-coercion'
 import { buildWorkItemListRequest } from './work-item-list-request'
 import { buildSearchQueryString } from './work-item-search-query'
@@ -29,25 +30,6 @@ const ISSUE_NODE_SELECTION = `__typename ... on Issue {
  assignees(first: 100) { nodes { __typename login avatarUrl } pageInfo { hasNextPage } }
 }`

-function restActor(actor: Actor | null): Record<string, unknown> | null {
-  if (!actor) {
-    return null
-  }
-  const login =
-    actor.__typename === 'Bot' && !actor.login.endsWith('[bot]')
-      ? `${actor.login}[bot]`
-      : actor.login
-  let avatar = actor.avatarUrl
-  if (avatar) {
-    const url = new URL(avatar)
-    if (url.hostname === 'avatars.githubusercontent.com') {
-      url.searchParams.delete('u')
-      avatar = url.toString()
-    }
-  }
-  return { login, avatar_url: avatar }
-}
-
 export async function listIssueWorkItemPage(args: {
  repoPath: string
  ownerRepo: OwnerRepo
@@ -110,9 +92,9 @@ export async function listIssueWorkItemPage(args: {
          mapIssueWorkItem({
            ...node,
            state: node.state.toLowerCase(),
-            user: restActor(node.author),
+            user: restIssueActor(node.author),
            labels: node.labels.nodes,
-            assignees: node.assignees.nodes.map(restActor)
+            assignees: node.assignees.nodes.map(restIssueActor)
          })
        )
      }
@@ -0,0 +1,20 @@
+type Actor = { __typename?: string; login: string; avatarUrl?: string }
+
+export function restIssueActor(actor: Actor | null): Record<string, unknown> | null {
+  if (!actor) {
+    return null
+  }
+  const login =
+    actor.__typename === 'Bot' && !actor.login.endsWith('[bot]')
+      ? `${actor.login}[bot]`
+      : actor.login
+  let avatar = actor.avatarUrl
+  if (avatar) {
+    const url = new URL(avatar)
+    if (url.hostname === 'avatars.githubusercontent.com') {
+      url.searchParams.delete('u')
+      avatar = url.toString()
+    }
+  }
+  return { login, avatar_url: avatar }
+}
@@ -1,16 +1,32 @@
 import type { GitHubIssueTimelineItem, PRComment } from '../../shared/github/comment-types'
 import type { GitHubAssignableUser } from '../../shared/github/pull-request-types'
+import { z } from 'zod'
+import { mapIssueWorkItem } from './client/map/work-item'
+import { restIssueActor } from './client/map/issue-actor'
+import type { MainWorkItem } from './client/map/work-item-field-coercion'
 import { ghExecFileAsync, ghRepoExecOptions, githubRepoContext } from './gh-utils'
 import type { LocalGitExecOptions } from './gh-utils'
 import { githubHostExecOptions, type GitHubApiRepository } from './github-api-repository'
 import { getIssueTimelineItems } from './issue-timeline'
 import { noteRepositoryRateLimitSpend, repositoryRateLimitGuard } from './rate-limit'

-const ISSUE_DETAILS_QUERY = `query($owner: String!, $repo: String!, $number: Int!) {
+const ISSUE_DETAILS_QUERY = `query($owner: String!, $repo: String!, $number: Int!, $includeItem: Boolean!) {
  repository(owner: $owner, name: $repo) {
    issue(number: $number) {
+      number @include(if: $includeItem) title @include(if: $includeItem)
+      state @include(if: $includeItem)
+      url @include(if: $includeItem)
+      updatedAt @include(if: $includeItem)
+      author @include(if: $includeItem) { __typename login avatarUrl(size: 48) }
+      labels(first: 100) @include(if: $includeItem) {
+        nodes { name }
+        pageInfo { hasNextPage }
+      }
      body
-      assignees(first: 50) { nodes { login avatarUrl(size: 48) ... on User { name } } }
+      assignees(first: 50) {
+        nodes { login avatarUrl(size: 48) ... on User { name } }
+        pageInfo { hasNextPage }
+      }
      participants(first: 100) {
        nodes { login avatarUrl(size: 48) ... on User { name } }
      }
@@ -31,12 +47,29 @@ const ISSUE_DETAILS_QUERY = `query($owner: String!, $repo: String!, $number: Int
  }
 }`

+const issueItemSchema = z.object({
+  number: z.number().int().positive(),
+  title: z.string(),
+  state: z.enum(['OPEN', 'CLOSED']),
+  url: z.string(),
+  updatedAt: z.string(),
+  author: z.object({ __typename: z.string(), login: z.string(), avatarUrl: z.string() }).nullable(),
+  assignees: z.object({
+    nodes: z.array(z.object({ login: z.string().min(1) })),
+    pageInfo: z.object({ hasNextPage: z.literal(false) })
+  }),
+  labels: z.object({
+    nodes: z.array(z.object({ name: z.string() })),
+    pageInfo: z.object({ hasNextPage: z.literal(false) })
+  })
+})
+
 type GraphQLIssueDetailsResponse = {
  data?: {
    repository?: {
      issue?: {
        body?: string | null
-        assignees?: { nodes?: { login?: string; avatarUrl?: string; name?: string | null }[] }
+        assignees?: { nodes?: (Partial<GitHubAssignableUser> | null)[] | null }
        participants?: { nodes?: GitHubAssignableUser[] }
        comments?: {
          nodes?: {
@@ -58,6 +91,7 @@ type GraphQLIssueDetailsResponse = {
 }

 export type CollapsedIssueDetails = {
+  item?: MainWorkItem
  body: string
  comments: PRComment[]
  assignees: string[]
@@ -71,7 +105,8 @@ export async function getIssueDetailsViaGraphQL(
  issueNumber: number,
  repository: GitHubApiRepository | null,
  connectionId?: string | null,
-  localGitOptions: LocalGitExecOptions = {}
+  localGitOptions: LocalGitExecOptions = {},
+  includeItem = false
 ): Promise<CollapsedIssueDetails | null> {
  const ghOptions = {
    ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
@@ -96,7 +131,9 @@ export async function getIssueDetailsViaGraphQL(
        '-f',
        `repo=${repository.repo}`,
        '-F',
-        `number=${issueNumber}`
+        `number=${issueNumber}`,
+        '-F',
+        `includeItem=${includeItem}`
      ],
      ghOptions
    )
@@ -108,6 +145,10 @@ export async function getIssueDetailsViaGraphQL(
    if (!issue) {
      return null
    }
+    const itemData = includeItem ? issueItemSchema.safeParse(issue) : undefined
+    if (itemData?.success && itemData.data.number !== issueNumber) {
+      return null
+    }
    const comments: PRComment[] = (issue.comments?.nodes ?? [])
      .filter((comment) => typeof comment.databaseId === 'number')
      .map((comment) => ({
@@ -121,7 +162,7 @@ export async function getIssueDetailsViaGraphQL(
      }))
    const assigneeUsers: GitHubAssignableUser[] = (issue.assignees?.nodes ?? [])
      .filter((assignee): assignee is { login: string; avatarUrl?: string; name?: string | null } =>
-        Boolean(assignee.login)
+        Boolean(assignee?.login)
      )
      .map((assignee) => ({
        login: assignee.login,
@@ -136,6 +177,17 @@ export async function getIssueDetailsViaGraphQL(
        avatarUrl: user.avatarUrl ?? ''
      }))
    return {
+      ...(itemData?.success
+        ? {
+            item: mapIssueWorkItem({
+              ...itemData.data,
+              state: itemData.data.state.toLowerCase(),
+              user: restIssueActor(itemData.data.author),
+              labels: itemData.data.labels.nodes,
+              assignees: assigneeUsers
+            })
+          }
+        : {}),
      body: issue.body ?? '',
      comments,
      assignees: assigneeUsers.map((assignee) => assignee.login),
@@ -154,12 +206,7 @@ export async function getIssueBodyAndComments(
  repository: GitHubApiRepository | null,
  connectionId?: string | null,
  localGitOptions: LocalGitExecOptions = {}
-): Promise<{
-  body: string
-  comments: PRComment[]
-  assignees: string[]
-  timelineItems: GitHubIssueTimelineItem[]
-}> {
+): Promise<Pick<CollapsedIssueDetails, 'body' | 'comments' | 'assignees' | 'timelineItems'>> {
  const ghOptions = {
    ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
    ...githubHostExecOptions(repository)
@@ -161,7 +161,9 @@ it.each(['issue', undefined] as const)(
    expect(details?.body).toBe('ORIGIN body')
    expect(details?.assignees).toEqual(['ORIGIN-assignee'])
    expect(details?.comments.map((comment) => comment.body)).toEqual(['ORIGIN comment'])
-    expect(fixture.requests[1].args).toContain('owner=fork-owner')
+    expect(
+      fixture.requests.filter(({ args }) => args.includes('graphql')).map(({ args }) => args)
+    ).toEqual([expect.arrayContaining(['owner=fork-owner'])])
  }
 )

@@ -0,0 +1,218 @@
+import { beforeEach, expect, it, vi } from 'vitest'
+import type { GitHubApiRepository } from './github-api-repository'
+import type { LocalGitExecOptions } from './gh-utils'
+
+const mocks = vi.hoisted(() => ({
+  exec: vi.fn(),
+  source: vi.fn(),
+  acquire: vi.fn(),
+  release: vi.fn()
+}))
+vi.mock('./gh-utils', () => ({
+  ghExecFileAsync: mocks.exec,
+  acquire: mocks.acquire,
+  release: mocks.release,
+  ghRepoExecOptions: (context: { repoPath: string; connectionId?: string; wslDistro?: string }) =>
+    context.connectionId
+      ? {}
+      : { cwd: context.repoPath, ...(context.wslDistro ? { wslDistro: context.wslDistro } : {}) },
+  githubRepoContext: (repoPath: string, connectionId?: string, options = {}) => ({
+    repoPath,
+    connectionId,
+    ...options
+  }),
+  classifyGhError: () => ({ type: 'not_found' })
+}))
+vi.mock('./github-api-repository', () => ({
+  resolveIssueGitHubApiRepositorySource: mocks.source,
+  githubHostExecOptions: (repository?: GitHubApiRepository) => ({ host: repository?.host })
+}))
+vi.mock('./client', async () => ({
+  ...(await import('./client/fetch/get-work-item')),
+  getPRChecks: vi.fn(),
+  getPRComments: vi.fn()
+}))
+vi.mock('./rate-limit', () => ({
+  repositoryRateLimitGuard: () => ({ blocked: false }),
+  noteRepositoryRateLimitSpend: vi.fn()
+}))
+
+import { getWorkItemDetails } from './work-item-details'
+import { getWorkItemWithRepository } from './client/fetch/get-work-item'
+
+const repository = { owner: 'team', repo: 'widgets', host: 'github.example.com:8443' }
+let issue: Record<string, unknown>
+beforeEach(() => {
+  vi.clearAllMocks()
+  mocks.source.mockResolvedValue({ source: repository, fellBack: false })
+  issue = {
+    number: 5,
+    title: 'Closed issue',
+    state: 'CLOSED',
+    url: 'https://github.example.com:8443/team/widgets/issues/5',
+    updatedAt: '2026-10-01T00:00:00Z',
+    author: { __typename: 'User', login: 'author', avatarUrl: 'https://avatars.example/author' },
+    labels: { nodes: [{ name: 'bug' }, { name: 'urgent' }], pageInfo: { hasNextPage: false } },
+    body: 'Description',
+    assignees: {
+      pageInfo: { hasNextPage: false },
+      nodes: [
+        { login: 'assigned', name: 'Assigned User', avatarUrl: 'https://avatars.example/assigned' }
+      ]
+    },
+    participants: { nodes: [] },
+    comments: {
+      nodes: [
+        {
+          databaseId: 10,
+          body: 'Comment',
+          createdAt: '2026-10-02T00:00:00Z',
+          url: 'https://example/comment',
+          author: { login: 'bot', __typename: 'Bot' }
+        }
+      ]
+    }
+  }
+  mocks.exec.mockImplementation(async (args: string[]) => ({
+    stdout: JSON.stringify(
+      args.includes('graphql')
+        ? { data: { repository: { issue } } }
+        : args.some((arg) => arg.includes('/timeline?'))
+          ? []
+          : {
+              number: 5,
+              title: 'REST issue',
+              state: 'open',
+              labels: [{ name: 'all labels' }],
+              assignees: [{ login: 'rest-assigned', name: 'REST Assigned User' }]
+            }
+    )
+  }))
+})
+
+it.each<{ connectionId?: string; options: LocalGitExecOptions; execution: object }>([
+  { options: {}, execution: { cwd: '/repo', host: repository.host } },
+  {
+    options: { wslDistro: 'Ubuntu' },
+    execution: { cwd: '/repo', wslDistro: 'Ubuntu', host: repository.host }
+  },
+  { connectionId: 'ssh-1', options: {}, execution: { host: repository.host } }
+])(
+  'loads issue metadata and conversation with one GraphQL read: $execution',
+  async ({ connectionId, options, execution }) => {
+    const details = await getWorkItemDetails('/repo', 5, 'issue', connectionId, options, 'origin')
+    expect(details?.item).toMatchObject({
+      title: 'Closed issue',
+      state: 'closed',
+      labels: ['bug', 'urgent'],
+      author: 'author',
+      assignees: [{ login: 'assigned', name: 'Assigned User' }]
+    })
+    expect(details).toMatchObject({
+      body: 'Description',
+      comments: [{ id: 10, isBot: true }],
+      assignees: ['assigned'],
+      timelineItems: []
+    })
+    expect(mocks.source).toHaveBeenCalledWith('/repo', 'origin', connectionId, options)
+    expect(mocks.exec).toHaveBeenCalledTimes(2)
+    expect(mocks.exec.mock.calls[0][0]).toContain('includeItem=true')
+    expect(mocks.exec.mock.calls[1][0]).toContain(
+      'repos/team/widgets/issues/5/timeline?per_page=100&page=1'
+    )
+    expect(
+      mocks.exec.mock.calls.every(
+        ([, options]) => JSON.stringify(options) === JSON.stringify(execution)
+      )
+    ).toBe(true)
+    expect(mocks.release).toHaveBeenCalledTimes(mocks.acquire.mock.calls.length)
+  }
+)
+
+it.each(['incomplete labels', 'invalid metadata', 'partial errors'])(
+  'retains the REST item fallback for %s',
+  async (reason) => {
+    if (reason === 'incomplete labels') {
+      issue.labels = { nodes: [{ name: 'partial' }], pageInfo: { hasNextPage: true } }
+    }
+    if (reason === 'invalid metadata') {
+      issue.number = 6
+    }
+    if (reason === 'partial errors') {
+      mocks.exec.mockResolvedValueOnce({
+        stdout: JSON.stringify({
+          data: { repository: { issue } },
+          errors: [{ message: 'Unavailable metadata' }]
+        })
+      })
+    }
+    const result = await getWorkItemWithRepository(
+      '/repo',
+      5,
+      'issue',
+      undefined,
+      {},
+      'origin',
+      true
+    )
+    expect(result.item).toMatchObject({ title: 'REST issue', labels: ['all labels'] })
+    if (reason === 'incomplete labels') {
+      expect(result.issueDetails).toMatchObject({ body: 'Description', timelineItems: [] })
+      expect(mocks.exec).toHaveBeenCalledTimes(3)
+    } else {
+      expect(result.issueDetails).toBeNull()
+      expect(mocks.exec).toHaveBeenCalledTimes(2)
+    }
+    expect(mocks.exec.mock.calls.at(-1)?.[0]).toEqual(['api', 'repos/team/widgets/issues/5'])
+  }
+)
+
+it('keeps ordinary metadata lookups on their existing REST path', async () => {
+  const result = await getWorkItemWithRepository('/repo', 5, 'issue')
+  expect(result.item?.title).toBe('REST issue')
+  expect(mocks.exec).toHaveBeenCalledTimes(1)
+  expect(mocks.exec.mock.calls[0][0]).toEqual(['api', 'repos/team/widgets/issues/5'])
+})
+
+it.each([
+  { reason: 'missing nodes', nodes: undefined },
+  { reason: 'null nodes', nodes: null },
+  { reason: 'null entry', nodes: [null] },
+  { reason: 'missing login', nodes: [{}] },
+  { reason: 'empty login', nodes: [{ login: '' }] }
+])('retains conversation and recovers issue assignees for $reason', async ({ nodes }) => {
+  issue.assignees = { nodes, pageInfo: { hasNextPage: false } }
+  const result = await getWorkItemWithRepository('/repo', 5, 'issue', undefined, {}, 'origin', true)
+  expect(result.item).toMatchObject({
+    title: 'REST issue',
+    assignees: [{ login: 'rest-assigned', name: 'REST Assigned User' }]
+  })
+  expect(result.issueDetails).toMatchObject({
+    body: 'Description',
+    comments: [{ id: 10, isBot: true }],
+    timelineItems: []
+  })
+  expect(mocks.exec.mock.calls.map(([args]) => args)).toEqual([
+    expect.arrayContaining(['api', 'graphql', 'includeItem=true']),
+    expect.arrayContaining(['api', 'repos/team/widgets/issues/5/timeline?per_page=100&page=1']),
+    ['api', 'repos/team/widgets/issues/5']
+  ])
+})
+
+it('does not query an unavailable explicitly selected source', async () => {
+  mocks.source.mockResolvedValue({ source: null, fellBack: false })
+  expect(
+    (await getWorkItemWithRepository('/repo', 5, 'issue', undefined, {}, 'origin', true)).item
+  ).toBeNull()
+  expect(mocks.exec).not.toHaveBeenCalled()
+})
+
+it('retains REST bot login normalization for issue metadata', async () => {
+  issue.author = {
+    __typename: 'Bot',
+    login: 'automation',
+    avatarUrl: 'https://avatars.example/bot'
+  }
+  const result = await getWorkItemWithRepository('/repo', 5, 'issue', undefined, {}, 'origin', true)
+  expect(result.item?.author).toBe('automation[bot]')
+})
@@ -5,7 +5,8 @@ import type { WorkItemRepositoryLookup } from './client/fetch/get-work-item'
 // Why: detail tests replace item lookup; retain its selected-source metadata.
 export function makeWorkItemDetailsLookupMock(lookup: typeof getWorkItem) {
  return async (...args: Parameters<typeof getWorkItem>): Promise<WorkItemRepositoryLookup> => {
-    const item = await lookup(...args)
+    const [repoPath, number, type, connectionId, localGitOptions, preference] = args
+    const item = await lookup(repoPath, number, type, connectionId, localGitOptions, preference)
    return {
      item,
      repository:
@@ -82,7 +82,11 @@ export async function getWorkItemDetails(
      return null
    }
  }
-  const { item, repository: selectedRepository } = issueRepositoryOverride
+  const {
+    item,
+    repository: selectedRepository,
+    issueDetails
+  } = issueRepositoryOverride
    ? {
        item: await getWorkItemByOwnerRepo(
          repoPath,
@@ -92,7 +96,8 @@ export async function getWorkItemDetails(
          connectionId,
          localGitOptions
        ),
-        repository: issueRepositoryOverride
+        repository: issueRepositoryOverride,
+        issueDetails: undefined
      }
    : await getWorkItemWithRepository(
        repoPath,
@@ -100,7 +105,8 @@ export async function getWorkItemDetails(
        type,
        connectionId,
        localGitOptions,
-        preference
+        preference,
+        type === 'issue'
      )
  if (!item) {
    return null
@@ -115,13 +121,16 @@ export async function getWorkItemDetails(

  if (item.type === 'issue') {
    return withWorkItemDetailsPermit(async () => {
-      const collapsed = await getIssueDetailsViaGraphQL(
-        repoPath,
-        item.number,
-        resolvedRepository,
-        connectionId,
-        localGitOptions
-      )
+      const collapsed =
+        issueDetails === undefined
+          ? await getIssueDetailsViaGraphQL(
+              repoPath,
+              item.number,
+              resolvedRepository,
+              connectionId,
+              localGitOptions
+            )
+          : issueDetails
      if (collapsed) {
        return {
          item: enrichItemDisplayAvatars(item, [