fix(git): skip upstream remote probes when the remote is absent (#18455)

* fix(git): skip upstream remote probes when the remote is absent

Issue and PR resolvers listed remotes by probing `git remote get-url
upstream` on every poll, including origin-only clones where that remote
cannot exist. List remotes once, cache against git config, and skip the
probe unless `upstream` is present.

* fix(git): avoid stale remote probe cache entries

* fix(github): observe origin repository probe failures

* fix(github): observe verified origin probe failures

* fix(github): skip missing upstream probe for PR lists

* test(github): scope the #9171 lazy-resolution guard to default-branch commands

The guard asserted that no git command runs for an open PR, using "no git at
all" as a proxy for "no default-branch resolution". Remote-name listing is a
separate concern, so allow it and keep every other command forbidden; the
symbolic-ref/rev-parse resolution this issue is about stays unreachable.

---------

Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
Wooseong Kim
2026-09-13 20:18:22 -07:00
committed by GitHub
co-authored by Neil
parent 16d1ab81d3
commit 31db2774f8
13 changed files with 1016 additions and 241 deletions
+200
View File
@@ -0,0 +1,200 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
gitExecFileAsyncMock,
getSshGitProviderMock,
getSshGitProviderGenerationMock,
readLocalGitConfigSignatureMock
} = vi.hoisted(() => ({
gitExecFileAsyncMock: vi.fn(),
getSshGitProviderMock: vi.fn(),
getSshGitProviderGenerationMock: vi.fn(() => 0),
readLocalGitConfigSignatureMock: vi.fn<() => Promise<string | undefined>>(async () => 'sig-1')
}))
vi.mock('./runner', () => ({ gitExecFileAsync: gitExecFileAsyncMock }))
vi.mock('../providers/ssh-git-dispatch', () => ({
getSshGitProvider: getSshGitProviderMock,
getSshGitProviderGeneration: getSshGitProviderGenerationMock
}))
vi.mock('../github/local-git-config-signature', () => ({
readLocalGitConfigSignature: readLocalGitConfigSignatureMock
}))
import { REMOTE_URL_PROBE_TIMEOUT_MS } from './remote-url-probe'
import {
_resetRemoteNameListingCache,
listCachedRemoteNames,
shouldProbeGitRemote
} from './remote-name-listing'
function remoteListCalls(): unknown[][] {
return gitExecFileAsyncMock.mock.calls.filter(
([args]) => Array.isArray(args) && args[0] === 'remote' && args[1] !== 'get-url'
)
}
describe('cached git remote name listing', () => {
beforeEach(() => {
_resetRemoteNameListingCache()
gitExecFileAsyncMock.mockReset()
getSshGitProviderMock.mockReset()
getSshGitProviderGenerationMock.mockReset()
getSshGitProviderGenerationMock.mockReturnValue(0)
readLocalGitConfigSignatureMock.mockReset()
readLocalGitConfigSignatureMock.mockImplementation(async () => 'sig-1')
})
it('skips probing upstream when listing only has origin', async () => {
gitExecFileAsyncMock.mockResolvedValue({ stdout: 'origin\n' })
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(false)
await expect(listCachedRemoteNames('/repo')).resolves.toEqual(['origin'])
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1)
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote'], {
cwd: '/repo',
timeout: REMOTE_URL_PROBE_TIMEOUT_MS
})
})
it('still probes upstream when listing includes that remote', async () => {
gitExecFileAsyncMock.mockResolvedValue({ stdout: 'origin\nupstream\n' })
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(true)
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1)
})
it('reuses a signed listing instead of spawning git remote again', async () => {
gitExecFileAsyncMock.mockResolvedValue({ stdout: 'origin\n' })
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(false)
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(false)
await expect(shouldProbeGitRemote('/repo', 'origin')).resolves.toBe(true)
expect(remoteListCalls()).toHaveLength(1)
})
it('re-lists as soon as the git config signature changes', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'origin\n' })
.mockResolvedValueOnce({ stdout: 'origin\nupstream\n' })
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(false)
readLocalGitConfigSignatureMock.mockImplementation(async () => 'sig-2')
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(true)
expect(remoteListCalls()).toHaveLength(2)
})
it('uses the short TTL when config changes during remote listing', async () => {
vi.useFakeTimers()
try {
readLocalGitConfigSignatureMock
.mockResolvedValueOnce('sig-1')
.mockResolvedValueOnce('sig-2')
.mockResolvedValue('sig-2')
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'origin\n' })
.mockResolvedValueOnce({ stdout: 'origin\nupstream\n' })
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(false)
await vi.advanceTimersByTimeAsync(30_001)
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(true)
expect(remoteListCalls()).toHaveLength(2)
} finally {
vi.useRealTimers()
}
})
it('expires an unsigned listing after the short TTL', async () => {
vi.useFakeTimers()
try {
readLocalGitConfigSignatureMock.mockImplementation(async () => undefined)
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'origin\n' })
.mockResolvedValueOnce({ stdout: 'origin\nupstream\n' })
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(false)
await vi.advanceTimersByTimeAsync(30_001)
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(true)
expect(remoteListCalls()).toHaveLength(2)
} finally {
vi.useRealTimers()
}
})
it('holds a signed listing past the unsigned TTL', async () => {
vi.useFakeTimers()
try {
gitExecFileAsyncMock.mockResolvedValue({ stdout: 'origin\n' })
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(false)
await vi.advanceTimersByTimeAsync(4 * 60_000)
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(false)
expect(remoteListCalls()).toHaveLength(1)
} finally {
vi.useRealTimers()
}
})
it('fails open and does not cache when listing throws', async () => {
gitExecFileAsyncMock
.mockRejectedValueOnce(new Error('git timed out.'))
.mockResolvedValueOnce({ stdout: 'origin\n' })
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(true)
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(false)
expect(remoteListCalls()).toHaveLength(2)
})
it('coalesces concurrent listings onto one spawn', async () => {
gitExecFileAsyncMock.mockImplementation(async () => {
await Promise.resolve()
return { stdout: 'origin\n' }
})
await expect(
Promise.all([
shouldProbeGitRemote('/repo', 'upstream'),
shouldProbeGitRemote('/repo', 'upstream'),
listCachedRemoteNames('/repo')
])
).resolves.toEqual([false, false, ['origin']])
expect(remoteListCalls()).toHaveLength(1)
})
it('keeps host and WSL listings separate', async () => {
gitExecFileAsyncMock.mockImplementation(
async (_args: string[], options: { wslDistro?: string } = {}) => ({
stdout: options.wslDistro ? 'origin\nupstream\n' : 'origin\n'
})
)
await expect(shouldProbeGitRemote('/repo', 'upstream')).resolves.toBe(false)
await expect(
shouldProbeGitRemote('/repo', 'upstream', null, { wslDistro: 'Ubuntu' })
).resolves.toBe(true)
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(2, ['remote'], {
cwd: '/repo',
timeout: REMOTE_URL_PROBE_TIMEOUT_MS,
wslDistro: 'Ubuntu'
})
})
it('lists remotes through the SSH git provider', async () => {
const exec = vi.fn(async () => ({ stdout: 'origin\n', stderr: '' }))
getSshGitProviderMock.mockReturnValue({ exec })
await expect(shouldProbeGitRemote('/remote/repo', 'upstream', 'ssh-1')).resolves.toBe(false)
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
expect(exec).toHaveBeenCalledWith(['remote'], '/remote/repo', {
signal: expect.any(AbortSignal)
})
})
it('fails open when the SSH git provider is missing instead of listing locally', async () => {
getSshGitProviderMock.mockReturnValue(undefined)
await expect(shouldProbeGitRemote('/remote/repo', 'upstream', 'ssh-1')).resolves.toBe(true)
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
})
})
+174
View File
@@ -0,0 +1,174 @@
import { readLocalGitConfigSignature } from '../github/local-git-config-signature'
import { getSshGitProvider, getSshGitProviderGeneration } from '../providers/ssh-git-dispatch'
import { runCoalescedProbe, type CoalescedProbes } from './coalesced-probe'
import type { GitAdmissionTier } from './command-runner/git-exec-options'
import { REMOTE_URL_PROBE_TIMEOUT_MS } from './remote-url-probe'
import { gitExecFileAsync } from './runner'
export type RemoteNameListingGitOptions = {
wslDistro?: string
admissionTier?: GitAdmissionTier
}
const SIGNED_REMOTE_NAME_LISTING_TTL_MS = 5 * 60_000
const UNSIGNED_REMOTE_NAME_LISTING_TTL_MS = 30_000
const REMOTE_NAME_LISTING_CACHE_MAX_ENTRIES = 512
type CachedRemoteNames = {
remotes: string[]
expiresAt: number
configSignature?: string
}
const remoteNameListingCache = new Map<string, CachedRemoteNames>()
const remoteNameListingInFlight: CoalescedProbes<string[] | null> = new Map()
/** @internal - exposed for tests only */
export function _resetRemoteNameListingCache(): void {
remoteNameListingCache.clear()
remoteNameListingInFlight.clear()
}
function parseRemoteNames(stdout: string): string[] {
return stdout
.split(/\r?\n/)
.map((line) => line.trim())
.filter(Boolean)
}
function remoteNameListingCacheKey(
repoPath: string,
connectionId?: string | null,
localGitOptions: RemoteNameListingGitOptions = {}
): string {
const runtimeKey = connectionId
? `ssh:${connectionId}:${getSshGitProviderGeneration(connectionId)}`
: `local:${localGitOptions.wslDistro ?? 'host'}`
return `${runtimeKey}\0${repoPath}`
}
function pruneRemoteNameListingCache(now: number): void {
for (const [key, entry] of remoteNameListingCache) {
if (entry.expiresAt <= now) {
remoteNameListingCache.delete(key)
}
}
while (remoteNameListingCache.size > REMOTE_NAME_LISTING_CACHE_MAX_ENTRIES) {
const oldestKey = remoteNameListingCache.keys().next().value
if (oldestKey === undefined) {
return
}
remoteNameListingCache.delete(oldestKey)
}
}
function listingGitConfigContext(
repoPath: string,
connectionId?: string | null,
localGitOptions: RemoteNameListingGitOptions = {}
): { repoPath: string; connectionId: string | null; wslDistro?: string } {
return {
repoPath,
connectionId: connectionId ?? null,
...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {})
}
}
/**
* `git remote` names for one repo/runtime. Failed listings are not cached: a
* missed `upstream` would otherwise send issue/PR resolvers to origin on a
* contributor clone (#7331).
*/
export async function listCachedRemoteNames(
repoPath: string,
connectionId?: string | null,
localGitOptions: RemoteNameListingGitOptions = {}
): Promise<string[] | null> {
const cacheKey = remoteNameListingCacheKey(repoPath, connectionId, localGitOptions)
const now = Date.now()
pruneRemoteNameListingCache(now)
const cached = remoteNameListingCache.get(cacheKey)
if (cached && cached.expiresAt > now) {
if (cached.configSignature !== undefined) {
const currentSignature = await readLocalGitConfigSignature(
listingGitConfigContext(repoPath, connectionId, localGitOptions)
)
if (currentSignature === cached.configSignature) {
return cached.remotes
}
remoteNameListingCache.delete(cacheKey)
} else {
return cached.remotes
}
}
return runCoalescedProbe(remoteNameListingInFlight, cacheKey, async (ownsKey) => {
const configContext = listingGitConfigContext(repoPath, connectionId, localGitOptions)
const configSignatureBefore = await readLocalGitConfigSignature(configContext)
const remotes = await listUncachedRemoteNames(repoPath, connectionId, localGitOptions)
if (remotes === null) {
return null
}
if (ownsKey()) {
const configSignatureAfter = await readLocalGitConfigSignature(configContext)
const configSignature =
configSignatureBefore !== undefined && configSignatureBefore === configSignatureAfter
? configSignatureAfter
: undefined
remoteNameListingCache.set(cacheKey, {
remotes,
expiresAt:
Date.now() +
(configSignature
? SIGNED_REMOTE_NAME_LISTING_TTL_MS
: UNSIGNED_REMOTE_NAME_LISTING_TTL_MS),
...(configSignature ? { configSignature } : {})
})
pruneRemoteNameListingCache(Date.now())
}
return remotes
})
}
async function listUncachedRemoteNames(
repoPath: string,
connectionId?: string | null,
localGitOptions: RemoteNameListingGitOptions = {}
): Promise<string[] | null> {
if (connectionId) {
const provider = getSshGitProvider(connectionId)
if (!provider) {
return null
}
try {
const { stdout } = await provider.exec(['remote'], repoPath, {
signal: AbortSignal.timeout(REMOTE_URL_PROBE_TIMEOUT_MS)
})
return parseRemoteNames(stdout)
} catch {
return null
}
}
try {
const { stdout } = await gitExecFileAsync(['remote'], {
cwd: repoPath,
timeout: REMOTE_URL_PROBE_TIMEOUT_MS,
...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}),
...(localGitOptions.admissionTier ? { admissionTier: localGitOptions.admissionTier } : {})
})
return parseRemoteNames(stdout)
} catch {
return null
}
}
/** Probe a named remote only when listing says it exists, or listing failed. */
export async function shouldProbeGitRemote(
repoPath: string,
remoteName: string,
connectionId?: string | null,
localGitOptions: RemoteNameListingGitOptions = {}
): Promise<boolean> {
const remotes = await listCachedRemoteNames(repoPath, connectionId, localGitOptions)
return remotes === null || remotes.includes(remoteName)
}
+27
View File
@@ -116,6 +116,7 @@ import {
_resetOwnerRepoCache
} from './client'
import { GITHUB_WORK_ITEMS_QUERY_MAX_BYTES } from '../../shared/github/work-items-query-bounds'
import { _resetRemoteNameListingCache } from '../git/remote-name-listing'
import { _resetOriginGitHubApiRepositoryCache } from './github-api-repository'
@@ -156,6 +157,7 @@ describe('listWorkItems', () => {
remoteName === 'origin' ? getOwnerRepoMock(repoPath, connectionId, opts) : null
)
_resetOwnerRepoCache()
_resetRemoteNameListingCache()
_resetMergeQueueCacheForTests()
})
@@ -383,6 +385,31 @@ describe('listWorkItems', () => {
)
})
it('skips upstream PR source probing when the clone only has origin', async () => {
getIssueOwnerRepoMock.mockResolvedValue(null)
getOwnerRepoMock.mockResolvedValue({ owner: 'fork', repo: 'orca' })
gitExecFileAsyncMock.mockResolvedValue({ stdout: 'origin\n' })
ghExecFileAsyncMock.mockResolvedValue({ stdout: '[]' })
await expect(listWorkItems('/origin-only-repo', 10, 'is:pr')).resolves.toMatchObject({
items: [],
sources: {
issues: null,
prs: { owner: 'fork', repo: 'orca' },
originCandidate: { owner: 'fork', repo: 'orca' },
upstreamCandidate: null
}
})
expect(getOwnerRepoForRemoteMock.mock.calls.map(([, remote]) => remote)).not.toContain(
'upstream'
)
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
['remote'],
expect.objectContaining({ cwd: '/origin-only-repo' })
)
})
it('rejects oversized queries before resolving repo sources or executing gh', async () => {
const secret = 'main-github-work-items-secret'
const oversizedQuery = secret + 'x'.repeat(GITHUB_WORK_ITEMS_QUERY_MAX_BYTES)
@@ -2,6 +2,7 @@ import type { ClassifiedError } from '../../../../shared/classified-error'
import type { IssueSourcePreference } from '../../../../shared/repo-types'
import type { ParsedTaskQuery } from '../../../../shared/task-query'
import { GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE } from '../../../../shared/work-items'
import { shouldProbeGitRemote } from '../../../git/remote-name-listing'
import type { LocalGitExecOptions, OwnerRepo } from '../../gh-utils'
import {
getGitHubApiRepositoryForRemote,
@@ -134,9 +135,27 @@ export async function resolvePrWorkItemSource(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<ResolvedPrWorkItemSource> {
const originCandidatePromise = getOriginGitHubApiRepository(
repoPath,
connectionId,
localGitOptions
)
// Why: PR list/count polling must not spawn a failing upstream lookup on
// origin-only clones, while still preserving upstream-first resolution when
// the remote is configured or remote discovery fails open.
const upstreamCandidatePromise = shouldProbeGitRemote(
repoPath,
'upstream',
connectionId,
localGitOptions
).then((shouldProbe) =>
shouldProbe
? getGitHubApiRepositoryForRemote(repoPath, 'upstream', connectionId, localGitOptions)
: null
)
const [originCandidate, upstreamCandidate] = await Promise.all([
getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions),
getGitHubApiRepositoryForRemote(repoPath, 'upstream', connectionId, localGitOptions)
originCandidatePromise,
upstreamCandidatePromise
])
// Why: fork-contribution PRs live on the upstream repo (the fork's own PR
// list is almost always empty), so 'auto' resolves upstream-first exactly
@@ -278,7 +278,13 @@ describe('issue #9171: default-branch checkout must not attach a stale non-open
expect(pr?.number).toBe(8)
expect(pr?.state).toBe('open')
// Open results never consult git for the default branch (lazy resolution).
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
// Remote-name listing is a separate concern from default-branch resolution,
// so allow it and keep every other git command forbidden here.
expect(
gitExecFileAsyncMock.mock.calls
.map(([args]) => args[0])
.filter((command) => command !== 'remote')
).toEqual([])
})
it('keeps a CLOSED PR on a feature branch visible (behavior preserved)', async () => {
+114 -53
View File
@@ -21,6 +21,7 @@ vi.mock('../providers/ssh-git-dispatch', () => ({
getSshGitProvider: getSshGitProviderMock
}))
import { _resetRemoteNameListingCache } from '../git/remote-name-listing'
import {
_getOwnerRepoCacheSize,
_resetOwnerRepoCache,
@@ -40,6 +41,35 @@ import {
} from './local-git-config-signature'
import { GITHUB_SEARCH_RESULT_WINDOW_ERROR_PATTERN } from '../../shared/github/work-items-query-bounds'
function mockGitRemoteCommands(remotes: Record<string, string>): void {
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
if (args[0] === 'remote' && args[1] !== 'get-url') {
return { stdout: `${Object.keys(remotes).join('\n')}\n` }
}
if (args[0] === 'remote' && args[1] === 'get-url') {
const url = remotes[args[2] ?? '']
if (!url) {
throw new Error(`fatal: No such remote '${args[2]}'`)
}
return { stdout: url }
}
throw new Error(`unexpected git ${args.join(' ')}`)
})
}
function gitRemoteGetUrlCalls(remoteName: string): unknown[][] {
return gitExecFileAsyncMock.mock.calls.filter(
([args]) =>
Array.isArray(args) && args[0] === 'remote' && args[1] === 'get-url' && args[2] === remoteName
)
}
function gitRemoteListCalls(): unknown[][] {
return gitExecFileAsyncMock.mock.calls.filter(
([args]) => Array.isArray(args) && args[0] === 'remote' && args[1] !== 'get-url'
)
}
describe('github owner/repo resolution', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
@@ -47,6 +77,7 @@ describe('github owner/repo resolution', () => {
getSshGitProviderGenerationMock.mockReturnValue(0)
getSshGitProviderMock.mockReset()
_resetOwnerRepoCache()
_resetRemoteNameListingCache()
__resetLocalGitConfigSignatureCacheForTests()
})
@@ -97,57 +128,46 @@ describe('github owner/repo resolution', () => {
})
it('prefers upstream for PR owner/repo resolution (#7331)', async () => {
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'git@github.com:stablyai/orca.git\n'
mockGitRemoteCommands({
origin: 'git@github.com:fork/orca.git\n',
upstream: 'git@github.com:stablyai/orca.git\n'
})
await expect(getOwnerRepo('/repo')).resolves.toEqual({ owner: 'stablyai', repo: 'orca' })
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', 'get-url', 'upstream'], {
cwd: '/repo',
timeout: 30_000
})
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(1)
})
it('resolves GitHub HTTPS origin remotes with user info and a default port', async () => {
gitExecFileAsyncMock
.mockRejectedValueOnce(new Error("fatal: No such remote 'upstream'"))
.mockResolvedValueOnce({
stdout: 'https://alice@github.com:443/acme/widgets.git\n'
})
it('does not spawn git remote get-url upstream on an origin-only clone', async () => {
mockGitRemoteCommands({
origin: 'https://alice@github.com:443/acme/widgets.git\n'
})
await expect(getOwnerRepo('/repo')).resolves.toEqual({ owner: 'acme', repo: 'widgets' })
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', 'get-url', 'origin'], {
cwd: '/repo',
timeout: 30_000
})
await expect(getOwnerRepo('/repo')).resolves.toEqual({ owner: 'acme', repo: 'widgets' })
expect(gitRemoteListCalls()).toHaveLength(1)
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(0)
expect(gitRemoteGetUrlCalls('origin')).toHaveLength(1)
})
it('prefers upstream for issue owner/repo resolution', async () => {
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'git@github.com:stablyai/orca.git\n'
mockGitRemoteCommands({
origin: 'git@github.com:fork/orca.git\n',
upstream: 'git@github.com:stablyai/orca.git\n'
})
await expect(getIssueOwnerRepo('/repo')).resolves.toEqual({ owner: 'stablyai', repo: 'orca' })
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', 'get-url', 'upstream'], {
cwd: '/repo',
timeout: 30_000
})
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(1)
})
it('falls back to origin when upstream is missing or non-GitHub', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'git@example.com:stablyai/orca.git\n' })
.mockResolvedValueOnce({ stdout: 'git@github.com:fork/orca.git\n' })
it('falls back to origin when upstream is present but non-GitHub', async () => {
mockGitRemoteCommands({
origin: 'git@github.com:fork/orca.git\n',
upstream: 'git@example.com:stablyai/orca.git\n'
})
await expect(getIssueOwnerRepo('/repo')).resolves.toEqual({ owner: 'fork', repo: 'orca' })
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(1, ['remote', 'get-url', 'upstream'], {
cwd: '/repo',
timeout: 30_000
})
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(2, ['remote', 'get-url', 'origin'], {
cwd: '/repo',
timeout: 30_000
})
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(1)
expect(gitRemoteGetUrlCalls('origin')).toHaveLength(1)
})
it('does not mix origin and upstream cache entries for the same repo path', async () => {
@@ -193,6 +213,9 @@ describe('github owner/repo resolution', () => {
it('resolves SSH repo remotes through the registered SSH git provider', async () => {
const sshProvider = {
exec: vi.fn(async (args: string[]) => {
if (args[0] === 'remote' && args[1] !== 'get-url') {
return { stdout: 'origin\n', stderr: '' }
}
if (args[2] === 'upstream') {
throw new Error("fatal: No such remote 'upstream'")
}
@@ -219,9 +242,14 @@ describe('github owner/repo resolution', () => {
it('keeps local and SSH owner/repo cache entries separate for the same path', async () => {
const sshProvider = {
exec: vi.fn().mockResolvedValue({ stdout: 'git@github.com:remote/orca.git\n', stderr: '' })
exec: vi.fn(async (args: string[]) => {
if (args[0] === 'remote' && args[1] !== 'get-url') {
return { stdout: 'origin\n', stderr: '' }
}
return { stdout: 'git@github.com:remote/orca.git\n', stderr: '' }
})
}
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'git@github.com:local/orca.git\n' })
mockGitRemoteCommands({ origin: 'git@github.com:local/orca.git\n' })
getSshGitProviderMock.mockReturnValue(sshProvider)
await expect(getOwnerRepo('/repo')).resolves.toEqual({ owner: 'local', repo: 'orca' })
@@ -231,6 +259,9 @@ describe('github owner/repo resolution', () => {
it('keeps local host and local WSL owner/repo cache entries separate for the same path', async () => {
gitExecFileAsyncMock.mockImplementation(
async (args: string[], options: { wslDistro?: string } = {}) => {
if (args[0] === 'remote' && args[1] !== 'get-url') {
return { stdout: 'origin\n' }
}
if (args[2] === 'upstream') {
throw new Error("fatal: No such remote 'upstream'")
}
@@ -252,8 +283,9 @@ describe('github owner/repo resolution', () => {
repo: 'orca'
})
// 2 runtimes x (1 upstream miss + 1 origin hit); repeat WSL call is cached.
// 2 runtimes x (1 remote list + 1 origin hit); repeat WSL call is cached.
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(4)
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(0)
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', 'get-url', 'origin'], {
cwd: '/repo',
timeout: 30_000
@@ -272,14 +304,20 @@ describe('github owner/repo resolution', () => {
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'git@github.com:stablyai/orca.git\n'
})
await expect(getOwnerRepo('/repo-a')).resolves.toEqual({ owner: 'stablyai', repo: 'orca' })
await expect(getOwnerRepoForRemote('/repo-a', 'origin')).resolves.toEqual({
owner: 'stablyai',
repo: 'orca'
})
expect(_getOwnerRepoCacheSize()).toBe(1)
nowSpy.mockReturnValue(32_000)
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'git@github.com:acme/widgets.git\n'
})
await expect(getOwnerRepo('/repo-b')).resolves.toEqual({ owner: 'acme', repo: 'widgets' })
await expect(getOwnerRepoForRemote('/repo-b', 'origin')).resolves.toEqual({
owner: 'acme',
repo: 'widgets'
})
expect(_getOwnerRepoCacheSize()).toBe(1)
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2)
@@ -289,20 +327,38 @@ describe('github owner/repo resolution', () => {
})
it('resolves PR candidates as upstream then origin and de-dupes matching slugs', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'git@github.com:Acme/Orca.git\n' })
.mockResolvedValueOnce({ stdout: 'git@github.com:acme/orca.git\n' })
mockGitRemoteCommands({
origin: 'git@github.com:acme/orca.git\n',
upstream: 'git@github.com:Acme/Orca.git\n'
})
await expect(resolvePRRepositoryCandidates('/repo')).resolves.toEqual({
candidates: [{ owner: 'Acme', repo: 'Orca' }],
headRepo: { owner: 'acme', repo: 'orca' }
})
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(1)
})
it('does not spawn git remote get-url upstream for origin-only PR candidates', async () => {
mockGitRemoteCommands({ origin: 'git@github.com:fork/orca.git\n' })
await expect(resolvePRRepositoryCandidates('/repo')).resolves.toEqual({
candidates: [{ owner: 'fork', repo: 'orca' }],
headRepo: { owner: 'fork', repo: 'orca' }
})
await expect(resolvePRRepositoryCandidates('/repo')).resolves.toEqual({
candidates: [{ owner: 'fork', repo: 'orca' }],
headRepo: { owner: 'fork', repo: 'orca' }
})
expect(gitRemoteListCalls()).toHaveLength(1)
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(0)
})
it('ignores non-GitHub upstream while keeping origin as the head repo', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'git@example.com:Acme/Orca.git\n' })
.mockResolvedValueOnce({ stdout: 'git@github.com:fork/orca.git\n' })
mockGitRemoteCommands({
origin: 'git@github.com:fork/orca.git\n',
upstream: 'git@example.com:Acme/Orca.git\n'
})
await expect(resolvePRRepositoryCandidates('/repo')).resolves.toEqual({
candidates: [{ owner: 'fork', repo: 'orca' }],
@@ -703,23 +759,27 @@ describe('resolveIssueSource', () => {
gitExecFileAsyncMock.mockReset()
getSshGitProviderMock.mockReset()
_resetOwnerRepoCache()
_resetRemoteNameListingCache()
})
it("'auto' + upstream exists → upstream, fellBack=false", async () => {
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'git@github.com:stablyai/orca.git\n'
mockGitRemoteCommands({
origin: 'git@github.com:fork/orca.git\n',
upstream: 'git@github.com:stablyai/orca.git\n'
})
await expect(resolveIssueSource('/repo', 'auto')).resolves.toEqual({
source: { owner: 'stablyai', repo: 'orca' },
fellBack: false
})
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(1)
})
it("'auto' + no upstream → origin, fellBack=false", async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'git@example.com:stablyai/orca.git\n' })
.mockResolvedValueOnce({ stdout: 'git@github.com:solo/orca.git\n' })
it("'auto' + no github upstream → origin, fellBack=false", async () => {
mockGitRemoteCommands({
origin: 'git@github.com:solo/orca.git\n',
upstream: 'git@example.com:stablyai/orca.git\n'
})
await expect(resolveIssueSource('/repo', 'auto')).resolves.toEqual({
source: { owner: 'solo', repo: 'orca' },
@@ -779,8 +839,9 @@ describe('resolveIssueSource', () => {
})
it('undefined preference is treated identically to auto', async () => {
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'git@github.com:stablyai/orca.git\n'
mockGitRemoteCommands({
origin: 'git@github.com:fork/orca.git\n',
upstream: 'git@github.com:stablyai/orca.git\n'
})
await expect(resolveIssueSource('/repo', undefined)).resolves.toEqual({
@@ -0,0 +1,132 @@
import type { GitHubApiRepository } from './github-api-repository'
import {
getOwnerRepoForRemote,
type GitHubRemoteIdentityProbeOptions,
type LocalGitExecOptions
} from './gh-utils'
import {
getEnterpriseGitHubRepoSlug,
getEnterpriseGitHubRepoSlugForRemote
} from './github-enterprise-repository'
import {
githubApiRepositoryProbeCacheKey,
resolveGitHubApiRepositoryProbe
} from './github-api-repository-probe'
// Why: cache the uncached Enterprise remote probe used by hot paths.
const ORIGIN_REPO_CACHE_TTL_MS = 30_000
const ORIGIN_REPO_CACHE_MAX_ENTRIES = 512
const originRepoCache = new Map<string, { value: GitHubApiRepository | null; expiresAt: number }>()
const originRepoInFlight = new Map<string, Promise<GitHubApiRepository | null>>()
/** @internal - exposed for tests only */
export function _resetOriginGitHubApiRepositoryCache(): void {
originRepoCache.clear()
originRepoInFlight.clear()
}
function pruneOriginRepoCache(now: number): void {
for (const [key, entry] of originRepoCache) {
if (entry.expiresAt <= now) {
originRepoCache.delete(key)
}
}
while (originRepoCache.size > ORIGIN_REPO_CACHE_MAX_ENTRIES) {
const oldestKey = originRepoCache.keys().next().value
if (oldestKey === undefined) {
return
}
originRepoCache.delete(oldestKey)
}
}
/**
* Host-qualified repository identity for one remote: github.com remotes come
* from the cached slug parser; any other GitHub-shaped host is auth-gated so a
* non-GitHub forge never routes to the GitHub provider.
*/
export async function getGitHubApiRepositoryForRemote(
repoPath: string,
remoteName: string,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {},
probeOptions: GitHubRemoteIdentityProbeOptions = {}
): Promise<GitHubApiRepository | null> {
// Why: generic PR resolution prefers upstream, but this API represents the
// caller-selected remote exactly (#7331).
const requireVerifiedSshProbe = probeOptions.requireVerifiedSshProbe === true
const verifiedIdentityArgs = requireVerifiedSshProbe ? ([probeOptions] as const) : []
const ownerRepo = await getOwnerRepoForRemote(
repoPath,
remoteName,
connectionId,
localGitOptions,
...verifiedIdentityArgs
)
if (ownerRepo) {
return { ...ownerRepo, host: 'github.com' }
}
const cacheKey = githubApiRepositoryProbeCacheKey(
repoPath,
remoteName,
connectionId,
localGitOptions,
requireVerifiedSshProbe
)
const now = Date.now()
pruneOriginRepoCache(now)
const cached = originRepoCache.get(cacheKey)
if (cached && cached.expiresAt > now) {
return cached.value
}
const inFlight = originRepoInFlight.get(cacheKey)
if (inFlight) {
return inFlight
}
const probe = (async () => {
const enterpriseOptions =
Object.keys(localGitOptions).length > 0 ? { localGitExecOptions: localGitOptions } : {}
const verifiedEnterpriseArgs = requireVerifiedSshProbe ? ([true] as const) : []
const slug =
remoteName === 'origin'
? await getEnterpriseGitHubRepoSlug(
repoPath,
connectionId,
enterpriseOptions,
...verifiedEnterpriseArgs
)
: await getEnterpriseGitHubRepoSlugForRemote(
repoPath,
remoteName,
connectionId,
enterpriseOptions,
...verifiedEnterpriseArgs
)
// Why: undefined means the gh auth inventory could not be read. Caching it
// as a negative would turn a transient spawn failure into a 30-second miss.
if (slug !== undefined) {
originRepoCache.set(cacheKey, {
value: slug,
expiresAt: Date.now() + ORIGIN_REPO_CACHE_TTL_MS
})
pruneOriginRepoCache(Date.now())
}
return resolveGitHubApiRepositoryProbe(slug, requireVerifiedSshProbe)
})()
originRepoInFlight.set(cacheKey, probe)
try {
return await probe
} finally {
if (originRepoInFlight.get(cacheKey) === probe) {
originRepoInFlight.delete(cacheKey)
}
}
}
export async function getOriginGitHubApiRepository(
repoPath: string,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubApiRepository | null> {
return getGitHubApiRepositoryForRemote(repoPath, 'origin', connectionId, localGitOptions)
}
+154 -2
View File
@@ -8,13 +8,15 @@ const {
getOwnerRepoMock,
getOwnerRepoForRemoteMock,
getSshGitProviderGenerationMock,
isGitHubHostAuthenticatedMock
isGitHubHostAuthenticatedMock,
shouldProbeGitRemoteMock
} = vi.hoisted(() => ({
getEnterpriseGitHubRepoSlugMock: vi.fn(),
getOwnerRepoMock: vi.fn(),
getOwnerRepoForRemoteMock: vi.fn(),
getSshGitProviderGenerationMock: vi.fn(() => 0),
isGitHubHostAuthenticatedMock: vi.fn()
isGitHubHostAuthenticatedMock: vi.fn(),
shouldProbeGitRemoteMock: vi.fn(async () => true)
}))
vi.mock('../providers/ssh-git-dispatch', async (importOriginal) => ({
@@ -35,12 +37,18 @@ vi.mock('./github-enterprise-repository', async (importOriginal) => ({
isGitHubHostAuthenticated: isGitHubHostAuthenticatedMock
}))
vi.mock('../git/remote-name-listing', () => ({
shouldProbeGitRemote: shouldProbeGitRemoteMock
}))
import {
_resetOriginGitHubApiRepositoryCache,
getGitHubApiRepositoryForRemote,
getIssueGitHubApiRepository,
getOriginGitHubApiRepository,
githubHostExecOptions,
resolveGitHubApiRepository,
resolveGitHubApiRepositoryCandidates,
resolveGitHubRepoExecution
} from './github-api-repository'
@@ -51,6 +59,7 @@ beforeEach(() => {
getOwnerRepoForRemoteMock.mockReset().mockResolvedValue(null)
getSshGitProviderGenerationMock.mockReset().mockReturnValue(0)
isGitHubHostAuthenticatedMock.mockReset().mockResolvedValue(false)
shouldProbeGitRemoteMock.mockReset().mockResolvedValue(true)
})
describe('githubHostExecOptions', () => {
@@ -346,3 +355,146 @@ describe('origin repository cache', () => {
expect(getEnterpriseGitHubRepoSlugMock).toHaveBeenCalledTimes(2)
})
})
describe('skip missing upstream remote probes', () => {
it('starts the issue origin probe before checking whether upstream exists', async () => {
let releaseRemoteProbe: (value: boolean) => void = () => undefined
shouldProbeGitRemoteMock.mockReturnValue(
new Promise<boolean>((resolve) => {
releaseRemoteProbe = resolve
})
)
let originStarted = false
getOwnerRepoForRemoteMock.mockImplementation(async (_path, remote) => {
if (remote === 'origin') {
originStarted = true
return { owner: 'fork', repo: 'orca' }
}
return { owner: 'stablyai', repo: 'orca' }
})
const resultPromise = getIssueGitHubApiRepository('/repo')
expect(originStarted).toBe(true)
releaseRemoteProbe(true)
await expect(resultPromise).resolves.toEqual({
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
})
})
it('does not probe upstream for issue identity when that remote is absent', async () => {
shouldProbeGitRemoteMock.mockResolvedValue(false)
getOwnerRepoForRemoteMock.mockImplementation(async (_path, remote) =>
remote === 'origin' ? { owner: 'acme', repo: 'widgets' } : null
)
await expect(getIssueGitHubApiRepository('/repo')).resolves.toEqual({
owner: 'acme',
repo: 'widgets',
host: 'github.com'
})
expect(getOwnerRepoForRemoteMock.mock.calls.map(([, remote]) => remote)).toEqual(['origin'])
})
it('still probes upstream for issue identity when that remote is present', async () => {
getOwnerRepoForRemoteMock.mockImplementation(async (_path, remote) =>
remote === 'upstream' ? { owner: 'stablyai', repo: 'orca' } : { owner: 'fork', repo: 'orca' }
)
await expect(getIssueGitHubApiRepository('/repo')).resolves.toEqual({
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
})
expect(getOwnerRepoForRemoteMock).toHaveBeenCalledWith('/repo', 'upstream', undefined, {})
})
it('observes a rejected origin probe when upstream resolves the issue repository', async () => {
const originError = new Error('origin probe failed')
getOwnerRepoForRemoteMock.mockImplementation(async (_path, remote) => {
if (remote === 'origin') {
throw originError
}
return { owner: 'stablyai', repo: 'orca' }
})
await expect(getIssueGitHubApiRepository('/repo')).resolves.toEqual({
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
})
})
it('preserves a rejected origin probe when upstream cannot resolve the issue repository', async () => {
const originError = new Error('origin probe failed')
getOwnerRepoForRemoteMock.mockImplementation(async (_path, remote) => {
if (remote === 'origin') {
throw originError
}
return null
})
await expect(getIssueGitHubApiRepository('/repo')).rejects.toBe(originError)
})
it('does not probe upstream for PR candidates when that remote is absent', async () => {
shouldProbeGitRemoteMock.mockResolvedValue(false)
getOwnerRepoForRemoteMock.mockResolvedValue({ owner: 'fork', repo: 'orca' })
await expect(resolveGitHubApiRepositoryCandidates('/repo')).resolves.toEqual({
candidates: [{ owner: 'fork', repo: 'orca', host: 'github.com' }],
headRepo: { owner: 'fork', repo: 'orca', host: 'github.com' }
})
expect(getOwnerRepoForRemoteMock.mock.calls.map(([, remote]) => remote)).toEqual(['origin'])
})
it('observes and propagates a verified origin probe failure while listing remotes', async () => {
let releaseRemoteProbe: (value: boolean) => void = () => undefined
shouldProbeGitRemoteMock.mockReturnValue(
new Promise<boolean>((resolve) => {
releaseRemoteProbe = resolve
})
)
const originError = new Error('origin probe failed')
getOwnerRepoForRemoteMock.mockImplementation(async (_path, remote) => {
if (remote === 'origin') {
throw originError
}
return { owner: 'stablyai', repo: 'orca' }
})
const resultPromise = resolveGitHubApiRepositoryCandidates('/repo')
await vi.waitFor(() =>
expect(getOwnerRepoForRemoteMock).toHaveBeenCalledWith(
'/repo',
'origin',
undefined,
{},
{ requireVerifiedSshProbe: true }
)
)
releaseRemoteProbe(true)
await expect(resultPromise).rejects.toBe(originError)
})
it('still probes upstream for PR candidates when that remote is present', async () => {
getOwnerRepoForRemoteMock.mockImplementation(async (_path, remote) =>
remote === 'upstream' ? { owner: 'Acme', repo: 'Orca' } : { owner: 'acme', repo: 'orca' }
)
await expect(resolveGitHubApiRepositoryCandidates('/repo')).resolves.toEqual({
candidates: [{ owner: 'Acme', repo: 'Orca', host: 'github.com' }],
headRepo: { owner: 'acme', repo: 'orca', host: 'github.com' }
})
expect(getOwnerRepoForRemoteMock).toHaveBeenCalledWith(
'/repo',
'upstream',
undefined,
{},
{ requireVerifiedSshProbe: true }
)
})
})
+52 -141
View File
@@ -4,27 +4,19 @@ import {
githubRepoIdentityKey,
isDefaultGitHubHost
} from '../../shared/github/repository-identity-key'
import {
getOwnerRepoForRemote,
ghRepoExecOptions,
githubRepoContext,
type GitHubRemoteIdentityProbeOptions,
type LocalGitExecOptions
} from './gh-utils'
import {
getEnterpriseGitHubRepoSlug,
getEnterpriseGitHubRepoSlugForRemote,
isGitHubHostAuthenticated
} from './github-enterprise-repository'
import { shouldProbeGitRemote } from '../git/remote-name-listing'
import { ghRepoExecOptions, githubRepoContext, type LocalGitExecOptions } from './gh-utils'
import { isGitHubHostAuthenticated } from './github-enterprise-repository'
import { githubHostExecOptions } from './github-repository-host'
import {
isValidGitHubApiRepository,
type GitHubApiRepositoryResolution
} from './github-api-repository-validation'
import {
githubApiRepositoryProbeCacheKey,
resolveGitHubApiRepositoryProbe
} from './github-api-repository-probe'
_resetOriginGitHubApiRepositoryCache,
getGitHubApiRepositoryForRemote,
getOriginGitHubApiRepository
} from './github-api-repository-remote-probe'
export {
githubHostExecOptions,
@@ -40,123 +32,10 @@ export type GitHubRepoExecution = {
ownerRepo: GitHubApiRepository | null
ghOptions: GitHubRepoExecOptions
}
// Why: cache the uncached Enterprise remote probe used by hot paths.
const ORIGIN_REPO_CACHE_TTL_MS = 30_000
const ORIGIN_REPO_CACHE_MAX_ENTRIES = 512
const originRepoCache = new Map<string, { value: GitHubApiRepository | null; expiresAt: number }>()
const originRepoInFlight = new Map<string, Promise<GitHubApiRepository | null>>()
/** @internal - exposed for tests only */
export function _resetOriginGitHubApiRepositoryCache(): void {
originRepoCache.clear()
originRepoInFlight.clear()
}
function pruneOriginRepoCache(now: number): void {
for (const [key, entry] of originRepoCache) {
if (entry.expiresAt <= now) {
originRepoCache.delete(key)
}
}
while (originRepoCache.size > ORIGIN_REPO_CACHE_MAX_ENTRIES) {
const oldestKey = originRepoCache.keys().next().value
if (oldestKey === undefined) {
return
}
originRepoCache.delete(oldestKey)
}
}
/**
* Host-qualified repository identity for one remote: github.com remotes come
* from the cached slug parser; any other GitHub-shaped host is auth-gated so a
* non-GitHub forge never routes to the GitHub provider.
*/
export async function getGitHubApiRepositoryForRemote(
repoPath: string,
remoteName: string,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {},
probeOptions: GitHubRemoteIdentityProbeOptions = {}
): Promise<GitHubApiRepository | null> {
// Why: generic PR resolution prefers upstream, but this API represents the
// caller-selected remote exactly (#7331).
const requireVerifiedSshProbe = probeOptions.requireVerifiedSshProbe === true
const verifiedIdentityArgs = requireVerifiedSshProbe ? ([probeOptions] as const) : []
const ownerRepo = await getOwnerRepoForRemote(
repoPath,
remoteName,
connectionId,
localGitOptions,
...verifiedIdentityArgs
)
if (ownerRepo) {
return { ...ownerRepo, host: 'github.com' }
}
const cacheKey = githubApiRepositoryProbeCacheKey(
repoPath,
remoteName,
connectionId,
localGitOptions,
requireVerifiedSshProbe
)
const now = Date.now()
pruneOriginRepoCache(now)
const cached = originRepoCache.get(cacheKey)
if (cached && cached.expiresAt > now) {
return cached.value
}
const inFlight = originRepoInFlight.get(cacheKey)
if (inFlight) {
return inFlight
}
const probe = (async () => {
const enterpriseOptions =
Object.keys(localGitOptions).length > 0 ? { localGitExecOptions: localGitOptions } : {}
const verifiedEnterpriseArgs = requireVerifiedSshProbe ? ([true] as const) : []
const slug =
remoteName === 'origin'
? await getEnterpriseGitHubRepoSlug(
repoPath,
connectionId,
enterpriseOptions,
...verifiedEnterpriseArgs
)
: await getEnterpriseGitHubRepoSlugForRemote(
repoPath,
remoteName,
connectionId,
enterpriseOptions,
...verifiedEnterpriseArgs
)
// Why: undefined means the gh auth inventory could not be read. Caching it
// as a negative would turn a transient spawn failure into a 30-second miss.
if (slug !== undefined) {
originRepoCache.set(cacheKey, {
value: slug,
expiresAt: Date.now() + ORIGIN_REPO_CACHE_TTL_MS
})
pruneOriginRepoCache(Date.now())
}
return resolveGitHubApiRepositoryProbe(slug, requireVerifiedSshProbe)
})()
originRepoInFlight.set(cacheKey, probe)
try {
return await probe
} finally {
if (originRepoInFlight.get(cacheKey) === probe) {
originRepoInFlight.delete(cacheKey)
}
}
}
export async function getOriginGitHubApiRepository(
repoPath: string,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubApiRepository | null> {
return getGitHubApiRepositoryForRemote(repoPath, 'origin', connectionId, localGitOptions)
export {
_resetOriginGitHubApiRepositoryCache,
getGitHubApiRepositoryForRemote,
getOriginGitHubApiRepository
}
/** Hosted mirror of getIssueOwnerRepo: issues prefer `upstream` over `origin`. */
@@ -165,16 +44,26 @@ export async function getIssueGitHubApiRepository(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubApiRepository | null> {
const upstream = await getGitHubApiRepositoryForRemote(
const originPromise = getGitHubApiRepositoryForRemote(
repoPath,
'upstream',
'origin',
connectionId,
localGitOptions
).then(
(value) => ({ status: 'fulfilled' as const, value }),
(reason: unknown) => ({ status: 'rejected' as const, reason })
)
const upstream = (await shouldProbeGitRemote(repoPath, 'upstream', connectionId, localGitOptions))
? await getGitHubApiRepositoryForRemote(repoPath, 'upstream', connectionId, localGitOptions)
: null
if (upstream) {
return upstream
}
return getGitHubApiRepositoryForRemote(repoPath, 'origin', connectionId, localGitOptions)
const origin = await originPromise
if (origin.status === 'rejected') {
throw origin.reason
}
return origin.value
}
export type GitHubApiRepositoryCandidates = {
@@ -188,14 +77,36 @@ export async function resolveGitHubApiRepositoryCandidates(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubApiRepositoryCandidates> {
const [upstream, origin] = await Promise.all([
getGitHubApiRepositoryForRemote(repoPath, 'upstream', connectionId, localGitOptions, {
const originPromise = getGitHubApiRepositoryForRemote(
repoPath,
'origin',
connectionId,
localGitOptions,
{
requireVerifiedSshProbe: true
}),
getGitHubApiRepositoryForRemote(repoPath, 'origin', connectionId, localGitOptions, {
requireVerifiedSshProbe: true
})
}
).then(
(value) => ({ status: 'fulfilled' as const, value }),
(reason: unknown) => ({ status: 'rejected' as const, reason })
)
const probeUpstream = await shouldProbeGitRemote(
repoPath,
'upstream',
connectionId,
localGitOptions
)
const [upstream, originResult] = await Promise.all([
probeUpstream
? getGitHubApiRepositoryForRemote(repoPath, 'upstream', connectionId, localGitOptions, {
requireVerifiedSshProbe: true
})
: null,
originPromise
])
if (originResult.status === 'rejected') {
throw originResult.reason
}
const origin = originResult.value
const seen = new Set<string>()
const candidates: GitHubApiRepository[] = []
for (const candidate of [upstream, origin]) {
+24 -6
View File
@@ -1,5 +1,6 @@
import type { IssueSourcePreference } from '../../shared/repo-types'
import { githubRepoIdentityKey } from '../../shared/github/repository-identity-key'
import { shouldProbeGitRemote } from '../git/remote-name-listing'
import {
getOwnerRepoForRemote,
type LocalGitExecOptions,
@@ -12,11 +13,19 @@ export async function getOwnerRepo(
localGitOptions: LocalGitExecOptions = {}
): Promise<OwnerRepo | null> {
// Why: on a fork checkout PRs live on the upstream parent, not origin (#7331).
const upstream = await getOwnerRepoForRemote(repoPath, 'upstream', connectionId, localGitOptions)
if (upstream) {
return upstream
const originPromise = getOwnerRepoForRemote(repoPath, 'origin', connectionId, localGitOptions)
if (await shouldProbeGitRemote(repoPath, 'upstream', connectionId, localGitOptions)) {
const upstream = await getOwnerRepoForRemote(
repoPath,
'upstream',
connectionId,
localGitOptions
)
if (upstream) {
return upstream
}
}
return getOwnerRepoForRemote(repoPath, 'origin', connectionId, localGitOptions)
return originPromise
}
export const getIssueOwnerRepo = getOwnerRepo
@@ -31,9 +40,18 @@ export async function resolvePRRepositoryCandidates(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<PRRepositoryCandidates> {
const originPromise = getOwnerRepoForRemote(repoPath, 'origin', connectionId, localGitOptions)
const probeUpstream = await shouldProbeGitRemote(
repoPath,
'upstream',
connectionId,
localGitOptions
)
const [upstream, origin] = await Promise.all([
getOwnerRepoForRemote(repoPath, 'upstream', connectionId, localGitOptions),
getOwnerRepoForRemote(repoPath, 'origin', connectionId, localGitOptions)
probeUpstream
? getOwnerRepoForRemote(repoPath, 'upstream', connectionId, localGitOptions)
: null,
originPromise
])
const seen = new Set<string>()
const candidates: OwnerRepo[] = []
@@ -28,6 +28,7 @@ vi.mock('./local-git-config-signature', () => ({
readLocalGitConfigSignature: readLocalGitConfigSignatureMock
}))
import { _resetRemoteNameListingCache } from '../git/remote-name-listing'
import { getOwnerRepoForRemote, _resetOwnerRepoCache } from './github-repository-identity'
import { getOwnerRepo, getIssueOwnerRepo } from './github-owner-repo-selection'
import { getRepoUpstream } from './client'
@@ -53,13 +54,17 @@ const REMOTE_URLS_BY_REPO: Record<string, Record<string, string>> = {
beforeEach(() => {
_resetOwnerRepoCache()
_resetRemoteNameListingCache()
gitExecFileAsyncMock.mockReset()
ghExecFileAsyncMock.mockReset()
gitExecFileAsyncMock.mockImplementation(
async (args: string[], options: { cwd?: string } = {}) => {
// getRemoteUrlForRepo calls: ['remote', 'get-url', <remoteName>]
const configured = REMOTE_URLS_BY_REPO[options.cwd ?? ''] ?? {}
if (args[0] === 'remote' && args[1] !== 'get-url') {
return { stdout: `${Object.keys(configured).join('\n')}\n` }
}
const remoteName = args[2]
const url = REMOTE_URLS_BY_REPO[options.cwd ?? '']?.[remoteName]
const url = configured[remoteName]
if (!url) {
const err = new Error(`fatal: No such remote '${remoteName}'`) as Error & { code?: number }
err.code = 128
@@ -92,16 +97,22 @@ describe('issue #7331: fork PR owner/repo resolution', () => {
expect(prRepo).toEqual({ owner: 'stablyai', repo: 'orca' })
})
it('caches the missing-upstream probe so repeat lookups skip the git spawn', async () => {
it('skips git remote get-url upstream on origin-only clones and caches the listing', async () => {
await getOwnerRepo(NON_FORK_PATH)
const upstreamProbes = (): number =>
gitExecFileAsyncMock.mock.calls.filter(([args]) => args[2] === 'upstream').length
expect(upstreamProbes()).toBe(1)
const upstreamGetUrl = (): number =>
gitExecFileAsyncMock.mock.calls.filter(
([args]) => args[1] === 'get-url' && args[2] === 'upstream'
).length
const listCalls = (): number =>
gitExecFileAsyncMock.mock.calls.filter(
([args]) => args[0] === 'remote' && args[1] !== 'get-url'
).length
expect(upstreamGetUrl()).toBe(0)
expect(listCalls()).toBe(1)
await getOwnerRepo(NON_FORK_PATH)
// Second lookup within the negative-cache TTL must not respawn git for
// the missing upstream remote.
expect(upstreamProbes()).toBe(1)
expect(upstreamGetUrl()).toBe(0)
expect(listCalls()).toBe(1)
})
it('resolves the upstream parent for SSH-style remote URLs', async () => {
@@ -1,5 +1,6 @@
import { glabExecFileAsync } from '../git/runner'
import type { GitAdmissionTier } from '../git/command-runner/git-exec-options'
import { shouldProbeGitRemote } from '../git/remote-name-listing'
import { isTransientGitProbeError, readRemoteUrl } from '../git/remote-url-probe'
import { NEGATIVE_ENTRY_TTL_MS } from '../git/remote-ref-probe-cache'
import { getSshGitProviderGeneration } from '../providers/ssh-git-dispatch'
@@ -180,17 +181,26 @@ export async function getIssueProjectRef(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<ProjectRef | null> {
const upstream = await getProjectRefForRemote(
const originPromise = getProjectRefForRemote(
repoPath,
'upstream',
'origin',
knownHosts,
connectionId,
localGitOptions
)
return (
upstream ??
getProjectRefForRemote(repoPath, 'origin', knownHosts, connectionId, localGitOptions)
)
if (await shouldProbeGitRemote(repoPath, 'upstream', connectionId, localGitOptions)) {
const upstream = await getProjectRefForRemote(
repoPath,
'upstream',
knownHosts,
connectionId,
localGitOptions
)
if (upstream) {
return upstream
}
}
return originPromise
}
export type ResolvedIssueSource = {
+75 -21
View File
@@ -33,9 +33,39 @@ import {
} from './gl-utils'
import { GlabNonListResponseError } from './glab-api-response'
import { registerSshGitProvider, unregisterSshGitProvider } from '../providers/ssh-git-dispatch'
import { _resetRemoteNameListingCache } from '../git/remote-name-listing'
import { REMOTE_URL_PROBE_TIMEOUT_MS } from '../git/remote-url-probe'
import { NEGATIVE_ENTRY_TTL_MS } from '../git/remote-ref-probe-cache'
function mockGitRemoteCommands(remotes: Record<string, string>): void {
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
if (args[0] === 'remote' && args[1] !== 'get-url') {
return { stdout: `${Object.keys(remotes).join('\n')}\n` }
}
if (args[0] === 'remote' && args[1] === 'get-url') {
const url = remotes[args[2] ?? '']
if (!url) {
throw new Error(`fatal: No such remote '${args[2]}'`)
}
return { stdout: url }
}
throw new Error(`unexpected git ${args.join(' ')}`)
})
}
function gitRemoteGetUrlCalls(remoteName: string): unknown[][] {
return gitExecFileAsyncMock.mock.calls.filter(
([args]) =>
Array.isArray(args) && args[0] === 'remote' && args[1] === 'get-url' && args[2] === remoteName
)
}
function gitRemoteListCalls(): unknown[][] {
return gitExecFileAsyncMock.mock.calls.filter(
([args]) => Array.isArray(args) && args[0] === 'remote' && args[1] !== 'get-url'
)
}
describe('gitlab project ref resolution', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
@@ -43,6 +73,7 @@ describe('gitlab project ref resolution', () => {
sshExecMock.mockReset()
unregisterSshGitProvider('conn-1')
_resetProjectRefCache()
_resetRemoteNameListingCache()
})
afterEach(() => {
@@ -66,35 +97,53 @@ describe('gitlab project ref resolution', () => {
})
it('prefers upstream for issue project ref resolution', async () => {
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'git@gitlab.com:stablyai/orca.git\n'
mockGitRemoteCommands({
origin: 'git@gitlab.com:fork/orca.git\n',
upstream: 'git@gitlab.com:stablyai/orca.git\n'
})
await expect(getIssueProjectRef('/repo')).resolves.toEqual({
host: 'gitlab.com',
path: 'stablyai/orca'
})
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', 'get-url', 'upstream'], {
cwd: '/repo',
timeout: REMOTE_URL_PROBE_TIMEOUT_MS
})
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(1)
})
it('falls back to origin when upstream is missing or non-GitLab', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'git@example.com:stablyai/orca.git\n' })
.mockResolvedValueOnce({ stdout: 'git@gitlab.com:fork/orca.git\n' })
it('does not spawn git remote get-url upstream on an origin-only clone', async () => {
mockGitRemoteCommands({ origin: 'git@gitlab.com:fork/orca.git\n' })
await expect(getIssueProjectRef('/repo')).resolves.toEqual({
host: 'gitlab.com',
path: 'fork/orca'
})
await expect(getIssueProjectRef('/repo')).resolves.toEqual({
host: 'gitlab.com',
path: 'fork/orca'
})
expect(gitRemoteListCalls()).toHaveLength(1)
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(0)
expect(gitRemoteGetUrlCalls('origin')).toHaveLength(1)
})
it('falls back to origin when upstream is present but non-GitLab', async () => {
mockGitRemoteCommands({
origin: 'git@gitlab.com:fork/orca.git\n',
upstream: 'git@example.com:stablyai/orca.git\n'
})
await expect(getIssueProjectRef('/repo')).resolves.toEqual({
host: 'gitlab.com',
path: 'fork/orca'
})
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(1)
expect(gitRemoteGetUrlCalls('origin')).toHaveLength(1)
})
it('does not mix origin and upstream cache entries for the same repo path', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'git@gitlab.com:fork/orca.git\n' })
.mockResolvedValueOnce({ stdout: 'git@gitlab.com:stablyai/orca.git\n' })
mockGitRemoteCommands({
origin: 'git@gitlab.com:fork/orca.git\n',
upstream: 'git@gitlab.com:stablyai/orca.git\n'
})
await expect(getProjectRef('/repo')).resolves.toEqual({
host: 'gitlab.com',
@@ -355,23 +404,27 @@ describe('resolveIssueSource', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
_resetProjectRefCache()
_resetRemoteNameListingCache()
})
it("'auto' + upstream exists → upstream, fellBack=false", async () => {
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'git@gitlab.com:stablyai/orca.git\n'
mockGitRemoteCommands({
origin: 'git@gitlab.com:fork/orca.git\n',
upstream: 'git@gitlab.com:stablyai/orca.git\n'
})
await expect(resolveIssueSource('/repo', 'auto')).resolves.toEqual({
source: { host: 'gitlab.com', path: 'stablyai/orca' },
fellBack: false
})
expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(1)
})
it("'auto' + no upstream → origin, fellBack=false", async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'git@example.com:stablyai/orca.git\n' })
.mockResolvedValueOnce({ stdout: 'git@gitlab.com:solo/orca.git\n' })
it("'auto' + no gitlab upstream → origin, fellBack=false", async () => {
mockGitRemoteCommands({
origin: 'git@gitlab.com:solo/orca.git\n',
upstream: 'git@example.com:stablyai/orca.git\n'
})
await expect(resolveIssueSource('/repo', 'auto')).resolves.toEqual({
source: { host: 'gitlab.com', path: 'solo/orca' },
@@ -407,8 +460,9 @@ describe('resolveIssueSource', () => {
})
it('undefined preference is treated identically to auto', async () => {
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'git@gitlab.com:stablyai/orca.git\n'
mockGitRemoteCommands({
origin: 'git@gitlab.com:fork/orca.git\n',
upstream: 'git@gitlab.com:stablyai/orca.git\n'
})
await expect(resolveIssueSource('/repo', undefined)).resolves.toEqual({