From ff9dce07882dda8e19d417526c2ca2620635e417 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 13 Jul 2026 15:49:08 -0700 Subject: [PATCH] perf(source-control): drop redundant GHES gh auth probe in eligibility Review follow-up. Detection only routes a GHES remote to the GitHub provider after getEnterpriseGitHubRepoSlug has confirmed gh is authenticated to its host, so isGitHubAuthenticated can trust a non-null slug as authenticated and skip a second, rate-limited `gh auth status` spawn per eligibility poll. Reaching the github.com probe now implies the remote is github.com. Tests assert the enterprise path fires no redundant gh probe. --- .../hosted-review-creation.test.ts | 16 ++++++---------- .../source-control/hosted-review-creation.ts | 15 +++++++++------ 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/main/source-control/hosted-review-creation.test.ts b/src/main/source-control/hosted-review-creation.test.ts index 7af904748dc..3ae82fd4010 100644 --- a/src/main/source-control/hosted-review-creation.test.ts +++ b/src/main/source-control/hosted-review-creation.test.ts @@ -388,11 +388,9 @@ describe('createHostedReview', () => { url: 'https://github.com/acme/orca/pull/12' }) - // The auth probe must target the GHES host, not the hardcoded github.com. - expect(ghExecFileAsyncMock).toHaveBeenCalledWith( - ['auth', 'status', '--hostname', 'github.acme-corp.com'], - { cwd: '/repo' } - ) + // Detection already confirmed gh is authed to the GHES host, so the auth + // gate must not fire a second (rate-limited) gh probe. + expect(ghExecFileAsyncMock).not.toHaveBeenCalled() expect(createGitHubPullRequestMock).toHaveBeenCalled() expect(createGiteaPullRequestMock).not.toHaveBeenCalled() }) @@ -704,11 +702,9 @@ describe('getHostedReviewCreationEligibility', () => { nextAction: null }) - // The auth gate must probe the GHES host, not the hardcoded github.com. - expect(ghExecFileAsyncMock).toHaveBeenCalledWith( - ['auth', 'status', '--hostname', 'github.acme-corp.com'], - { cwd: '/repo' } - ) + // Enterprise auth was already confirmed during detection; the gate must not + // fire a redundant gh probe. + expect(ghExecFileAsyncMock).not.toHaveBeenCalled() }) it('resolves remote eligibility through SSH repo metadata without generating PR copy', async () => { diff --git a/src/main/source-control/hosted-review-creation.ts b/src/main/source-control/hosted-review-creation.ts index 0b0f8f9cb13..01caac804d8 100644 --- a/src/main/source-control/hosted-review-creation.ts +++ b/src/main/source-control/hosted-review-creation.ts @@ -60,15 +60,18 @@ async function isGitHubAuthenticated( connectionId?: string | null, options: HostedReviewExecutionOptions = {} ): Promise { - // Why: a GHES repo authenticates against its own custom host, so a hardcoded - // github.com probe reports "not authenticated" for Enterprise users (#8312). - // Resolve the repo's actual GitHub host, defaulting to github.com. - const enterprise = await getEnterpriseGitHubRepoSlug(repoPath, connectionId, options) - const hostname = enterprise?.host ?? 'github.com' + // Why: a GHES remote is only routed to the GitHub provider once detection has + // confirmed gh is authenticated to its enterprise host, so a non-null slug + // already means authenticated — skip a redundant, rate-limited gh probe. + // Reaching the github.com check below therefore means the remote is github.com + // (its own custom host would have resolved above) (#8312). + if (await getEnterpriseGitHubRepoSlug(repoPath, connectionId, options)) { + return true + } await acquire() try { await ghExecFileAsync( - ['auth', 'status', '--hostname', hostname], + ['auth', 'status', '--hostname', 'github.com'], connectionId ? {} : { cwd: repoPath, ...getHostedReviewLocalGitOptions(options) } ) return true