Share repository assignee reads across task rows (#35564)

* Share repository assignees across row-specific selections

* test(github): trim redundant cases

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>

src/renderer/src/components/github/PRAssigneesPanel.metadata.test.tsx

@@ -0,0 +1,98 @@
+// @vitest-environment happy-dom
+
+import { act } from 'react'
+import { createRoot, type Root } from 'react-dom/client'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type { GitHubWorkItem } from '../../../../shared/github/work-item-types'
+import { clearGitHubSlugMetadataCache } from '@/hooks/useGitHubSlugMetadata'
+import { PRAssigneesPanel } from './PRAssigneesPanel'
+
+type PanelStoreState = { repos: [] }
+
+vi.mock('@/store', () => ({
+  useAppStore: (select: (state: PanelStoreState) => unknown) => select({ repos: [] })
+}))
+vi.mock('@/lib/github-source-runtime-context', () => ({
+  getGitHubRepoRoutingTarget: () => ({ kind: 'local' })
+}))
+vi.mock('@/hooks/useIssueMetadata', async () => {
+  const { useRepoAssignees } = await import('@/hooks/useGitHubRepoMetadata')
+  return { useRepoAssignees, useImmediateMutation: () => ({ isPending: () => false }) }
+})
+vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback }))
+
+const bySlug = vi.fn()
+const byPath = vi.fn()
+let root: Root
+
+async function render(repoId: string, url: string, repoPath: string | null): Promise<void> {
+  const item: GitHubWorkItem = {
+    id: 'pr:42',
+    repoId,
+    type: 'pr',
+    number: 42,
+    title: 'Example',
+    state: 'open',
+    url,
+    labels: [],
+    updatedAt: '',
+    author: null
+  }
+  await act(async () => {
+    root.render(
+      <PRAssigneesPanel
+        item={item}
+        repoPath={repoPath}
+        projectOrigin={undefined}
+        onMutated={() => {}}
+      />
+    )
+  })
+}
+
+describe('assignee panel metadata source selection', () => {
+  beforeEach(() => {
+    vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
+    clearGitHubSlugMetadataCache()
+    bySlug.mockReset().mockResolvedValue({ ok: true, users: [] })
+    byPath.mockReset().mockResolvedValue([])
+    Object.defineProperty(window, 'api', {
+      configurable: true,
+      value: { gh: { listAssignableUsersBySlug: bySlug, listAssignableUsers: byPath } }
+    })
+    const container = document.createElement('div')
+    document.body.append(container)
+    root = createRoot(container)
+  })
+
+  afterEach(() => {
+    act(() => root.unmount())
+    document.body.replaceChildren()
+    vi.unstubAllGlobals()
+  })
+
+  it('loads only the slug source even when a registered repo and path are available', async () => {
+    await render('registered-with-url', 'https://github.com/acme/orca/pull/42', '/repo')
+
+    expect(bySlug).toHaveBeenCalledExactlyOnceWith({
+      owner: 'acme',
+      repo: 'orca',
+      host: 'github.com'
+    })
+    expect(byPath).not.toHaveBeenCalled()
+  })
+
+  it('retains path loading when the item has no repository slug', async () => {
+    await render('path-only', '', '/folder')
+
+    expect(byPath).toHaveBeenCalledExactlyOnceWith({ repoId: 'path-only', repoPath: '/folder' })
+    expect(bySlug).not.toHaveBeenCalled()
+  })
+
+  it('retains repo-id loading without a checkout path or slug', async () => {
+    await render('id-only', '', null)
+
+    expect(byPath).toHaveBeenCalledExactlyOnceWith({ repoId: 'id-only', repoPath: '' })
+    expect(bySlug).not.toHaveBeenCalled()
+  })
+})

src/renderer/src/components/github/PRAssigneesPanel.tsx

