starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-18 08:03:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(github): load PR diffs for Enterprise remotes

Browse Source
This commit is contained in:
wonjerry
2026-07-20 12:35:24 -07:00
committed by Jinjing
parent 4f81dfc128
commit c89313cff2
2 changed files with 221 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/github/work-item-details.test.ts
+151 -1
View File
@@ -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 () => {
src/main/github/work-item-details.ts
+70 -34
View File
@@ -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<GitHubOwnerRepoSlug | null> {
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<GitHubPRFile[] | null> {
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<PRFileViewedStatesResult | null> {
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<string> {
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<GitHubPRFileContents> {
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
})