fix(github): keep failed SSH discovery unverifiable

This commit is contained in:
Brennan Benson
2026-08-16 19:59:46 -07:00
parent 9a2d0d572c
commit a875d0af2d
4 changed files with 51 additions and 8 deletions
+9
View File
@@ -97,6 +97,15 @@ describe('remote URL probe', () => {
await expect(assertRemoteUrlReadable({ repoPath: '/repo' })).resolves.toBeUndefined()
})
it('keeps a failed SSH command unverifiable even without a transport error string', async () => {
const failure = new Error('relay request failed')
getSshGitProviderMock.mockReturnValue({ exec: vi.fn().mockRejectedValue(failure) })
await expect(
assertRemoteUrlReadable({ repoPath: '/repo', connectionId: 'ssh-1' })
).rejects.toBe(failure)
})
it('does not turn a missing SSH provider into a false readable result', async () => {
getSshGitProviderMock.mockReturnValue(null)
+7 -6
View File
@@ -3,6 +3,7 @@ import {
SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE
} from '../providers/ssh-git-dispatch'
import { gitExecFileAsync } from './runner'
import { isStableMissingGitRemoteError } from './stable-missing-git-remote-error'
/**
* The `git remote get-url` probe every forge integration runs to decide whether
@@ -91,10 +92,9 @@ export function isTransientGitProbeError(error: unknown): boolean {
}
/**
* Throws when the remote could not be read because the probe was wedged or its
* transport was gone, and returns normally for every answer — including a repo
* with no remote at all. Lets a caller that treats "nothing found" as a cacheable
* answer tell that apart from a lookup that never got to ask.
* Throws when the remote could not be read, and returns normally for every
* answer — including a repo with no remote at all. Lets a caller that treats
* "nothing found" as cacheable tell that apart from a lookup that never got to ask.
*/
export async function assertRemoteUrlReadable(
context: RemoteUrlProbeContext,
@@ -106,8 +106,9 @@ export async function assertRemoteUrlReadable(
try {
await readRemoteUrl(context, remoteName)
} catch (error) {
if (isTransientGitProbeError(error)) {
throw error
if (isStableMissingGitRemoteError(error)) {
return
}
throw error
}
}
@@ -29,8 +29,13 @@ vi.mock('./github-api-repository', async (importOriginal) =>
import { getPRForBranch, getPRForBranchOutcome } from './client'
import { resetPRForBranchMocks } from './client-test-harness'
const { ghExecFileAsyncMock, getOwnerRepoMock, gitExecFileAsyncMock, getSshGitProviderMock } =
clientMocks
const {
ghExecFileAsyncMock,
getOwnerRepoMock,
gitExecFileAsyncMock,
getSshGitProviderMock,
resolvePRRepositoryCandidatesMock
} = clientMocks
const MERGED_BRANCH = 'fix-tab-strip-layout-test'
const MERGED_HEAD_OID = 'current-head-oid'
@@ -127,6 +132,30 @@ describe('getPRForBranch SSH execution boundary', () => {
expect(outcome).toMatchObject({ kind: 'upstream-error', errorType: 'unknown' })
})
it('reports an upstream error when candidate discovery finds no SSH provider', async () => {
resolvePRRepositoryCandidatesMock.mockResolvedValue({ candidates: [], headRepo: null })
getSshGitProviderMock.mockReturnValue(undefined)
const outcome = await getPRForBranchOutcome('/remote/repo', MERGED_BRANCH, null, 'ssh-1')
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
expect(outcome).toMatchObject({ kind: 'upstream-error', errorType: 'unknown' })
})
it('reports an upstream error when candidate discovery loses SSH mid-flight', async () => {
resolvePRRepositoryCandidatesMock.mockResolvedValue({ candidates: [], headRepo: null })
getSshGitProviderMock.mockReturnValue({
exec: vi.fn().mockRejectedValue(new Error('SSH transport closed'))
})
const outcome = await getPRForBranchOutcome('/remote/repo', MERGED_BRANCH, null, 'ssh-1')
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
expect(outcome).toMatchObject({ kind: 'upstream-error', errorType: 'unknown' })
})
it('rev-parses HEAD through the SSH provider when it is registered', async () => {
const sshGitProvider = {
exec: vi.fn(async (args: string[]) =>
+4
View File
@@ -77,6 +77,7 @@ import {
getSshGitProvider,
SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE
} from '../providers/ssh-git-dispatch'
import { assertRemoteUrlReadable } from '../git/remote-url-probe'
import {
hasHostedReviewLocalGitOptions,
getHostedReviewLocalGitOptions,
@@ -3268,6 +3269,9 @@ export async function getPRForBranchOutcome(
// Why: connection-backed gh runs without a repository cwd. A bare lookup
// here can honor process GH_REPO/GH_HOST and return an unrelated PR.
if (connectionId && candidates.length === 0) {
// Why: candidate discovery intentionally tolerates unknown forges, but a
// lost SSH probe is unverifiable and must not clear cached review state.
await assertRemoteUrlReadable(context)
return { kind: 'no-pr', fetchedAt: Date.now() }
}
// Why (#11532): account every lookup, not just the coordinator's queue —