diff --git a/src/main/git/remote-url-probe.test.ts b/src/main/git/remote-url-probe.test.ts index 8a7672cb229..f9e0348c3ac 100644 --- a/src/main/git/remote-url-probe.test.ts +++ b/src/main/git/remote-url-probe.test.ts @@ -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) diff --git a/src/main/git/remote-url-probe.ts b/src/main/git/remote-url-probe.ts index 7b18c1deac9..d237bdd98fa 100644 --- a/src/main/git/remote-url-probe.ts +++ b/src/main/git/remote-url-probe.ts @@ -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 } } diff --git a/src/main/github/client-ssh-provider-execution-boundary.test.ts b/src/main/github/client-ssh-provider-execution-boundary.test.ts index b37901744fe..1e0ac6b8ebc 100644 --- a/src/main/github/client-ssh-provider-execution-boundary.test.ts +++ b/src/main/github/client-ssh-provider-execution-boundary.test.ts @@ -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[]) => diff --git a/src/main/github/client.ts b/src/main/github/client.ts index b9e8d94226f..997007f19b3 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -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 —