From c89313cff25c1246861b7f22be21dd9dbfbb14f9 Mon Sep 17 00:00:00 2001 From: wonjerry Date: Thu, 16 Jul 2026 10:22:28 +0900 Subject: [PATCH] fix(github): load PR diffs for Enterprise remotes --- src/main/github/work-item-details.test.ts | 152 +++++++++++++++++++++- src/main/github/work-item-details.ts | 104 ++++++++++----- 2 files changed, 221 insertions(+), 35 deletions(-) diff --git a/src/main/github/work-item-details.test.ts b/src/main/github/work-item-details.test.ts index 4fd21131c23..cc46b2b3091 100644 --- a/src/main/github/work-item-details.test.ts +++ b/src/main/github/work-item-details.test.ts @@ -8,6 +8,7 @@ const { ghExecFileAsyncMock, getOwnerRepoMock, getIssueOwnerRepoMock, + getEnterpriseGitHubRepoSlugMock, getWorkItemMock, getPRChecksMock, getPRCommentsMock, @@ -21,6 +22,7 @@ const { ghExecFileAsyncMock: vi.fn(), getOwnerRepoMock: vi.fn(), getIssueOwnerRepoMock: vi.fn(), + getEnterpriseGitHubRepoSlugMock: vi.fn(), getWorkItemMock: vi.fn(), getPRChecksMock: vi.fn(), getPRCommentsMock: vi.fn(), @@ -56,18 +58,24 @@ vi.mock('./client', () => ({ getPRComments: getPRCommentsMock })) +vi.mock('./github-enterprise-repository', () => ({ + getEnterpriseGitHubRepoSlug: getEnterpriseGitHubRepoSlugMock +})) + vi.mock('./rate-limit', () => ({ rateLimitGuard: rateLimitGuardMock, noteRateLimitSpend: noteRateLimitSpendMock })) -import { getWorkItemDetails } from './work-item-details' +import { getPRFileContents, getWorkItemDetails } from './work-item-details' describe('getWorkItemDetails', () => { beforeEach(() => { ghExecFileAsyncMock.mockReset() getOwnerRepoMock.mockReset() getIssueOwnerRepoMock.mockReset() + getEnterpriseGitHubRepoSlugMock.mockReset() + getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null) getWorkItemMock.mockReset() getPRChecksMock.mockReset() getPRCommentsMock.mockReset() @@ -549,6 +557,148 @@ describe('getWorkItemDetails', () => { ) }) + it('uses the GitHub Enterprise host for PR files in work item details', async () => { + getWorkItemMock.mockResolvedValueOnce({ + id: 'pr:7', + type: 'pr', + number: 7, + title: 'Enterprise PR files', + state: 'open', + url: 'https://github.acme-corp.com/team/orca/pull/7', + labels: [], + updatedAt: '2026-07-16T00:00:00Z', + author: 'pr-author' + }) + getOwnerRepoMock.mockResolvedValue(null) + getEnterpriseGitHubRepoSlugMock.mockResolvedValue({ + owner: 'team', + repo: 'orca', + host: 'github.acme-corp.com' + }) + getPRCommentsMock.mockResolvedValue([]) + getPRChecksMock.mockResolvedValue([]) + ghExecFileAsyncMock.mockImplementation(async (args: string[]) => { + const endpoint = args.find((arg) => arg.startsWith('repos/')) ?? '' + if (endpoint === 'repos/team/orca/pulls/7') { + return { + stdout: JSON.stringify({ + body: 'Enterprise PR body', + head: { sha: 'head-sha' }, + base: { sha: 'base-sha' } + }) + } + } + if (endpoint === 'repos/team/orca/pulls/7/files?per_page=100') { + return { + stdout: JSON.stringify([ + { + filename: 'src/enterprise.ts', + status: 'modified', + additions: 2, + deletions: 1, + changes: 3, + patch: '@@ -1 +1 @@' + } + ]) + } + } + const query = args.find((arg) => arg.startsWith('query=')) ?? '' + if (query.includes('viewerViewedState')) { + return { + stdout: JSON.stringify({ + data: { + repository: { + pullRequest: { + id: 'PR_enterprise', + files: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [{ path: 'src/enterprise.ts', viewerViewedState: 'VIEWED' }] + } + } + } + } + }) + } + } + if (query.includes('participants(first: 100)')) { + return { + stdout: JSON.stringify({ + data: { repository: { pullRequest: { participants: { nodes: [] } } } } + }) + } + } + throw new Error(`unexpected gh call: ${args.join(' ')}`) + }) + + const details = await getWorkItemDetails('/repo-root', 7, 'pr') + + expect(details?.body).toBe('Enterprise PR body') + expect(details?.headSha).toBe('head-sha') + expect(details?.baseSha).toBe('base-sha') + expect(details?.filesUnavailable).toBe(false) + expect(details?.files).toEqual([ + { + path: 'src/enterprise.ts', + oldPath: undefined, + status: 'modified', + additions: 2, + deletions: 1, + isBinary: false, + reviewCommentLineNumbers: [], + viewerViewedState: 'VIEWED' + } + ]) + const apiCalls = ghExecFileAsyncMock.mock.calls + .map(([args]) => args as string[]) + .filter((args) => args[0] === 'api') + expect(apiCalls.length).toBeGreaterThan(0) + expect(apiCalls.every((args) => args.includes('--hostname'))).toBe(true) + expect( + apiCalls.every((args) => args[args.indexOf('--hostname') + 1] === 'github.acme-corp.com') + ).toBe(true) + }) + + it('uses the GitHub Enterprise host when fetching PR file contents', async () => { + getOwnerRepoMock.mockResolvedValue(null) + getEnterpriseGitHubRepoSlugMock.mockResolvedValue({ + owner: 'team', + repo: 'orca', + host: 'github.acme-corp.com' + }) + ghExecFileAsyncMock.mockImplementation(async (args: string[]) => { + const endpoint = args.find((arg) => arg.startsWith('repos/')) ?? '' + if (endpoint === 'repos/team/orca/contents/src/enterprise.ts?ref=base-sha') { + return { stdout: 'base content' } + } + if (endpoint === 'repos/team/orca/contents/src/enterprise.ts?ref=head-sha') { + return { stdout: 'head content' } + } + throw new Error(`unexpected gh call: ${args.join(' ')}`) + }) + + const contents = await getPRFileContents({ + repoPath: '/repo-root', + prNumber: 7, + path: 'src/enterprise.ts', + status: 'modified', + headSha: 'head-sha', + baseSha: 'base-sha' + }) + + expect(contents).toMatchObject({ + original: 'base content', + modified: 'head content', + originalIsBinary: false, + modifiedIsBinary: false + }) + const apiCalls = ghExecFileAsyncMock.mock.calls.map(([args]) => args as string[]) + expect(apiCalls).toHaveLength(2) + expect(apiCalls.every((args) => args.includes('--hostname'))).toBe(true) + expect( + apiCalls.every((args) => args[args.indexOf('--hostname') + 1] === 'github.acme-corp.com') + ).toBe(true) + }) + // Why: a rate-limited/auth-failed file fetch must not render as an empty PR; // the Files tab keys its retry state off details.filesUnavailable. it('flags filesUnavailable when the PR file fetch fails but leaves the PR empty otherwise intact', async () => { diff --git a/src/main/github/work-item-details.ts b/src/main/github/work-item-details.ts index 87cdbe52eaa..17fe68745d8 100644 --- a/src/main/github/work-item-details.ts +++ b/src/main/github/work-item-details.ts @@ -22,6 +22,7 @@ import { type LocalGitExecOptions } from './gh-utils' import { getWorkItem, getPRChecks, getPRComments } from './client' +import { getEnterpriseGitHubRepoSlug } from './github-enterprise-repository' import { noteRateLimitSpend, rateLimitGuard } from './rate-limit' import { getPRReviewCommentLineNumbersFromPatch } from './pr-review-comment-lines' import { isMaxBufferOverflowError } from '../git/max-buffer-overflow' @@ -38,6 +39,36 @@ function localGitOptionArgs(options: LocalGitExecOptions = {}): [] | [LocalGitEx return Object.keys(options).length > 0 ? [options] : [] } +function hostedReviewLocalGitOptions(options: LocalGitExecOptions = {}): { + localGitExecOptions?: LocalGitExecOptions +} { + return Object.keys(options).length > 0 ? { localGitExecOptions: options } : {} +} + +async function getOriginApiOwnerRepo( + repoPath: string, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const ownerRepo = await getOwnerRepo( + repoPath, + connectionId, + ...localGitOptionArgs(localGitOptions) + ) + return ( + ownerRepo ?? + (await getEnterpriseGitHubRepoSlug( + repoPath, + connectionId, + hostedReviewLocalGitOptions(localGitOptions) + )) + ) +} + +function ghApiHostArgs(ownerRepo: GitHubOwnerRepoSlug): string[] { + return ownerRepo.host && ownerRepo.host !== 'github.com' ? ['--hostname', ownerRepo.host] : [] +} + const PR_FILE_VIEWED_STATES_QUERY = `query($owner: String!, $repo: String!, $number: Int!, $after: String) { repository(owner: $owner, name: $repo) { pullRequest(number: $number) { @@ -120,7 +151,7 @@ type GraphQLIssueDetailsResponse = { errors?: { message?: string }[] } -type GitHubOwnerRepoSlug = { owner: string; repo: string } +type GitHubOwnerRepoSlug = { owner: string; repo: string; host?: string } type RestTimelineUser = { login?: string | null @@ -280,6 +311,7 @@ async function getIssueTimelineItems( const { stdout } = await ghExecFileAsync( [ 'api', + ...ghApiHostArgs(ownerRepo), '--cache', '60s', `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}/timeline?per_page=${GITHUB_REST_PAGE_SIZE}&page=${page}`, @@ -483,15 +515,17 @@ async function getPRHeadBaseSha( localGitOptions: LocalGitExecOptions = {} ): Promise<{ headSha: string; baseSha: string } | null> { const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)) - const ownerRepo = await getOwnerRepo( - repoPath, - connectionId, - ...localGitOptionArgs(localGitOptions) - ) + const ownerRepo = await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions) try { if (ownerRepo) { const { stdout } = await ghExecFileAsync( - ['api', '--cache', '60s', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}`], + [ + 'api', + ...ghApiHostArgs(ownerRepo), + '--cache', + '60s', + `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}` + ], ghOptions ) const data = JSON.parse(stdout) as { @@ -528,11 +562,7 @@ async function getPRFiles( localGitOptions: LocalGitExecOptions = {} ): Promise { const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)) - const ownerRepo = await getOwnerRepo( - repoPath, - connectionId, - ...localGitOptionArgs(localGitOptions) - ) + const ownerRepo = await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions) if (!ownerRepo) { return null } @@ -540,6 +570,7 @@ async function getPRFiles( const { stdout } = await ghExecFileAsync( [ 'api', + ...ghApiHostArgs(ownerRepo), '--cache', '60s', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}/files?per_page=100` @@ -573,11 +604,7 @@ async function getPRFileViewedStates( localGitOptions: LocalGitExecOptions = {} ): Promise { const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)) - const ownerRepo = await getOwnerRepo( - repoPath, - connectionId, - ...localGitOptionArgs(localGitOptions) - ) + const ownerRepo = await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions) if (!ownerRepo) { return null } @@ -592,6 +619,7 @@ async function getPRFileViewedStates( for (let fetched = 0; fetched < MAX_PR_FILES; fetched += 100) { const args = [ 'api', + ...ghApiHostArgs(ownerRepo), 'graphql', '-f', `query=${PR_FILE_VIEWED_STATES_QUERY}`, @@ -766,15 +794,17 @@ async function getPRBody( localGitOptions: LocalGitExecOptions = {} ): Promise { const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)) - const ownerRepo = await getOwnerRepo( - repoPath, - connectionId, - ...localGitOptionArgs(localGitOptions) - ) + const ownerRepo = await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions) try { if (ownerRepo) { const { stdout } = await ghExecFileAsync( - ['api', '--cache', '60s', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}`], + [ + 'api', + ...ghApiHostArgs(ownerRepo), + '--cache', + '60s', + `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}` + ], ghOptions ) const data = JSON.parse(stdout) as { body?: string | null } @@ -801,7 +831,7 @@ async function getWorkItemParticipants( const ownerRepo = item.type === 'issue' ? await getIssueOwnerRepo(repoPath, connectionId, ...localGitOptionArgs(localGitOptions)) - : await getOwnerRepo(repoPath, connectionId, ...localGitOptionArgs(localGitOptions)) + : await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions) if (!ownerRepo) { return [] } @@ -813,6 +843,7 @@ async function getWorkItemParticipants( const { stdout } = await ghExecFileAsync( [ 'api', + ...ghApiHostArgs(ownerRepo), 'graphql', '-f', `query=${WORK_ITEM_PARTICIPANTS_QUERY}`, @@ -874,6 +905,7 @@ async function getGitHubUsersByLogin( ) return [] } + const ownerRepo = await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions) const fields = uniqueLogins .map( (login, index) => @@ -883,7 +915,13 @@ async function getGitHubUsersByLogin( try { noteRateLimitSpend('graphql') const { stdout } = await ghExecFileAsync( - ['api', 'graphql', '-f', `query=query { ${fields} }`], + [ + 'api', + ...(ownerRepo ? ghApiHostArgs(ownerRepo) : []), + 'graphql', + '-f', + `query=query { ${fields} }` + ], ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)) ) const data = JSON.parse(stdout) as { @@ -1134,8 +1172,7 @@ async function fetchContentAtRef(args: { repoPath: string connectionId?: string | null localGitOptions?: LocalGitExecOptions - owner: string - repo: string + ownerRepo: GitHubOwnerRepoSlug path: string ref: string }): Promise<{ content: string; isBinary: boolean; tooLarge?: boolean }> { @@ -1147,7 +1184,8 @@ async function fetchContentAtRef(args: { '300s', '-H', 'Accept: application/vnd.github.raw', - `repos/${args.owner}/${args.repo}/contents/${encodeURI(args.path)}?ref=${encodeURIComponent(args.ref)}` + ...ghApiHostArgs(args.ownerRepo), + `repos/${args.ownerRepo.owner}/${args.ownerRepo.repo}/contents/${encodeURI(args.path)}?ref=${encodeURIComponent(args.ref)}` ], { ...ghRepoExecOptions( @@ -1181,10 +1219,10 @@ export async function getPRFileContents(args: { headSha: string baseSha: string }): Promise { - const ownerRepo = await getOwnerRepo( + const ownerRepo = await getOriginApiOwnerRepo( args.repoPath, args.connectionId, - ...localGitOptionArgs(args.localGitOptions) + args.localGitOptions ) if (!ownerRepo) { return { @@ -1209,8 +1247,7 @@ export async function getPRFileContents(args: { repoPath: args.repoPath, connectionId: args.connectionId, localGitOptions: args.localGitOptions, - owner: ownerRepo.owner, - repo: ownerRepo.repo, + ownerRepo, path: originalPath, ref: originalRef }) @@ -1223,8 +1260,7 @@ export async function getPRFileContents(args: { repoPath: args.repoPath, connectionId: args.connectionId, localGitOptions: args.localGitOptions, - owner: ownerRepo.owner, - repo: ownerRepo.repo, + ownerRepo, path: args.path, ref: args.headSha })