diff --git a/src/main/github/client.ts b/src/main/github/client.ts index b4702f28128..31dbc519571 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -79,6 +79,7 @@ import { rememberGhCwdResolutionFailure } from './gh-cwd-repo-negative-cache' import type { GitHubRepoContext } from './github-repository-identity' +import { getEnterpriseGitHubRepoSlug } from './github-enterprise-repository' export { _resetOwnerRepoCache } from './gh-utils' export { getIssue, @@ -1844,11 +1845,11 @@ export async function createGitHubPullRequest( } } - const ownerRepo = await getOwnerRepo( - repoPath, - connectionId, - ...hostedReviewLocalGitOptionArgs(options) - ) + // Why: github.com-only slug parsing returns null for GHES, so fall back to the + // enterprise resolver (gh-authenticated custom host) before giving up (#8312). + const ownerRepo = + (await getOwnerRepo(repoPath, connectionId, ...hostedReviewLocalGitOptionArgs(options))) ?? + (await getEnterpriseGitHubRepoSlug(repoPath, connectionId, options)) if (!ownerRepo) { return { ok: false, diff --git a/src/main/github/github-enterprise-repository.test.ts b/src/main/github/github-enterprise-repository.test.ts new file mode 100644 index 00000000000..cdc409f6322 --- /dev/null +++ b/src/main/github/github-enterprise-repository.test.ts @@ -0,0 +1,142 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { ghExecFileAsyncMock, gitExecFileAsyncMock } = vi.hoisted(() => ({ + ghExecFileAsyncMock: vi.fn(), + gitExecFileAsyncMock: vi.fn() +})) + +// Mock only the exec boundary so the real remote-identity parsing and +// `gh auth status` parsing run against controlled command output. +vi.mock('../git/runner', () => ({ + ghExecFileAsync: ghExecFileAsyncMock, + gitExecFileAsync: gitExecFileAsyncMock +})) + +import { + _resetAuthenticatedGitHubHostsCache, + getAuthenticatedGitHubHosts, + getEnterpriseGitHubRepoSlug +} from './github-enterprise-repository' + +function mockOriginRemote(url: string): void { + gitExecFileAsyncMock.mockImplementation(async (args: string[]) => { + if (args[0] === 'remote' && args[1] === 'get-url') { + return { stdout: `${url}\n`, stderr: '' } + } + return { stdout: '', stderr: '' } + }) +} + +function mockGhAuthStatus(hosts: string[]): void { + const text = hosts + .map( + (host) => + `${host}\n ✓ Logged in to ${host} account kelora (keyring)\n - Active account: true\n - Token scopes: 'repo', 'read:org'` + ) + .join('\n') + ghExecFileAsyncMock.mockResolvedValue({ stdout: text, stderr: '' }) +} + +describe('getEnterpriseGitHubRepoSlug', () => { + beforeEach(() => { + ghExecFileAsyncMock.mockReset() + gitExecFileAsyncMock.mockReset() + _resetAuthenticatedGitHubHostsCache() + }) + + it('resolves a GHES remote whose host the user is gh-authenticated to (#8312)', async () => { + mockOriginRemote('https://github.acme-corp.com/team/orca.git') + mockGhAuthStatus(['github.acme-corp.com']) + + await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toEqual({ + owner: 'team', + repo: 'orca', + host: 'github.acme-corp.com' + }) + }) + + it('resolves a GHES SCP-style SSH remote', async () => { + mockOriginRemote('git@github.acme-corp.com:team/orca.git') + mockGhAuthStatus(['github.acme-corp.com']) + + await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toEqual({ + owner: 'team', + repo: 'orca', + host: 'github.acme-corp.com' + }) + }) + + it('leaves github.com to getOwnerRepo without probing gh auth', async () => { + mockOriginRemote('https://github.com/team/orca.git') + + await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toBeNull() + expect(ghExecFileAsyncMock).not.toHaveBeenCalled() + }) + + it('declines a custom host the user is not gh-authenticated to (leaves it for Gitea)', async () => { + mockOriginRemote('https://gitea.example.com/team/orca.git') + mockGhAuthStatus(['github.com']) + + await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toBeNull() + }) + + it('returns null for an unparseable remote', async () => { + mockOriginRemote('not-a-remote-url') + mockGhAuthStatus(['github.acme-corp.com']) + + await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toBeNull() + }) + + it('returns null when the origin remote lookup fails', async () => { + gitExecFileAsyncMock.mockRejectedValue(new Error('no such remote')) + + await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toBeNull() + }) +}) + +describe('getAuthenticatedGitHubHosts', () => { + beforeEach(() => { + ghExecFileAsyncMock.mockReset() + gitExecFileAsyncMock.mockReset() + _resetAuthenticatedGitHubHostsCache() + }) + + it('parses and lowercases the logged-in hosts', async () => { + mockGhAuthStatus(['github.com', 'GitHub.Acme-Corp.com']) + + const hosts = await getAuthenticatedGitHubHosts() + expect(hosts.has('github.com')).toBe(true) + expect(hosts.has('github.acme-corp.com')).toBe(true) + }) + + it('caches a non-empty result to avoid re-spawning gh per detection poll', async () => { + mockGhAuthStatus(['github.acme-corp.com']) + + await getAuthenticatedGitHubHosts() + await getAuthenticatedGitHubHosts() + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + }) + + it('recovers hosts from gh output even when gh exits non-zero', async () => { + ghExecFileAsyncMock.mockRejectedValue( + Object.assign(new Error('exit 1'), { + stdout: + 'github.acme-corp.com\n ✓ Logged in to github.acme-corp.com account kelora (keyring)', + stderr: '' + }) + ) + + const hosts = await getAuthenticatedGitHubHosts() + expect(hosts.has('github.acme-corp.com')).toBe(true) + }) + + it('does not cache an empty result so a later gh login is discovered', async () => { + ghExecFileAsyncMock.mockRejectedValueOnce( + Object.assign(new Error('not installed'), { stdout: '', stderr: '' }) + ) + expect((await getAuthenticatedGitHubHosts()).size).toBe(0) + + mockGhAuthStatus(['github.acme-corp.com']) + expect((await getAuthenticatedGitHubHosts()).has('github.acme-corp.com')).toBe(true) + }) +}) diff --git a/src/main/github/github-enterprise-repository.ts b/src/main/github/github-enterprise-repository.ts new file mode 100644 index 00000000000..8542343ef0e --- /dev/null +++ b/src/main/github/github-enterprise-repository.ts @@ -0,0 +1,103 @@ +import { ghExecFileAsync } from '../git/runner' +import type { GitHubOwnerRepo } from '../../shared/types' +import { + getHostedReviewLocalGitOptions, + type HostedReviewExecutionOptions +} from '../source-control/hosted-review-git-options' +import { parseAuthStatus } from './auth-diagnose' +import { + getRemoteUrlForRepo, + githubRepoContext, + parseGitHubRemoteIdentity +} from './github-repository-identity' + +export type GitHubEnterpriseRepoSlug = GitHubOwnerRepo & { host: string } + +// Why: `gh` only ever manages github.com / GitHub Enterprise credentials, so a +// host that `gh auth status` reports as logged-in is definitively a GitHub host. +// This is the same signal `glab auth status` provides for GitLab self-hosted +// detection, mirrored here so GHES is not left to fall through to Gitea (#8312). +const AUTHENTICATED_HOSTS_TTL_MS = 60_000 + +type AuthenticatedHostsCacheEntry = { + hosts: ReadonlySet + expiresAt: number +} + +const authenticatedHostsCache = new Map() + +function connectionCacheKey(connectionId?: string | null): string { + return connectionId ?? 'local' +} + +/** @internal - exposed for tests only */ +export function _resetAuthenticatedGitHubHostsCache(): void { + authenticatedHostsCache.clear() +} + +/** + * Hosts the local `gh` CLI is authenticated to, lowercased. Cached briefly so a + * `gh auth login --hostname ` performed after startup is picked up without + * spawning `gh auth status` on every provider-detection poll. Failures are not + * cached so a later probe (gh installed, tunnel ready) can still discover hosts. + */ +export async function getAuthenticatedGitHubHosts( + connectionId?: string | null +): Promise> { + const key = connectionCacheKey(connectionId) + const now = Date.now() + const cached = authenticatedHostsCache.get(key) + if (cached && cached.expiresAt > now) { + return cached.hosts + } + let text = '' + try { + // Why: `gh auth status` reads gh's own config, so it is host-scoped rather + // than cwd-scoped — no repo context is needed to enumerate logged-in hosts. + const { stdout, stderr } = await ghExecFileAsync(['auth', 'status']) + text = `${stdout}\n${stderr}` + } catch (err) { + // gh exits non-zero when any host has a token problem but still prints the + // per-host status we parse; recover its output before giving up. + const execErr = err as { stdout?: unknown; stderr?: unknown } + text = `${String(execErr?.stdout ?? '')}\n${String(execErr?.stderr ?? '')}`.trim() + if (!text) { + return new Set() + } + } + const hosts = new Set(parseAuthStatus(text).map((account) => account.host.toLowerCase())) + if (hosts.size === 0) { + return hosts + } + authenticatedHostsCache.set(key, { hosts, expiresAt: now + AUTHENTICATED_HOSTS_TTL_MS }) + return hosts +} + +/** + * Resolve owner/repo for a GitHub Enterprise Server `origin` remote — a custom + * host the user is gh-authenticated to. Returns null for github.com (already + * handled by {@link getOwnerRepo}) and for hosts gh is not logged in to + * (Gitea/Forgejo/self-hosted GitLab/etc.), so GHES routes to the GitHub provider + * without a GitHub provider stealing another forge's remote. + */ +export async function getEnterpriseGitHubRepoSlug( + repoPath: string, + connectionId?: string | null, + options: HostedReviewExecutionOptions = {} +): Promise { + const context = githubRepoContext(repoPath, connectionId, getHostedReviewLocalGitOptions(options)) + let remoteUrl: string | null + try { + remoteUrl = await getRemoteUrlForRepo(context, 'origin') + } catch { + return null + } + const identity = remoteUrl ? parseGitHubRemoteIdentity(remoteUrl) : null + if (!identity || identity.host === 'github.com') { + return null + } + const authenticatedHosts = await getAuthenticatedGitHubHosts(connectionId) + return authenticatedHosts.has(identity.host) + ? { owner: identity.owner, repo: identity.repo, host: identity.host } + : null +} diff --git a/src/main/source-control/forge-provider.test.ts b/src/main/source-control/forge-provider.test.ts index 0317d6fc9e9..95c5c9d2676 100644 --- a/src/main/source-control/forge-provider.test.ts +++ b/src/main/source-control/forge-provider.test.ts @@ -11,7 +11,8 @@ const { getMergeRequestForBranchMock, getProjectSlugMock, getPRForBranchOutcomeMock, - getRepoSlugMock + getRepoSlugMock, + getEnterpriseGitHubRepoSlugMock } = vi.hoisted(() => ({ createGitHubPullRequestMock: vi.fn(), createGitLabMergeRequestMock: vi.fn(), @@ -23,7 +24,8 @@ const { getMergeRequestForBranchMock: vi.fn(), getProjectSlugMock: vi.fn(), getPRForBranchOutcomeMock: vi.fn(), - getRepoSlugMock: vi.fn() + getRepoSlugMock: vi.fn(), + getEnterpriseGitHubRepoSlugMock: vi.fn() })) vi.mock('../gitlab/client', () => ({ @@ -42,6 +44,10 @@ vi.mock('../github/client', () => ({ getPRForBranchOutcome: getPRForBranchOutcomeMock })) +vi.mock('../github/github-enterprise-repository', () => ({ + getEnterpriseGitHubRepoSlug: getEnterpriseGitHubRepoSlugMock +})) + vi.mock('../bitbucket/client', () => ({ getBitbucketRepoSlug: getBitbucketRepoSlugMock, getBitbucketPullRequestForBranch: vi.fn(), @@ -88,6 +94,7 @@ describe('forge provider interface', () => { getProjectSlugMock.mockReset() getPRForBranchOutcomeMock.mockReset() getRepoSlugMock.mockReset() + getEnterpriseGitHubRepoSlugMock.mockReset() }) it('preserves the existing hosted provider detection order', async () => { @@ -101,6 +108,45 @@ describe('forge provider interface', () => { expect(getRepoSlugMock).not.toHaveBeenCalled() }) + it('detects a GitHub Enterprise Server remote as the GitHub provider, not Gitea', async () => { + // Regression for #8312: a GHES host is not github.com, so github.com-only + // slug parsing returns null. Detection must claim it via the enterprise + // resolver instead of falling through to Gitea's demand for ORCA_GITEA_TOKEN. + getProjectSlugMock.mockResolvedValue(null) + getRepoSlugMock.mockResolvedValue(null) + getEnterpriseGitHubRepoSlugMock.mockResolvedValue({ + owner: 'team', + repo: 'orca', + host: 'github.acme-corp.com' + }) + + await expect(detectHostedReviewProvider({ repoPath: '/repo' })).resolves.toBe('github') + await expect(getForgeProviderForRepository({ repoPath: '/repo' })).resolves.toMatchObject({ + id: 'github' + }) + // Gitea must never be consulted once GitHub claims the enterprise host. + expect(getGiteaRepoSlugMock).not.toHaveBeenCalled() + }) + + it('leaves a genuinely non-GitHub remote for later providers when gh is not authenticated', async () => { + getProjectSlugMock.mockResolvedValue(null) + getRepoSlugMock.mockResolvedValue(null) + // gh is not logged in to this host, so the enterprise resolver declines and + // the Gitea provider is free to claim its own self-hosted remote. + getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null) + getBitbucketRepoSlugMock.mockResolvedValue(null) + getAzureDevOpsRepoSlugMock.mockResolvedValue(null) + getGiteaRepoSlugMock.mockResolvedValue({ + host: 'gitea.example.com', + owner: 'team', + repo: 'orca', + apiBaseUrl: 'https://gitea.example.com/api/v1', + webBaseUrl: 'https://gitea.example.com' + }) + + await expect(detectHostedReviewProvider({ repoPath: '/repo' })).resolves.toBe('gitea') + }) + it('keeps review creation capability scoped to providers with creation support', async () => { expect( FORGE_PROVIDERS.map((provider) => [provider.id, provider.supportsReviewCreation]) diff --git a/src/main/source-control/forge-provider.ts b/src/main/source-control/forge-provider.ts index d87efe9dcf1..a7d22ad48ae 100644 --- a/src/main/source-control/forge-provider.ts +++ b/src/main/source-control/forge-provider.ts @@ -22,6 +22,7 @@ import { } from '../gitea/client' import { createGiteaPullRequest } from '../gitea/pull-request-creation' import { createGitHubPullRequest, getPRForBranchOutcome, getRepoSlug } from '../github/client' +import { getEnterpriseGitHubRepoSlug } from '../github/github-enterprise-repository' import { getMergeRequest, getMergeRequestForBranch, getProjectSlug } from '../gitlab/client' import { createGitLabMergeRequest } from '../gitlab/merge-request-creation' import { @@ -122,8 +123,25 @@ function unwrapGitHubPRForBranchOutcome( const gitHubForgeProvider = { id: 'github', supportsReviewCreation: true, - resolveRepository: (context) => - getRepoSlug(context.repoPath, context.connectionId, ...hostedReviewExecutionArgs(context)), + resolveRepository: async (context) => { + const slug = await getRepoSlug( + context.repoPath, + context.connectionId, + ...hostedReviewExecutionArgs(context) + ) + if (slug) { + return slug + } + // Why: GHES remotes live on a custom host, so github.com-only slug parsing + // misses them and detection would otherwise fall through to Gitea (#8312). + // Claim the repo when gh is authenticated to its host — the same signal + // GitLab uses for self-hosted instances. + return getEnterpriseGitHubRepoSlug( + context.repoPath, + context.connectionId, + ...hostedReviewExecutionArgs(context) + ) + }, async getReviewForBranch(input) { const fallbackReviewNumber = input.linkedReviewNumber == null ? (input.fallbackReviewNumber ?? null) : null diff --git a/src/main/source-control/hosted-review-creation.test.ts b/src/main/source-control/hosted-review-creation.test.ts index c03447d4043..7af904748dc 100644 --- a/src/main/source-control/hosted-review-creation.test.ts +++ b/src/main/source-control/hosted-review-creation.test.ts @@ -17,7 +17,8 @@ const { glabExecFileAsyncMock, gitExecFileAsyncMock, getUpstreamStatusMock, - getSshGitProviderMock + getSshGitProviderMock, + getEnterpriseGitHubRepoSlugMock } = vi.hoisted(() => ({ createGitHubPullRequestMock: vi.fn(), createGitLabMergeRequestMock: vi.fn(), @@ -35,7 +36,8 @@ const { glabExecFileAsyncMock: vi.fn(), gitExecFileAsyncMock: vi.fn(), getUpstreamStatusMock: vi.fn(), - getSshGitProviderMock: vi.fn() + getSshGitProviderMock: vi.fn(), + getEnterpriseGitHubRepoSlugMock: vi.fn() })) vi.mock('../github/client', () => ({ @@ -44,6 +46,10 @@ vi.mock('../github/client', () => ({ getPRForBranch: vi.fn() })) +vi.mock('../github/github-enterprise-repository', () => ({ + getEnterpriseGitHubRepoSlug: getEnterpriseGitHubRepoSlugMock +})) + vi.mock('../gitlab/client', () => ({ getProjectSlug: getProjectSlugMock, getMergeRequestForBranch: vi.fn(), @@ -129,7 +135,8 @@ function resetMocks(): void { glabExecFileAsyncMock, gitExecFileAsyncMock, getUpstreamStatusMock, - getSshGitProviderMock + getSshGitProviderMock, + getEnterpriseGitHubRepoSlugMock ]) { mock.mockReset() } @@ -141,6 +148,22 @@ function mockGitHubProvider(): void { getBitbucketRepoSlugMock.mockResolvedValue(null) getAzureDevOpsRepoSlugMock.mockResolvedValue(null) getGiteaRepoSlugMock.mockResolvedValue(null) + getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null) +} + +// GHES: github.com-only slug parsing misses the custom host, so the enterprise +// resolver claims the repo and reports the host for the gh auth probe (#8312). +function mockGitHubEnterpriseProvider(): void { + getProjectSlugMock.mockResolvedValue(null) + getRepoSlugMock.mockResolvedValue(null) + getBitbucketRepoSlugMock.mockResolvedValue(null) + getAzureDevOpsRepoSlugMock.mockResolvedValue(null) + getGiteaRepoSlugMock.mockResolvedValue(null) + getEnterpriseGitHubRepoSlugMock.mockResolvedValue({ + owner: 'acme', + repo: 'orca', + host: 'github.acme-corp.com' + }) } function mockGitLabProvider(): void { @@ -349,6 +372,31 @@ describe('createHostedReview', () => { ) }) + it('creates a pull request on a GitHub Enterprise Server remote (#8312)', async () => { + mockGitHubEnterpriseProvider() + + await expect( + createHostedReview('/repo', { + provider: 'github', + base: 'main', + head: 'feature', + title: 'Feature' + }) + ).resolves.toEqual({ + ok: true, + number: 12, + 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' } + ) + expect(createGitHubPullRequestMock).toHaveBeenCalled() + expect(createGiteaPullRequestMock).not.toHaveBeenCalled() + }) + it('creates a GitLab merge request after fresh main-process validation passes', async () => { mockGitLabProvider() @@ -636,6 +684,33 @@ describe('getHostedReviewCreationEligibility', () => { }) }) + it('detects a GitHub Enterprise Server branch as the GitHub provider (#8312)', async () => { + mockGitHubEnterpriseProvider() + + await expect( + getHostedReviewCreationEligibility({ + repoPath: '/repo', + branch: 'feature/create-pr', + base: 'origin/main', + hasUncommittedChanges: false, + hasUpstream: true, + ahead: 0, + behind: 0 + }) + ).resolves.toMatchObject({ + provider: 'github', + canCreate: true, + blockedReason: null, + 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' } + ) + }) + it('resolves remote eligibility through SSH repo metadata without generating PR copy', async () => { const remoteGit = { exec: vi.fn(async () => ({ stdout: '', stderr: '' })) diff --git a/src/main/source-control/hosted-review-creation.ts b/src/main/source-control/hosted-review-creation.ts index 7b6709d2dd3..0b0f8f9cb13 100644 --- a/src/main/source-control/hosted-review-creation.ts +++ b/src/main/source-control/hosted-review-creation.ts @@ -18,6 +18,7 @@ import { } from '../../shared/hosted-review-creation-providers' import { isAzureDevOpsReviewCreationAuthenticated } from '../azure-devops/pull-request-creation' import { isGiteaReviewCreationAuthenticated } from '../gitea/pull-request-creation' +import { getEnterpriseGitHubRepoSlug } from '../github/github-enterprise-repository' import { acquire, ghExecFileAsync, gitExecFileAsync, release } from '../github/gh-utils' import { isNoUpstreamError, normalizeGitErrorMessage } from '../../shared/git-remote-error' import type { GitUpstreamStatus } from '../../shared/types' @@ -59,10 +60,15 @@ 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' await acquire() try { await ghExecFileAsync( - ['auth', 'status', '--hostname', 'github.com'], + ['auth', 'status', '--hostname', hostname], connectionId ? {} : { cwd: repoPath, ...getHostedReviewLocalGitOptions(options) } ) return true