@@ -80,7 +80,12 @@ export function PRAssigneesPanel({
     ownerTarget,
     projectOrigin?.host ?? assigneeSlug?.host
   )
-  const repoAssigneesByPath = useRepoAssignees(repoPath, item.repoId, { target: ownerTarget })
+  const usesSlug = Boolean(slugOwner && slugRepo)
+  const repoAssigneesByPath = useRepoAssignees(
+    usesSlug ? null : repoPath,
+    usesSlug ? null : item.repoId,
+    { target: ownerTarget }
+  )
   const repoAssignees = slugOwner && slugRepo ? repoAssigneesBySlug : repoAssigneesByPath
   const canEditAssignees = Boolean(projectOrigin || repoPath)
   const assigneesByLogin = useMemo(

src/renderer/src/hooks/useGitHubSlugMetadata.assignee-reuse.test.tsx

@@ -0,0 +1,158 @@
+// @vitest-environment happy-dom
+
+import { act } from 'react'
+import { createRoot, type Root } from 'react-dom/client'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type { GitHubAssignableUser } from '../../../shared/github/pull-request-types'
+import type { RuntimeClientTarget } from '../../../shared/runtime-client-target'
+import { callRuntimeRpc } from '@/runtime/runtime-rpc-client'
+import { clearGitHubSlugMetadataCache, useRepoAssigneesBySlug } from './useGitHubSlugMetadata'
+import type { MetadataListState } from './useMetadataListRequest'
+
+vi.mock('@/runtime/runtime-rpc-client', () => ({ callRuntimeRpc: vi.fn() }))
+
+const listUsers = vi.fn()
+const baseUser = { login: 'member', name: 'Member Name', avatarUrl: 'https://example.test/a' }
+const results = new Map<string, MetadataListState<GitHubAssignableUser>>()
+let root: Root
+
+function Probe({
+  id = 'row',
+  seeds,
+  repo = 'orca',
+  host,
+  target
+}: {
+  id?: string
+  seeds?: string[]
+  repo?: string
+  host?: string
+  target?: RuntimeClientTarget
+}): null {
+  results.set(id, useRepoAssigneesBySlug('stablyai', repo, seeds, target, host))
+  return null
+}
+
+async function render(element: React.ReactNode): Promise<void> {
+  await act(async () => {
+    root.render(element)
+  })
+}
+
+function logins(id = 'row'): string[] | undefined {
+  return results.get(id)?.data.map((user) => user.login)
+}
+
+describe('repository assignee reuse across row seeds', () => {
+  beforeEach(() => {
+    vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
+    clearGitHubSlugMetadataCache()
+    listUsers
+      .mockReset()
+      .mockImplementation(async ({ seedLogins = [] }: { seedLogins?: string[] }) => {
+        const extras = [...new Set(seedLogins)].filter((login) => login !== baseUser.login)
+        return {
+          ok: true,
+          users: [baseUser, ...extras.map((login) => ({ login, name: null, avatarUrl: '' }))]
+        }
+      })
+    vi.mocked(callRuntimeRpc)
+      .mockReset()
+      .mockResolvedValue({ ok: true, users: [baseUser] })
+    Object.defineProperty(window, 'api', {
+      configurable: true,
+      value: { gh: { listAssignableUsersBySlug: listUsers } }
+    })
+    results.clear()
+    const container = document.createElement('div')
+    document.body.append(container)
+    root = createRoot(container)
+  })
+
+  afterEach(() => {
+    act(() => root.unmount())
+    document.body.replaceChildren()
+    vi.restoreAllMocks()
+    vi.unstubAllGlobals()
+  })
+
+  it('loads one base list for concurrent rows without sharing their extra users', async () => {
+    await render(
+      <>
+        <Probe id="a" seeds={['zeta', 'member', 'alpha', 'alpha']} />
+        <Probe id="b" seeds={['beta']} />
+      </>
+    )
+
+    expect(listUsers).toHaveBeenCalledExactlyOnceWith({
+      owner: 'stablyai',
+      repo: 'orca',
+      host: 'github.com'
+    })
+    expect(logins('a')).toEqual(['member', 'alpha', 'zeta'])
+    expect(logins('b')).toEqual(['member', 'beta'])
+    expect(results.get('a')?.data[0]).toEqual(baseUser)
+    expect(results.get('a')?.data[1]).toEqual({ login: 'alpha', name: null, avatarUrl: '' })
+  })
+
+  it('updates seeds in place without a fetch or loading transition', async () => {
+    await render(<Probe seeds={['first']} />)
+    await render(<Probe seeds={['second']} />)
+
+    expect(logins()).toEqual(['member', 'second'])
+    expect(results.get('row')).toMatchObject({ loading: false, error: null })
+    expect(listUsers).toHaveBeenCalledTimes(1)
+  })
+
+  it('adds seeds to a successful empty list but not to loading or failed results', async () => {
+    let finish: (value: { ok: boolean; users: GitHubAssignableUser[] }) => void = () => {}
+    listUsers.mockReturnValueOnce(new Promise((resolve) => (finish = resolve)))
+    await render(<Probe seeds={['existing']} />)
+    expect(results.get('row')).toMatchObject({ data: [], loading: true, error: null })
+    await act(async () => finish({ ok: true, users: [] }))
+    expect(logins()).toEqual(['existing'])
+
+    listUsers.mockResolvedValueOnce({ ok: false, error: { message: 'Forbidden' } })
+    await render(<Probe repo="other" seeds={['unavailable']} />)
+    expect(results.get('row')).toMatchObject({ data: [], loading: false, error: 'Forbidden' })
+  })
+
+  it('retains seed projection after expiry and fetches fresh metadata for a new reader', async () => {
+    const now = Date.now()
+    const time = vi.spyOn(Date, 'now').mockReturnValue(now)
+    await render(<Probe seeds={['existing']} />)
+    time.mockReturnValue(now + 300_001)
+    await render(<Probe seeds={['updated']} />)
+    expect(logins()).toEqual(['member', 'updated'])
+    expect(listUsers).toHaveBeenCalledTimes(1)
+
+    await render(<Probe key="new-reader" seeds={['fresh']} />)
+    expect(logins()).toEqual(['member', 'fresh'])
+    expect(listUsers).toHaveBeenCalledTimes(2)
+  })
+
+  it('isolates repository, host and remote runtime scopes and honors cache clearing', async () => {
+    await render(<Probe />)
+    await render(<Probe repo="other" />)
+    await render(<Probe host="ghe.example.test" />)
+    await render(<Probe target={{ kind: 'environment', environmentId: 'remote-a' }} />)
+    await render(<Probe target={{ kind: 'environment', environmentId: 'remote-b' }} />)
+
+    expect(listUsers).toHaveBeenCalledTimes(3)
+    expect(listUsers).toHaveBeenLastCalledWith({
+      owner: 'stablyai',
+      repo: 'orca',
+      host: 'ghe.example.test'
+    })
+    expect(callRuntimeRpc).toHaveBeenCalledTimes(2)
+    expect(callRuntimeRpc).toHaveBeenLastCalledWith(
+      { kind: 'environment', environmentId: 'remote-b' },
+      'github.project.listAssignableUsersBySlug',
+      { owner: 'stablyai', repo: 'orca', host: 'github.com' },
+      { timeoutMs: 30_000 }
+    )
+    clearGitHubSlugMetadataCache()
+    await render(<Probe key="after-auth-change" />)
+    expect(listUsers).toHaveBeenCalledTimes(4)
+  })
+})

src/renderer/src/hooks/useGitHubSlugMetadata.test.tsx

@@ -109,8 +109,7 @@ describe('useGitHubSlugMetadata', () => {
     expect(apiMocks.listAssignableUsersBySlug).toHaveBeenCalledExactlyOnceWith({
       owner: 'stablyai',
       repo: 'orca',
-      host: 'github.com',
-      seedLogins: ['jinwoo']
+      host: 'github.com'
     })
     expect(renders).toBeLessThanOrEqual(4)
   })

src/renderer/src/hooks/useGitHubSlugMetadata.ts

@@ -1,3 +1,4 @@
+import { useMemo } from 'react'
 import { callRuntimeRpc } from '@/runtime/runtime-rpc-client'
 import type { RuntimeClientTarget } from '../../../shared/runtime-client-target'
 import type { GitHubAssignableUser } from '../../../shared/github/pull-request-types'
@@ -73,17 +74,16 @@ export function useRepoAssigneesBySlug(
   const repositoryKey = owner && repo ? githubRepoIdentityKey({ owner, repo, host }) : null
   const cacheKey = repositoryKey
     ? target.kind === 'environment'
-      ? `runtime:${target.environmentId}:${repositoryKey}#${seedKey}`
-      : `${repositoryKey}#${seedKey}`
+      ? `runtime:${target.environmentId}:${repositoryKey}`
+      : repositoryKey
     : null
   const args = {
     owner: selectedOwner,
     repo: selectedRepo,
-    host: githubProjectHost(host),
-    ...(seedKey ? { seedLogins: seedKey.split(',') } : {})
+    host: githubProjectHost(host)
   }
 
-  return useMetadataListRequest({
+  const base = useMetadataListRequest({
     cacheKey,
     store: slugAssigneeStore,
     errorFallback: 'Failed to load assignees',
@@ -103,4 +103,19 @@ export function useRepoAssigneesBySlug(
         return res.users
       })
   })
+  const data = useMemo(() => {
+    if (!cacheKey || base.resolvedKey !== cacheKey || !seedKey) {
+      return base.data
+    }
+    const users = [...base.data]
+    const seen = new Set(users.map((user) => user.login))
+    for (const login of seedKey.split(',')) {
+      if (!seen.has(login)) {
+        users.push({ login, name: null, avatarUrl: '' })
+        seen.add(login)
+      }
+    }
+    return users
+  }, [base.data, base.resolvedKey, cacheKey, seedKey])
+  return { data, loading: base.loading, error: base.error }
 }

src/renderer/src/hooks/useMetadataListRequest.test.tsx

@@ -63,6 +63,11 @@ describe('useMetadataListRequest', () => {
       first.resolve(['first'])
       await first.promise
     })
-    expect(view.result.current).toEqual({ data: ['second'], loading: false, error: null })
+    expect(view.result.current).toEqual({
+      data: ['second'],
+      loading: false,
+      error: null,
+      resolvedKey: 'second'
+    })
   })
 })

