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

Route gh host qualification through runner options instead of argv sniff

Browse Source 
Move GHES/GH_HOST resolution from parsing --hostname/--repo out of gh argv to an explicit options.host passed through ghExecFileAsync, since SSH-backed repos spawn gh with no cwd and argv sniffing couldn't reliably detect the target host. The runner now injects --hostname and qualifies --repo/-R at spawn time from options.host, and rate-limit scoping/guards use the same explicit host instead of inferring it. Also adds a shared githubRepoIdentityKey helper to keep cache/store keys consistent with the new host-aware repository identity.
This commit is contained in:
Jinjing
2026-07-20 12:35:24 -07:00
parent 7b49c5739d
commit 93d8a04992
28 changed files with 587 additions and 327 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/git/runner-gh-host-args.test.ts
+106
View File
@@ -0,0 +1,106 @@
import { describe, expect, it } from 'vitest'
import { applyGhHostToArgs } from './runner'
describe('applyGhHostToArgs', () => {
it('returns args unchanged when no host is given', () => {
const args = ['api', 'repos/a/b/pulls']
expect(applyGhHostToArgs(args)).toBe(args)
expect(applyGhHostToArgs(['pr', 'list', '--repo', 'a/b'])).toEqual([
'pr',
'list',
'--repo',
'a/b'
])
})
it('injects --hostname right after `api` for any host', () => {
expect(applyGhHostToArgs(['api', 'rate_limit'], 'github.com')).toEqual([
'api',
'--hostname',
'github.com',
'rate_limit'
])
expect(applyGhHostToArgs(['api', 'repos/a/b/pulls'], 'github.acme-corp.com')).toEqual([
'api',
'--hostname',
'github.acme-corp.com',
'repos/a/b/pulls'
])
})
it('does not double-inject when --hostname is already present', () => {
expect(
applyGhHostToArgs(['api', '--hostname', 'github.com', 'rate_limit'], 'github.acme-corp.com')
).toEqual(['api', '--hostname', 'github.com', 'rate_limit'])
expect(
applyGhHostToArgs(['api', '--hostname=github.com', 'rate_limit'], 'github.acme-corp.com')
).toEqual(['api', '--hostname=github.com', 'rate_limit'])
})
it('qualifies bare owner/repo in --repo / -R for a GHES host', () => {
expect(applyGhHostToArgs(['pr', 'list', '--repo', 'a/b'], 'github.acme-corp.com')).toEqual([
'pr',
'list',
'--repo',
'github.acme-corp.com/a/b'
])
expect(applyGhHostToArgs(['pr', 'list', '-R', 'a/b'], 'github.acme-corp.com')).toEqual([
'pr',
'list',
'-R',
'github.acme-corp.com/a/b'
])
})
it('qualifies the inline --repo= form for a GHES host', () => {
expect(applyGhHostToArgs(['pr', 'list', '--repo=a/b'], 'github.acme-corp.com')).toEqual([
'pr',
'list',
'--repo=github.acme-corp.com/a/b'
])
})
it('leaves --repo untouched for the github.com host (gh default resolution)', () => {
expect(applyGhHostToArgs(['pr', 'list', '--repo', 'a/b'], 'github.com')).toEqual([
'pr',
'list',
'--repo',
'a/b'
])
expect(applyGhHostToArgs(['pr', 'list', '--repo', 'a/b'], 'GitHub.com')).toEqual([
'pr',
'list',
'--repo',
'a/b'
])
})
it('passes through URL and already-qualified 3-part --repo values', () => {
expect(
applyGhHostToArgs(
['pr', 'list', '--repo', 'https://github.acme-corp.com/a/b'],
'github.acme-corp.com'
)
).toEqual(['pr', 'list', '--repo', 'https://github.acme-corp.com/a/b'])
expect(
applyGhHostToArgs(
['pr', 'list', '--repo', 'github.acme-corp.com/a/b'],
'github.acme-corp.com'
)
).toEqual(['pr', 'list', '--repo', 'github.acme-corp.com/a/b'])
})
it('injects --hostname and qualifies --repo together for GHES api calls', () => {
expect(
applyGhHostToArgs(['api', 'repos/a/b/pulls', '--repo', 'a/b'], 'github.acme-corp.com')
).toEqual([
'api',
'--hostname',
'github.acme-corp.com',
'repos/a/b/pulls',
'--repo',
'github.acme-corp.com/a/b'
])
})
})
src/main/git/runner-gh-rate-limit-breaker.test.ts
+4 -2
View File
@@ -75,12 +75,14 @@ describe('ghExecFileAsync rate-limit breaker', () => {
mockGhFailure(PRIMARY_RATE_LIMIT_STDERR)
await expect(ghExecFileAsync(['api', 'repos/a/b/pulls'])).rejects.toThrow()
// The breaker scope now comes from options.host / GH_HOST / wslDistro, not
// from sniffing argv — so an Enterprise host must be routed via options.host.
mockGhSuccess('[]')
await expect(
ghExecFileAsync(['api', '--hostname', 'github.acme-corp.com', 'repos/a/b/pulls'])
ghExecFileAsync(['api', 'repos/a/b/pulls'], { host: 'github.acme-corp.com' })
).resolves.toMatchObject({ stdout: '[]' })
await expect(
ghExecFileAsync(['pr', 'list', '--repo', 'github.acme-corp.com/a/b'])
ghExecFileAsync(['pr', 'list', '--repo', 'a/b'], { host: 'github.acme-corp.com' })
).resolves.toMatchObject({ stdout: '[]' })
await expect(
ghExecFileAsync(['api', 'repos/a/b/pulls'], {
src/main/git/runner.ts
+49 -31
View File
@@ -1116,6 +1116,11 @@ type GhExecOptions = Omit<GitExecOptions, 'cwd'> & {
cwd?: string
wslDistro?: string
idempotent?: boolean
// Why: `gh api` and `--repo OWNER/REPO` shorthand resolve against gh's
// default host, not the repo's remote. Carrying the host here lets the
// runner qualify every spawn once, so call sites can't silently fall back
// to github.com for GHES repos; it also scopes the rate-limit breaker.
host?: string
}
const NON_IDEMPOTENT_METHODS = new Set(['POST', 'PATCH', 'PUT', 'DELETE'])
@@ -1253,42 +1258,54 @@ function nonInteractiveGhEnv(env: NodeJS.ProcessEnv = process.env): NodeJS.Proce
}
}
function ghFlagValue(args: readonly string[], names: readonly string[]): string | undefined {
for (const name of names) {
const index = args.indexOf(name)
if (index >= 0 && args[index + 1]) {
return args[index + 1]
}
const prefix = `${name}=`
const inline = args.find((arg) => arg.startsWith(prefix))
if (inline) {
return inline.slice(prefix.length)
}
}
return undefined
function hasGhHostnameFlag(args: readonly string[]): boolean {
return args.some((arg) => arg === '--hostname' || arg.startsWith('--hostname='))
}
function ghRequestHost(args: readonly string[], env?: NodeJS.ProcessEnv): string {
const explicitHost = ghFlagValue(args, ['--hostname'])
if (explicitHost) {
return explicitHost.toLowerCase()
function hostQualifiedGhRepoValue(value: string, host: string): string {
// URLs and already-qualified HOST/OWNER/REPO values pass through untouched.
if (value.includes('://') || value.split('/').length !== 2) {
return value
}
const repository = ghFlagValue(args, ['--repo', '-R'])
const repositoryParts = repository?.split('/') ?? []
if (repositoryParts.length >= 3) {
return repositoryParts[0].toLowerCase()
}
return (env?.GH_HOST ?? process.env.GH_HOST ?? 'github.com').toLowerCase()
return `${host}/${value}`
}
function ghRateLimitScope(
args: readonly string[],
options: GhExecOptions,
resolved: ResolvedCommand
): string {
/**
* Host-qualify a gh invocation from `options.host`: `--hostname` for `api`
* calls, `HOST/OWNER/REPO` for `--repo`/`-R` shorthand. SSH-backed repos run
* gh with no cwd, so this is their only host signal (#8312).
*
* @internal exported for tests.
*/
export function applyGhHostToArgs(args: string[], host?: string): string[] {
if (!host) {
return args
}
let result = args
if (result[0] === 'api' && !hasGhHostnameFlag(result)) {
result = ['api', '--hostname', host, ...result.slice(1)]
}
// github.com is gh's default resolution for bare OWNER/REPO shorthand.
if (host.toLowerCase() === 'github.com') {
return result
}
return result.map((arg, i) => {
const prev = result[i - 1]
if (prev === '--repo' || prev === '-R') {
return hostQualifiedGhRepoValue(arg, host)
}
if (arg.startsWith('--repo=')) {
return `--repo=${hostQualifiedGhRepoValue(arg.slice('--repo='.length), host)}`
}
return arg
})
}
function ghRateLimitScope(options: GhExecOptions, resolved: ResolvedCommand): string {
const distro = resolved.wsl?.distro ?? options.wslDistro
const runtime = distro ? `wsl:${distro.toLowerCase()}` : 'native'
return `${runtime}:${ghRequestHost(args, options.env)}`
const host = options.host ?? options.env?.GH_HOST ?? process.env.GH_HOST ?? 'github.com'
return `${runtime}:${host.toLowerCase()}`
}
/**
@@ -1302,11 +1319,12 @@ export async function ghExecFileAsync(
args: string[],
options: GhExecOptions = {}
): Promise<{ stdout: string; stderr: string }> {
args = applyGhHostToArgs(args, options.host)
let resolved = resolveCommand('gh', args, options.cwd, options.wslDistro)
// Why: while a bucket is rate-limited every spawn returns 403 — fail fast; the probe is exempt so the breaker can learn the reset.
// Why: scope by runtime and host so unrelated github.com, GHES, and WSL quotas cannot block each other.
const rateLimitBucket = classifyGhRateLimitBucket(args)
const initialRateLimitScope = ghRateLimitScope(args, options, resolved)
const initialRateLimitScope = ghRateLimitScope(options, resolved)
if (!isGhRateLimitProbe(args)) {
const blockedUntilMs = getGhRateLimitBlockedUntilMs(
rateLimitBucket,
@@ -1335,7 +1353,7 @@ export async function ghExecFileAsync(
lastError = err
const { stderr } = extractExecError(err)
if (isGhPrimaryRateLimitStderr(stderr)) {
notifyGhPrimaryRateLimit(rateLimitBucket, ghRateLimitScope(args, options, resolved))
notifyGhPrimaryRateLimit(rateLimitBucket, ghRateLimitScope(options, resolved))
}
if (
process.platform === 'win32' &&
src/main/github/client-create-pr.test.ts
+10 -8
View File
@@ -167,7 +167,7 @@ describe('createGitHubPullRequest', () => {
expect(args[args.indexOf('--head') + 1]).toBe('my-branch')
})
it('host-qualifies --repo for a GHES remote so gh targets the Enterprise server (#8312)', async () => {
it('routes --repo to the Enterprise server via options.host for a GHES remote (#8312)', async () => {
// github.com-only slug parsing misses GHES, so creation comes from the
// enterprise resolver, which carries the host.
getOwnerRepoMock.mockResolvedValueOnce(null)
@@ -194,13 +194,14 @@ describe('createGitHubPullRequest', () => {
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')
const [args, options] = ghExecFileAsyncMock.mock.calls[0]
// The runner host-qualifies argv at spawn time from options.host, so the
// mocked call sees a bare owner/repo plus the host in exec options.
expect(args[args.indexOf('--repo') + 1]).toBe('team/orca')
expect(options).toMatchObject({ host: 'github.acme-corp.com' })
})
it('host-qualifies --repo for the GHES existing-PR fallback lookup (#8312)', async () => {
it('routes the GHES existing-PR fallback lookup through options.host (#8312)', async () => {
getOwnerRepoMock.mockResolvedValue(null)
getEnterpriseGitHubRepoSlugMock.mockResolvedValue({
owner: 'team',
@@ -234,9 +235,10 @@ describe('createGitHubPullRequest', () => {
existingReview: { number: 9, url: 'https://github.acme-corp.com/team/orca/pull/9' }
})
const [listArgs] = ghExecFileAsyncMock.mock.calls[1]
const [listArgs, listOptions] = ghExecFileAsyncMock.mock.calls[1]
expect(listArgs).toEqual(expect.arrayContaining(['pr', 'list']))
expect(listArgs[listArgs.indexOf('--repo') + 1]).toBe('github.acme-corp.com/team/orca')
expect(listArgs[listArgs.indexOf('--repo') + 1]).toBe('team/orca')
expect(listOptions).toMatchObject({ host: 'github.acme-corp.com' })
})
it('runs local WSL project pull request creation through the selected distro', async () => {
src/main/github/client-file-viewed.test.ts
+6 -8
View File
@@ -134,15 +134,13 @@ describe('setPRFileViewed', () => {
})
).resolves.toBe(true)
// The runner injects --hostname at spawn time from options.host, so the
// mocked call sees unqualified argv plus the Enterprise host in options.
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
expect.arrayContaining([
'api',
'--hostname',
'github.acme-corp.com',
'graphql',
'pullRequestId=PR_enterprise'
]),
{}
expect.arrayContaining(['api', 'graphql', 'pullRequestId=PR_enterprise']),
{ host: 'github.acme-corp.com' }
)
const [args] = ghExecFileAsyncMock.mock.calls[0]
expect(args).not.toContain('--hostname')
})
})
src/main/github/client-issue-source.test.ts
+4 -1
View File
@@ -44,7 +44,10 @@ vi.mock('./gh-utils', async () => {
vi.mock('./rate-limit', () => ({
rateLimitGuard: rateLimitGuardMock,
noteRateLimitSpend: noteRateLimitSpendMock,
getRateLimit: vi.fn(async () => ({ ok: false, error: 'not probed in tests' }))
getRateLimit: vi.fn(async () => ({ ok: false, error: 'not probed in tests' })),
repositoryRateLimitGuard: vi.fn(() => ({ blocked: false })),
noteRepositoryRateLimitSpend: vi.fn(),
spendsSharedGitHubComQuota: () => true
}))
import { countWorkItems, getWorkItem, listWorkItems, _resetOwnerRepoCache } from './client'
src/main/github/client-pr-check-details.test.ts
+7 -4
View File
@@ -28,7 +28,10 @@ vi.mock('../git/runner', () => ({
vi.mock('./rate-limit', () => ({
rateLimitGuard: rateLimitGuardMock,
noteRateLimitSpend: vi.fn()
noteRateLimitSpend: vi.fn(),
repositoryRateLimitGuard: vi.fn(() => ({ blocked: false })),
noteRepositoryRateLimitSpend: vi.fn(),
spendsSharedGitHubComQuota: vi.fn(() => true)
}))
import { getPRCheckDetails, _resetOwnerRepoCache } from './client'
@@ -212,9 +215,9 @@ describe('getPRCheckDetails', () => {
})
it('isolates failed-job log tails for the same job ID on different GitHub hosts', async () => {
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
ghExecFileAsyncMock.mockImplementation(async (args: string[], options?: { host?: string }) => {
const endpoint = args.find((arg) => arg.startsWith('repos/')) ?? ''
const enterprise = args.includes('github.acme-corp.com')
const enterprise = options?.host === 'github.acme-corp.com'
if (endpoint.endsWith('/check-runs/99')) {
return {
stdout: JSON.stringify({
@@ -264,6 +267,6 @@ describe('getPRCheckDetails', () => {
args.some((arg) => arg.endsWith('/actions/jobs/9901/logs'))
)
expect(logCalls).toHaveLength(2)
expect(logCalls[1]?.[0]).toEqual(expect.arrayContaining(['--hostname', 'github.acme-corp.com']))
expect(logCalls[1]?.[1]).toEqual(expect.objectContaining({ host: 'github.acme-corp.com' }))
})
})
src/main/github/client-pr-checks.test.ts
+10 -1
View File
@@ -10,6 +10,9 @@ const {
getRateLimitMock,
rateLimitGuardMock,
noteRateLimitSpendMock,
repositoryRateLimitGuardMock,
noteRepositoryRateLimitSpendMock,
spendsSharedGitHubComQuotaMock,
acquireMock,
releaseMock
} = vi.hoisted(() => ({
@@ -31,6 +34,9 @@ const {
getRateLimitMock: vi.fn(),
rateLimitGuardMock: vi.fn(() => ({ blocked: false })),
noteRateLimitSpendMock: vi.fn(),
repositoryRateLimitGuardMock: vi.fn(() => ({ blocked: false })),
noteRepositoryRateLimitSpendMock: vi.fn(),
spendsSharedGitHubComQuotaMock: vi.fn(() => true),
acquireMock: vi.fn(),
releaseMock: vi.fn()
}))
@@ -66,7 +72,10 @@ vi.mock('../git/runner', () => ({
vi.mock('./rate-limit', () => ({
getRateLimit: getRateLimitMock,
rateLimitGuard: rateLimitGuardMock,
noteRateLimitSpend: noteRateLimitSpendMock
noteRateLimitSpend: noteRateLimitSpendMock,
repositoryRateLimitGuard: repositoryRateLimitGuardMock,
noteRepositoryRateLimitSpend: noteRepositoryRateLimitSpendMock,
spendsSharedGitHubComQuota: spendsSharedGitHubComQuotaMock
}))
import { getPRChecks, rerunPRChecks, _resetOwnerRepoCache } from './client'
src/main/github/client-pr-local-runtime.test.ts
+16 -10
View File
@@ -93,7 +93,15 @@ vi.mock('./conflict-summary', () => ({
vi.mock('./rate-limit', () => ({
getRateLimit: getRateLimitMock,
rateLimitGuard: rateLimitGuardMock,
noteRateLimitSpend: noteRateLimitSpendMock
noteRateLimitSpend: noteRateLimitSpendMock,
repositoryRateLimitGuard: vi.fn(() => ({ blocked: false })),
noteRepositoryRateLimitSpend: vi.fn(),
spendsSharedGitHubComQuota: (
repository?: { host?: string } | null,
executionOptions?: { wslDistro?: string }
) =>
(!repository?.host || repository.host.toLowerCase() === 'github.com') &&
!executionOptions?.wslDistro
}))
import {
@@ -488,16 +496,14 @@ describe('GitHub PR local runtime routing', () => {
const prViewCall = ghExecFileAsyncMock.mock.calls.find(
([args]) => args[0] === 'pr' && args[1] === 'view'
)
expect(prViewCall?.[0]).toEqual(
expect.arrayContaining(['--repo', 'github.acme-corp.com/team/orca'])
)
expect(prViewCall?.[1]).toEqual({})
// The runner host-qualifies argv at spawn time from options.host, so the
// mocked call sees the unqualified --repo plus the host in exec options.
expect(prViewCall?.[0]).toEqual(expect.arrayContaining(['--repo', 'team/orca']))
expect(prViewCall?.[1]).toEqual({ host: 'github.acme-corp.com' })
const prCalls = ghExecFileAsyncMock.mock.calls.filter(([args]) => args[0] === 'pr')
expect(
prCalls.every(
([args]) =>
args.includes('--repo') &&
args[args.indexOf('--repo') + 1] === 'github.acme-corp.com/team/orca'
([args]) => args.includes('--repo') && args[args.indexOf('--repo') + 1] === 'team/orca'
)
).toBe(true)
const apiCalls = ghExecFileAsyncMock.mock.calls.filter(([args]) => args[0] === 'api')
@@ -505,8 +511,8 @@ describe('GitHub PR local runtime routing', () => {
expect(
apiCalls.every(
([args, options]) =>
args.includes('--hostname') &&
args[args.indexOf('--hostname') + 1] === 'github.acme-corp.com' &&
!args.includes('--hostname') &&
options.host === 'github.acme-corp.com' &&
options.cwd === undefined &&
options.wslDistro === undefined
)
src/main/github/client-work-items.test.ts
+4 -1
View File
@@ -64,7 +64,10 @@ vi.mock('../git/runner', () => ({
vi.mock('./rate-limit', () => ({
rateLimitGuard: rateLimitGuardMock,
noteRateLimitSpend: noteRateLimitSpendMock,
getRateLimit: vi.fn(async () => ({ ok: false, error: 'not probed in tests' }))
getRateLimit: vi.fn(async () => ({ ok: false, error: 'not probed in tests' })),
repositoryRateLimitGuard: vi.fn(() => ({ blocked: false })),
noteRepositoryRateLimitSpend: vi.fn(),
spendsSharedGitHubComQuota: () => true
}))
import {
src/main/github/client.test.ts
+11 -6
View File
@@ -101,7 +101,14 @@ vi.mock('./local-git-config-signature', () => ({
vi.mock('./rate-limit', () => ({
getRateLimit: getRateLimitMock,
rateLimitGuard: rateLimitGuardMock,
noteRateLimitSpend: noteRateLimitSpendMock
noteRateLimitSpend: noteRateLimitSpendMock,
// Why: the repository-scoped guards share the same bucket-keyed budget as the
// legacy ones, so delegate to the existing mocks to keep per-bucket blocking
// and spend assertions working unchanged.
repositoryRateLimitGuard: (_repository: unknown, bucket: string) => rateLimitGuardMock(bucket),
noteRepositoryRateLimitSpend: (_repository: unknown, bucket: string) =>
noteRateLimitSpendMock(bucket),
spendsSharedGitHubComQuota: () => true
}))
import {
@@ -4200,9 +4207,9 @@ describe('GitHub GraphQL rate-limit guard', () => {
baseRefName: 'main',
headRefOid: 'head-oid'
}
ghExecFileAsyncMock.mockImplementation(async (args) => {
ghExecFileAsyncMock.mockImplementation(async (args, options) => {
if (args.includes('graphql')) {
const enterprise = args.includes('github.acme-corp.com')
const enterprise = options?.host === 'github.acme-corp.com'
return {
stdout: JSON.stringify({
data: {
@@ -4234,9 +4241,7 @@ describe('GitHub GraphQL rate-limit guard', () => {
expect(enterprise?.autoMergeAllowed).toBe(false)
const graphqlCalls = ghExecFileAsyncMock.mock.calls.filter(([args]) => args.includes('graphql'))
expect(graphqlCalls).toHaveLength(2)
expect(graphqlCalls[1]?.[0]).toEqual(
expect.arrayContaining(['--hostname', 'github.acme-corp.com'])
)
expect(graphqlCalls[1]?.[1]).toEqual(expect.objectContaining({ host: 'github.acme-corp.com' }))
})
it('returns conflicting file details instead of running gh merge when PR is dirty', async () => {
src/main/github/client.ts
+114 -145
View File
@@ -88,13 +88,12 @@ import type { GitHubRepoContext } from './github-repository-identity'
import { getEnterpriseGitHubRepoSlug } from './github-enterprise-repository'
import {
getOriginGitHubApiRepository,
githubApiHostArgs,
githubCliRepositoryArgument,
githubHostExecOptions,
githubRepositoryWebHost,
isGitHubDotComRepository,
resolveGitHubApiRepository,
type GitHubApiRepository
} from './github-api-repository'
import { githubRepoIdentityKey } from '../../shared/github-repository-identity-key'
export { _resetOwnerRepoCache } from './gh-utils'
export {
getIssue,
@@ -119,11 +118,14 @@ import { mapGraphQLReactionGroups, type GitHubGraphQLReactionGroup } from './com
import {
getRateLimit,
noteRateLimitSpend,
noteRepositoryRateLimitSpend,
rateLimitGuard,
repositoryRateLimitGuard,
spendsSharedGitHubComQuota,
type RateLimitBucketKind
} from './rate-limit'
type GhExecOptions = ReturnType<typeof ghRepoExecOptions>
type GhExecOptions = ReturnType<typeof ghRepoExecOptions> & { host?: string }
type HostedReviewLocalGitOptions = ReturnType<typeof getHostedReviewLocalGitOptions>
const ORCA_REPO = 'stablyai/orca'
@@ -189,9 +191,7 @@ async function assertRateLimitBudget(
repository?: GitHubApiRepository | null,
executionOptions?: Pick<GhExecOptions, 'wslDistro'>
): Promise<void> {
// The global snapshot is for the default github.com account. Applying it to
// GHES or WSL can block a healthy runtime based on an unrelated quota.
if ((repository && !isGitHubDotComRepository(repository)) || executionOptions?.wslDistro) {
if (!spendsSharedGitHubComQuota(repository, executionOptions)) {
return
}
await getRateLimit()
@@ -203,27 +203,6 @@ async function assertRateLimitBudget(
}
}
function noteRepositoryRateLimitSpend(
repository: GitHubApiRepository | null,
bucket: RateLimitBucketKind,
cost = 1,
executionOptions?: Pick<GhExecOptions, 'wslDistro'>
): void {
if ((!repository || isGitHubDotComRepository(repository)) && !executionOptions?.wslDistro) {
noteRateLimitSpend(bucket, cost)
}
}
function repositoryRateLimitGuard(
repository: GitHubApiRepository | null,
bucket: RateLimitBucketKind,
executionOptions?: Pick<GhExecOptions, 'wslDistro'>
): ReturnType<typeof rateLimitGuard> {
return (!repository || isGitHubDotComRepository(repository)) && !executionOptions?.wslDistro
? rateLimitGuard(bucket)
: { blocked: false }
}
function prRefreshUpstreamError(
err: unknown
): Extract<PRRefreshOutcome, { kind: 'upstream-error' }> {
@@ -839,14 +818,13 @@ async function fetchIssueWorkItem(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<MainWorkItem | null> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
if (ownerRepo) {
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${number}`
],
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${number}`],
ghOptions
)
const item = JSON.parse(stdout) as Record<string, unknown>
@@ -878,7 +856,7 @@ async function fetchPullRequestReviewFields(
'view',
String(number),
'--repo',
githubCliRepositoryArgument(ownerRepo),
`${ownerRepo.owner}/${ownerRepo.repo}`,
'--json',
WORK_ITEM_PR_REVIEW_JSON_FIELDS
]
@@ -905,7 +883,10 @@ async function fetchPullRequestWorkItem(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<MainWorkItem> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
if (ownerRepo) {
try {
const { stdout } = await ghExecFileAsync(
@@ -914,7 +895,7 @@ async function fetchPullRequestWorkItem(
'view',
String(number),
'--repo',
githubCliRepositoryArgument(ownerRepo),
`${ownerRepo.owner}/${ownerRepo.repo}`,
'--json',
WORK_ITEM_PR_DETAIL_JSON_FIELDS
],
@@ -941,11 +922,7 @@ async function fetchPullRequestWorkItem(
}
} catch {
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${number}`
],
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${number}`],
ghOptions
)
const mapped = mapPullRequestWorkItem(
@@ -1738,7 +1715,7 @@ function parseCreatePRPayload(stdout: string): { number: number; url: string } |
async function findOpenPRByHeadBase(args: {
repoPath: string
repoArg: string
repo: GitHubApiRepository
head: string
base: string
connectionId?: string | null
@@ -1750,7 +1727,7 @@ async function findOpenPRByHeadBase(args: {
'pr',
'list',
'--repo',
args.repoArg,
`${args.repo.owner}/${args.repo.repo}`,
'--head',
args.head,
'--base',
@@ -1764,7 +1741,8 @@ async function findOpenPRByHeadBase(args: {
],
{
...ghRepoExecOptions(context),
...(args.connectionId ? {} : getHostedReviewLocalGitOptions(args.options))
...(args.connectionId ? {} : getHostedReviewLocalGitOptions(args.options)),
...githubHostExecOptions(args.repo)
}
)
const list = JSON.parse(stdout) as { number?: number; url?: string }[]
@@ -1839,8 +1817,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 = githubCliRepositoryArgument(ownerRepo)
// The runner host-qualifies --repo from options.host for GHES (#8312).
const repoArg = `${ownerRepo.owner}/${ownerRepo.repo}`
const base = normalizeHostedReviewBaseRef(input.base)
const head = input.head ? normalizeHostedReviewHeadRef(input.head) || undefined : undefined
@@ -1892,6 +1870,7 @@ export async function createGitHubPullRequest(
const { stdout } = await ghExecFileAsync(createArgs, {
...ghRepoExecOptions(context),
...(connectionId ? {} : getHostedReviewLocalGitOptions(options)),
...githubHostExecOptions(ownerRepo),
timeout: 60_000,
idempotent: false
})
@@ -1902,7 +1881,7 @@ export async function createGitHubPullRequest(
const found = head
? await findOpenPRByHeadBase({
repoPath,
repoArg,
repo: ownerRepo,
head,
base,
connectionId,
@@ -1926,7 +1905,7 @@ export async function createGitHubPullRequest(
) {
const existing = await findOpenPRByHeadBase({
repoPath,
repoArg,
repo: ownerRepo,
head,
base,
connectionId,
@@ -2194,8 +2173,7 @@ async function detectRepositoryMergeMetadata(
branchName: string | undefined,
ghOptions: GhExecOptions
): Promise<GitHubRepositoryMergeMetadata> {
const host = ownerRepo.host?.toLowerCase() ?? 'github.com'
const cacheKey = `${host}/${ownerRepo.owner.toLowerCase()}/${ownerRepo.repo.toLowerCase()}:${branchName ?? '__repo__'}`
const cacheKey = `${githubRepoIdentityKey(ownerRepo)}:${branchName ?? '__repo__'}`
pruneRepositoryMergeMetadataCache()
const cached = repositoryMergeMetadataCache.get(cacheKey)
if (cached) {
@@ -2229,7 +2207,6 @@ async function detectRepositoryMergeMetadata(
noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, ghOptions)
const args = [
'api',
...githubApiHostArgs(ownerRepo),
'graphql',
'-f',
`query=${query}`,
@@ -2241,7 +2218,10 @@ async function detectRepositoryMergeMetadata(
if (branchName) {
args.push('-f', `branch=${branchName}`)
}
const { stdout } = await ghExecFileAsync(args, ghOptions)
const { stdout } = await ghExecFileAsync(args, {
...ghOptions,
...githubHostExecOptions(ownerRepo)
})
const parsed = JSON.parse(stdout) as {
data?: {
repository?: {
@@ -2326,12 +2306,8 @@ async function getRestPRForBranch(
): Promise<PullRequestLookupData | null> {
const head = encodeURIComponent(`${headOwner}:${branchName}`)
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(prRepo),
`repos/${prRepo.owner}/${prRepo.repo}/pulls?head=${head}&state=all&per_page=1`
],
ghOptions
['api', `repos/${prRepo.owner}/${prRepo.repo}/pulls?head=${head}&state=all&per_page=1`],
{ ...ghOptions, ...githubHostExecOptions(prRepo) }
)
const list = JSON.parse(stdout) as RestPullRequest[]
const pr = list[0]
@@ -2348,7 +2324,7 @@ async function getFallbackPRListForBranch(
'pr',
'list',
'--repo',
githubCliRepositoryArgument(prRepo),
`${prRepo.owner}/${prRepo.repo}`,
'--head',
branchName,
'--state',
@@ -2358,7 +2334,7 @@ async function getFallbackPRListForBranch(
'--json',
PR_BRANCH_LIST_JSON_FIELDS
],
ghOptions
{ ...ghOptions, ...githubHostExecOptions(prRepo) }
)
const list = JSON.parse(stdout) as PullRequestLookupData[]
return list[0] ?? null
@@ -2752,12 +2728,8 @@ async function getRestPRByNumber(
ghOptions: ReturnType<typeof ghRepoExecOptions>
): Promise<PullRequestLookupData | null> {
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${number}`
],
ghOptions
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${number}`],
{ ...ghOptions, ...githubHostExecOptions(ownerRepo) }
)
return mapRestPullRequest(JSON.parse(stdout) as RestPullRequest)
}
@@ -2774,11 +2746,11 @@ async function getPRByNumber(
'view',
String(number),
'--repo',
githubCliRepositoryArgument(ownerRepo),
`${ownerRepo.owner}/${ownerRepo.repo}`,
'--json',
PR_LOOKUP_JSON_FIELDS
],
ghOptions
{ ...ghOptions, ...githubHostExecOptions(ownerRepo) }
)
return hydratePullRequestLookupData(
ownerRepo,
@@ -3458,7 +3430,6 @@ async function getPRChecksViaRestFallback(
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
...cacheArgs,
`repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodedHeadSha}/check-runs?per_page=100`
],
@@ -3476,7 +3447,6 @@ async function getPRChecksViaRestFallback(
const statusResult = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
...cacheArgs,
`repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodedHeadSha}/status?per_page=100`
],
@@ -3499,7 +3469,6 @@ async function getPRChecksViaRestFallback(
const suitesResult = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
...cacheArgs,
`repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodedHeadSha}/check-suites?per_page=100`
],
@@ -3546,13 +3515,16 @@ export async function getPRChecks(
localGitOptions: LocalGitExecOptions = {}
): Promise<PRCheckDetail[]> {
void headSha
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
connectionId,
localGitOptions
)
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
if (connectionId && !ownerRepo) {
throw new Error(GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE)
}
@@ -3562,7 +3534,7 @@ export async function getPRChecks(
try {
const fallbackArgs = ['pr', 'checks', String(prNumber), '--json', 'name,state,link']
if (ownerRepo) {
fallbackArgs.push('--repo', githubCliRepositoryArgument(ownerRepo))
fallbackArgs.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
}
const { stdout } = await ghExecFileAsync(fallbackArgs, ghOptions).catch((err: unknown) => {
const { stderr } = extractExecError(err)
@@ -3602,7 +3574,6 @@ export async function getPRChecks(
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'graphql',
...cacheArgs,
'-f',
@@ -3776,9 +3747,7 @@ async function attachFailedJobLogTails(
// Why: cap log fetches so failed-job details stay a bounded follow-up, not a burst of hosted log downloads.
for (const job of failedJobs) {
const jobCacheKey = getCheckJobLogTailCacheKey(job)
const cacheKey = jobCacheKey
? `${githubRepositoryWebHost(ownerRepo).toLowerCase()}/${ownerRepo.owner.toLowerCase()}/${ownerRepo.repo.toLowerCase()}:${jobCacheKey}`
: null
const cacheKey = jobCacheKey ? `${githubRepoIdentityKey(ownerRepo)}:${jobCacheKey}` : null
if (!cacheKey) {
continue
}
@@ -3788,11 +3757,7 @@ async function attachFailedJobLogTails(
}
try {
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/actions/jobs/${job.id}/logs`
],
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/actions/jobs/${job.id}/logs`],
ghOptions
)
job.logTail = sliceCheckLogTail(stdout)
@@ -3831,7 +3796,6 @@ export async function getPRCheckDetails(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<PRCheckRunDetails | null> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
args.prRepo,
@@ -3841,6 +3805,10 @@ export async function getPRCheckDetails(
if (!ownerRepo) {
return null
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
@@ -3848,11 +3816,7 @@ export async function getPRCheckDetails(
let annotations: PRCheckRunDetails['annotations'] = []
if (args.checkRunId) {
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/check-runs/${args.checkRunId}`
],
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/check-runs/${args.checkRunId}`],
ghOptions
)
checkRun = JSON.parse(stdout) as Record<string, unknown>
@@ -3860,7 +3824,6 @@ export async function getPRCheckDetails(
const annotationsResult = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/check-runs/${args.checkRunId}/annotations?per_page=20`
],
ghOptions
@@ -3878,7 +3841,6 @@ export async function getPRCheckDetails(
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/actions/runs/${workflowRunId}/jobs?per_page=100`
],
ghOptions
@@ -3935,11 +3897,14 @@ export async function rerunPRChecks(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubRerunPRChecksResult> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
const checks = await getPRChecks(
repoPath,
@@ -3983,7 +3948,7 @@ export async function rerunPRChecks(
const endpoint = options.failedOnly
? `repos/${ownerRepo.owner}/${ownerRepo.repo}/actions/runs/${runId}/rerun-failed-jobs`
: `repos/${ownerRepo.owner}/${ownerRepo.repo}/actions/runs/${runId}/rerun`
await ghExecFileAsync(['api', ...githubApiHostArgs(ownerRepo), '-X', 'POST', endpoint], {
await ghExecFileAsync(['api', '-X', 'POST', endpoint], {
...ghOptions,
env: { ...process.env, GH_PROMPT_DISABLED: '1' }
})
@@ -3993,7 +3958,6 @@ export async function rerunPRChecks(
await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'-X',
'POST',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/check-runs/${checkRunId}/rerequest`
@@ -4074,13 +4038,16 @@ export async function getPRComments(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<PRComment[]> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
options?.prRepo,
connectionId,
localGitOptions
)
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
if (connectionId && !ownerRepo) {
throw new Error(GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE)
}
@@ -4104,7 +4071,6 @@ export async function getPRComments(
reviewThreadsFetch = ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'graphql',
'-f',
`query=${REVIEW_THREADS_QUERY}`,
@@ -4120,23 +4086,13 @@ export async function getPRComments(
}
const [issueResult, threadsResult, reviewsResult] = await Promise.allSettled([
ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
...cacheArgs,
`${base}/issues/${prNumber}/comments?per_page=100`
],
['api', ...cacheArgs, `${base}/issues/${prNumber}/comments?per_page=100`],
ghOptions
),
reviewThreadsFetch,
// Why: review summaries (approve/request-changes/general) live under pulls/{n}/reviews, not issue comments or threads.
ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
...cacheArgs,
`${base}/pulls/${prNumber}/reviews?per_page=100`
],
['api', ...cacheArgs, `${base}/pulls/${prNumber}/reviews?per_page=100`],
ghOptions
)
])
@@ -4326,9 +4282,6 @@ export async function setPRFileViewed(args: {
path: string
viewed: boolean
}): Promise<boolean> {
const ghOptions = ghRepoExecOptions(
githubRepoContext(args.repoPath, args.connectionId, args.localGitOptions)
)
const ownerRepo = await getOriginGitHubApiRepository(
args.repoPath,
args.connectionId,
@@ -4337,6 +4290,10 @@ export async function setPRFileViewed(args: {
if (args.connectionId && !ownerRepo) {
return false
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(args.repoPath, args.connectionId, args.localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
const mutation = args.viewed ? 'markFileAsViewed' : 'unmarkFileAsViewed'
const query = `mutation($pullRequestId: ID!, $path: String!) {
${mutation}(input: { pullRequestId: $pullRequestId, path: $path }) {
@@ -4348,7 +4305,6 @@ export async function setPRFileViewed(args: {
await ghExecFileAsync(
[
'api',
...(ownerRepo ? githubApiHostArgs(ownerRepo) : []),
'graphql',
'-f',
`query=${query}`,
@@ -4380,11 +4336,14 @@ export async function resolveReviewThread(
): Promise<boolean> {
const mutation = resolve ? 'resolveReviewThread' : 'unresolveReviewThread'
const query = `mutation($threadId: ID!) { ${mutation}(input: { threadId: $threadId }) { thread { isResolved } } }`
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
if (connectionId && !ownerRepo) {
return false
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
const guard = repositoryRateLimitGuard(ownerRepo, 'graphql', ghOptions)
if (guard.blocked) {
console.warn(
@@ -4396,15 +4355,7 @@ export async function resolveReviewThread(
try {
noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, ghOptions)
await ghExecFileAsync(
[
'api',
...(ownerRepo ? githubApiHostArgs(ownerRepo) : []),
'graphql',
'-f',
`query=${query}`,
'-f',
`threadId=${threadId}`
],
['api', 'graphql', '-f', `query=${query}`, '-f', `threadId=${threadId}`],
ghOptions
)
return true
@@ -4459,7 +4410,6 @@ export async function addPRReviewCommentReply(
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubCommentResult> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
@@ -4469,12 +4419,15 @@ export async function addPRReviewCommentReply(
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'-X',
'POST',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}/comments/${commentId}/replies`,
@@ -4505,9 +4458,6 @@ export async function addPRReviewComment(
localGitOptions?: LocalGitExecOptions
}
): Promise<GitHubCommentResult> {
const ghOptions = ghRepoExecOptions(
githubRepoContext(args.repoPath, args.connectionId, args.localGitOptions)
)
const ownerRepo = await getOriginGitHubApiRepository(
args.repoPath,
args.connectionId,
@@ -4516,11 +4466,14 @@ export async function addPRReviewComment(
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(args.repoPath, args.connectionId, args.localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const fields = [
'api',
...githubApiHostArgs(ownerRepo),
'-X',
'POST',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${args.prNumber}/comments`,
@@ -4574,7 +4527,6 @@ export async function mergePR(
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
@@ -4584,6 +4536,10 @@ export async function mergePR(
if (connectionId && !ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const mergeBlocker = await getPRMergeBlocker(
@@ -4601,7 +4557,7 @@ export async function mergePR(
// Don't use --delete-branch: it deletes the local branch, which fails while the worktree is checked out on it.
const args = ['pr', 'merge', String(prNumber), `--${method}`]
if (ownerRepo) {
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
}
await ghExecFileAsync(args, {
...ghOptions,
@@ -4626,7 +4582,6 @@ export async function setPRAutoMerge(
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
@@ -4636,6 +4591,10 @@ export async function setPRAutoMerge(
if (connectionId && !ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
if (enabled) {
@@ -4643,7 +4602,7 @@ export async function setPRAutoMerge(
}
const args = ['pr', 'merge', String(prNumber), '--disable-auto']
if (ownerRepo) {
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
}
await ghExecFileAsync(args, {
...ghOptions,
@@ -4680,7 +4639,7 @@ async function getPRAutoMergeIdentity(
): Promise<PRAutoMergeIdentity | null> {
const args = ['pr', 'view', String(prNumber), '--json', PR_AUTO_MERGE_IDENTITY_JSON_FIELDS]
if (ownerRepo) {
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
}
const { stdout } = await ghExecFileAsync(args, ghOptions)
const data = JSON.parse(stdout) as PRAutoMergeIdentity
@@ -4699,7 +4658,7 @@ async function runPRAutoMergeCommand(
): Promise<void> {
const args = ['pr', 'merge', String(prNumber), '--auto', `--${method}`]
if (ownerRepo) {
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
}
await ghExecFileAsync(args, {
...ghOptions,
@@ -4744,7 +4703,6 @@ async function enablePRAutoMerge(
}`
const args = [
'api',
...(ownerRepo ? githubApiHostArgs(ownerRepo) : []),
'graphql',
'-f',
`query=${query}`,
@@ -4837,18 +4795,18 @@ export async function updatePRState(
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const ghOptions = ghRepoExecOptions(context)
const ownerRepo = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = { ...ghRepoExecOptions(context), ...githubHostExecOptions(ownerRepo) }
await acquire()
try {
const cmd = updates.state === 'closed' ? 'close' : 'reopen'
// Why: gh's PR commands use GitHub's supported reopen flow; REST state PATCH can 422 on reopen.
await ghExecFileAsync(
['pr', cmd, String(prNumber), '--repo', githubCliRepositoryArgument(ownerRepo)],
['pr', cmd, String(prNumber), '--repo', `${ownerRepo.owner}/${ownerRepo.repo}`],
{
...ghOptions
}
@@ -4874,16 +4832,19 @@ export async function requestPRReviewers(
if (logins.length === 0) {
return { ok: false, error: 'Enter at least one reviewer' }
}
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
if (connectionId && !ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const args = ['pr', 'edit', String(prNumber), '--add-reviewer', logins.join(',')]
if (ownerRepo) {
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
}
await ghExecFileAsync(args, {
...ghOptions,
@@ -4910,16 +4871,19 @@ export async function removePRReviewers(
if (logins.length === 0) {
return { ok: false, error: 'Enter at least one reviewer' }
}
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
if (connectionId && !ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const args = ['pr', 'edit', String(prNumber), '--remove-reviewer', logins.join(',')]
if (ownerRepo) {
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
}
await ghExecFileAsync(args, {
...ghOptions,
@@ -4946,7 +4910,6 @@ export async function updatePRTitle(
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<boolean> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
@@ -4956,11 +4919,15 @@ export async function updatePRTitle(
if (connectionId && !ownerRepo) {
return false
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const args = ['pr', 'edit', String(prNumber), '--title', title]
if (ownerRepo) {
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
}
await ghExecFileAsync(args, {
...ghOptions
@@ -4982,7 +4949,6 @@ export async function updatePRDetails(
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
@@ -4992,6 +4958,10 @@ export async function updatePRDetails(
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
const fields: string[] = []
if (updates.title !== undefined) {
@@ -5013,7 +4983,6 @@ export async function updatePRDetails(
await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'-X',
'PATCH',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}`,
src/main/github/github-api-repository.ts
+8 -11
View File
@@ -45,21 +45,18 @@ export async function resolveGitHubApiRepository(
return connectionId ? null : repository
}
export function githubApiHostArgs(repository: GitHubApiRepository): string[] {
const host = repository.host
return host && host.toLowerCase() !== 'github.com' ? ['--hostname', host] : []
}
export function isGitHubDotComRepository(repository: GitHubApiRepository): boolean {
return !repository.host || repository.host.toLowerCase() === 'github.com'
}
// Why: owner/repo shorthand targets gh's default host, and SSH-backed repos
// have no local cwd from which gh can infer an Enterprise hostname.
export function githubCliRepositoryArgument(repository: GitHubApiRepository): string {
return !isGitHubDotComRepository(repository)
? `${repository.host}/${repository.owner}/${repository.repo}`
: `${repository.owner}/${repository.repo}`
// Why: the gh runner host-qualifies argv (`--hostname`, `HOST/OWNER/REPO`)
// from `options.host`, so merging this into a call's exec options is the one
// step a call site needs to target GHES. github.com stays unset to keep gh's
// default-host resolution (and existing argv) unchanged.
export function githubHostExecOptions(repository: GitHubApiRepository | null | undefined): {
host?: string
} {
return repository?.host && !isGitHubDotComRepository(repository) ? { host: repository.host } : {}
}
export function githubRepositoryWebHost(repository: GitHubApiRepository): string {
src/main/github/github-enterprise-repository.test.ts
+5 -4
View File
@@ -61,10 +61,11 @@ describe('getEnterpriseGitHubRepoSlug', () => {
repo: 'orca',
host: 'github.acme-corp.com'
})
// The auth probe targets the remote's host, not a hardcoded github.com.
// The auth probe targets the remote's host, not a hardcoded github.com, and
// scopes any breaker trip to that host via the exec options.
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
['auth', 'status', '--hostname', 'github.acme-corp.com'],
{ cwd: '/repo' }
{ cwd: '/repo', host: 'github.acme-corp.com' }
)
})
@@ -89,7 +90,7 @@ describe('getEnterpriseGitHubRepoSlug', () => {
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
['auth', 'status', '--hostname', 'github.acme-corp.com'],
{ cwd: '/repo', wslDistro: 'Ubuntu' }
{ cwd: '/repo', wslDistro: 'Ubuntu', host: 'github.acme-corp.com' }
)
})
@@ -137,7 +138,7 @@ describe('isGitHubHostAuthenticated', () => {
).resolves.toBe(true)
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
['auth', 'status', '--hostname', 'github.acme-corp.com'],
{}
{ host: 'github.acme-corp.com' }
)
})
src/main/github/github-enterprise-repository.ts
+6 -3
View File
@@ -83,9 +83,12 @@ export async function isGitHubHostAuthenticated(
// Why: provider detection and review loading can probe the same runtime at
// once; coalesce them so one host never spawns duplicate auth subprocesses.
const probe = (async () => {
const execOptions = ghRepoExecOptions(
githubRepoContext(repoPath, connectionId, localGitOptions)
)
const execOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
// Why: scope any rate-limit breaker trip from this probe to the probed
// host, not to gh's default account.
host: normalizedHost
}
let authenticated: boolean
try {
await ghExecFileAsync(['auth', 'status', '--hostname', normalizedHost], execOptions)
src/main/github/issues.test.ts
+1 -3
View File
@@ -160,15 +160,13 @@ describe('issue source operations', () => {
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
[
'api',
'--hostname',
'github.acme-corp.com',
'-X',
'POST',
'repos/team/orca/issues/7/comments',
'--raw-field',
'body=Enterprise comment'
],
{}
expect.objectContaining({ host: 'github.acme-corp.com' })
)
})
src/main/github/issues.ts
+2 -3
View File
@@ -15,7 +15,7 @@ import type {
} from '../../shared/types'
import { mapIssueInfo } from './mappers'
import type { LocalGitExecOptions, OwnerRepo } from './gh-utils'
import { githubApiHostArgs, resolveGitHubApiRepository } from './github-api-repository'
import { githubHostExecOptions, resolveGitHubApiRepository } from './github-api-repository'
// prettier-ignore
import { ghExecFileAsync, acquire, release, getIssueOwnerRepo, resolveIssueSource, classifyGhError, classifyListIssuesError, ghRepoExecOptions, githubRepoContext, extractExecError } from './gh-utils'
@@ -411,19 +411,18 @@ export async function addIssueComment(
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubCommentResult> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const ghOptions = ghRepoExecOptions(context)
const ownerRepo = ownerRepoOverride
? await resolveGitHubApiRepository(repoPath, ownerRepoOverride, connectionId, localGitOptions)
: await getIssueOwnerRepo(repoPath, connectionId, localGitOptions)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = { ...ghRepoExecOptions(context), ...githubHostExecOptions(ownerRepo) }
await acquire()
try {
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'-X',
'POST',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}/comments`,
src/main/github/merged-pr-commit-membership.ts
+9 -6
View File
@@ -1,5 +1,7 @@
import { ghExecFileAsync } from './gh-utils'
import { noteRateLimitSpend, rateLimitGuard } from './rate-limit'
import { noteRepositoryRateLimitSpend, repositoryRateLimitGuard } from './rate-limit'
import { githubHostExecOptions } from './github-api-repository'
import { githubRepoIdentityKey } from '../../shared/github-repository-identity-key'
import type { OwnerRepo } from './github-repository-identity'
type GhExecOptions = Parameters<typeof ghExecFileAsync>[1]
@@ -59,7 +61,8 @@ export async function isCommitPartOfMergedPR(args: {
}
const owner = args.ownerRepo.owner
const repo = args.ownerRepo.repo
const cacheKey = `${owner.toLowerCase()}/${repo.toLowerCase()}#${args.prNumber}@${oid}`
const cacheKey = `${githubRepoIdentityKey(args.ownerRepo)}#${args.prNumber}@${oid}`
const ghOptions = { ...args.ghOptions, ...githubHostExecOptions(args.ownerRepo) }
const now = Date.now()
pruneMergedPRCommitMembershipCache(now)
const cached = membershipCache.get(cacheKey)
@@ -68,7 +71,7 @@ export async function isCommitPartOfMergedPR(args: {
}
// Why blocked stays unknown: hiding a transient branch match is safe, but
// callers must not clear a durable linked PR when the probe never ran.
if (rateLimitGuard('core').blocked) {
if (repositoryRateLimitGuard(args.ownerRepo, 'core', ghOptions).blocked) {
return 'unknown'
}
try {
@@ -79,20 +82,20 @@ export async function isCommitPartOfMergedPR(args: {
// found (contained) or a short page proves absence (not-contained); only the
// pathological all-full case up to the cap stays 'unknown'.
for (let page = 1; page <= COMMIT_PULLS_MAX_PAGES; page += 1) {
if (page > 1 && rateLimitGuard('core').blocked) {
if (page > 1 && repositoryRateLimitGuard(args.ownerRepo, 'core', ghOptions).blocked) {
membershipCache.set(cacheKey, {
value: 'unknown',
expiresAt: now + MEMBERSHIP_ERROR_TTL_MS
})
return 'unknown'
}
noteRateLimitSpend('core')
noteRepositoryRateLimitSpend(args.ownerRepo, 'core', 1, ghOptions)
const { stdout } = await ghExecFileAsync(
[
'api',
`repos/${owner}/${repo}/commits/${oid}/pulls?per_page=${COMMIT_PULLS_PAGE_SIZE}&page=${page}`
],
args.ghOptions
ghOptions
)
const parsed = JSON.parse(stdout) as unknown
// Why: a non-array success payload is a shape mismatch, not an empty page;
src/main/github/rate-limit.test.ts
+93 -5
View File
@@ -13,7 +13,15 @@ vi.mock('./gh-utils', () => ({
release: vi.fn()
}))
import { getRateLimit, rateLimitGuard, _resetRateLimitCache } from './rate-limit'
import {
getRateLimit,
rateLimitGuard,
noteRateLimitSpend,
spendsSharedGitHubComQuota,
repositoryRateLimitGuard,
noteRepositoryRateLimitSpend,
_resetRateLimitCache
} from './rate-limit'
const PAYLOAD = JSON.stringify({
resources: {
@@ -49,10 +57,12 @@ describe('getRateLimit', () => {
fetchedAt: 1_000
}
})
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
['api', '--hostname', 'github.com', 'rate_limit'],
{ encoding: 'utf-8' }
)
// The runner injects --hostname at spawn from the `host` option, so the
// probe passes bare argv and pins the host to github.com through options.
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(['api', 'rate_limit'], {
encoding: 'utf-8',
host: 'github.com'
})
})
it('caches a failed probe for the TTL instead of re-spawning gh', async () => {
@@ -139,3 +149,81 @@ describe('rateLimitGuard', () => {
expect(rateLimitGuard('core')).toEqual({ blocked: false })
})
})
// Why: only default github.com traffic on the native runtime shares this
// snapshot's budget. GHES hosts and WSL distros run against separate quotas,
// so the shared guard must bypass them entirely.
describe('shared-quota host scoping', () => {
beforeEach(() => {
vi.useFakeTimers()
vi.setSystemTime(1_000)
ghExecFileAsyncMock.mockReset()
_resetRateLimitCache()
})
afterEach(() => {
vi.useRealTimers()
})
async function cacheExhaustedSnapshot(resetEpochSeconds: number): Promise<void> {
ghExecFileAsyncMock.mockResolvedValueOnce({
stdout: JSON.stringify({
resources: {
core: { limit: 5000, remaining: 3, reset: resetEpochSeconds },
search: { limit: 30, remaining: 28, reset: resetEpochSeconds },
graphql: { limit: 5000, remaining: 4900, reset: resetEpochSeconds }
}
})
})
expect((await getRateLimit()).ok).toBe(true)
}
it('treats github.com / no host on the native runtime as sharing the budget', () => {
expect(spendsSharedGitHubComQuota(undefined)).toBe(true)
expect(spendsSharedGitHubComQuota(null)).toBe(true)
expect(spendsSharedGitHubComQuota({ host: 'github.com' })).toBe(true)
expect(spendsSharedGitHubComQuota({ host: 'GitHub.com' })).toBe(true)
})
it('excludes GHES hosts and WSL runtimes from the shared budget', () => {
expect(spendsSharedGitHubComQuota({ host: 'github.acme-corp.com' })).toBe(false)
expect(spendsSharedGitHubComQuota({ host: 'github.com' }, { wslDistro: 'Ubuntu' })).toBe(false)
expect(spendsSharedGitHubComQuota(undefined, { wslDistro: 'Ubuntu' })).toBe(false)
})
it('applies the shared guard for github.com but bypasses non-shared scopes', async () => {
await cacheExhaustedSnapshot(61)
expect(repositoryRateLimitGuard({ host: 'github.com' }, 'core')).toEqual({
blocked: true,
remaining: 3,
limit: 5000,
resetAt: 61
})
expect(repositoryRateLimitGuard({ host: 'github.acme-corp.com' }, 'core')).toEqual({
blocked: false
})
expect(
repositoryRateLimitGuard({ host: 'github.com' }, 'core', { wslDistro: 'Ubuntu' })
).toEqual({ blocked: false })
})
it('debits shared-budget spend but no-ops for non-shared scopes', async () => {
await cacheExhaustedSnapshot(61)
noteRepositoryRateLimitSpend({ host: 'github.acme-corp.com' }, 'core')
const afterEnterprise = rateLimitGuard('core')
expect(afterEnterprise.blocked && afterEnterprise.remaining).toBe(3)
noteRepositoryRateLimitSpend({ host: 'github.com' }, 'core')
const afterShared = rateLimitGuard('core')
expect(afterShared.blocked && afterShared.remaining).toBe(2)
})
it('matches noteRateLimitSpend when the scope is shared github.com', async () => {
await cacheExhaustedSnapshot(61)
noteRateLimitSpend('core')
const guard = rateLimitGuard('core')
expect(guard.blocked && guard.remaining).toBe(2)
})
})
src/main/github/rate-limit.ts
+33 -2
View File
@@ -120,6 +120,36 @@ export function noteRateLimitSpend(bucket: RateLimitBucketKind, cost = 1): void
}
}
// Why: this snapshot covers native github.com only; GHES and WSL use quotas protected by the runner's scoped breaker.
export function spendsSharedGitHubComQuota(
repository: { host?: string } | null | undefined,
executionOptions?: { wslDistro?: string }
): boolean {
const host = repository?.host
return (!host || host.toLowerCase() === 'github.com') && !executionOptions?.wslDistro
}
export function repositoryRateLimitGuard(
repository: { host?: string } | null | undefined,
bucket: RateLimitBucketKind,
executionOptions?: { wslDistro?: string }
): ReturnType<typeof rateLimitGuard> {
return spendsSharedGitHubComQuota(repository, executionOptions)
? rateLimitGuard(bucket)
: { blocked: false }
}
export function noteRepositoryRateLimitSpend(
repository: { host?: string } | null | undefined,
bucket: RateLimitBucketKind,
cost = 1,
executionOptions?: { wslDistro?: string }
): void {
if (spendsSharedGitHubComQuota(repository, executionOptions)) {
noteRateLimitSpend(bucket, cost)
}
}
// Why: the breaker only knows "blocked", not the reset time; one exempt probe refines it to the real reset or clears a stale block (single-flight so a 403 burst probes once).
let resetRefinementInFlight: Promise<void> | null = null
@@ -178,8 +208,9 @@ async function fetchRateLimitSnapshot(): Promise<GetRateLimitResult> {
try {
// Why: this singleton snapshot guards native github.com traffic. Pin the
// host so a process-level GH_HOST cannot make it describe a GHES account.
const { stdout } = await ghExecFileAsync(['api', '--hostname', 'github.com', 'rate_limit'], {
encoding: 'utf-8'
const { stdout } = await ghExecFileAsync(['api', 'rate_limit'], {
encoding: 'utf-8',
host: 'github.com'
})
const parsed = JSON.parse(stdout) as GhRateLimitPayload
const snapshot: GitHubRateLimitSnapshot = {
src/main/github/work-item-details-file-viewed.test.ts
+10 -1
View File
@@ -13,6 +13,8 @@ const {
getPRCommentsMock,
rateLimitGuardMock,
noteRateLimitSpendMock,
repositoryRateLimitGuardMock,
noteRepositoryRateLimitSpendMock,
acquireMock,
releaseMock
} = vi.hoisted(() => ({
@@ -24,6 +26,8 @@ const {
getPRCommentsMock: vi.fn(),
rateLimitGuardMock: vi.fn<() => RateLimitGuardResult>(() => ({ blocked: false })),
noteRateLimitSpendMock: vi.fn(),
repositoryRateLimitGuardMock: vi.fn<() => RateLimitGuardResult>(() => ({ blocked: false })),
noteRepositoryRateLimitSpendMock: vi.fn(),
acquireMock: vi.fn(),
releaseMock: vi.fn()
}))
@@ -50,7 +54,9 @@ vi.mock('./client', () => ({
vi.mock('./rate-limit', () => ({
rateLimitGuard: rateLimitGuardMock,
noteRateLimitSpend: noteRateLimitSpendMock
noteRateLimitSpend: noteRateLimitSpendMock,
repositoryRateLimitGuard: repositoryRateLimitGuardMock,
noteRepositoryRateLimitSpend: noteRepositoryRateLimitSpendMock
}))
import { getWorkItemDetails } from './work-item-details'
@@ -66,6 +72,9 @@ describe('getWorkItemDetails PR file viewed state', () => {
rateLimitGuardMock.mockReset()
rateLimitGuardMock.mockReturnValue({ blocked: false })
noteRateLimitSpendMock.mockReset()
repositoryRateLimitGuardMock.mockReset()
repositoryRateLimitGuardMock.mockReturnValue({ blocked: false })
noteRepositoryRateLimitSpendMock.mockReset()
acquireMock.mockReset()
releaseMock.mockReset()
acquireMock.mockResolvedValue(undefined)
src/main/github/work-item-details-pr-files.test.ts
+3 -1
View File
@@ -41,7 +41,9 @@ vi.mock('./github-enterprise-repository', () => ({
vi.mock('./rate-limit', () => ({
rateLimitGuard: vi.fn(() => ({ blocked: false })),
noteRateLimitSpend: vi.fn()
noteRateLimitSpend: vi.fn(),
repositoryRateLimitGuard: vi.fn(() => ({ blocked: false })),
noteRepositoryRateLimitSpend: vi.fn()
}))
import { getWorkItemDetails } from './work-item-details'
src/main/github/work-item-details.test.ts
+27 -8
View File
@@ -15,6 +15,8 @@ const {
getPRCommentsMock,
rateLimitGuardMock,
noteRateLimitSpendMock,
repositoryRateLimitGuardMock,
noteRepositoryRateLimitSpendMock,
ghRepoExecOptionsMock,
githubRepoContextMock,
acquireMock,
@@ -30,6 +32,8 @@ const {
getPRCommentsMock: vi.fn(),
rateLimitGuardMock: vi.fn<() => RateLimitGuardResult>(() => ({ blocked: false })),
noteRateLimitSpendMock: vi.fn(),
repositoryRateLimitGuardMock: vi.fn<() => RateLimitGuardResult>(() => ({ blocked: false })),
noteRepositoryRateLimitSpendMock: vi.fn(),
ghRepoExecOptionsMock: vi.fn((context) =>
context.connectionId
? {}
@@ -67,7 +71,9 @@ vi.mock('./github-enterprise-repository', () => ({
vi.mock('./rate-limit', () => ({
rateLimitGuard: rateLimitGuardMock,
noteRateLimitSpend: noteRateLimitSpendMock
noteRateLimitSpend: noteRateLimitSpendMock,
repositoryRateLimitGuard: repositoryRateLimitGuardMock,
noteRepositoryRateLimitSpend: noteRepositoryRateLimitSpendMock
}))
import { getPRFileContents, getWorkItemDetails } from './work-item-details'
@@ -86,6 +92,9 @@ describe('getWorkItemDetails', () => {
rateLimitGuardMock.mockReset()
rateLimitGuardMock.mockReturnValue({ blocked: false })
noteRateLimitSpendMock.mockReset()
repositoryRateLimitGuardMock.mockReset()
repositoryRateLimitGuardMock.mockReturnValue({ blocked: false })
noteRepositoryRateLimitSpendMock.mockReset()
ghRepoExecOptionsMock.mockClear()
githubRepoContextMock.mockClear()
acquireMock.mockReset()
@@ -410,12 +419,20 @@ describe('getWorkItemDetails', () => {
})
it('skips optional GraphQL issue detail calls when the cached GraphQL budget is low', async () => {
// github.com repos share the singleton quota, so both the raw and
// repository-scoped guards report the same low-budget block.
rateLimitGuardMock.mockReturnValue({
blocked: true,
remaining: 3,
limit: 5000,
resetAt: 1_800_000_000
})
repositoryRateLimitGuardMock.mockReturnValue({
blocked: true,
remaining: 3,
limit: 5000,
resetAt: 1_800_000_000
})
getWorkItemMock.mockResolvedValueOnce({
id: 'issue:923',
type: 'issue',
@@ -438,6 +455,7 @@ describe('getWorkItemDetails', () => {
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(3)
expect(ghExecFileAsyncMock.mock.calls.some((call) => call[0][1] === 'graphql')).toBe(false)
expect(noteRateLimitSpendMock).not.toHaveBeenCalled()
expect(noteRepositoryRateLimitSpendMock).not.toHaveBeenCalled()
expect(details?.body).toBe('Issue body')
expect(details?.participants).toEqual([])
})
@@ -687,12 +705,11 @@ describe('getWorkItemDetails', () => {
.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.includes('--hostname'))).toBe(true)
expect(
apiCalls.every((args) => args[args.indexOf('--hostname') + 1] === 'github.acme-corp.com')
).toBe(true)
expect(
ghExecFileAsyncMock.mock.calls.every(([, options]) => Object.keys(options).length === 0)
ghExecFileAsyncMock.mock.calls.every(
([, options]) => options?.host === 'github.acme-corp.com'
)
).toBe(true)
})
@@ -768,9 +785,11 @@ describe('getWorkItemDetails', () => {
})
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.includes('--hostname'))).toBe(true)
expect(
apiCalls.every((args) => args[args.indexOf('--hostname') + 1] === 'github.acme-corp.com')
ghExecFileAsyncMock.mock.calls.every(
([, options]) => options?.host === 'github.acme-corp.com'
)
).toBe(true)
})
src/main/github/work-item-details.ts
+31 -48
View File
@@ -23,11 +23,16 @@ import {
import { getWorkItem, getWorkItemByOwnerRepo, getPRChecks, getPRComments } from './client'
import {
getOriginGitHubApiRepository,
githubApiHostArgs,
githubHostExecOptions,
isGitHubDotComRepository,
type GitHubApiRepository
} from './github-api-repository'
import { noteRateLimitSpend, rateLimitGuard } from './rate-limit'
import {
noteRateLimitSpend,
noteRepositoryRateLimitSpend,
rateLimitGuard,
repositoryRateLimitGuard
} from './rate-limit'
import { getPRReviewCommentLineNumbersFromPatch } from './pr-review-comment-lines'
import { isMaxBufferOverflowError } from '../git/max-buffer-overflow'
@@ -47,27 +52,6 @@ function encodeGitHubContentPath(path: string): string {
return path.split('/').map(encodeURIComponent).join('/')
}
function repositoryRateLimitGuard(
repository: GitHubApiRepository | null,
bucket: 'core' | 'graphql' | 'search',
executionOptions: Pick<LocalGitExecOptions, 'wslDistro'> = {}
): ReturnType<typeof rateLimitGuard> {
return (!repository || isGitHubDotComRepository(repository)) && !executionOptions.wslDistro
? rateLimitGuard(bucket)
: { blocked: false }
}
function noteRepositoryRateLimitSpend(
repository: GitHubApiRepository | null,
bucket: 'core' | 'graphql' | 'search',
cost = 1,
executionOptions: Pick<LocalGitExecOptions, 'wslDistro'> = {}
): void {
if ((!repository || isGitHubDotComRepository(repository)) && !executionOptions.wslDistro) {
noteRateLimitSpend(bucket, cost)
}
}
const PR_FILE_VIEWED_STATES_QUERY = `query($owner: String!, $repo: String!, $number: Int!, $after: String) {
repository(owner: $owner, name: $repo) {
pullRequest(number: $number) {
@@ -308,14 +292,13 @@ async function getIssueTimelineItems(
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'--cache',
'60s',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}/timeline?per_page=${GITHUB_REST_PAGE_SIZE}&page=${page}`,
'--jq',
'.[] | @json'
],
ghOptions
{ ...ghOptions, ...githubHostExecOptions(ownerRepo) }
)
// Why: --jq emits NDJSON and explicit paging lets us stop once we hit the drawer cap.
const pageEvents = parseRestTimelineEventLines(stdout)
@@ -512,17 +495,14 @@ async function getPRMetadata(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ body: string; headSha?: string; baseSha?: string }> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
try {
if (ownerRepo) {
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'--cache',
'60s',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}`
],
['api', '--cache', '60s', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}`],
ghOptions
)
const data = JSON.parse(stdout) as {
@@ -563,10 +543,13 @@ async function getPRFiles(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubPRFile[] | null> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
if (!ownerRepo) {
return null
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
try {
const data: RESTPRFile[] = []
for (let page = 1; data.length < MAX_PR_FILES; page += 1) {
@@ -574,7 +557,6 @@ async function getPRFiles(
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'--cache',
'60s',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}/files?per_page=100${pageSuffix}`
@@ -613,10 +595,13 @@ async function getPRFileViewedStates(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<PRFileViewedStatesResult | null> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
if (!ownerRepo) {
return null
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
if (repositoryRateLimitGuard(ownerRepo, 'graphql', localGitOptions).blocked) {
return null
}
@@ -628,7 +613,6 @@ async function getPRFileViewedStates(
for (let fetched = 0; fetched < MAX_PR_FILES; fetched += 100) {
const args = [
'api',
...githubApiHostArgs(ownerRepo),
'graphql',
'-f',
`query=${PR_FILE_VIEWED_STATES_QUERY}`,
@@ -819,7 +803,6 @@ async function getWorkItemParticipants(
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'graphql',
'-f',
`query=${WORK_ITEM_PARTICIPANTS_QUERY}`,
@@ -832,7 +815,10 @@ async function getWorkItemParticipants(
'-F',
`isPr=${item.type === 'pr'}`
],
ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
{
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
)
const data = JSON.parse(stdout) as {
data?: {
@@ -892,14 +878,11 @@ async function getGitHubUsersByLogin(
try {
noteRepositoryRateLimitSpend(repository, 'graphql', 1, localGitOptions)
const { stdout } = await ghExecFileAsync(
[
'api',
...(repository ? githubApiHostArgs(repository) : []),
'graphql',
'-f',
`query=query { ${fields} }`
],
ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
['api', 'graphql', '-f', `query=query { ${fields} }`],
{
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(repository)
}
)
const data = JSON.parse(stdout) as {
data?: Record<
@@ -1204,13 +1187,13 @@ async function fetchContentAtRef(args: {
'300s',
'-H',
'Accept: application/vnd.github.raw',
...githubApiHostArgs(args.ownerRepo),
`repos/${args.ownerRepo.owner}/${args.ownerRepo.repo}/contents/${encodeGitHubContentPath(args.path)}?ref=${encodeURIComponent(args.ref)}`
],
{
...ghRepoExecOptions(
githubRepoContext(args.repoPath, args.connectionId, args.localGitOptions)
),
...githubHostExecOptions(args.ownerRepo),
maxBuffer: GITHUB_RAW_CONTENT_MAX_BUFFER_BYTES
}
)
src/renderer/src/components/right-sidebar/checks-panel-async-result-key.ts
+2 -6
View File
@@ -1,12 +1,8 @@
import type { GitHubOwnerRepo } from '../../../../shared/types'
import { githubRepoIdentityKey } from '../../../../shared/github-repository-identity-key'
function normalizedPRRepoIdentity(prRepo?: GitHubOwnerRepo | null): string {
if (!prRepo) {
return 'none'
}
const slug = `${prRepo.owner.toLowerCase()}/${prRepo.repo.toLowerCase()}`
const host = prRepo.host?.toLowerCase()
return host && host !== 'github.com' ? `${host}/${slug}` : slug
return prRepo ? githubRepoIdentityKey(prRepo) : 'none'
}
export function checksPanelAsyncResultKey(
src/renderer/src/store/slices/github-checks.ts
+2 -6
View File
@@ -1,6 +1,7 @@
import type { AppState } from '../types'
import type { PRCheckDetail, CheckStatus, GitHubOwnerRepo } from '../../../../shared/types'
import { getGitHubPRCacheKey } from './github-cache-key'
import { githubRepoIdentityKey } from '../../../../shared/github-repository-identity-key'
export function normalizeBranchName(branch: string): string {
return branch.replace(/^refs\/heads\//, '')
@@ -97,12 +98,7 @@ export function syncPRChecksStatus(
}
function normalizedPRRepo(repo?: GitHubOwnerRepo | null): string | null {
if (!repo) {
return null
}
const slug = `${repo.owner.toLowerCase()}/${repo.repo.toLowerCase()}`
const host = repo.host?.toLowerCase()
return host && host !== 'github.com' ? `${host}/${slug}` : slug
return repo ? githubRepoIdentityKey(repo) : null
}
function samePRRepo(left?: GitHubOwnerRepo | null, right?: GitHubOwnerRepo | null): boolean {
src/renderer/src/store/slices/github.ts
+2 -3
View File
@@ -2,6 +2,7 @@
import type { StateCreator } from 'zustand'
import { toast } from 'sonner'
import type { AppState } from '../types'
import { githubRepoIdentityKey } from '../../../../shared/github-repository-identity-key'
import type {
ClassifiedError,
GitHubOwnerRepo,
@@ -859,9 +860,7 @@ function evictRepoCacheEntries<T>(
}
function normalizedRepoIdentity(repo: GitHubOwnerRepo): string {
const slug = `${repo.owner.toLowerCase()}/${repo.repo.toLowerCase()}`
const host = repo.host?.toLowerCase()
return host && host !== 'github.com' ? `${host}/${slug}` : slug
return githubRepoIdentityKey(repo)
}
function normalizedHeadSha(headSha?: string): string | null {
src/shared/github-repository-identity-key.ts
+12
View File
@@ -0,0 +1,12 @@
// Why: cache keys and equality checks for GitHub repos must include the host,
// or a GHES repo and a same-named github.com repo would collide. github.com is
// omitted so pre-Enterprise host-less keys stay stable.
export function githubRepoIdentityKey(repo: {
owner: string
repo: string
host?: string
}): string {
const slug = `${repo.owner.toLowerCase()}/${repo.repo.toLowerCase()}`
const host = repo.host?.toLowerCase()
return host && host !== 'github.com' ? `${host}/${slug}` : slug
}