diff --git a/src/main/github/client-create-pr.test.ts b/src/main/github/client-create-pr.test.ts index 6c64e4b8066..79d614eb0d0 100644 --- a/src/main/github/client-create-pr.test.ts +++ b/src/main/github/client-create-pr.test.ts @@ -4,6 +4,7 @@ import { readFile } from 'node:fs/promises' const { ghExecFileAsyncMock, getOwnerRepoMock, + getEnterpriseGitHubRepoSlugMock, extractExecErrorMock, acquireMock, releaseMock, @@ -11,6 +12,7 @@ const { } = vi.hoisted(() => ({ ghExecFileAsyncMock: vi.fn(), getOwnerRepoMock: vi.fn(), + getEnterpriseGitHubRepoSlugMock: vi.fn(), extractExecErrorMock: vi.fn((error: unknown) => { const value = error as { stderr?: string; stdout?: string; message?: string } return { @@ -52,12 +54,18 @@ vi.mock('../git/runner', () => ({ gitExecFileAsync: vi.fn() })) +vi.mock('./github-enterprise-repository', () => ({ + getEnterpriseGitHubRepoSlug: getEnterpriseGitHubRepoSlugMock +})) + import { createGitHubPullRequest } from './client' describe('createGitHubPullRequest', () => { beforeEach(() => { ghExecFileAsyncMock.mockReset() getOwnerRepoMock.mockReset() + getEnterpriseGitHubRepoSlugMock.mockReset() + getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null) extractExecErrorMock.mockClear() acquireMock.mockReset() releaseMock.mockReset() @@ -115,6 +123,78 @@ describe('createGitHubPullRequest', () => { expect(releaseMock).toHaveBeenCalledOnce() }) + it('host-qualifies --repo for a GHES remote so gh targets the Enterprise server (#8312)', async () => { + // github.com-only slug parsing misses GHES, so creation comes from the + // enterprise resolver, which carries the host. + getOwnerRepoMock.mockResolvedValueOnce(null) + getEnterpriseGitHubRepoSlugMock.mockResolvedValueOnce({ + owner: 'team', + repo: 'orca', + host: 'github.acme-corp.com' + }) + // gh prints the PR URL (not JSON); the GHES host must still parse directly. + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'https://github.acme-corp.com/team/orca/pull/7\n' + }) + + await expect( + createGitHubPullRequest('/repo-root', { + provider: 'github', + base: 'main', + head: 'feature/create-pr', + title: 'GHES PR' + }) + ).resolves.toEqual({ + ok: true, + number: 7, + url: 'https://github.acme-corp.com/team/orca/pull/7' + }) + + const [args] = ghExecFileAsyncMock.mock.calls[0] + // Bare "team/orca" would resolve against gh's default host (github.com); + // the host prefix pins the command to the Enterprise server. + expect(args[args.indexOf('--repo') + 1]).toBe('github.acme-corp.com/team/orca') + }) + + it('host-qualifies --repo for the GHES existing-PR fallback lookup (#8312)', async () => { + getOwnerRepoMock.mockResolvedValue(null) + getEnterpriseGitHubRepoSlugMock.mockResolvedValue({ + owner: 'team', + repo: 'orca', + host: 'github.acme-corp.com' + }) + // Create reports "already exists", forcing the pr-list fallback. + ghExecFileAsyncMock + .mockRejectedValueOnce( + Object.assign(new Error('exists'), { + stderr: 'a pull request for branch "feature/create-pr" already exists', + stdout: '' + }) + ) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { number: 9, url: 'https://github.acme-corp.com/team/orca/pull/9' } + ]) + }) + + await expect( + createGitHubPullRequest('/repo-root', { + provider: 'github', + base: 'main', + head: 'feature/create-pr', + title: 'GHES PR' + }) + ).resolves.toMatchObject({ + ok: false, + code: 'already_exists', + existingReview: { number: 9, url: 'https://github.acme-corp.com/team/orca/pull/9' } + }) + + const [listArgs] = ghExecFileAsyncMock.mock.calls[1] + expect(listArgs).toEqual(expect.arrayContaining(['pr', 'list'])) + expect(listArgs[listArgs.indexOf('--repo') + 1]).toBe('github.acme-corp.com/team/orca') + }) + it('runs local WSL project pull request creation through the selected distro', async () => { getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) ghExecFileAsyncMock.mockResolvedValueOnce({ diff --git a/src/main/github/client.ts b/src/main/github/client.ts index b4702f28128..5848454e6dd 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, @@ -1750,16 +1751,29 @@ function parseCreatePRPayload(stdout: string): { number: number; url: string } | } catch { // Fall through to URL parsing for older gh versions without --json support. } - const urlMatch = trimmed.match(/https:\/\/github\.com\/[^/\s]+\/[^/\s]+\/pull\/(\d+)/) + // Why: gh prints the PR URL (not JSON) here; match any host, not just + // github.com, so a GitHub Enterprise Server URL still parses directly (#8312). + const urlMatch = trimmed.match(/https?:\/\/[^\s/]+\/[^\s/]+\/[^\s/]+\/pull\/(\d+)/) if (!urlMatch) { return null } return { number: Number(urlMatch[1]), url: urlMatch[0] } } +// Why: `gh --repo OWNER/REPO` resolves the shorthand against gh's default host +// (usually github.com), not the repo's remote — so a GHES repo would target a +// same-named github.com repo, or fail. Qualify with the host for GHES so gh hits +// the Enterprise server; this is the only host signal for SSH repos, which run +// gh with no cwd context (#8312). github.com keeps the bare shorthand. +function ghRepoArg(slug: { owner: string; repo: string; host?: string }): string { + return slug.host && slug.host.toLowerCase() !== 'github.com' + ? `${slug.host}/${slug.owner}/${slug.repo}` + : `${slug.owner}/${slug.repo}` +} + async function findOpenPRByHeadBase(args: { repoPath: string - ownerRepo: OwnerRepo + repoArg: string head: string base: string connectionId?: string | null @@ -1771,7 +1785,7 @@ async function findOpenPRByHeadBase(args: { 'pr', 'list', '--repo', - `${args.ownerRepo.owner}/${args.ownerRepo.repo}`, + args.repoArg, '--head', args.head, '--base', @@ -1844,11 +1858,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, @@ -1856,6 +1870,8 @@ export async function createGitHubPullRequest( error: 'Creating pull requests requires a GitHub remote.' } } + // Host-qualified for GHES so gh targets the Enterprise server, not github.com. + const repoArg = ghRepoArg(ownerRepo) const base = normalizeHostedReviewBaseRef(input.base) const head = input.head ? normalizeHostedReviewHeadRef(input.head) || undefined : undefined @@ -1888,7 +1904,7 @@ export async function createGitHubPullRequest( 'pr', 'create', '--repo', - `${ownerRepo.owner}/${ownerRepo.repo}`, + repoArg, '--base', base, '--title', @@ -1917,7 +1933,7 @@ export async function createGitHubPullRequest( const found = head ? await findOpenPRByHeadBase({ repoPath, - ownerRepo, + repoArg, head, base, connectionId, @@ -1941,7 +1957,7 @@ export async function createGitHubPullRequest( ) { const existing = await findOpenPRByHeadBase({ repoPath, - ownerRepo, + repoArg, head, base, connectionId, 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..73de02ddae9 --- /dev/null +++ b/src/main/github/github-enterprise-repository.test.ts @@ -0,0 +1,181 @@ +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, runtime +// option resolution, and `gh auth status` parsing run against controlled output. +vi.mock('../git/runner', () => ({ + ghExecFileAsync: ghExecFileAsyncMock, + gitExecFileAsync: gitExecFileAsyncMock +})) + +import { + _resetGitHubHostAuthCache, + getEnterpriseGitHubRepoSlug, + isGitHubHostAuthenticated +} 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: '' } + }) +} + +// gh exit 0 for `auth status --hostname ` means logged in to that host. +function mockHostAuthenticated(host = 'github.acme-corp.com'): void { + ghExecFileAsyncMock.mockResolvedValue({ + stdout: `${host}\n ✓ Logged in to ${host} account kelora (keyring)`, + stderr: '' + }) +} + +// gh exits non-zero and reports no matching host when not logged in. +function mockHostNotAuthenticated(): void { + ghExecFileAsyncMock.mockRejectedValue( + Object.assign(new Error('exit 1'), { + stdout: '', + stderr: 'You are not logged into any GitHub hosts. To log in, run: gh auth login' + }) + ) +} + +describe('getEnterpriseGitHubRepoSlug', () => { + beforeEach(() => { + ghExecFileAsyncMock.mockReset() + gitExecFileAsyncMock.mockReset() + _resetGitHubHostAuthCache() + }) + + it('resolves a GHES remote whose host the user is gh-authenticated to (#8312)', async () => { + mockOriginRemote('https://github.acme-corp.com/team/orca.git') + mockHostAuthenticated() + + await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toEqual({ + owner: 'team', + repo: 'orca', + host: 'github.acme-corp.com' + }) + // The auth probe targets the remote's host, not a hardcoded github.com. + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + ['auth', 'status', '--hostname', 'github.acme-corp.com'], + { cwd: '/repo' } + ) + }) + + it('resolves a GHES SCP-style SSH remote', async () => { + mockOriginRemote('git@github.acme-corp.com:team/orca.git') + mockHostAuthenticated() + + await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toEqual({ + owner: 'team', + repo: 'orca', + host: 'github.acme-corp.com' + }) + }) + + it('probes gh in the repository WSL runtime, not the host/default distro', async () => { + mockOriginRemote('https://github.acme-corp.com/team/orca.git') + mockHostAuthenticated() + + await getEnterpriseGitHubRepoSlug('/repo', null, { + localGitExecOptions: { wslDistro: 'Ubuntu' } + }) + + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + ['auth', 'status', '--hostname', 'github.acme-corp.com'], + { cwd: '/repo', wslDistro: 'Ubuntu' } + ) + }) + + 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') + mockHostNotAuthenticated() + + await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toBeNull() + }) + + it('returns null for an unparseable remote', async () => { + mockOriginRemote('not-a-remote-url') + mockHostAuthenticated() + + await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toBeNull() + expect(ghExecFileAsyncMock).not.toHaveBeenCalled() + }) + + it('returns null when the origin remote lookup fails', async () => { + gitExecFileAsyncMock.mockRejectedValue(new Error('no such remote')) + + await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toBeNull() + }) +}) + +describe('isGitHubHostAuthenticated', () => { + beforeEach(() => { + ghExecFileAsyncMock.mockReset() + gitExecFileAsyncMock.mockReset() + _resetGitHubHostAuthCache() + }) + + it('runs gh in the SSH-local runtime (no cwd) for connection-backed repos', async () => { + mockHostAuthenticated() + + await expect( + isGitHubHostAuthenticated('github.acme-corp.com', '/remote/repo', 'ssh-1') + ).resolves.toBe(true) + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + ['auth', 'status', '--hostname', 'github.acme-corp.com'], + {} + ) + }) + + it('caches per runtime+host so detection polling does not re-spawn gh', async () => { + mockHostAuthenticated() + + await isGitHubHostAuthenticated('github.acme-corp.com', '/repo') + await isGitHubHostAuthenticated('github.acme-corp.com', '/repo') + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + }) + + it('does not share cache state across WSL distros', async () => { + mockHostAuthenticated() + + await isGitHubHostAuthenticated('github.acme-corp.com', '/repo', null, { wslDistro: 'Ubuntu' }) + await isGitHubHostAuthenticated('github.acme-corp.com', '/repo', null, { wslDistro: 'Debian' }) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2) + }) + + it('treats a listed host as authenticated even when gh exits non-zero', async () => { + ghExecFileAsyncMock.mockRejectedValue( + Object.assign(new Error('exit 1'), { + stdout: '', + stderr: + 'github.acme-corp.com\n ✓ Logged in to github.acme-corp.com account kelora (keyring)\n X github.com: token expired' + }) + ) + + await expect(isGitHubHostAuthenticated('github.acme-corp.com', '/repo')).resolves.toBe(true) + }) + + it('does not cache a hard gh failure so a later probe can recover', async () => { + ghExecFileAsyncMock.mockRejectedValueOnce( + Object.assign(new Error('not installed'), { stdout: '', stderr: '' }) + ) + expect(await isGitHubHostAuthenticated('github.acme-corp.com', '/repo')).toBe(false) + + mockHostAuthenticated() + expect(await isGitHubHostAuthenticated('github.acme-corp.com', '/repo')).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..94cffe725fa --- /dev/null +++ b/src/main/github/github-enterprise-repository.ts @@ -0,0 +1,130 @@ +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 { + ghRepoExecOptions, + getRemoteUrlForRepo, + githubRepoContext, + parseGitHubRemoteIdentity, + type LocalGitExecOptions +} from './github-repository-identity' + +export type GitHubEnterpriseRepoSlug = GitHubOwnerRepo & { host: string } + +// Why: `gh` only ever manages github.com / GitHub Enterprise credentials, so a +// host `gh auth status` reports as logged-in is definitively a GitHub host. This +// mirrors the `glab auth status` signal GitLab self-hosted detection uses, so a +// GHES remote is not left to fall through to Gitea (#8312). +const HOST_AUTH_TTL_MS = 60_000 + +type HostAuthCacheEntry = { + authenticated: boolean + expiresAt: number +} + +const hostAuthCache = new Map() + +// Why: gh's authenticated hosts live in per-runtime config — a WSL distro and an +// SSH host each carry their own `hosts.yml` — so cache state must be keyed by the +// runtime that executes gh, not shared under one "local" bucket. Mirrors the +// runtime scoping used by owner/repo resolution. +function runtimeCacheKey(connectionId?: string | null, wslDistro?: string): string { + return connectionId ?? `local:${wslDistro ?? 'host'}` +} + +/** @internal - exposed for tests only */ +export function _resetGitHubHostAuthCache(): void { + hostAuthCache.clear() +} + +// Only gh's own stdout/stderr — not the Error.message — counts as an +// authoritative answer. A spawn failure (gh missing, ENOENT) carries just a +// message and no command output, and must stay indeterminate rather than be +// read as "host not authenticated". +function ghCommandOutput(error: unknown): string { + const execErr = error as { stdout?: unknown; stderr?: unknown } + return [execErr?.stdout, execErr?.stderr] + .filter((value): value is string => typeof value === 'string' && value.trim().length > 0) + .join('\n') +} + +/** + * Whether `gh` is authenticated to `host` from the repository's own runtime. + * + * The probe runs `gh auth status --hostname ` with the repo's execution + * options (cwd / WSL distro, or SSH-local like the create path), so a GHES login + * stored only in that runtime's gh config — or a `GH_ENTERPRISE_TOKEN` inferred + * from it — is honored instead of the host/default-distro gh. Cached briefly per + * runtime+host so provider-detection polling does not re-spawn gh each time. + */ +export async function isGitHubHostAuthenticated( + host: string, + repoPath: string, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const normalizedHost = host.toLowerCase() + const cacheKey = `${runtimeCacheKey(connectionId, localGitOptions.wslDistro)}\0${normalizedHost}` + const now = Date.now() + const cached = hostAuthCache.get(cacheKey) + if (cached && cached.expiresAt > now) { + return cached.authenticated + } + const execOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)) + let authenticated: boolean + try { + await ghExecFileAsync(['auth', 'status', '--hostname', normalizedHost], execOptions) + authenticated = true + } catch (error) { + const output = ghCommandOutput(error) + if (!output) { + // Indeterminate (gh missing / spawn failure) — do not cache so a later + // probe (gh installed, tunnel ready, token added) can recover. + return false + } + // gh exits non-zero when a host has a token problem but still prints the + // per-host status; treat the host as GitHub only when it is actually listed. + authenticated = parseAuthStatus(output).some( + (account) => account.host.toLowerCase() === normalizedHost + ) + } + hostAuthCache.set(cacheKey, { authenticated, expiresAt: now + HOST_AUTH_TTL_MS }) + return authenticated +} + +/** + * 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 localGitOptions = getHostedReviewLocalGitOptions(options) + const context = githubRepoContext(repoPath, connectionId, localGitOptions) + 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 authenticated = await isGitHubHostAuthenticated( + identity.host, + repoPath, + connectionId, + localGitOptions + ) + return authenticated ? { 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..3ae82fd4010 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,29 @@ 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' + }) + + // 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() + }) + it('creates a GitLab merge request after fresh main-process validation passes', async () => { mockGitLabProvider() @@ -636,6 +682,31 @@ 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 + }) + + // 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 () => { 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..01caac804d8 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,6 +60,14 @@ async function isGitHubAuthenticated( connectionId?: string | null, options: HostedReviewExecutionOptions = {} ): Promise { + // 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(