src/renderer/src/hooks/useMetadataListRequest.ts

@@ -19,11 +19,12 @@ export function useMetadataListRequest<T>({
   store,
   load,
   errorFallback
-}: MetadataListRequest<T>): MetadataListState<T> {
-  const [state, setState] = useState<MetadataListState<T>>({
+}: MetadataListRequest<T>): MetadataListState<T> & { resolvedKey: string | null } {
+  const [state, setState] = useState<MetadataListState<T> & { resolvedKey: string | null }>({
     data: [],
     loading: false,
-    error: null
+    error: null,
+    resolvedKey: null
   })
   const activeKeyRef = useRef<string | null>(null)
   const loadLatest = useEffectEvent(load)
@@ -36,7 +37,7 @@ export function useMetadataListRequest<T>({
     const cached = getFreshMetadata(store, cacheKey)
     if (cached) {
       if (activeKeyRef.current !== cacheKey) {
-        setState({ data: cached.data, loading: false, error: null })
+        setState({ data: cached.data, loading: false, error: null, resolvedKey: cacheKey })
       }
       activeKeyRef.current = cacheKey
       return
@@ -47,12 +48,13 @@ export function useMetadataListRequest<T>({
       ...current,
       data: current.data.length ? [] : current.data,
       loading: true,
-      error: null
+      error: null,
+      resolvedKey: null
     }))
     void loadMetadata(store, cacheKey, () => loadLatest())
       .then((data) => {
         if (activeKeyRef.current === requestKey) {
-          setState({ data, loading: false, error: null })
+          setState({ data, loading: false, error: null, resolvedKey: requestKey })
         }
       })
       .catch((error: unknown) => {