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

Thread GitHub host through issue detail requests

Browse Source 
Incorporates ghes-issue-host-support (ed6bb96ef): one hosted issue
repository identity is resolved before the details fan-out so comments,
timeline, participants, and mention lookups cannot drift across hosts,
with SSH guards so unresolved issue/PR repositories never fall through
to gh's default host.

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinjing
2026-07-16 22:32:04 -07:00
co-authored by Orca
parent 756979215c
commit 215d42e597
2 changed files with 149 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/github/work-item-details.test.ts
+95 -15
View File
@@ -7,8 +7,9 @@ type RateLimitGuardResult =
const {
ghExecFileAsyncMock,
getOwnerRepoMock,
getIssueOwnerRepoMock,
getOwnerRepoForRemoteMock,
getEnterpriseGitHubRepoSlugMock,
getEnterpriseGitHubRepoSlugForRemoteMock,
getWorkItemMock,
getWorkItemByOwnerRepoMock,
getPRChecksMock,
@@ -24,8 +25,9 @@ const {
} = vi.hoisted(() => ({
ghExecFileAsyncMock: vi.fn(),
getOwnerRepoMock: vi.fn(),
getIssueOwnerRepoMock: vi.fn(),
getOwnerRepoForRemoteMock: vi.fn(),
getEnterpriseGitHubRepoSlugMock: vi.fn(),
getEnterpriseGitHubRepoSlugForRemoteMock: vi.fn(),
getWorkItemMock: vi.fn(),
getWorkItemByOwnerRepoMock: vi.fn(),
getPRChecksMock: vi.fn(),
@@ -51,7 +53,7 @@ const {
vi.mock('./gh-utils', () => ({
ghExecFileAsync: ghExecFileAsyncMock,
getOwnerRepo: getOwnerRepoMock,
getIssueOwnerRepo: getIssueOwnerRepoMock,
getOwnerRepoForRemote: getOwnerRepoForRemoteMock,
ghRepoExecOptions: ghRepoExecOptionsMock,
githubRepoContext: githubRepoContextMock,
acquire: acquireMock,
@@ -66,7 +68,8 @@ vi.mock('./client', () => ({
}))
vi.mock('./github-enterprise-repository', () => ({
getEnterpriseGitHubRepoSlug: getEnterpriseGitHubRepoSlugMock
getEnterpriseGitHubRepoSlug: getEnterpriseGitHubRepoSlugMock,
getEnterpriseGitHubRepoSlugForRemote: getEnterpriseGitHubRepoSlugForRemoteMock
}))
vi.mock('./rate-limit', () => ({
@@ -90,9 +93,11 @@ describe('getWorkItemDetails', () => {
beforeEach(() => {
ghExecFileAsyncMock.mockReset()
getOwnerRepoMock.mockReset()
getIssueOwnerRepoMock.mockReset()
getOwnerRepoForRemoteMock.mockReset()
getEnterpriseGitHubRepoSlugMock.mockReset()
getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null)
getEnterpriseGitHubRepoSlugForRemoteMock.mockReset()
getEnterpriseGitHubRepoSlugForRemoteMock.mockResolvedValue(null)
getWorkItemMock.mockReset()
getWorkItemByOwnerRepoMock.mockReset()
getPRChecksMock.mockReset()
@@ -122,7 +127,7 @@ describe('getWorkItemDetails', () => {
updatedAt: '2026-04-01T00:00:00Z',
author: 'issue-author'
})
getIssueOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
getOwnerRepoForRemoteMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
const timelineEvents = [
{
id: 101,
@@ -307,7 +312,7 @@ describe('getWorkItemDetails', () => {
updatedAt: '2026-04-01T00:00:00Z',
author: 'issue-author'
})
getIssueOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
getOwnerRepoForRemoteMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
const makeTimelineEvent = (page: number, index: number, event: string): string =>
JSON.stringify({
id: `${page}:${index}`,
@@ -383,7 +388,7 @@ describe('getWorkItemDetails', () => {
updatedAt: '2026-04-01T00:00:00Z',
author: 'issue-author'
})
getIssueOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
getOwnerRepoForRemoteMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
// Collapsed GraphQL throws → fallback path picks up.
ghExecFileAsyncMock
.mockRejectedValueOnce(new Error('GraphQL error'))
@@ -404,12 +409,12 @@ describe('getWorkItemDetails', () => {
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
2,
['api', '--cache', '60s', 'repos/acme/widgets/issues/923'],
{ cwd: '/repo-root' }
{ cwd: '/repo-root', host: 'github.com' }
)
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
3,
['api', '--cache', '60s', 'repos/acme/widgets/issues/923/comments?per_page=100'],
{ cwd: '/repo-root' }
{ cwd: '/repo-root', host: 'github.com' }
)
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
4,
@@ -421,7 +426,7 @@ describe('getWorkItemDetails', () => {
'--jq',
'.[] | @json'
],
{ cwd: '/repo-root' }
{ cwd: '/repo-root', host: 'github.com' }
)
expect(details?.body).toBe('Issue body')
})
@@ -452,7 +457,7 @@ describe('getWorkItemDetails', () => {
updatedAt: '2026-04-01T00:00:00Z',
author: 'issue-author'
})
getIssueOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
getOwnerRepoForRemoteMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
ghExecFileAsyncMock
.mockResolvedValueOnce({ stdout: JSON.stringify({ body: 'Issue body', assignees: [] }) })
.mockResolvedValueOnce({ stdout: '[]' })
@@ -480,7 +485,7 @@ describe('getWorkItemDetails', () => {
updatedAt: '2026-04-01T00:00:00Z',
author: 'issue-author'
})
getIssueOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
getOwnerRepoForRemoteMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
ghExecFileAsyncMock
.mockResolvedValueOnce({
stdout: JSON.stringify({
@@ -501,11 +506,86 @@ describe('getWorkItemDetails', () => {
const details = await getWorkItemDetails('/home/tester/widgets', 923, 'issue', 'ssh-test-1')
expect(getWorkItemMock).toHaveBeenCalledWith('/home/tester/widgets', 923, 'issue', 'ssh-test-1')
expect(getIssueOwnerRepoMock).toHaveBeenCalledWith('/home/tester/widgets', 'ssh-test-1')
expect(ghExecFileAsyncMock.mock.calls[0][1]).toEqual({})
expect(getOwnerRepoForRemoteMock).toHaveBeenCalledWith(
'/home/tester/widgets',
'upstream',
'ssh-test-1',
{}
)
expect(ghExecFileAsyncMock.mock.calls[0][1]).toEqual({ host: 'github.com' })
expect(details?.body).toBe('Remote issue body')
})
it('uses the GitHub Enterprise host for SSH-backed issue work item details', async () => {
const enterpriseRepository = {
owner: 'team',
repo: 'orca',
host: 'github.acme-corp.com'
}
getWorkItemMock.mockResolvedValueOnce({
id: 'issue:7',
type: 'issue',
number: 7,
title: 'Enterprise issue',
state: 'open',
url: 'https://github.acme-corp.com/team/orca/issues/7',
labels: [],
updatedAt: '2026-07-16T00:00:00Z',
author: 'issue-author'
})
getOwnerRepoMock.mockResolvedValue(null)
getEnterpriseGitHubRepoSlugMock.mockResolvedValue(enterpriseRepository)
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
const query = args.find((arg) => arg.startsWith('query=')) ?? ''
if (query.includes('comments(first: 100)')) {
return {
stdout: JSON.stringify({
data: {
repository: {
issue: {
body: 'Enterprise issue body',
assignees: { nodes: [] },
participants: { nodes: [] },
comments: { nodes: [] }
}
}
}
})
}
}
const endpoint = args.find((arg) => arg.startsWith('repos/')) ?? ''
if (endpoint === 'repos/team/orca/issues/7/timeline?per_page=100&page=1') {
return { stdout: '' }
}
throw new Error(`unexpected gh call: ${args.join(' ')}`)
})
const details = await getWorkItemDetails('/remote/repo', 7, 'issue', 'ssh-1')
expect(details?.body).toBe('Enterprise issue body')
expect(getWorkItemMock).toHaveBeenCalledWith('/remote/repo', 7, 'issue', 'ssh-1')
expect(getWorkItemByOwnerRepoMock).not.toHaveBeenCalled()
expect(getEnterpriseGitHubRepoSlugMock).toHaveBeenCalledTimes(1)
expect(repositoryRateLimitGuardMock).toHaveBeenCalledWith(enterpriseRepository, 'graphql', {})
expect(noteRepositoryRateLimitSpendMock).toHaveBeenCalled()
expect(
ghExecFileAsyncMock.mock.calls.every(
([, options]) => options?.host === 'github.acme-corp.com'
)
).toBe(true)
})
it('does not query the default host when an SSH issue repository is unresolved', async () => {
getOwnerRepoMock.mockResolvedValue(null)
getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null)
await expect(getWorkItemDetails('/remote/repo', 7, 'issue', 'ssh-1')).resolves.toBeNull()
expect(getWorkItemMock).not.toHaveBeenCalled()
expect(getWorkItemByOwnerRepoMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
})
it('routes local WSL PR detail fan-out through the selected distro', async () => {
const localGitOptions = { wslDistro: 'Ubuntu' }
rateLimitGuardMock.mockReturnValue({
src/main/github/work-item-details.ts
+54 -38
View File
@@ -17,24 +17,19 @@ import {
ghExecFileAsync,
acquire,
release,
getIssueOwnerRepo,
ghRepoExecOptions,
githubRepoContext,
type LocalGitExecOptions
} from './gh-utils'
import { getWorkItem, getWorkItemByOwnerRepo, getPRChecks, getPRComments } from './client'
import {
getIssueGitHubApiRepository,
getOriginGitHubApiRepository,
githubHostExecOptions,
isGitHubDotComRepository,
type GitHubApiRepository
} from './github-api-repository'
import {
noteRateLimitSpend,
noteRepositoryRateLimitSpend,
rateLimitGuard,
repositoryRateLimitGuard
} from './rate-limit'
import { noteRepositoryRateLimitSpend, repositoryRateLimitGuard } from './rate-limit'
import { getPRReviewCommentLineNumbersFromPatch } from './pr-review-comment-lines'
import { isMaxBufferOverflowError } from '../git/max-buffer-overflow'
@@ -344,6 +339,7 @@ async function getIssueTimelineItems(
async function getIssueDetailsViaGraphQL(
repoPath: string,
issueNumber: number,
ownerRepo: GitHubApiRepository | null,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{
@@ -357,20 +353,18 @@ async function getIssueDetailsViaGraphQL(
participants: GitHubAssignableUser[]
timelineItems: GitHubIssueTimelineItem[]
} | null> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getIssueOwnerRepo(
repoPath,
connectionId,
...localGitOptionArgs(localGitOptions)
)
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
if (!ownerRepo) {
return null
}
if (rateLimitGuard('graphql').blocked) {
if (repositoryRateLimitGuard(ownerRepo, 'graphql', localGitOptions).blocked) {
return null
}
try {
noteRateLimitSpend('graphql')
noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, localGitOptions)
const { stdout } = await ghExecFileAsync(
[
'api',
@@ -711,6 +705,7 @@ function mergePRFileViewedStates(
async function getIssueBodyAndComments(
repoPath: string,
issueNumber: number,
ownerRepo: GitHubApiRepository | null,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{
@@ -719,12 +714,10 @@ async function getIssueBodyAndComments(
assignees: string[]
timelineItems: GitHubIssueTimelineItem[]
}> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getIssueOwnerRepo(
repoPath,
connectionId,
...localGitOptionArgs(localGitOptions)
)
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
try {
if (ownerRepo) {
const [issueResult, commentsResult, timelineItems] = await Promise.all([
@@ -773,6 +766,11 @@ async function getIssueBodyAndComments(
const assignees = (issue.assignees ?? []).map((a) => a.login)
return { body: issue.body ?? '', comments, assignees, timelineItems }
}
if (connectionId) {
// Why: connection-backed gh has no cwd. A bare issue lookup could honor
// process GH_REPO/GH_HOST and return an unrelated repository's issue.
return { body: '', comments: [], assignees: [], timelineItems: [] }
}
// Fallback: non-GitHub remote
const { stdout } = await ghExecFileAsync(
['issue', 'view', String(issueNumber), '--json', 'body,comments,assignees'],
@@ -812,13 +810,9 @@ async function getWorkItemParticipants(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubAssignableUser[]> {
// Why: issues in a fork live on the upstream remote, so participants must be
// resolved via getIssueOwnerRepo to stay consistent with getIssueBodyAndComments.
// PRs remain tied to origin via getOwnerRepo.
const ownerRepo =
item.type === 'issue'
? await getIssueOwnerRepo(repoPath, connectionId, ...localGitOptionArgs(localGitOptions))
: resolvedRepository
// Why: one repository identity is resolved before the details fan-out so
// comments, participants, mentions, and PR files cannot drift across hosts.
const ownerRepo = resolvedRepository
if (!ownerRepo) {
return []
}
@@ -1090,6 +1084,15 @@ export async function getWorkItemDetails(
if (connectionId && type !== 'issue' && !originApiRepository) {
return null
}
const issueApiRepository =
type === 'issue'
? await getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions)
: null
// Why: the same cwd-less fallthrough exists for issues — an unresolved issue
// repository over SSH must not surface a same-numbered github.com issue.
if (connectionId && type === 'issue' && !issueApiRepository) {
return null
}
// Why: connection-backed repos have no local cwd for gh to inspect, so GHES
// must use its host-qualified repository identity during the initial lookup.
const useExplicitEnterpriseRepository =
@@ -1125,6 +1128,12 @@ export async function getWorkItemDetails(
return null
}
const resolvedRepository =
item.type === 'issue'
? (issueApiRepository ??
(await getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions)))
: (item.prRepo ?? originApiRepository)
await acquire()
try {
if (item.type === 'issue') {
@@ -1138,6 +1147,7 @@ export async function getWorkItemDetails(
const collapsed = await getIssueDetailsViaGraphQL(
repoPath,
item.number,
resolvedRepository,
connectionId,
localGitOptions
)
@@ -1160,15 +1170,21 @@ export async function getWorkItemDetails(
// Why: fall back to body/comments and GraphQL participants in parallel;
// the mention-participant merge is a cheap local operation afterward.
const [{ body, comments, assignees, timelineItems }, participants] = await Promise.all([
getIssueBodyAndComments(repoPath, item.number, connectionId, localGitOptions),
getWorkItemParticipants(repoPath, item, null, connectionId, localGitOptions)
getIssueBodyAndComments(
repoPath,
item.number,
resolvedRepository,
connectionId,
localGitOptions
),
getWorkItemParticipants(repoPath, item, resolvedRepository, connectionId, localGitOptions)
])
const mentionParticipants = await getMentionParticipants(
repoPath,
item,
comments,
participants,
null,
resolvedRepository,
connectionId,
localGitOptions
)
@@ -1184,23 +1200,23 @@ export async function getWorkItemDetails(
// PR: fetch metadata + comments + files + viewed states + participants in parallel.
const [metadata, comments, files, viewedStates, participants] = await Promise.all([
getPRMetadata(repoPath, item.number, originApiRepository, connectionId, localGitOptions),
getPRMetadata(repoPath, item.number, resolvedRepository, connectionId, localGitOptions),
getPRComments(
repoPath,
item.number,
{ prRepo: originApiRepository },
{ prRepo: resolvedRepository },
connectionId,
...localGitOptionArgs(localGitOptions)
),
getPRFiles(repoPath, item.number, originApiRepository, connectionId, localGitOptions),
getPRFiles(repoPath, item.number, resolvedRepository, connectionId, localGitOptions),
getPRFileViewedStates(
repoPath,
item.number,
originApiRepository,
resolvedRepository,
connectionId,
localGitOptions
),
getWorkItemParticipants(repoPath, item, originApiRepository, connectionId, localGitOptions)
getWorkItemParticipants(repoPath, item, resolvedRepository, connectionId, localGitOptions)
])
// Why: run the mention-author GraphQL lookup in parallel with the final
@@ -1212,7 +1228,7 @@ export async function getWorkItemDetails(
item,
comments,
participants,
originApiRepository,
resolvedRepository,
connectionId,
localGitOptions
),
@@ -1220,7 +1236,7 @@ export async function getWorkItemDetails(
repoPath,
item.number,
metadata.headSha,
originApiRepository,
resolvedRepository,
connectionId,
localGitOptions
)