fix(github): main finds a twin project's registered row by id and path, and a refresh by its host (#31274)

validateRegisteredRepo took the first row with the requested id, so for a repo id
registered locally and over SSH it read the other twin and refused the second
twin's fetch as a path mismatch. It now matches id and path together, uses the
GitHub source host when one is named, and lets a PR-refresh candidate's host pick
between twins at one path; a lone row's own fields still win over stale candidate
fields.

src/main/ipc/github-pr-refresh-handlers.ts

@@ -21,6 +21,7 @@ import type { StatsCollector } from '../stats/collector'
 import {
   applyRegisteredRepoToPRRefreshCandidate,
   assertRegisteredGitHubRepo,
+  assertRegisteredPRRefreshCandidateRepo,
   getGitHubLocalGitOptionArgs,
   getGitHubRepoConnectionId,
   isPRRefreshCandidateRegistered,
@@ -115,7 +116,7 @@ export function registerGitHubPRRefreshHandlers(store: Store, stats: StatsCollec
       _event,
       args: { candidate: GitHubPRRefreshCandidate; reason?: GitHubPRRefreshReason }
     ) => {
-      const repo = assertRegisteredGitHubRepo(args.candidate, store)
+      const repo = assertRegisteredPRRefreshCandidateRepo(args.candidate, store)
       const outcome = await refreshPRNow(
         applyRegisteredRepoToPRRefreshCandidate(store, repo, args.candidate),
         args.reason

src/main/ipc/github-repo-routing-twin-host.test.ts

@@ -0,0 +1,106 @@
+import { describe, expect, it, vi } from 'vitest'
+import type { GitHubPRRefreshCandidate } from '../../shared/github/pull-request-refresh-types'
+import type { Repo } from '../../shared/repo-types'
+import type { Store } from '../persistence'
+
+vi.mock('../project-runtime-git-options', () => ({
+  getLocalProjectGhExecOptions: () => ({})
+}))
+
+import {
+  assertRegisteredGitHubRepo,
+  assertRegisteredPRRefreshCandidateRepo,
+  validateAutomaticPRRefreshCandidate
+} from './github-repo-routing'
+
+const localTwin: Repo = {
+  id: 'twin',
+  path: '/laptop/twin',
+  displayName: 'Twin',
+  badgeColor: '#000',
+  addedAt: 1
+}
+const sshTwin: Repo = { ...localTwin, path: '/remote/twin', connectionId: 'builder' }
+const samePathSshTwin: Repo = { ...sshTwin, path: localTwin.path }
+
+function storeWith(repos: Repo[]): Store {
+  // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: these paths read only getRepos.
+  return { getRepos: () => repos } as unknown as Store
+}
+
+function candidate(overrides: Partial<GitHubPRRefreshCandidate>): GitHubPRRefreshCandidate {
+  return {
+    cacheKey: 'twin::branch',
+    repoPath: sshTwin.path,
+    branch: 'feature',
+    repoKind: 'git',
+    repoId: 'twin',
+    connectionId: 'builder',
+    ...overrides
+  }
+}
+
+describe('registered GitHub repo lookup for a repo id on two hosts', () => {
+  it("accepts the second twin's fetch by its own path", () => {
+    const store = storeWith([localTwin, sshTwin])
+
+    expect(assertRegisteredGitHubRepo({ repoPath: '/remote/twin', repoId: 'twin' }, store)).toBe(
+      sshTwin
+    )
+    expect(assertRegisteredGitHubRepo({ repoPath: '/laptop/twin', repoId: 'twin' }, store)).toBe(
+      localTwin
+    )
+  })
+
+  it("queues the SSH twin's PR refresh on the SSH row, even when it is listed second", () => {
+    const result = validateAutomaticPRRefreshCandidate(
+      candidate({}),
+      storeWith([localTwin, sshTwin])
+    )
+
+    expect(result).toMatchObject({
+      kind: 'ok',
+      candidate: { repoPath: '/remote/twin', connectionId: 'builder' }
+    })
+  })
+
+  it('picks the candidate host when the twins share a path', () => {
+    const store = storeWith([localTwin, samePathSshTwin])
+
+    expect(
+      validateAutomaticPRRefreshCandidate(candidate({ repoPath: localTwin.path }), store)
+    ).toMatchObject({ kind: 'ok', candidate: { connectionId: 'builder' } })
+    expect(
+      validateAutomaticPRRefreshCandidate(
+        candidate({ repoPath: localTwin.path, connectionId: null }),
+        store
+      )
+    ).toMatchObject({ kind: 'ok', candidate: { connectionId: null } })
+  })
+
+  it('resolves a manual PR refresh to the same twin as the automatic queue', () => {
+    const store = storeWith([localTwin, samePathSshTwin])
+
+    expect(
+      assertRegisteredPRRefreshCandidateRepo(candidate({ repoPath: localTwin.path }), store)
+    ).toBe(samePathSshTwin)
+  })
+
+  it("keeps a lone row's own host over a stale candidate host", () => {
+    const result = validateAutomaticPRRefreshCandidate(
+      candidate({ repoPath: localTwin.path, connectionId: 'gone' }),
+      storeWith([localTwin])
+    )
+
+    expect(result).toMatchObject({ kind: 'ok', candidate: { connectionId: null } })
+  })
+
+  it('still refuses a path that no row with that id holds', () => {
+    expect(() =>
+      assertRegisteredGitHubRepo(
+        { repoPath: '/elsewhere', repoId: 'twin' },
+        storeWith([localTwin, sshTwin])
+      )
+    ).toThrow('repository path does not match repo id')
+  })
+})

src/main/ipc/github-repo-routing.ts

@@ -26,54 +26,101 @@ type RegisteredRepoValidationResult =
   | { kind: 'ok'; repo: Repo }
   | { kind: 'denied'; reason: PRRefreshValidationDenialReason; message: string }
 
+/** A registered-repo lookup: the caller's path and id, plus the host it believes owns the row. */
+type RegisteredRepoQuery = {
+  repoPath: string
+  repoId?: string | null
+  /** Picks between twins (one id on two hosts); a lone row's own fields still win (they may be stale). */
+  hostHint?: Parameters<typeof getRepoExecutionHostId>[0] | null
+  sourceContext?: TaskSourceContext | null
+}
+
 function validateRegisteredRepo(
-  args: string | GitHubRepoScopedArgs,
-  store: Store,
-  repos = store.getRepos()
+  query: RegisteredRepoQuery,
+  repos: readonly Repo[]
 ): RegisteredRepoValidationResult {
-  const repoPath = typeof args === 'string' ? args : args.repoPath
-  const repoId = typeof args === 'string' ? undefined : args.repoId
-  const resolvedRepoPath = resolve(repoPath)
-  const repo = repos.find((candidate) =>
-    repoId ? candidate.id === repoId : resolve(candidate.path) === resolvedRepoPath
-  )
-  if (!repo) {
+  const { repoId, sourceContext } = query
+  const resolvedRepoPath = resolve(query.repoPath)
+  const idRows = repoId ? repos.filter((candidate) => candidate.id === repoId) : []
+  if (repoId && idRows.length === 0) {
     return {
       kind: 'denied',
       reason: 'unknown-repo',
       message: 'Access denied: unknown repository path'
     }
   }
-  if (repoId && resolve(repo.path) !== resolvedRepoPath) {
-    return {
-      kind: 'denied',
-      reason: 'repo-path-mismatch',
-      message: 'Access denied: repository path does not match repo id'
-    }
+  // Why id and path together: a twin's rows share the id, so the first id match can be the other host.
+  const pathRows = (repoId ? idRows : repos).filter(
+    (candidate) => resolve(candidate.path) === resolvedRepoPath
+  )
+  if (pathRows.length === 0) {
+    return repoId
+      ? {
+          kind: 'denied',
+          reason: 'repo-path-mismatch',
+          message: 'Access denied: repository path does not match repo id'
+        }
+      : {
+          kind: 'denied',
+          reason: 'unknown-repo',
+          message: 'Access denied: unknown repository path'
+        }
   }
-  if (
-    typeof args !== 'string' &&
-    args.sourceContext?.provider === 'github' &&
-    args.sourceContext.hostId !== getRepoExecutionHostId(repo)
-  ) {
-    return {
-      kind: 'denied',
-      reason: 'host-mismatch',
-      message: 'Access denied: GitHub source host does not match repository host'
-    }
+  if (sourceContext?.provider === 'github') {
+    const repo = pathRows.find(
+      (candidate) => getRepoExecutionHostId(candidate) === sourceContext.hostId
+    )
+    return repo
+      ? { kind: 'ok', repo }
+      : {
+          kind: 'denied',
+          reason: 'host-mismatch',
+          message: 'Access denied: GitHub source host does not match repository host'
+        }
   }
+  const hintHostId = query.hostHint ? getRepoExecutionHostId(query.hostHint) : null
+  const repo =
+    (hintHostId
+      ? pathRows.find((candidate) => getRepoExecutionHostId(candidate) === hintHostId)
+      : undefined) ?? pathRows[0]
   return { kind: 'ok', repo }
 }
 
+function registeredRepoQuery(args: string | GitHubRepoScopedArgs): RegisteredRepoQuery {
+  return typeof args === 'string'
+    ? { repoPath: args }
+    : { repoPath: args.repoPath, repoId: args.repoId, sourceContext: args.sourceContext }
+}
+
+function prRefreshCandidateQuery(candidate: GitHubPRRefreshCandidate): RegisteredRepoQuery {
+  return {
+    repoPath: candidate.repoPath,
+    repoId: candidate.repoId,
+    hostHint: { connectionId: candidate.connectionId, executionHostId: candidate.executionHostId }
+  }
+}
+
+function assertRegisteredRepo(query: RegisteredRepoQuery, store: Store): Repo {
+  const result = validateRegisteredRepo(query, store.getRepos())
+  if (result.kind === 'denied') {
+    throw new Error(result.message)
+  }
+  return result.repo
+}
+
 export function assertRegisteredGitHubRepo(
   args: string | GitHubRepoScopedArgs,
   store: Store
 ): Repo {
-  const result = validateRegisteredRepo(args, store)
-  if (result.kind === 'denied') {
-    throw new Error(result.message)
-  }
-  return result.repo
+  return assertRegisteredRepo(registeredRepoQuery(args), store)
+}
+
+/** A manual PR refresh resolves its twin the way the automatic queue does. */
+export function assertRegisteredPRRefreshCandidateRepo(
+  candidate: GitHubPRRefreshCandidate,
+  store: Store
+): Repo {
+  return assertRegisteredRepo(prRefreshCandidateQuery(candidate), store)
 }
 
 /** Pure: a project is still registered at the candidate's path on its execution host (the queue key's identity). */
@@ -129,7 +176,7 @@ export function validateAutomaticPRRefreshCandidate(
 ):
   | { kind: 'ok'; candidate: GitHubPRRefreshCandidate }
   | { kind: 'skipped'; result: Extract<GitHubPRRefreshEnqueueResult, { kind: 'skipped' }> } {
-  const result = validateRegisteredRepo(candidate, store, repos)
+  const result = validateRegisteredRepo(prRefreshCandidateQuery(candidate), repos)
   if (result.kind === 'denied') {
     const skippedReason = notePRRefreshValidationDenial({
       repoId: candidate.repoId,