Fix PR review actions failing on GitHub Enterprise remotes

- Threads GitHub host identity (not just owner/repo) through the client,
  work-item-details, issues, and RPC layers so gh commands target the
  correct Enterprise server instead of silently falling back to github.com
- Adds a shared github-api-repository helper to resolve/host-qualify repo
  identity consistently across REST, GraphQL, and CLI shorthand calls
- Scopes the gh rate-limit breaker and singleton rate-limit snapshot by
  host/runtime so a github.com block or probe can't affect GHES or WSL
- Coalesces concurrent host-auth probes and paginates PR file fetching
  beyond 100 results
- Propagates `host` through renderer PR caches, checks-panel keys, and
  preload IPC types so Enterprise and github.com data never collide
This commit is contained in:
Jinjing
2026-07-16 22:32:03 -07:00
parent 8b0201f11a
commit c8edd223ad
31 changed files with 1547 additions and 306 deletions
@@ -41,6 +41,7 @@ describe('isGhRateLimitProbe', () => {
it('recognizes the exempt rate_limit endpoint only', () => {
expect(isGhRateLimitProbe(['api', 'rate_limit'])).toBe(true)
expect(isGhRateLimitProbe(['api', '/rate_limit'])).toBe(true)
expect(isGhRateLimitProbe(['api', '--hostname', 'github.com', 'rate_limit'])).toBe(true)
expect(isGhRateLimitProbe(['api', 'search/issues?q=x'])).toBe(false)
expect(isGhRateLimitProbe(['pr', 'list'])).toBe(false)
})
@@ -76,6 +77,15 @@ describe('breaker state', () => {
expect(getGhRateLimitBlockedUntilMs('core', now)).toBe(now + 60_000)
})
it('isolates blocks by host and execution runtime scope', () => {
const now = 1_000_000
recordGhPrimaryRateLimit('core', now + 60_000, 'native:github.com')
expect(getGhRateLimitBlockedUntilMs('core', now, 'native:github.com')).toBe(now + 60_000)
expect(getGhRateLimitBlockedUntilMs('core', now, 'native:github.acme-corp.com')).toBeNull()
expect(getGhRateLimitBlockedUntilMs('core', now, 'wsl:ubuntu:github.com')).toBeNull()
})
it('notifyGhPrimaryRateLimit applies a fallback block and fires the reset probe', () => {
const probe = vi.fn()
registerGhRateLimitResetProbe(probe)
@@ -84,6 +94,18 @@ describe('breaker state', () => {
expect(getGhRateLimitBlockedUntilMs('search')).toBeGreaterThan(Date.now())
})
it('does not refine a non-default scope with the native github.com probe', () => {
const probe = vi.fn()
registerGhRateLimitResetProbe(probe)
notifyGhPrimaryRateLimit('core', 'native:github.acme-corp.com')
expect(probe).not.toHaveBeenCalled()
expect(
getGhRateLimitBlockedUntilMs('core', Date.now(), 'native:github.acme-corp.com')
).not.toBeNull()
})
it('creates an error that classifies as rate_limited, not permission_denied', () => {
const error = createGhRateLimitBlockedError('search', Date.now() + 30_000)
expect(error.stderr.toLowerCase()).toContain('rate limit')
+29 -12
View File
@@ -25,9 +25,14 @@ const FALLBACK_BLOCK_MS: Record<GhRateLimitBucket, number> = {
graphql: 5 * 60_000
}
const blockedUntilMsByBucket = new Map<GhRateLimitBucket, number>()
const DEFAULT_SCOPE = 'native:github.com'
const blockedUntilMsByScopeAndBucket = new Map<string, number>()
let resetProbe: ((bucket: GhRateLimitBucket) => void) | null = null
function breakerKey(bucket: GhRateLimitBucket, scope = DEFAULT_SCOPE): string {
return `${scope}\0${bucket}`
}
// gh api flags that take a separate value, so the endpoint arg can be found.
const GH_API_VALUE_FLAGS = new Set([
'--cache',
@@ -103,25 +108,32 @@ export function isGhPrimaryRateLimitStderr(stderr: string): boolean {
return s.includes('api rate limit exceeded') && !s.includes('secondary rate limit')
}
export function recordGhPrimaryRateLimit(bucket: GhRateLimitBucket, blockedUntilMs: number): void {
const existing = blockedUntilMsByBucket.get(bucket) ?? 0
blockedUntilMsByBucket.set(bucket, Math.max(existing, blockedUntilMs))
export function recordGhPrimaryRateLimit(
bucket: GhRateLimitBucket,
blockedUntilMs: number,
scope = DEFAULT_SCOPE
): void {
const key = breakerKey(bucket, scope)
const existing = blockedUntilMsByScopeAndBucket.get(key) ?? 0
blockedUntilMsByScopeAndBucket.set(key, Math.max(existing, blockedUntilMs))
}
export function clearGhRateLimitBlock(bucket: GhRateLimitBucket): void {
blockedUntilMsByBucket.delete(bucket)
export function clearGhRateLimitBlock(bucket: GhRateLimitBucket, scope = DEFAULT_SCOPE): void {
blockedUntilMsByScopeAndBucket.delete(breakerKey(bucket, scope))
}
export function getGhRateLimitBlockedUntilMs(
bucket: GhRateLimitBucket,
nowMs: number = Date.now()
nowMs: number = Date.now(),
scope = DEFAULT_SCOPE
): number | null {
const blockedUntil = blockedUntilMsByBucket.get(bucket)
const key = breakerKey(bucket, scope)
const blockedUntil = blockedUntilMsByScopeAndBucket.get(key)
if (blockedUntil === undefined) {
return null
}
if (blockedUntil <= nowMs) {
blockedUntilMsByBucket.delete(bucket)
blockedUntilMsByScopeAndBucket.delete(key)
return null
}
return blockedUntil
@@ -136,8 +148,13 @@ export function registerGhRateLimitResetProbe(
}
/** Called by the runner when a gh spawn came back with a primary 403. */
export function notifyGhPrimaryRateLimit(bucket: GhRateLimitBucket): void {
recordGhPrimaryRateLimit(bucket, Date.now() + FALLBACK_BLOCK_MS[bucket])
export function notifyGhPrimaryRateLimit(bucket: GhRateLimitBucket, scope = DEFAULT_SCOPE): void {
recordGhPrimaryRateLimit(bucket, Date.now() + FALLBACK_BLOCK_MS[bucket], scope)
// The registered probe uses native github.com credentials; it cannot refine
// a GHES or WSL bucket without querying the wrong account/runtime.
if (scope !== DEFAULT_SCOPE) {
return
}
try {
resetProbe?.(bucket)
} catch {
@@ -161,6 +178,6 @@ export function createGhRateLimitBlockedError(
/** @internal — test-only */
export function _resetGhRateLimitBreaker(): void {
blockedUntilMsByBucket.clear()
blockedUntilMsByScopeAndBucket.clear()
resetProbe = null
}
@@ -71,11 +71,35 @@ describe('ghExecFileAsync rate-limit breaker', () => {
expect(execFileMock).toHaveBeenCalledTimes(2)
})
it('keeps other GitHub hosts and WSL runtimes working when github.com is blocked', async () => {
mockGhFailure(PRIMARY_RATE_LIMIT_STDERR)
await expect(ghExecFileAsync(['api', 'repos/a/b/pulls'])).rejects.toThrow()
mockGhSuccess('[]')
await expect(
ghExecFileAsync(['api', '--hostname', 'github.acme-corp.com', 'repos/a/b/pulls'])
).resolves.toMatchObject({ stdout: '[]' })
await expect(
ghExecFileAsync(['pr', 'list', '--repo', 'github.acme-corp.com/a/b'])
).resolves.toMatchObject({ stdout: '[]' })
await expect(
ghExecFileAsync(['api', 'repos/a/b/pulls'], {
env: { ...process.env, GH_HOST: 'github.acme-corp.com' }
})
).resolves.toMatchObject({ stdout: '[]' })
await expect(
ghExecFileAsync(['api', 'repos/a/b/pulls'], { wslDistro: 'Ubuntu' })
).resolves.toMatchObject({ stdout: '[]' })
expect(execFileMock).toHaveBeenCalledTimes(5)
})
it('never blocks the exempt rate_limit probe', async () => {
mockGhFailure(PRIMARY_RATE_LIMIT_STDERR)
await expect(ghExecFileAsync(['api', 'repos/a/b/pulls'])).rejects.toThrow()
mockGhSuccess('{"resources":{}}')
await expect(ghExecFileAsync(['api', 'rate_limit'])).resolves.toMatchObject({
await expect(
ghExecFileAsync(['api', '--hostname', 'github.com', 'rate_limit'])
).resolves.toMatchObject({
stdout: '{"resources":{}}'
})
expect(execFileMock).toHaveBeenCalledTimes(2)
+48 -3
View File
@@ -1402,6 +1402,44 @@ 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 ghRequestHost(args: readonly string[], env?: NodeJS.ProcessEnv): string {
const explicitHost = ghFlagValue(args, ['--hostname'])
if (explicitHost) {
return explicitHost.toLowerCase()
}
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()
}
function ghRateLimitScope(
args: readonly string[],
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)}`
}
/**
* Async gh CLI execution. Drop-in replacement for
* `execFileAsync('gh', args, { cwd, encoding, ... })`.
@@ -1414,17 +1452,24 @@ export async function ghExecFileAsync(
args: string[],
options: GhExecOptions = {}
): Promise<{ stdout: string; stderr: string }> {
let resolved = resolveCommand('gh', args, options.cwd, options.wslDistro)
// Why: while a bucket's primary rate limit is exhausted, every spawn would
// return the same 403 — fail fast without paying the subprocess cost. The
// rate_limit probe itself is exempt so the breaker can learn the reset time.
// Scope by execution runtime and host so github.com, GHES, and WSL accounts
// cannot block one another after an unrelated quota exhaustion.
const rateLimitBucket = classifyGhRateLimitBucket(args)
const initialRateLimitScope = ghRateLimitScope(args, options, resolved)
if (!isGhRateLimitProbe(args)) {
const blockedUntilMs = getGhRateLimitBlockedUntilMs(rateLimitBucket)
const blockedUntilMs = getGhRateLimitBlockedUntilMs(
rateLimitBucket,
Date.now(),
initialRateLimitScope
)
if (blockedUntilMs !== null) {
throw createGhRateLimitBlockedError(rateLimitBucket, blockedUntilMs)
}
}
let resolved = resolveCommand('gh', args, options.cwd, options.wslDistro)
let lastError: unknown
let attemptedHostFallback = false
let attemptedDefaultWslFallback = false
@@ -1444,7 +1489,7 @@ export async function ghExecFileAsync(
lastError = err
const { stderr } = extractExecError(err)
if (isGhPrimaryRateLimitStderr(stderr)) {
notifyGhPrimaryRateLimit(rateLimitBucket)
notifyGhPrimaryRateLimit(rateLimitBucket, ghRateLimitScope(args, options, resolved))
}
if (
process.platform === 'win32' &&
+49 -2
View File
@@ -1,7 +1,15 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { ghExecFileAsyncMock, acquireMock, releaseMock } = vi.hoisted(() => ({
const {
ghExecFileAsyncMock,
getOwnerRepoMock,
getEnterpriseGitHubRepoSlugMock,
acquireMock,
releaseMock
} = vi.hoisted(() => ({
ghExecFileAsyncMock: vi.fn(),
getOwnerRepoMock: vi.fn(),
getEnterpriseGitHubRepoSlugMock: vi.fn(),
acquireMock: vi.fn(),
releaseMock: vi.fn()
}))
@@ -10,7 +18,7 @@ vi.mock('./gh-utils', () => ({
execFileAsync: vi.fn(),
ghExecFileAsync: ghExecFileAsyncMock,
gitExecFileAsync: vi.fn(),
getOwnerRepo: vi.fn(),
getOwnerRepo: getOwnerRepoMock,
getIssueOwnerRepo: vi.fn(),
getOwnerRepoForRemote: vi.fn(),
resolveIssueSource: vi.fn(),
@@ -31,11 +39,19 @@ vi.mock('./gh-utils', () => ({
_resetOwnerRepoCache: vi.fn()
}))
vi.mock('./github-enterprise-repository', () => ({
getEnterpriseGitHubRepoSlug: getEnterpriseGitHubRepoSlugMock
}))
import { setPRFileViewed } from './client'
describe('setPRFileViewed', () => {
beforeEach(() => {
ghExecFileAsyncMock.mockReset()
getOwnerRepoMock.mockReset()
getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'orca' })
getEnterpriseGitHubRepoSlugMock.mockReset()
getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null)
acquireMock.mockReset()
releaseMock.mockReset()
acquireMock.mockResolvedValue(undefined)
@@ -98,4 +114,35 @@ describe('setPRFileViewed', () => {
wslDistro: 'Ubuntu'
})
})
it('routes SSH-backed GitHub Enterprise file-viewed mutations to the Enterprise host', async () => {
getOwnerRepoMock.mockResolvedValueOnce(null)
getEnterpriseGitHubRepoSlugMock.mockResolvedValueOnce({
owner: 'team',
repo: 'orca',
host: 'github.acme-corp.com'
})
ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '{}' })
await expect(
setPRFileViewed({
repoPath: '/remote/repo',
connectionId: 'ssh-1',
pullRequestId: 'PR_enterprise',
path: 'src/enterprise.ts',
viewed: true
})
).resolves.toBe(true)
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
expect.arrayContaining([
'api',
'--hostname',
'github.acme-corp.com',
'graphql',
'pullRequestId=PR_enterprise'
]),
{}
)
})
})
@@ -210,4 +210,60 @@ describe('getPRCheckDetails', () => {
)
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(4)
})
it('isolates failed-job log tails for the same job ID on different GitHub hosts', async () => {
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
const endpoint = args.find((arg) => arg.startsWith('repos/')) ?? ''
const enterprise = args.includes('github.acme-corp.com')
if (endpoint.endsWith('/check-runs/99')) {
return {
stdout: JSON.stringify({
name: 'verify',
status: 'completed',
conclusion: 'failure',
check_suite: { workflow_run: { id: 77 } }
})
}
}
if (endpoint.endsWith('/check-runs/99/annotations?per_page=20')) {
return { stdout: '[]' }
}
if (endpoint.endsWith('/actions/runs/77/jobs?per_page=100')) {
return {
stdout: JSON.stringify({
jobs: [
{
id: 9901,
name: 'verify',
status: 'completed',
conclusion: 'failure',
steps: []
}
]
})
}
}
if (endpoint.endsWith('/actions/jobs/9901/logs')) {
return { stdout: enterprise ? 'enterprise log' : 'github.com log' }
}
throw new Error(`unexpected gh call: ${args.join(' ')}`)
})
const githubDotCom = await getPRCheckDetails('/repo-root', {
checkRunId: 99,
prRepo: { owner: 'acme', repo: 'widgets', host: 'github.com' }
})
const enterprise = await getPRCheckDetails('/repo-root', {
checkRunId: 99,
prRepo: { owner: 'acme', repo: 'widgets', host: 'github.acme-corp.com' }
})
expect(githubDotCom?.jobs[0]?.logTail).toBe('github.com log')
expect(enterprise?.jobs[0]?.logTail).toBe('enterprise log')
const logCalls = ghExecFileAsyncMock.mock.calls.filter(([args]) =>
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']))
})
})
+5 -1
View File
@@ -591,7 +591,11 @@ describe('getPRChecks', () => {
stdout: JSON.stringify([{ name: 'lint', state: 'PASS', link: 'https://example.com/lint' }])
})
await getPRChecks('/repo-root', 42, undefined, { owner: 'acme', repo: 'widgets' })
await getPRChecks('/repo-root', 42, undefined, {
owner: 'acme',
repo: 'widgets',
host: 'github.com'
})
expect(getOwnerRepoMock).not.toHaveBeenCalled()
expectGraphQLRollupCall()
@@ -10,6 +10,7 @@ const {
getOwnerRepoMock,
getIssueOwnerRepoMock,
getOwnerRepoForRemoteMock,
getEnterpriseGitHubRepoSlugMock,
resolvePRRepositoryCandidatesMock,
getRemoteUrlForRepoMock,
gitExecFileAsyncMock,
@@ -24,6 +25,7 @@ const {
getOwnerRepoMock: vi.fn(),
getIssueOwnerRepoMock: vi.fn(),
getOwnerRepoForRemoteMock: vi.fn(),
getEnterpriseGitHubRepoSlugMock: vi.fn(),
resolvePRRepositoryCandidatesMock: vi.fn(),
getRemoteUrlForRepoMock: vi.fn(),
gitExecFileAsyncMock: vi.fn(),
@@ -76,6 +78,10 @@ vi.mock('../git/runner', () => ({
gitExecFileAsync: gitExecFileAsyncMock
}))
vi.mock('./github-enterprise-repository', () => ({
getEnterpriseGitHubRepoSlug: getEnterpriseGitHubRepoSlugMock
}))
vi.mock('../providers/ssh-git-dispatch', () => ({
getSshGitProvider: vi.fn()
}))
@@ -93,9 +99,13 @@ vi.mock('./rate-limit', () => ({
import {
addPRReviewComment,
addPRReviewCommentReply,
getPRCheckDetails,
getPRChecks,
getWorkItemByOwnerRepo,
getPRComments,
mergePR,
removePRReviewers,
rerunPRChecks,
requestPRReviewers,
resolveReviewThread,
setPRAutoMerge,
@@ -111,6 +121,8 @@ describe('GitHub PR local runtime routing', () => {
getOwnerRepoMock.mockReset()
getIssueOwnerRepoMock.mockReset()
getOwnerRepoForRemoteMock.mockReset()
getEnterpriseGitHubRepoSlugMock.mockReset()
getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null)
resolvePRRepositoryCandidatesMock.mockReset()
getRemoteUrlForRepoMock.mockReset()
gitExecFileAsyncMock.mockReset()
@@ -127,12 +139,28 @@ describe('GitHub PR local runtime routing', () => {
it('routes PR details and mutations through the selected WSL distro', async () => {
const localGitOptions = { wslDistro: 'Ubuntu' }
const prRepo = { owner: 'acme', repo: 'orca' }
rateLimitGuardMock.mockReturnValue({
blocked: true,
remaining: 0,
limit: 5000,
resetAt: 1_800_000_000
})
getOwnerRepoMock.mockResolvedValue(prRepo)
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
const endpoint = args.find((arg) => arg.startsWith('repos/acme/orca/')) ?? ''
const query = args.find((arg) => arg.startsWith('query=')) ?? ''
if (args[0] === 'pr' && args[1] === 'view') {
const jsonFields = args[args.indexOf('--json') + 1]
if (jsonFields === 'id,headRefOid,baseRefName') {
return {
stdout: JSON.stringify({
id: 'PR_local',
headRefOid: 'head-oid',
baseRefName: 'main'
})
}
}
return {
stdout: JSON.stringify({
id: 'PR_kwDO123',
@@ -238,5 +266,250 @@ describe('GitHub PR local runtime routing', () => {
expect(ghExecFileAsyncMock.mock.calls.every((call) => call[1]?.wslDistro === 'Ubuntu')).toBe(
true
)
expect(getRateLimitMock).not.toHaveBeenCalled()
expect(rateLimitGuardMock).not.toHaveBeenCalled()
expect(noteRateLimitSpendMock).not.toHaveBeenCalled()
})
it('never falls through to the default gh host for an unresolved SSH repository', async () => {
const legacyRepo = { owner: 'team', repo: 'orca' }
getOwnerRepoMock.mockResolvedValue(null)
getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null)
await expect(getPRComments('/remote/repo', 7, { prRepo: legacyRepo }, 'ssh-1')).rejects.toThrow(
'GitHub remote'
)
await expect(
getPRChecks('/remote/repo', 7, undefined, legacyRepo, undefined, 'ssh-1')
).rejects.toThrow('GitHub remote')
await expect(mergePR('/remote/repo', 7, 'squash', 'ssh-1', legacyRepo)).resolves.toMatchObject({
ok: false
})
await expect(
setPRAutoMerge('/remote/repo', 7, true, 'squash', 'ssh-1', legacyRepo)
).resolves.toMatchObject({ ok: false })
await expect(updatePRTitle('/remote/repo', 7, 'New title', 'ssh-1', legacyRepo)).resolves.toBe(
false
)
await expect(requestPRReviewers('/remote/repo', 7, ['octo'], 'ssh-1')).resolves.toMatchObject({
ok: false
})
await expect(removePRReviewers('/remote/repo', 7, ['octo'], 'ssh-1')).resolves.toMatchObject({
ok: false
})
await expect(
addPRReviewCommentReply(
'/remote/repo',
7,
11,
'Reply',
undefined,
undefined,
undefined,
'ssh-1',
legacyRepo
)
).resolves.toMatchObject({ ok: false })
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
})
it('host-qualifies SSH-backed GitHub Enterprise review reads and mutations', async () => {
const enterpriseRepo = {
owner: 'team',
repo: 'orca',
host: 'github.acme-corp.com'
}
getOwnerRepoMock.mockResolvedValue(null)
getEnterpriseGitHubRepoSlugMock.mockResolvedValue(enterpriseRepo)
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
const endpoint = args.find((arg) => arg.startsWith('repos/team/orca/')) ?? ''
const query = args.find((arg) => arg.startsWith('query=')) ?? ''
if (args[0] === 'pr' && args[1] === 'checks') {
return { stdout: '[]' }
}
if (args[0] === 'pr' && args[1] === 'view') {
if (args.includes('id,headRefOid,baseRefName')) {
return {
stdout: JSON.stringify({
id: 'PR_enterprise',
headRefOid: 'head-sha',
baseRefName: 'main'
})
}
}
return {
stdout: JSON.stringify({
number: 7,
title: 'Enterprise PR',
state: 'OPEN',
url: 'https://github.acme-corp.com/team/orca/pull/7',
labels: [],
updatedAt: '2026-07-16T00:00:00Z',
author: { login: 'pr-author' },
headRefName: 'feature',
baseRefName: 'main'
})
}
}
if (query.includes('reviewThreads')) {
return {
stdout: JSON.stringify({
data: {
repository: {
pullRequest: {
reviewThreads: { nodes: [] },
comments: { nodes: [] }
}
}
}
})
}
}
if (
endpoint.endsWith('/issues/7/comments?per_page=100') ||
endpoint.endsWith('/pulls/7/reviews?per_page=100')
) {
return { stdout: '[]' }
}
if (endpoint.endsWith('/commits/head-sha/check-runs?per_page=100')) {
return {
stdout: JSON.stringify({
check_runs: [
{
id: 88,
name: 'lint',
status: 'completed',
conclusion: 'failure',
details_url: 'https://github.acme-corp.com/team/orca/actions/runs/77/job/88'
}
]
})
}
}
if (endpoint.endsWith('/commits/head-sha/status?per_page=100')) {
return { stdout: JSON.stringify({ statuses: [] }) }
}
if (endpoint.endsWith('/commits/head-sha/check-suites?per_page=100')) {
return { stdout: JSON.stringify({ check_suites: [] }) }
}
if (endpoint.endsWith('/check-runs/88')) {
return {
stdout: JSON.stringify({
id: 88,
name: 'lint',
status: 'completed',
conclusion: 'failure',
details_url: 'https://github.acme-corp.com/team/orca/actions/runs/77/job/88',
output: { title: 'Lint failed', summary: 'One error' }
})
}
}
if (endpoint.endsWith('/check-runs/88/annotations?per_page=20')) {
return { stdout: '[]' }
}
if (query) {
return { stdout: JSON.stringify({ data: { repository: {} } }) }
}
return {
stdout: JSON.stringify({
id: 13,
user: null,
body: 'Enterprise inline comment'
})
}
})
await expect(
getWorkItemByOwnerRepo('/remote/repo', enterpriseRepo, 7, 'pr', 'ssh-1')
).resolves.toMatchObject({ number: 7, title: 'Enterprise PR' })
await expect(
getPRComments('/remote/repo', 7, { prRepo: enterpriseRepo }, 'ssh-1')
).resolves.toEqual([])
await expect(
getPRChecks('/remote/repo', 7, undefined, enterpriseRepo, undefined, 'ssh-1')
).resolves.toEqual([])
await expect(
getPRCheckDetails('/remote/repo', { checkRunId: 88, prRepo: enterpriseRepo }, 'ssh-1')
).resolves.toMatchObject({ name: 'lint', conclusion: 'failure' })
await expect(
rerunPRChecks('/remote/repo', 7, { headSha: 'head-sha', failedOnly: true }, 'ssh-1')
).resolves.toEqual({ ok: true, count: 1 })
await expect(resolveReviewThread('/remote/repo', 'thread-1', true, 'ssh-1')).resolves.toBe(true)
await expect(
addPRReviewCommentReply(
'/remote/repo',
7,
11,
'Enterprise reply',
'thread-1',
'src/enterprise.ts',
10,
'ssh-1',
enterpriseRepo
)
).resolves.toMatchObject({ ok: true })
await expect(
addPRReviewComment({
repoPath: '/remote/repo',
connectionId: 'ssh-1',
prNumber: 7,
body: 'Enterprise inline comment',
commitId: 'head-sha',
path: 'src/enterprise.ts',
line: 10
})
).resolves.toMatchObject({ ok: true })
await expect(
updatePRTitle('/remote/repo', 7, 'New title', 'ssh-1', enterpriseRepo)
).resolves.toBe(true)
await expect(
updatePRDetails('/remote/repo', 7, { body: 'New body' }, 'ssh-1', enterpriseRepo)
).resolves.toEqual({ ok: true })
await expect(updatePRState('/remote/repo', 7, { state: 'closed' }, 'ssh-1')).resolves.toEqual({
ok: true
})
await expect(requestPRReviewers('/remote/repo', 7, ['octo'], 'ssh-1')).resolves.toEqual({
ok: true
})
await expect(removePRReviewers('/remote/repo', 7, ['octo'], 'ssh-1')).resolves.toEqual({
ok: true
})
await expect(
setPRAutoMerge('/remote/repo', 7, true, 'squash', 'ssh-1', enterpriseRepo)
).resolves.toEqual({ ok: true })
await expect(
setPRAutoMerge('/remote/repo', 7, false, 'squash', 'ssh-1', enterpriseRepo)
).resolves.toEqual({ ok: true })
await expect(mergePR('/remote/repo', 7, 'squash', 'ssh-1', enterpriseRepo)).resolves.toEqual({
ok: true
})
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({})
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'
)
).toBe(true)
const apiCalls = ghExecFileAsyncMock.mock.calls.filter(([args]) => args[0] === 'api')
expect(apiCalls.length).toBeGreaterThan(0)
expect(
apiCalls.every(
([args, options]) =>
args.includes('--hostname') &&
args[args.indexOf('--hostname') + 1] === 'github.acme-corp.com' &&
options.cwd === undefined &&
options.wslDistro === undefined
)
).toBe(true)
})
})
+93 -9
View File
@@ -111,6 +111,7 @@ import {
getPRForBranchOutcome,
getRepoUpstream,
getWorkItem,
getWorkItemByOwnerRepo,
getPullRequestPushTarget,
mergePR,
resolveReviewThread,
@@ -3739,7 +3740,12 @@ describe('GitHub GraphQL rate-limit guard', () => {
})
.mockResolvedValueOnce({ stdout: '[]' })
await getPRComments('/repo-root', 7, { prRepo: { owner: 'stablyai', repo: 'orca' } }, undefined)
await getPRComments(
'/repo-root',
7,
{ prRepo: { owner: 'stablyai', repo: 'orca', host: 'github.com' } },
undefined
)
expect(getOwnerRepoMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
@@ -3774,10 +3780,18 @@ describe('GitHub GraphQL rate-limit guard', () => {
.mockResolvedValue({ stdout: '', stderr: '' })
await expect(
mergePR('/repo-root', 7, 'squash', undefined, { owner: 'stablyai', repo: 'orca' })
mergePR('/repo-root', 7, 'squash', undefined, {
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
})
).resolves.toEqual({ ok: true })
await expect(
updatePRTitle('/repo-root', 7, 'New title', undefined, { owner: 'stablyai', repo: 'orca' })
updatePRTitle('/repo-root', 7, 'New title', undefined, {
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
})
).resolves.toBe(true)
expect(getOwnerRepoMock).not.toHaveBeenCalled()
@@ -3819,13 +3833,15 @@ describe('GitHub GraphQL rate-limit guard', () => {
await expect(
setPRAutoMerge('/remote/repo-root', 7, true, 'squash', 'ssh-1', {
owner: 'stablyai',
repo: 'orca'
repo: 'orca',
host: 'github.com'
})
).resolves.toEqual({ ok: true })
await expect(
setPRAutoMerge('/remote/repo-root', 7, false, 'squash', 'ssh-1', {
owner: 'stablyai',
repo: 'orca'
repo: 'orca',
host: 'github.com'
})
).resolves.toEqual({ ok: true })
@@ -3963,7 +3979,11 @@ describe('GitHub GraphQL rate-limit guard', () => {
})
await expect(
mergePR('/repo-root', 7, 'squash', undefined, { owner: 'stablyai', repo: 'orca' })
mergePR('/repo-root', 7, 'squash', undefined, {
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
})
).resolves.toEqual({
ok: false,
error: 'This pull request requires review approval before it can be merged.'
@@ -3998,7 +4018,11 @@ describe('GitHub GraphQL rate-limit guard', () => {
.mockResolvedValueOnce({ stdout: JSON.stringify(prView) })
await expect(
mergePR('/repo-root', 7, 'squash', undefined, { owner: 'stablyai', repo: 'orca' })
mergePR('/repo-root', 7, 'squash', undefined, {
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
})
).resolves.toEqual({
ok: false,
error:
@@ -4087,6 +4111,58 @@ describe('GitHub GraphQL rate-limit guard', () => {
expect(_getMergeQueueCacheSizeForTests()).toBe(256)
})
it('isolates merge metadata for the same slug on different GitHub hosts', async () => {
const prView = {
number: 7,
title: 'PR',
state: 'OPEN',
url: 'https://github.com/acme/widgets/pull/7',
statusCheckRollup: [],
updatedAt: '2026-07-16T00:00:00Z',
isDraft: false,
mergeable: 'MERGEABLE',
baseRefName: 'main',
headRefOid: 'head-oid'
}
ghExecFileAsyncMock.mockImplementation(async (args) => {
if (args.includes('graphql')) {
const enterprise = args.includes('github.acme-corp.com')
return {
stdout: JSON.stringify({
data: {
repository: {
mergeQueue: null,
autoMergeAllowed: !enterprise
}
}
})
}
}
return { stdout: JSON.stringify(prView) }
})
const githubDotCom = await getWorkItemByOwnerRepo(
'/repo-root',
{ owner: 'acme', repo: 'widgets', host: 'github.com' },
7,
'pr'
)
const enterprise = await getWorkItemByOwnerRepo(
'/repo-root',
{ owner: 'acme', repo: 'widgets', host: 'github.acme-corp.com' },
7,
'pr'
)
expect(githubDotCom?.autoMergeAllowed).toBe(true)
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'])
)
})
it('returns conflicting file details instead of running gh merge when PR is dirty', async () => {
ghExecFileAsyncMock.mockResolvedValueOnce({
stdout: JSON.stringify({
@@ -4111,7 +4187,11 @@ describe('GitHub GraphQL rate-limit guard', () => {
.mockResolvedValueOnce({ stdout: 'result-tree-oid\u0000src/conflict.ts\u0000' })
await expect(
mergePR('/repo-root', 7, 'squash', undefined, { owner: 'stablyai', repo: 'orca' })
mergePR('/repo-root', 7, 'squash', undefined, {
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
})
).resolves.toEqual({
ok: false,
error:
@@ -4144,7 +4224,11 @@ describe('GitHub GraphQL rate-limit guard', () => {
.mockResolvedValueOnce({ stdout: '', stderr: '' })
await expect(
mergePR('/remote/repo-root', 7, 'squash', 'ssh-1', { owner: 'stablyai', repo: 'orca' })
mergePR('/remote/repo-root', 7, 'squash', 'ssh-1', {
owner: 'stablyai',
repo: 'orca',
host: 'github.com'
})
).resolves.toEqual({ ok: true })
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2)
+269 -98
View File
@@ -80,6 +80,15 @@ import {
} from './gh-cwd-repo-negative-cache'
import type { GitHubRepoContext } from './github-repository-identity'
import { getEnterpriseGitHubRepoSlug } from './github-enterprise-repository'
import {
getOriginGitHubApiRepository,
githubApiHostArgs,
githubCliRepositoryArgument,
githubRepositoryWebHost,
isGitHubDotComRepository,
resolveGitHubApiRepository,
type GitHubApiRepository
} from './github-api-repository'
export { _resetOwnerRepoCache } from './gh-utils'
export {
getIssue,
@@ -170,7 +179,16 @@ function pruneRepositoryMergeMetadataCache(now = Date.now()): void {
}
}
async function assertRateLimitBudget(bucket: RateLimitBucketKind): Promise<void> {
async function assertRateLimitBudget(
bucket: RateLimitBucketKind,
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) {
return
}
await getRateLimit()
const guard = rateLimitGuard(bucket)
if (guard.blocked) {
@@ -180,6 +198,27 @@ async function assertRateLimitBudget(bucket: RateLimitBucketKind): Promise<void>
}
}
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 classifyPRRefreshError(
err: unknown
): Extract<PRRefreshOutcome, { kind: 'upstream-error' }>['errorType'] {
@@ -772,7 +811,15 @@ function mapPullRequestWorkItem(
? String((item.base as { ref?: unknown }).ref ?? '')
: String(item.baseRefName ?? ''),
...(headSha ? { headSha } : {}),
...(baseOwnerRepo ? { prRepo: { owner: baseOwnerRepo.owner, repo: baseOwnerRepo.repo } } : {}),
...(baseOwnerRepo
? {
prRepo: {
owner: baseOwnerRepo.owner,
repo: baseOwnerRepo.repo,
...(baseOwnerRepo.host ? { host: baseOwnerRepo.host } : {})
}
}
: {}),
...(additions !== undefined ? { additions } : {}),
...(deletions !== undefined ? { deletions } : {}),
...(changedFiles !== undefined ? { changedFiles } : {}),
@@ -837,7 +884,7 @@ async function hydrateWorkItemRepositoryMergeMetadata(
async function fetchIssueWorkItem(
repoPath: string,
ownerRepo: OwnerRepo | null,
ownerRepo: GitHubApiRepository | null,
number: number,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
@@ -845,7 +892,11 @@ async function fetchIssueWorkItem(
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
if (ownerRepo) {
const { stdout } = await ghExecFileAsync(
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${number}`],
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${number}`
],
ghOptions
)
const item = JSON.parse(stdout) as Record<string, unknown>
@@ -869,7 +920,7 @@ const WORK_ITEM_PR_REVIEW_JSON_FIELDS = 'reviewRequests,latestReviews'
async function fetchPullRequestReviewFields(
number: number,
ownerRepo: OwnerRepo | null,
ownerRepo: GitHubApiRepository | null,
ghOptions: GhExecOptions
): Promise<Pick<MainWorkItem, 'reviewRequests' | 'latestReviews'>> {
try {
@@ -879,7 +930,7 @@ async function fetchPullRequestReviewFields(
'view',
String(number),
'--repo',
`${ownerRepo.owner}/${ownerRepo.repo}`,
githubCliRepositoryArgument(ownerRepo),
'--json',
WORK_ITEM_PR_REVIEW_JSON_FIELDS
]
@@ -901,7 +952,7 @@ async function fetchPullRequestReviewFields(
async function fetchPullRequestWorkItem(
repoPath: string,
ownerRepo: OwnerRepo | null,
ownerRepo: GitHubApiRepository | null,
number: number,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
@@ -915,7 +966,7 @@ async function fetchPullRequestWorkItem(
'view',
String(number),
'--repo',
`${ownerRepo.owner}/${ownerRepo.repo}`,
githubCliRepositoryArgument(ownerRepo),
'--json',
WORK_ITEM_PR_DETAIL_JSON_FIELDS
],
@@ -944,7 +995,11 @@ async function fetchPullRequestWorkItem(
}
} catch {
const { stdout } = await ghExecFileAsync(
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${number}`],
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${number}`
],
ghOptions
)
const mapped = mapPullRequestWorkItem(
@@ -1756,17 +1811,6 @@ function parseCreatePRPayload(stdout: string): { number: number; url: string } |
return { number: Number(urlMatch[1]), url: urlMatch[0] }
}
// Why: `gh --repo OWNER/REPO` resolves the shorthand against gh's default host
// (usually github.com), not the repo's remote — so a GHES repo would target a
// same-named github.com repo, or fail. Qualify with the host for GHES so gh hits
// the Enterprise server; this is the only host signal for SSH repos, which run
// gh with no cwd context (#8312). github.com keeps the bare shorthand.
function ghRepoArg(slug: { owner: string; repo: string; host?: string }): string {
return slug.host && slug.host.toLowerCase() !== 'github.com'
? `${slug.host}/${slug.owner}/${slug.repo}`
: `${slug.owner}/${slug.repo}`
}
async function findOpenPRByHeadBase(args: {
repoPath: string
repoArg: string
@@ -1867,7 +1911,7 @@ export async function createGitHubPullRequest(
}
}
// Host-qualified for GHES so gh targets the Enterprise server, not github.com.
const repoArg = ghRepoArg(ownerRepo)
const repoArg = githubCliRepositoryArgument(ownerRepo)
const base = normalizeHostedReviewBaseRef(input.base)
const head = input.head ? normalizeHostedReviewHeadRef(input.head) || undefined : undefined
@@ -2043,7 +2087,7 @@ export async function getWorkItem(
export async function getWorkItemByOwnerRepo(
repoPath: string,
ownerRepo: OwnerRepo,
ownerRepo: GitHubApiRepository,
number: number,
type: 'issue' | 'pr',
connectionId?: string | null,
@@ -2227,19 +2271,18 @@ function cacheRepositoryMergeMetadata(
}
async function detectRepositoryMergeMetadata(
ownerRepo: OwnerRepo,
ownerRepo: GitHubApiRepository,
branchName: string | undefined,
ghOptions: GhExecOptions
): Promise<GitHubRepositoryMergeMetadata> {
const cacheKey = `${ownerRepo.owner.toLowerCase()}/${ownerRepo.repo.toLowerCase()}:${
branchName ?? '__repo__'
}`
const host = ownerRepo.host?.toLowerCase() ?? 'github.com'
const cacheKey = `${host}/${ownerRepo.owner.toLowerCase()}/${ownerRepo.repo.toLowerCase()}:${branchName ?? '__repo__'}`
pruneRepositoryMergeMetadataCache()
const cached = repositoryMergeMetadataCache.get(cacheKey)
if (cached) {
return cached.value
}
const guard = rateLimitGuard('graphql')
const guard = repositoryRateLimitGuard(ownerRepo, 'graphql', ghOptions)
if (guard.blocked) {
return { mergeQueueRequired: null, autoMergeAllowed: null }
}
@@ -2264,9 +2307,10 @@ async function detectRepositoryMergeMetadata(
}
}`
try {
noteRateLimitSpend('graphql')
noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, ghOptions)
const args = [
'api',
...githubApiHostArgs(ownerRepo),
'graphql',
'-f',
`query=${query}`,
@@ -2357,14 +2401,18 @@ async function hydrateBranchLookupWithExactPR(
}
async function getRestPRForBranch(
prRepo: OwnerRepo,
prRepo: GitHubApiRepository,
headOwner: string,
branchName: string,
ghOptions: ReturnType<typeof ghRepoExecOptions>
): Promise<PullRequestLookupData | null> {
const head = encodeURIComponent(`${headOwner}:${branchName}`)
const { stdout } = await ghExecFileAsync(
['api', `repos/${prRepo.owner}/${prRepo.repo}/pulls?head=${head}&state=all&per_page=1`],
[
'api',
...githubApiHostArgs(prRepo),
`repos/${prRepo.owner}/${prRepo.repo}/pulls?head=${head}&state=all&per_page=1`
],
ghOptions
)
const list = JSON.parse(stdout) as RestPullRequest[]
@@ -2373,7 +2421,7 @@ async function getRestPRForBranch(
}
async function getFallbackPRListForBranch(
prRepo: OwnerRepo,
prRepo: GitHubApiRepository,
branchName: string,
ghOptions: ReturnType<typeof ghRepoExecOptions>
): Promise<PullRequestLookupData | null> {
@@ -2382,7 +2430,7 @@ async function getFallbackPRListForBranch(
'pr',
'list',
'--repo',
`${prRepo.owner}/${prRepo.repo}`,
githubCliRepositoryArgument(prRepo),
'--head',
branchName,
'--state',
@@ -2790,19 +2838,23 @@ async function lookupPRByBranchName(args: {
}
async function getRestPRByNumber(
ownerRepo: OwnerRepo,
ownerRepo: GitHubApiRepository,
number: number,
ghOptions: ReturnType<typeof ghRepoExecOptions>
): Promise<PullRequestLookupData | null> {
const { stdout } = await ghExecFileAsync(
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${number}`],
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${number}`
],
ghOptions
)
return mapRestPullRequest(JSON.parse(stdout) as RestPullRequest)
}
async function getPRByNumber(
ownerRepo: OwnerRepo,
ownerRepo: GitHubApiRepository,
number: number,
ghOptions: ReturnType<typeof ghRepoExecOptions>
): Promise<PullRequestLookupData | null> {
@@ -2813,7 +2865,7 @@ async function getPRByNumber(
'view',
String(number),
'--repo',
`${ownerRepo.owner}/${ownerRepo.repo}`,
githubCliRepositoryArgument(ownerRepo),
'--json',
PR_LOOKUP_JSON_FIELDS
],
@@ -3419,7 +3471,7 @@ function mapRestCommitStatus(status: RestCommitStatus): PRCheckDetail | null {
}
function mapGraphQLPendingApprovalCheckSuite(
ownerRepo: OwnerRepo,
ownerRepo: GitHubApiRepository,
suite: GraphQLCheckSuite,
headSha: string | null | undefined,
index: number
@@ -3437,7 +3489,7 @@ function mapGraphQLPendingApprovalCheckSuite(
}
function mapGraphQLPRChecksResponse(
ownerRepo: OwnerRepo,
ownerRepo: GitHubApiRepository,
response: GraphQLPRChecksResponse
): PRCheckDetail[] | null {
const pullRequest = response.data?.repository?.pullRequest
@@ -3480,7 +3532,7 @@ function mapGraphQLPRChecksResponse(
}
async function getPRChecksViaRestFallback(
ownerRepo: OwnerRepo,
ownerRepo: GitHubApiRepository,
headSha: string | undefined,
ghOptions: GhExecOptions,
noCache?: boolean
@@ -3489,7 +3541,7 @@ async function getPRChecksViaRestFallback(
return null
}
try {
await assertRateLimitBudget('core')
await assertRateLimitBudget('core', ownerRepo, ghOptions)
} catch (err) {
console.warn('getPRChecks skipped REST fallback, falling back to gh pr checks:', err)
return null
@@ -3502,12 +3554,13 @@ async function getPRChecksViaRestFallback(
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
...cacheArgs,
`repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodedHeadSha}/check-runs?per_page=100`
],
ghOptions
)
noteRateLimitSpend('core')
noteRepositoryRateLimitSpend(ownerRepo, 'core', 1, ghOptions)
const checkRunData = JSON.parse(stdout) as {
check_runs?: RestCheckRun[]
}
@@ -3519,12 +3572,13 @@ async function getPRChecksViaRestFallback(
const statusResult = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
...cacheArgs,
`repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodedHeadSha}/status?per_page=100`
],
ghOptions
)
noteRateLimitSpend('core')
noteRepositoryRateLimitSpend(ownerRepo, 'core', 1, ghOptions)
const statusData = JSON.parse(statusResult.stdout) as {
statuses?: RestCommitStatus[]
}
@@ -3542,12 +3596,13 @@ async function getPRChecksViaRestFallback(
const suitesResult = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
...cacheArgs,
`repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodedHeadSha}/check-suites?per_page=100`
],
ghOptions
)
noteRateLimitSpend('core')
noteRepositoryRateLimitSpend(ownerRepo, 'core', 1, ghOptions)
const suitesData = JSON.parse(suitesResult.stdout) as {
check_suites?: RestCheckSuite[]
}
@@ -3582,21 +3637,29 @@ export async function getPRChecks(
repoPath: string,
prNumber: number,
headSha?: string,
prRepo?: OwnerRepo | null,
prRepo?: GitHubApiRepository | null,
options?: { noCache?: boolean },
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<PRCheckDetail[]> {
void headSha
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = prRepo ?? (await getOwnerRepo(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
connectionId,
localGitOptions
)
if (connectionId && !ownerRepo) {
throw new Error(GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE)
}
const fallbackToPRChecks = async (): Promise<PRCheckDetail[]> => {
await assertRateLimitBudget('graphql')
await assertRateLimitBudget('graphql', ownerRepo, ghOptions)
await acquire()
try {
const fallbackArgs = ['pr', 'checks', String(prNumber), '--json', 'name,state,link']
if (ownerRepo) {
fallbackArgs.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
fallbackArgs.push('--repo', githubCliRepositoryArgument(ownerRepo))
}
const { stdout } = await ghExecFileAsync(fallbackArgs, ghOptions).catch((err: unknown) => {
const { stderr } = extractExecError(err)
@@ -3607,7 +3670,7 @@ export async function getPRChecks(
}
throw err
})
noteRateLimitSpend('graphql')
noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, ghOptions)
const data = JSON.parse(stdout) as { name: string; state: string; link: string }[]
return data.map((d) => ({
name: d.name,
@@ -3624,7 +3687,7 @@ export async function getPRChecks(
if (ownerRepo) {
let canUseGraphQLRollup = true
try {
await assertRateLimitBudget('graphql')
await assertRateLimitBudget('graphql', ownerRepo, ghOptions)
} catch (err) {
canUseGraphQLRollup = false
console.warn('getPRChecks skipped GraphQL rollup, falling back to gh pr checks:', err)
@@ -3638,6 +3701,7 @@ export async function getPRChecks(
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'graphql',
...cacheArgs,
'-f',
@@ -3651,7 +3715,7 @@ export async function getPRChecks(
],
ghOptions
)
noteRateLimitSpend('graphql')
noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, ghOptions)
const checks = mapGraphQLPRChecksResponse(
ownerRepo,
JSON.parse(stdout) as GraphQLPRChecksResponse
@@ -3712,11 +3776,11 @@ function getPendingApprovalCheckSuiteName(
}
function getPendingApprovalCheckSuiteUrl(
ownerRepo: OwnerRepo,
ownerRepo: GitHubApiRepository,
headSha: string,
suiteId: number | null | undefined
): string {
const base = `https://github.com/${ownerRepo.owner}/${ownerRepo.repo}/commits/${headSha}/checks`
const base = `https://${githubRepositoryWebHost(ownerRepo)}/${ownerRepo.owner}/${ownerRepo.repo}/commits/${headSha}/checks`
return typeof suiteId === 'number' && Number.isFinite(suiteId)
? `${base}#check-suite-${suiteId}`
: base
@@ -3799,7 +3863,7 @@ function getCheckJobLogTailCacheKey(job: PRCheckRunDetails['jobs'][number]): str
async function attachFailedJobLogTails(
jobs: PRCheckRunDetails['jobs'],
ownerRepo: OwnerRepo,
ownerRepo: GitHubApiRepository,
ghOptions: GhExecOptions
): Promise<void> {
const failedJobs = jobs
@@ -3812,7 +3876,10 @@ async function attachFailedJobLogTails(
// Why: failed workflows can have many jobs; cap log fetches so details remain
// a lazy, bounded follow-up request instead of a burst of hosted log downloads.
for (const job of failedJobs) {
const cacheKey = getCheckJobLogTailCacheKey(job)
const jobCacheKey = getCheckJobLogTailCacheKey(job)
const cacheKey = jobCacheKey
? `${githubRepositoryWebHost(ownerRepo).toLowerCase()}/${ownerRepo.owner.toLowerCase()}/${ownerRepo.repo.toLowerCase()}:${jobCacheKey}`
: null
if (!cacheKey) {
continue
}
@@ -3822,7 +3889,11 @@ async function attachFailedJobLogTails(
}
try {
const { stdout } = await ghExecFileAsync(
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/actions/jobs/${job.id}/logs`],
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/actions/jobs/${job.id}/logs`
],
ghOptions
)
job.logTail = sliceCheckLogTail(stdout)
@@ -3856,13 +3927,18 @@ export async function getPRCheckDetails(
workflowRunId?: number
checkName?: string
url?: string | null
prRepo?: OwnerRepo | null
prRepo?: GitHubApiRepository | null
},
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<PRCheckRunDetails | null> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = args.prRepo ?? (await getOwnerRepo(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
args.prRepo,
connectionId,
localGitOptions
)
if (!ownerRepo) {
return null
}
@@ -3873,7 +3949,11 @@ export async function getPRCheckDetails(
let annotations: PRCheckRunDetails['annotations'] = []
if (args.checkRunId) {
const { stdout } = await ghExecFileAsync(
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/check-runs/${args.checkRunId}`],
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/check-runs/${args.checkRunId}`
],
ghOptions
)
checkRun = JSON.parse(stdout) as Record<string, unknown>
@@ -3881,6 +3961,7 @@ 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
@@ -3898,6 +3979,7 @@ export async function getPRCheckDetails(
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
`repos/${ownerRepo.owner}/${ownerRepo.repo}/actions/runs/${workflowRunId}/jobs?per_page=100`
],
ghOptions
@@ -3955,7 +4037,7 @@ export async function rerunPRChecks(
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubRerunPRChecksResult> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getOwnerRepo(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' }
}
@@ -4002,7 +4084,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', '-X', 'POST', endpoint], {
await ghExecFileAsync(['api', ...githubApiHostArgs(ownerRepo), '-X', 'POST', endpoint], {
...ghOptions,
env: { ...process.env, GH_PROMPT_DISABLED: '1' }
})
@@ -4012,6 +4094,7 @@ export async function rerunPRChecks(
await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'-X',
'POST',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/check-runs/${checkRunId}/rerequest`
@@ -4090,14 +4173,22 @@ query($owner: String!, $repo: String!, $pr: Int!) {
export async function getPRComments(
repoPath: string,
prNumber: number,
options?: { noCache?: boolean; prRepo?: OwnerRepo | null },
options?: { noCache?: boolean; prRepo?: GitHubApiRepository | null },
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<PRComment[]> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = options?.prRepo ?? (await getOwnerRepo(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
options?.prRepo,
connectionId,
localGitOptions
)
if (connectionId && !ownerRepo) {
throw new Error(GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE)
}
if (ownerRepo) {
await assertRateLimitBudget('core')
await assertRateLimitBudget('core', ownerRepo, ghOptions)
}
await acquire()
try {
@@ -4111,15 +4202,16 @@ export async function getPRComments(
// permissions, transient network error) doesn't blank out all comments.
// Each source is parsed independently; failed sources contribute zero
// comments instead of aborting the entire fetch.
const reviewThreadsGuard = rateLimitGuard('graphql')
const reviewThreadsGuard = repositoryRateLimitGuard(ownerRepo, 'graphql', ghOptions)
let reviewThreadsFetch: Promise<{ stdout: string; stderr: string } | null>
if (reviewThreadsGuard.blocked) {
reviewThreadsFetch = Promise.resolve(null)
} else {
noteRateLimitSpend('graphql')
noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, ghOptions)
reviewThreadsFetch = ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'graphql',
'-f',
`query=${REVIEW_THREADS_QUERY}`,
@@ -4135,7 +4227,12 @@ export async function getPRComments(
}
const [issueResult, threadsResult, reviewsResult] = await Promise.allSettled([
ghExecFileAsync(
['api', ...cacheArgs, `${base}/issues/${prNumber}/comments?per_page=100`],
[
'api',
...githubApiHostArgs(ownerRepo),
...cacheArgs,
`${base}/issues/${prNumber}/comments?per_page=100`
],
ghOptions
),
reviewThreadsFetch,
@@ -4144,11 +4241,16 @@ export async function getPRComments(
// Without this, a reviewer who submits "LGTM" without inline threads
// would have their comment silently dropped from the panel.
ghExecFileAsync(
['api', ...cacheArgs, `${base}/pulls/${prNumber}/reviews?per_page=100`],
[
'api',
...githubApiHostArgs(ownerRepo),
...cacheArgs,
`${base}/pulls/${prNumber}/reviews?per_page=100`
],
ghOptions
)
])
noteRateLimitSpend('core', 2)
noteRepositoryRateLimitSpend(ownerRepo, 'core', 2, ghOptions)
// Parse issue comments (REST)
type RESTComment = {
@@ -4340,6 +4442,14 @@ export async function setPRFileViewed(args: {
const ghOptions = ghRepoExecOptions(
githubRepoContext(args.repoPath, args.connectionId, args.localGitOptions)
)
const ownerRepo = await getOriginGitHubApiRepository(
args.repoPath,
args.connectionId,
args.localGitOptions
)
if (args.connectionId && !ownerRepo) {
return false
}
const mutation = args.viewed ? 'markFileAsViewed' : 'unmarkFileAsViewed'
const query = `mutation($pullRequestId: ID!, $path: String!) {
${mutation}(input: { pullRequestId: $pullRequestId, path: $path }) {
@@ -4351,6 +4461,7 @@ export async function setPRFileViewed(args: {
await ghExecFileAsync(
[
'api',
...(ownerRepo ? githubApiHostArgs(ownerRepo) : []),
'graphql',
'-f',
`query=${query}`,
@@ -4383,7 +4494,11 @@ export async function resolveReviewThread(
const mutation = resolve ? 'resolveReviewThread' : 'unresolveReviewThread'
const query = `mutation($threadId: ID!) { ${mutation}(input: { threadId: $threadId }) { thread { isResolved } } }`
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const guard = rateLimitGuard('graphql')
const ownerRepo = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
if (connectionId && !ownerRepo) {
return false
}
const guard = repositoryRateLimitGuard(ownerRepo, 'graphql', ghOptions)
if (guard.blocked) {
console.warn(
`${mutation} skipped: GitHub GraphQL rate limit nearly exhausted (${guard.remaining}/${guard.limit})`
@@ -4392,9 +4507,17 @@ export async function resolveReviewThread(
}
await acquire()
try {
noteRateLimitSpend('graphql')
noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, ghOptions)
await ghExecFileAsync(
['api', 'graphql', '-f', `query=${query}`, '-f', `threadId=${threadId}`],
[
'api',
...(ownerRepo ? githubApiHostArgs(ownerRepo) : []),
'graphql',
'-f',
`query=${query}`,
'-f',
`threadId=${threadId}`
],
ghOptions
)
return true
@@ -4446,11 +4569,16 @@ export async function addPRReviewCommentReply(
path?: string,
line?: number,
connectionId?: string | null,
prRepo?: OwnerRepo | null,
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubCommentResult> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = prRepo ?? (await getOwnerRepo(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
connectionId,
localGitOptions
)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
@@ -4459,6 +4587,7 @@ export async function addPRReviewCommentReply(
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'-X',
'POST',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}/comments/${commentId}/replies`,
@@ -4492,7 +4621,11 @@ export async function addPRReviewComment(
const ghOptions = ghRepoExecOptions(
githubRepoContext(args.repoPath, args.connectionId, args.localGitOptions)
)
const ownerRepo = await getOwnerRepo(args.repoPath, args.connectionId, args.localGitOptions)
const ownerRepo = await getOriginGitHubApiRepository(
args.repoPath,
args.connectionId,
args.localGitOptions
)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
@@ -4500,6 +4633,7 @@ export async function addPRReviewComment(
try {
const fields = [
'api',
...githubApiHostArgs(ownerRepo),
'-X',
'POST',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${args.prNumber}/comments`,
@@ -4550,11 +4684,19 @@ export async function mergePR(
prNumber: number,
method: 'merge' | 'squash' | 'rebase' = 'squash',
connectionId?: string | null,
prRepo?: OwnerRepo | null,
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = prRepo ?? (await getOwnerRepo(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
connectionId,
localGitOptions
)
if (connectionId && !ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
await acquire()
try {
const mergeBlocker = await getPRMergeBlocker(
@@ -4574,7 +4716,7 @@ export async function mergePR(
// is handled by worktree deletion (local) and GitHub's auto-delete setting (remote).
const args = ['pr', 'merge', String(prNumber), `--${method}`]
if (ownerRepo) {
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
}
await ghExecFileAsync(args, {
...ghOptions,
@@ -4596,11 +4738,19 @@ export async function setPRAutoMerge(
enabled: boolean,
method: GitHubPRMergeMethod = 'squash',
connectionId?: string | null,
prRepo?: OwnerRepo | null,
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = prRepo ?? (await getOwnerRepo(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
connectionId,
localGitOptions
)
if (connectionId && !ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
await acquire()
try {
if (enabled) {
@@ -4608,7 +4758,7 @@ export async function setPRAutoMerge(
}
const args = ['pr', 'merge', String(prNumber), '--disable-auto']
if (ownerRepo) {
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
}
await ghExecFileAsync(args, {
...ghOptions,
@@ -4642,12 +4792,12 @@ type PRAutoMergeIdentity = {
async function getPRAutoMergeIdentity(
prNumber: number,
ownerRepo: OwnerRepo | null,
ownerRepo: GitHubApiRepository | null,
ghOptions: GhExecOptions
): Promise<PRAutoMergeIdentity | null> {
const args = ['pr', 'view', String(prNumber), '--json', PR_AUTO_MERGE_IDENTITY_JSON_FIELDS]
if (ownerRepo) {
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
}
const { stdout } = await ghExecFileAsync(args, ghOptions)
const data = JSON.parse(stdout) as PRAutoMergeIdentity
@@ -4661,12 +4811,12 @@ async function getPRAutoMergeIdentity(
async function runPRAutoMergeCommand(
prNumber: number,
method: GitHubPRMergeMethod,
ownerRepo: OwnerRepo | null,
ownerRepo: GitHubApiRepository | null,
ghOptions: GhExecOptions
): Promise<void> {
const args = ['pr', 'merge', String(prNumber), '--auto', `--${method}`]
if (ownerRepo) {
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
}
await ghExecFileAsync(args, {
...ghOptions,
@@ -4676,7 +4826,7 @@ async function runPRAutoMergeCommand(
async function shouldUseMergeQueueAutoMerge(
pr: PRAutoMergeIdentity,
ownerRepo: OwnerRepo | null,
ownerRepo: GitHubApiRepository | null,
ghOptions: GhExecOptions
): Promise<boolean> {
if (!ownerRepo || !pr.baseRefName) {
@@ -4689,7 +4839,7 @@ async function shouldUseMergeQueueAutoMerge(
async function enablePRAutoMerge(
prNumber: number,
method: GitHubPRMergeMethod,
ownerRepo: OwnerRepo | null,
ownerRepo: GitHubApiRepository | null,
ghOptions: GhExecOptions
): Promise<{ ok: true } | { ok: false; error: string }> {
const pr = await getPRAutoMergeIdentity(prNumber, ownerRepo, ghOptions)
@@ -4711,6 +4861,7 @@ async function enablePRAutoMerge(
}`
const args = [
'api',
...(ownerRepo ? githubApiHostArgs(ownerRepo) : []),
'graphql',
'-f',
`query=${query}`,
@@ -4734,7 +4885,7 @@ async function enablePRAutoMerge(
async function getPRMergeBlocker(
repoPath: string,
prNumber: number,
ownerRepo: OwnerRepo | null,
ownerRepo: GitHubApiRepository | null,
ghOptions: GhExecOptions,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
@@ -4807,7 +4958,7 @@ export async function updatePRState(
): Promise<{ ok: true } | { ok: false; error: string }> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const ghOptions = ghRepoExecOptions(context)
const ownerRepo = await getOwnerRepo(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' }
}
@@ -4818,7 +4969,7 @@ export async function updatePRState(
// Why: GitHub can reject REST pull state PATCHes for reopen paths with a
// generic 422; gh's PR commands use GitHub's supported reopen flow.
await ghExecFileAsync(
['pr', cmd, String(prNumber), '--repo', `${ownerRepo.owner}/${ownerRepo.repo}`],
['pr', cmd, String(prNumber), '--repo', githubCliRepositoryArgument(ownerRepo)],
{
...ghOptions
}
@@ -4845,12 +4996,15 @@ export async function requestPRReviewers(
return { ok: false, error: 'Enter at least one reviewer' }
}
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getOwnerRepo(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' }
}
await acquire()
try {
const args = ['pr', 'edit', String(prNumber), '--add-reviewer', logins.join(',')]
if (ownerRepo) {
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
}
await ghExecFileAsync(args, {
...ghOptions,
@@ -4878,12 +5032,15 @@ export async function removePRReviewers(
return { ok: false, error: 'Enter at least one reviewer' }
}
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getOwnerRepo(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' }
}
await acquire()
try {
const args = ['pr', 'edit', String(prNumber), '--remove-reviewer', logins.join(',')]
if (ownerRepo) {
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
}
await ghExecFileAsync(args, {
...ghOptions,
@@ -4907,16 +5064,24 @@ export async function updatePRTitle(
prNumber: number,
title: string,
connectionId?: string | null,
prRepo?: OwnerRepo | null,
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<boolean> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = prRepo ?? (await getOwnerRepo(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
connectionId,
localGitOptions
)
if (connectionId && !ownerRepo) {
return false
}
await acquire()
try {
const args = ['pr', 'edit', String(prNumber), '--title', title]
if (ownerRepo) {
args.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`)
args.push('--repo', githubCliRepositoryArgument(ownerRepo))
}
await ghExecFileAsync(args, {
...ghOptions
@@ -4935,11 +5100,16 @@ export async function updatePRDetails(
prNumber: number,
updates: { title?: string; body?: string },
connectionId?: string | null,
prRepo?: OwnerRepo | null,
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = prRepo ?? (await getOwnerRepo(repoPath, connectionId, localGitOptions))
const ownerRepo = await resolveGitHubApiRepository(
repoPath,
prRepo,
connectionId,
localGitOptions
)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
@@ -4964,6 +5134,7 @@ export async function updatePRDetails(
await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'-X',
'PATCH',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}`,
+67
View File
@@ -0,0 +1,67 @@
import type { GitHubOwnerRepo } from '../../shared/types'
import { getOwnerRepo, type LocalGitExecOptions } from './gh-utils'
import { getEnterpriseGitHubRepoSlug } from './github-enterprise-repository'
export type GitHubApiRepository = GitHubOwnerRepo
export async function getOriginGitHubApiRepository(
repoPath: string,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubApiRepository | null> {
const ownerRepo = await getOwnerRepo(repoPath, connectionId, localGitOptions)
if (ownerRepo) {
return { ...ownerRepo, host: 'github.com' }
}
const enterpriseOptions =
Object.keys(localGitOptions).length > 0 ? { localGitExecOptions: localGitOptions } : {}
return getEnterpriseGitHubRepoSlug(repoPath, connectionId, enterpriseOptions)
}
export async function resolveGitHubApiRepository(
repoPath: string,
repository?: GitHubApiRepository | null,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubApiRepository | null> {
if (repository?.host) {
return repository
}
const originRepository = await getOriginGitHubApiRepository(
repoPath,
connectionId,
localGitOptions
)
if (!repository) {
return originRepository
}
// Why: older clients only send owner/repo. The origin still supplies the
// execution host for fork-base slugs on the same GitHub Enterprise server.
if (originRepository?.host) {
return { ...repository, host: originRepository.host }
}
// Why: connection-backed gh has no cwd to infer a host from. An unhosted
// legacy identity is unsafe there because gh would target its default account.
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}`
}
export function githubRepositoryWebHost(repository: GitHubApiRepository): string {
return repository.host ?? 'github.com'
}
@@ -149,6 +149,23 @@ describe('isGitHubHostAuthenticated', () => {
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1)
})
it('coalesces concurrent probes for the same runtime and host', async () => {
let finishProbe: (() => void) | undefined
ghExecFileAsyncMock.mockImplementation(
() =>
new Promise((resolve) => {
finishProbe = () => resolve({ stdout: '', stderr: '' })
})
)
const first = isGitHubHostAuthenticated('github.acme-corp.com', '/repo')
const second = isGitHubHostAuthenticated('github.acme-corp.com', '/repo')
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1)
finishProbe?.()
await expect(Promise.all([first, second])).resolves.toEqual([true, true])
})
it('does not share cache state across WSL distros', async () => {
mockHostAuthenticated()
+37 -17
View File
@@ -27,6 +27,7 @@ type HostAuthCacheEntry = {
}
const hostAuthCache = new Map<string, HostAuthCacheEntry>()
const hostAuthInFlight = new Map<string, Promise<boolean>>()
// Why: gh's authenticated hosts live in per-runtime config — a WSL distro and an
// SSH host each carry their own `hosts.yml` — so cache state must be keyed by the
@@ -39,6 +40,7 @@ function runtimeCacheKey(connectionId?: string | null, wslDistro?: string): stri
/** @internal - exposed for tests only */
export function _resetGitHubHostAuthCache(): void {
hostAuthCache.clear()
hostAuthInFlight.clear()
}
// Only gh's own stdout/stderr — not the Error.message — counts as an
@@ -74,26 +76,44 @@ export async function isGitHubHostAuthenticated(
if (cached && cached.expiresAt > now) {
return cached.authenticated
}
const execOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
let authenticated: boolean
const inFlight = hostAuthInFlight.get(cacheKey)
if (inFlight) {
return inFlight
}
// 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)
)
let authenticated: boolean
try {
await ghExecFileAsync(['auth', 'status', '--hostname', normalizedHost], execOptions)
authenticated = true
} catch (error) {
const output = ghCommandOutput(error)
if (!output) {
// Indeterminate (gh missing / spawn failure) — do not cache so a later
// probe (gh installed, tunnel ready, token added) can recover.
return false
}
// gh exits non-zero when a host has a token problem but still prints the
// per-host status; treat the host as GitHub only when it is actually listed.
authenticated = parseAuthStatus(output).some(
(account) => account.host.toLowerCase() === normalizedHost
)
}
hostAuthCache.set(cacheKey, { authenticated, expiresAt: Date.now() + HOST_AUTH_TTL_MS })
return authenticated
})()
hostAuthInFlight.set(cacheKey, probe)
try {
await ghExecFileAsync(['auth', 'status', '--hostname', normalizedHost], execOptions)
authenticated = true
} catch (error) {
const output = ghCommandOutput(error)
if (!output) {
// Indeterminate (gh missing / spawn failure) — do not cache so a later
// probe (gh installed, tunnel ready, token added) can recover.
return false
return await probe
} finally {
if (hostAuthInFlight.get(cacheKey) === probe) {
hostAuthInFlight.delete(cacheKey)
}
// gh exits non-zero when a host has a token problem but still prints the
// per-host status; treat the host as GitHub only when it is actually listed.
authenticated = parseAuthStatus(output).some(
(account) => account.host.toLowerCase() === normalizedHost
)
}
hostAuthCache.set(cacheKey, { authenticated, expiresAt: now + HOST_AUTH_TTL_MS })
return authenticated
}
/**
+34
View File
@@ -138,6 +138,40 @@ describe('issue source operations', () => {
)
})
it('routes PR conversation comments to the supplied Enterprise host', async () => {
ghExecFileAsyncMock.mockResolvedValueOnce({
stdout: JSON.stringify({
id: 9,
user: { login: 'octo', avatar_url: '', type: 'User' },
body: 'Enterprise comment',
created_at: '2026-07-16T00:00:00.000Z',
html_url: 'https://github.acme-corp.com/team/orca/pull/7#issuecomment-9'
})
})
await expect(
addIssueComment('/remote/repo', 7, 'Enterprise comment', 'ssh-1', {
owner: 'team',
repo: 'orca',
host: 'github.acme-corp.com'
})
).resolves.toMatchObject({ ok: true })
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
[
'api',
'--hostname',
'github.acme-corp.com',
'-X',
'POST',
'repos/team/orca/issues/7/comments',
'--raw-field',
'body=Enterprise comment'
],
{}
)
})
it('lists issues from the issue owner/repo', async () => {
getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' })
+5 -2
View File
@@ -15,6 +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'
// prettier-ignore
import { ghExecFileAsync, acquire, release, getIssueOwnerRepo, resolveIssueSource, classifyGhError, classifyListIssuesError, ghRepoExecOptions, githubRepoContext, extractExecError } from './gh-utils'
@@ -411,8 +412,9 @@ export async function addIssueComment(
): Promise<GitHubCommentResult> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const ghOptions = ghRepoExecOptions(context)
const ownerRepo =
ownerRepoOverride ?? (await getIssueOwnerRepo(repoPath, connectionId, localGitOptions))
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' }
}
@@ -421,6 +423,7 @@ export async function addIssueComment(
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'-X',
'POST',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}/comments`,
+6 -3
View File
@@ -49,12 +49,15 @@ describe('getRateLimit', () => {
fetchedAt: 1_000
}
})
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
['api', '--hostname', 'github.com', 'rate_limit'],
{ encoding: 'utf-8' }
)
})
it('caches a failed probe for the TTL instead of re-spawning gh', async () => {
// Why: regression for #7553 — GHES with rate limiting disabled fails every
// probe. Refreshes fail open past the failure, so without a negative cache
// each queued refresh would spawn its own doomed gh subprocess.
// Why: refreshes fail open past a probe failure, so without a negative cache
// an auth or transport failure would spawn the same doomed gh call repeatedly.
ghExecFileAsyncMock.mockRejectedValue(new Error('HTTP 404: Rate limiting is not enabled.'))
const first = await getRateLimit()
+5 -1
View File
@@ -222,7 +222,11 @@ export async function getRateLimit(options?: { force?: boolean }): Promise<GetRa
async function fetchRateLimitSnapshot(): Promise<GetRateLimitResult> {
await acquire()
try {
const { stdout } = await ghExecFileAsync(['api', 'rate_limit'], { encoding: 'utf-8' })
// 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 parsed = JSON.parse(stdout) as GhRateLimitPayload
const snapshot: GitHubRateLimitSnapshot = {
core: parseBucket(parsed.resources?.core),
@@ -43,6 +43,7 @@ vi.mock('./gh-utils', () => ({
vi.mock('./client', () => ({
getWorkItem: getWorkItemMock,
getWorkItemByOwnerRepo: vi.fn(),
getPRChecks: getPRChecksMock,
getPRComments: getPRCommentsMock
}))
@@ -162,7 +163,7 @@ describe('getWorkItemDetails PR file viewed state', () => {
'/repo-root',
42,
'head-sha',
null,
{ owner: 'stablyai', repo: 'orca', host: 'github.com' },
undefined,
undefined
)
@@ -0,0 +1,183 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
ghExecFileAsyncMock,
getOwnerRepoMock,
getWorkItemMock,
getPRChecksMock,
getPRCommentsMock
} = vi.hoisted(() => ({
ghExecFileAsyncMock: vi.fn(),
getOwnerRepoMock: vi.fn(),
getWorkItemMock: vi.fn(),
getPRChecksMock: vi.fn(),
getPRCommentsMock: vi.fn()
}))
vi.mock('./gh-utils', () => ({
ghExecFileAsync: ghExecFileAsyncMock,
getOwnerRepo: getOwnerRepoMock,
getIssueOwnerRepo: vi.fn(),
ghRepoExecOptions: vi.fn((context) => ({ cwd: context.repoPath })),
githubRepoContext: vi.fn((repoPath, connectionId, localGitOptions) => ({
repoPath,
connectionId: connectionId ?? null,
...localGitOptions
})),
acquire: vi.fn(),
release: vi.fn()
}))
vi.mock('./client', () => ({
getWorkItem: getWorkItemMock,
getWorkItemByOwnerRepo: vi.fn(),
getPRChecks: getPRChecksMock,
getPRComments: getPRCommentsMock
}))
vi.mock('./github-enterprise-repository', () => ({
getEnterpriseGitHubRepoSlug: vi.fn().mockResolvedValue(null)
}))
vi.mock('./rate-limit', () => ({
rateLimitGuard: vi.fn(() => ({ blocked: false })),
noteRateLimitSpend: vi.fn()
}))
import { getWorkItemDetails } from './work-item-details'
function pullRequestItem(number: number, title: string): Record<string, unknown> {
return {
id: `pr:${number}`,
type: 'pr',
number,
title,
state: 'open',
url: `https://github.com/acme/widgets/pull/${number}`,
labels: [],
updatedAt: '2026-07-16T00:00:00Z',
author: 'pr-author'
}
}
function auxiliaryPRResponse(args: string[]): { stdout: string } {
const query = args.find((arg) => arg.startsWith('query=')) ?? ''
if (query.includes('viewerViewedState')) {
return {
stdout: JSON.stringify({
data: {
repository: {
pullRequest: {
id: 'PR_file_list',
files: { pageInfo: { hasNextPage: false }, nodes: [] }
}
}
}
})
}
}
if (query.includes('participants(first: 100)')) {
return {
stdout: JSON.stringify({
data: { repository: { pullRequest: { participants: { nodes: [] } } } }
})
}
}
return { stdout: JSON.stringify({ data: {} }) }
}
describe('getWorkItemDetails PR file listing', () => {
beforeEach(() => {
ghExecFileAsyncMock.mockReset()
getOwnerRepoMock.mockReset()
getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
getWorkItemMock.mockReset()
getPRChecksMock.mockReset()
getPRChecksMock.mockResolvedValue([])
getPRCommentsMock.mockReset()
getPRCommentsMock.mockResolvedValue([])
})
it('loads files beyond the first 100-result REST page', async () => {
getWorkItemMock.mockResolvedValueOnce(pullRequestItem(108, 'Large PR'))
const restFile = (index: number) => ({
filename: `src/file-${index}.ts`,
status: 'modified',
additions: 1,
deletions: 0,
changes: 1,
patch: '@@ -1 +1 @@'
})
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
const endpoint = args.find((arg) => arg.startsWith('repos/')) ?? ''
if (endpoint === 'repos/acme/widgets/pulls/108') {
return { stdout: JSON.stringify({ head: { sha: 'head' }, base: { sha: 'base' } }) }
}
if (endpoint === 'repos/acme/widgets/pulls/108/files?per_page=100') {
return {
stdout: JSON.stringify(Array.from({ length: 100 }, (_, index) => restFile(index)))
}
}
if (endpoint === 'repos/acme/widgets/pulls/108/files?per_page=100&page=2') {
return { stdout: JSON.stringify([restFile(100)]) }
}
return auxiliaryPRResponse(args)
})
const details = await getWorkItemDetails('/repo-root', 108, 'pr')
expect(details?.files).toHaveLength(101)
expect(details?.files?.at(-1)?.path).toBe('src/file-100.ts')
const fileEndpoints = ghExecFileAsyncMock.mock.calls
.map(([args]) => (args as string[]).find((arg) => arg.includes('/files?')))
.filter(Boolean)
expect(fileEndpoints).toEqual([
'repos/acme/widgets/pulls/108/files?per_page=100',
'repos/acme/widgets/pulls/108/files?per_page=100&page=2'
])
})
// 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 file fetch fails', async () => {
getWorkItemMock.mockResolvedValueOnce(pullRequestItem(8305, 'Files fetch fails'))
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
const target = args.at(-1)
if (target === 'repos/acme/widgets/pulls/8305') {
return {
stdout: JSON.stringify({ head: { sha: 'head-sha' }, base: { sha: 'base-sha' } })
}
}
if (target === 'repos/acme/widgets/pulls/8305/files?per_page=100') {
throw new Error('gh: API rate limit exceeded (403)')
}
return auxiliaryPRResponse(args)
})
const details = await getWorkItemDetails('/repo-root', 8305, 'pr')
expect(details?.filesUnavailable).toBe(true)
expect(details?.files).toBeUndefined()
})
it('preserves an empty file list as an available result', async () => {
getWorkItemMock.mockResolvedValueOnce(pullRequestItem(8306, 'Empty PR'))
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
const target = args.at(-1)
if (target === 'repos/acme/widgets/pulls/8306') {
return {
stdout: JSON.stringify({ head: { sha: 'head-sha' }, base: { sha: 'base-sha' } })
}
}
if (target === 'repos/acme/widgets/pulls/8306/files?per_page=100') {
return { stdout: '[]' }
}
return auxiliaryPRResponse(args)
})
const details = await getWorkItemDetails('/repo-root', 8306, 'pr')
expect(details?.filesUnavailable).toBe(false)
expect(details?.files).toEqual([])
})
})
+80 -5
View File
@@ -10,6 +10,7 @@ const {
getIssueOwnerRepoMock,
getEnterpriseGitHubRepoSlugMock,
getWorkItemMock,
getWorkItemByOwnerRepoMock,
getPRChecksMock,
getPRCommentsMock,
rateLimitGuardMock,
@@ -24,6 +25,7 @@ const {
getIssueOwnerRepoMock: vi.fn(),
getEnterpriseGitHubRepoSlugMock: vi.fn(),
getWorkItemMock: vi.fn(),
getWorkItemByOwnerRepoMock: vi.fn(),
getPRChecksMock: vi.fn(),
getPRCommentsMock: vi.fn(),
rateLimitGuardMock: vi.fn<() => RateLimitGuardResult>(() => ({ blocked: false })),
@@ -54,6 +56,7 @@ vi.mock('./gh-utils', () => ({
vi.mock('./client', () => ({
getWorkItem: getWorkItemMock,
getWorkItemByOwnerRepo: getWorkItemByOwnerRepoMock,
getPRChecks: getPRChecksMock,
getPRComments: getPRCommentsMock
}))
@@ -77,6 +80,7 @@ describe('getWorkItemDetails', () => {
getEnterpriseGitHubRepoSlugMock.mockReset()
getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null)
getWorkItemMock.mockReset()
getWorkItemByOwnerRepoMock.mockReset()
getPRChecksMock.mockReset()
getPRCommentsMock.mockReset()
rateLimitGuardMock.mockReset()
@@ -478,6 +482,12 @@ describe('getWorkItemDetails', () => {
it('routes local WSL PR detail fan-out through the selected distro', async () => {
const localGitOptions = { wslDistro: 'Ubuntu' }
rateLimitGuardMock.mockReturnValue({
blocked: true,
remaining: 0,
limit: 5000,
resetAt: 1_800_000_000
})
getWorkItemMock.mockResolvedValueOnce({
id: 'pr:42',
type: 'pr',
@@ -539,7 +549,7 @@ describe('getWorkItemDetails', () => {
expect(getPRCommentsMock).toHaveBeenCalledWith(
'/repo-root',
42,
undefined,
{ prRepo: { owner: 'acme', repo: 'widgets', host: 'github.com' } },
null,
localGitOptions
)
@@ -547,7 +557,7 @@ describe('getWorkItemDetails', () => {
'/repo-root',
42,
'head-sha',
null,
{ owner: 'acme', repo: 'widgets', host: 'github.com' },
undefined,
null,
localGitOptions
@@ -555,10 +565,12 @@ describe('getWorkItemDetails', () => {
expect(ghExecFileAsyncMock.mock.calls.every((call) => call[1]?.wslDistro === 'Ubuntu')).toBe(
true
)
expect(rateLimitGuardMock).not.toHaveBeenCalled()
expect(noteRateLimitSpendMock).not.toHaveBeenCalled()
})
it('uses the GitHub Enterprise host for PR files in work item details', async () => {
getWorkItemMock.mockResolvedValueOnce({
it('uses the GitHub Enterprise host for SSH-backed PR work item details', async () => {
getWorkItemByOwnerRepoMock.mockResolvedValueOnce({
id: 'pr:7',
type: 'pr',
number: 7,
@@ -630,7 +642,7 @@ describe('getWorkItemDetails', () => {
throw new Error(`unexpected gh call: ${args.join(' ')}`)
})
const details = await getWorkItemDetails('/repo-root', 7, 'pr')
const details = await getWorkItemDetails('/remote/repo', 7, 'pr', 'ssh-1')
expect(details?.body).toBe('Enterprise PR body')
expect(details?.headSha).toBe('head-sha')
@@ -648,6 +660,29 @@ describe('getWorkItemDetails', () => {
viewerViewedState: 'VIEWED'
}
])
expect(getWorkItemMock).not.toHaveBeenCalled()
expect(getWorkItemByOwnerRepoMock).toHaveBeenCalledWith(
'/remote/repo',
{ owner: 'team', repo: 'orca', host: 'github.acme-corp.com' },
7,
'pr',
'ssh-1'
)
expect(getEnterpriseGitHubRepoSlugMock).toHaveBeenCalledTimes(1)
expect(getPRCommentsMock).toHaveBeenCalledWith(
'/remote/repo',
7,
{ prRepo: { owner: 'team', repo: 'orca', host: 'github.acme-corp.com' } },
'ssh-1'
)
expect(getPRChecksMock).toHaveBeenCalledWith(
'/remote/repo',
7,
'head-sha',
{ owner: 'team', repo: 'orca', host: 'github.acme-corp.com' },
undefined,
'ssh-1'
)
const apiCalls = ghExecFileAsyncMock.mock.calls
.map(([args]) => args as string[])
.filter((args) => args[0] === 'api')
@@ -656,6 +691,46 @@ describe('getWorkItemDetails', () => {
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)
).toBe(true)
})
it('does not fall back to the default host after an Enterprise PR lookup fails', async () => {
getOwnerRepoMock.mockResolvedValue(null)
getEnterpriseGitHubRepoSlugMock.mockResolvedValue({
owner: 'team',
repo: 'orca',
host: 'github.acme-corp.com'
})
getWorkItemByOwnerRepoMock.mockResolvedValue(null)
getWorkItemMock.mockResolvedValue({
id: 'pr:7',
type: 'pr',
number: 7,
title: 'Wrong github.com PR',
state: 'open',
url: 'https://github.com/team/orca/pull/7',
labels: [],
updatedAt: '2026-07-16T00:00:00Z',
author: 'wrong-author'
})
await expect(getWorkItemDetails('/remote/repo', 7, 'pr', 'ssh-1')).resolves.toBeNull()
expect(getWorkItemByOwnerRepoMock).toHaveBeenCalledTimes(1)
expect(getWorkItemMock).not.toHaveBeenCalled()
})
it('does not query the default host when an SSH PR repository is unresolved', async () => {
getOwnerRepoMock.mockResolvedValue(null)
getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null)
await expect(getWorkItemDetails('/remote/repo', 7, 'pr', 'ssh-1')).resolves.toBeNull()
expect(getWorkItemMock).not.toHaveBeenCalled()
expect(getWorkItemByOwnerRepoMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
})
it('uses the GitHub Enterprise host when fetching PR file contents', async () => {
+152 -136
View File
@@ -17,14 +17,18 @@ import {
ghExecFileAsync,
acquire,
release,
getOwnerRepo,
getIssueOwnerRepo,
ghRepoExecOptions,
githubRepoContext,
type LocalGitExecOptions
} from './gh-utils'
import { getWorkItem, getPRChecks, getPRComments } from './client'
import { getEnterpriseGitHubRepoSlug } from './github-enterprise-repository'
import { getWorkItem, getWorkItemByOwnerRepo, getPRChecks, getPRComments } from './client'
import {
getOriginGitHubApiRepository,
githubApiHostArgs,
isGitHubDotComRepository,
type GitHubApiRepository
} from './github-api-repository'
import { noteRateLimitSpend, rateLimitGuard } from './rate-limit'
import { getPRReviewCommentLineNumbersFromPatch } from './pr-review-comment-lines'
import { isMaxBufferOverflowError } from '../git/max-buffer-overflow'
@@ -46,40 +50,31 @@ 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] : []
}
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) {
@@ -167,8 +162,6 @@ type GraphQLIssueDetailsResponse = {
errors?: { message?: string }[]
}
type GitHubOwnerRepoSlug = { owner: string; repo: string; host?: string }
type RestTimelineUser = {
login?: string | null
avatar_url?: string | null
@@ -317,7 +310,7 @@ function parseRestTimelineEventLines(stdout: string): RestTimelineEvent[] {
}
async function getIssueTimelineItems(
ownerRepo: GitHubOwnerRepoSlug,
ownerRepo: GitHubApiRepository,
issueNumber: number,
ghOptions: ReturnType<typeof ghRepoExecOptions>
): Promise<GitHubIssueTimelineItem[]> {
@@ -327,7 +320,7 @@ async function getIssueTimelineItems(
const { stdout } = await ghExecFileAsync(
[
'api',
...ghApiHostArgs(ownerRepo),
...githubApiHostArgs(ownerRepo),
'--cache',
'60s',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}/timeline?per_page=${GITHUB_REST_PAGE_SIZE}&page=${page}`,
@@ -535,20 +528,20 @@ function isBinaryHint(file: RESTPRFile): boolean {
return file.patch === undefined && file.changes > 0
}
async function getPRHeadBaseSha(
async function getPRMetadata(
repoPath: string,
prNumber: number,
ownerRepo: GitHubApiRepository | null,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ headSha: string; baseSha: string } | null> {
): Promise<{ body: string; headSha?: string; baseSha?: string }> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions)
try {
if (ownerRepo) {
const { stdout } = await ghExecFileAsync(
[
'api',
...ghApiHostArgs(ownerRepo),
...githubApiHostArgs(ownerRepo),
'--cache',
'60s',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}`
@@ -556,28 +549,32 @@ async function getPRHeadBaseSha(
ghOptions
)
const data = JSON.parse(stdout) as {
body?: string | null
head?: { sha?: string }
base?: { sha?: string }
}
if (data.head?.sha && data.base?.sha) {
return { headSha: data.head.sha, baseSha: data.base.sha }
return {
body: data.body ?? '',
...(data.head?.sha ? { headSha: data.head.sha } : {}),
...(data.base?.sha ? { baseSha: data.base.sha } : {})
}
return null
}
const { stdout } = await ghExecFileAsync(
['pr', 'view', String(prNumber), '--json', 'headRefOid,baseRefOid'],
['pr', 'view', String(prNumber), '--json', 'body,headRefOid,baseRefOid'],
ghOptions
)
const data = JSON.parse(stdout) as {
body?: string
headRefOid?: string
baseRefOid?: string
}
if (data.headRefOid && data.baseRefOid) {
return { headSha: data.headRefOid, baseSha: data.baseRefOid }
return {
body: data.body ?? '',
...(data.headRefOid ? { headSha: data.headRefOid } : {}),
...(data.baseRefOid ? { baseSha: data.baseRefOid } : {})
}
return null
} catch {
return null
return { body: '' }
}
}
@@ -587,27 +584,35 @@ async function getPRHeadBaseSha(
async function getPRFiles(
repoPath: string,
prNumber: number,
ownerRepo: GitHubApiRepository | null,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubPRFile[] | null> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions)
if (!ownerRepo) {
return null
}
try {
const { stdout } = await ghExecFileAsync(
[
'api',
...ghApiHostArgs(ownerRepo),
'--cache',
'60s',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}/files?per_page=100`
],
ghOptions
)
const data = JSON.parse(stdout) as RESTPRFile[]
return data.slice(0, MAX_PR_FILES).map((file) => ({
const data: RESTPRFile[] = []
for (let page = 1; data.length < MAX_PR_FILES; page += 1) {
const pageSuffix = page === 1 ? '' : `&page=${page}`
const { stdout } = await ghExecFileAsync(
[
'api',
...githubApiHostArgs(ownerRepo),
'--cache',
'60s',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}/files?per_page=100${pageSuffix}`
],
ghOptions
)
const pageData = JSON.parse(stdout) as RESTPRFile[]
data.push(...pageData.slice(0, MAX_PR_FILES - data.length))
if (pageData.length < 100) {
break
}
}
return data.map((file) => ({
path: file.filename,
oldPath: file.previous_filename,
status: mapFileStatus(file.status),
@@ -629,15 +634,15 @@ type PRFileViewedStatesResult = {
async function getPRFileViewedStates(
repoPath: string,
prNumber: number,
ownerRepo: GitHubApiRepository | null,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<PRFileViewedStatesResult | null> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions)
if (!ownerRepo) {
return null
}
if (rateLimitGuard('graphql').blocked) {
if (repositoryRateLimitGuard(ownerRepo, 'graphql', localGitOptions).blocked) {
return null
}
const viewedStates = new Map<string, GitHubPRFileViewedState>()
@@ -648,7 +653,7 @@ async function getPRFileViewedStates(
for (let fetched = 0; fetched < MAX_PR_FILES; fetched += 100) {
const args = [
'api',
...ghApiHostArgs(ownerRepo),
...githubApiHostArgs(ownerRepo),
'graphql',
'-f',
`query=${PR_FILE_VIEWED_STATES_QUERY}`,
@@ -662,7 +667,7 @@ async function getPRFileViewedStates(
if (after) {
args.push('-f', `after=${after}`)
}
noteRateLimitSpend('graphql')
noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, localGitOptions)
const { stdout } = await ghExecFileAsync(args, ghOptions)
const parsed = JSON.parse(stdout) as {
data?: {
@@ -816,43 +821,10 @@ async function getIssueBodyAndComments(
}
}
async function getPRBody(
repoPath: string,
prNumber: number,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<string> {
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions))
const ownerRepo = await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions)
try {
if (ownerRepo) {
const { stdout } = await ghExecFileAsync(
[
'api',
...ghApiHostArgs(ownerRepo),
'--cache',
'60s',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}`
],
ghOptions
)
const data = JSON.parse(stdout) as { body?: string | null }
return data.body ?? ''
}
const { stdout } = await ghExecFileAsync(
['pr', 'view', String(prNumber), '--json', 'body'],
ghOptions
)
const data = JSON.parse(stdout) as { body?: string }
return data.body ?? ''
} catch {
return ''
}
}
async function getWorkItemParticipants(
repoPath: string,
item: Pick<GitHubWorkItem, 'number' | 'type'>,
resolvedRepository: GitHubApiRepository | null,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubAssignableUser[]> {
@@ -862,19 +834,19 @@ async function getWorkItemParticipants(
const ownerRepo =
item.type === 'issue'
? await getIssueOwnerRepo(repoPath, connectionId, ...localGitOptionArgs(localGitOptions))
: await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions)
: resolvedRepository
if (!ownerRepo) {
return []
}
if (rateLimitGuard('graphql').blocked) {
if (repositoryRateLimitGuard(ownerRepo, 'graphql', localGitOptions).blocked) {
return []
}
try {
noteRateLimitSpend('graphql')
noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, localGitOptions)
const { stdout } = await ghExecFileAsync(
[
'api',
...ghApiHostArgs(ownerRepo),
...githubApiHostArgs(ownerRepo),
'graphql',
'-f',
`query=${WORK_ITEM_PARTICIPANTS_QUERY}`,
@@ -920,6 +892,7 @@ async function getWorkItemParticipants(
async function getGitHubUsersByLogin(
repoPath: string,
logins: string[],
repository: GitHubApiRepository | null,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubAssignableUser[]> {
@@ -929,7 +902,8 @@ async function getGitHubUsersByLogin(
if (uniqueLogins.length === 0) {
return []
}
if (rateLimitGuard('graphql').blocked) {
const guard = repositoryRateLimitGuard(repository, 'graphql', localGitOptions)
if (guard.blocked) {
// Why: surface the skip. Callers degrade silently to login-based avatar URLs
// (blank on GHE), so without this log "why are avatars missing" is untraceable.
console.warn(
@@ -937,7 +911,6 @@ async function getGitHubUsersByLogin(
)
return []
}
const ownerRepo = await getOriginApiOwnerRepo(repoPath, connectionId, localGitOptions)
const fields = uniqueLogins
.map(
(login, index) =>
@@ -945,11 +918,11 @@ async function getGitHubUsersByLogin(
)
.join('\n')
try {
noteRateLimitSpend('graphql')
noteRepositoryRateLimitSpend(repository, 'graphql', 1, localGitOptions)
const { stdout } = await ghExecFileAsync(
[
'api',
...(ownerRepo ? ghApiHostArgs(ownerRepo) : []),
...(repository ? githubApiHostArgs(repository) : []),
'graphql',
'-f',
`query=query { ${fields} }`
@@ -1059,6 +1032,7 @@ async function getMentionParticipants(
>,
comments: PRComment[],
participants: GitHubAssignableUser[],
repository: GitHubApiRepository | null,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubAssignableUser[]> {
@@ -1083,6 +1057,7 @@ async function getMentionParticipants(
const graphQlUsers = await getGitHubUsersByLogin(
repoPath,
visibleLogins,
repository,
connectionId,
localGitOptions
)
@@ -1093,6 +1068,7 @@ async function getPRChecksForDetails(
repoPath: string,
prNumber: number,
headSha: string | undefined,
repository: GitHubApiRepository | null,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<PRCheckDetail[]> {
@@ -1101,7 +1077,7 @@ async function getPRChecksForDetails(
repoPath,
prNumber,
headSha,
null,
repository,
undefined,
connectionId,
...localGitOptionArgs(localGitOptions)
@@ -1121,16 +1097,35 @@ export async function getWorkItemDetails(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubWorkItemDetails | null> {
// Why: getWorkItem already handles acquire/release. We call it first (outside
// our semaphore) so the known-cheap lookup doesn't compete with the richer
// detail fetches that follow.
const item: Omit<GitHubWorkItem, 'repoId'> | null = await getWorkItem(
repoPath,
number,
type,
connectionId,
...localGitOptionArgs(localGitOptions)
)
const originApiRepository =
type === 'issue'
? null
: await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
// Why: SSH-backed gh runs without a cwd, so an unresolved PR repository must
// not fall through to gh's default host and show a same-number github.com PR.
if (connectionId && type === 'pr' && !originApiRepository) {
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 =
type === 'pr' && originApiRepository !== null && !isGitHubDotComRepository(originApiRepository)
const item: Omit<GitHubWorkItem, 'repoId'> | null = useExplicitEnterpriseRepository
? await getWorkItemByOwnerRepo(
repoPath,
originApiRepository,
number,
'pr',
connectionId,
...localGitOptionArgs(localGitOptions)
)
: await getWorkItem(
repoPath,
number,
type,
connectionId,
...localGitOptionArgs(localGitOptions)
)
if (!item) {
return null
}
@@ -1171,13 +1166,14 @@ export async function getWorkItemDetails(
// 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, connectionId, localGitOptions)
getWorkItemParticipants(repoPath, item, null, connectionId, localGitOptions)
])
const mentionParticipants = await getMentionParticipants(
repoPath,
item,
comments,
participants,
null,
connectionId,
localGitOptions
)
@@ -1191,36 +1187,56 @@ export async function getWorkItemDetails(
}
}
// PR: fetch body + comments + checks + files + head/base SHAs in parallel.
const [body, comments, shas, files, viewedStates, participants] = await Promise.all([
getPRBody(repoPath, item.number, connectionId, localGitOptions),
// 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),
getPRComments(
repoPath,
item.number,
undefined,
{ prRepo: originApiRepository },
connectionId,
...localGitOptionArgs(localGitOptions)
),
getPRHeadBaseSha(repoPath, item.number, connectionId, localGitOptions),
getPRFiles(repoPath, item.number, connectionId, localGitOptions),
getPRFileViewedStates(repoPath, item.number, connectionId, localGitOptions),
getWorkItemParticipants(repoPath, item, connectionId, localGitOptions)
getPRFiles(repoPath, item.number, originApiRepository, connectionId, localGitOptions),
getPRFileViewedStates(
repoPath,
item.number,
originApiRepository,
connectionId,
localGitOptions
),
getWorkItemParticipants(repoPath, item, originApiRepository, connectionId, localGitOptions)
])
// Why: run the mention-author GraphQL lookup in parallel with the final
// checks fetch instead of serially — both depend only on data from the
// Promise.all above, so there's no ordering requirement between them.
const [mentionParticipants, checks] = await Promise.all([
getMentionParticipants(repoPath, item, comments, participants, connectionId, localGitOptions),
getPRChecksForDetails(repoPath, item.number, shas?.headSha, connectionId, localGitOptions)
getMentionParticipants(
repoPath,
item,
comments,
participants,
originApiRepository,
connectionId,
localGitOptions
),
getPRChecksForDetails(
repoPath,
item.number,
metadata.headSha,
originApiRepository,
connectionId,
localGitOptions
)
])
return {
item: enrichItemDisplayAvatars(item, mentionParticipants),
body,
body: metadata.body,
comments,
headSha: shas?.headSha,
baseSha: shas?.baseSha,
headSha: metadata.headSha,
baseSha: metadata.baseSha,
pullRequestId: viewedStates?.pullRequestId,
checks,
// Why: distinguish a failed file fetch (null) from an empty PR so the
@@ -1242,7 +1258,7 @@ async function fetchContentAtRef(args: {
repoPath: string
connectionId?: string | null
localGitOptions?: LocalGitExecOptions
ownerRepo: GitHubOwnerRepoSlug
ownerRepo: GitHubApiRepository
path: string
ref: string
}): Promise<{ content: string; isBinary: boolean; tooLarge?: boolean }> {
@@ -1254,7 +1270,7 @@ async function fetchContentAtRef(args: {
'300s',
'-H',
'Accept: application/vnd.github.raw',
...ghApiHostArgs(args.ownerRepo),
...githubApiHostArgs(args.ownerRepo),
`repos/${args.ownerRepo.owner}/${args.ownerRepo.repo}/contents/${encodeGitHubContentPath(args.path)}?ref=${encodeURIComponent(args.ref)}`
],
{
@@ -1291,7 +1307,7 @@ export async function getPRFileContents(args: {
headSha: string
baseSha: string
}): Promise<GitHubPRFileContents> {
const ownerRepo = await getOriginApiOwnerRepo(
const ownerRepo = await getOriginGitHubApiRepository(
args.repoPath,
args.connectionId,
args.localGitOptions
+2 -2
View File
@@ -175,7 +175,7 @@ describe('github RPC methods', () => {
repo: 'repo-1',
prNumber: 7,
headSha: 'abc123',
prRepo: { owner: 'acme', repo: 'widgets' },
prRepo: { owner: 'acme', repo: 'widgets', host: 'github.acme-corp.com' },
noCache: true
})
)
@@ -184,7 +184,7 @@ describe('github RPC methods', () => {
'repo-1',
7,
'abc123',
{ owner: 'acme', repo: 'widgets' },
{ owner: 'acme', repo: 'widgets', host: 'github.acme-corp.com' },
{
noCache: true
}
+4 -1
View File
@@ -42,7 +42,10 @@ const RateLimit = z.object({
const SlugRepo = z.object({
owner: requiredString('Missing owner'),
repo: requiredString('Missing repo')
repo: requiredString('Missing repo'),
// Why: Enterprise host identity must survive RPC parsing; Zod strips
// undeclared fields before the runtime can host-qualify gh requests.
host: OptionalString
})
const SlugAssignableUsers = SlugRepo.extend({
+9 -8
View File
@@ -27,6 +27,7 @@ import type {
GitHubAssignableUser,
GitHubCommentResult,
GitHubCreateIssueResult,
GitHubOwnerRepo,
GitHubWorkItem,
JiraProjectStatusOrder,
GitPushTarget,
@@ -1340,7 +1341,7 @@ const api = {
sourceContext?: TaskSourceContext | null
prNumber: number
headSha?: string
prRepo?: { owner: string; repo: string } | null
prRepo?: GitHubOwnerRepo | null
noCache?: boolean
}): Promise<unknown[]> => ipcRenderer.invoke('gh:prChecks', args),
@@ -1352,7 +1353,7 @@ const api = {
workflowRunId?: number
checkName?: string
url?: string | null
prRepo?: { owner: string; repo: string } | null
prRepo?: GitHubOwnerRepo | null
}): Promise<unknown | null> => ipcRenderer.invoke('gh:prCheckDetails', args),
rerunPRChecks: (args: {
@@ -1370,7 +1371,7 @@ const api = {
repoId?: string
sourceContext?: TaskSourceContext | null
prNumber: number
prRepo?: { owner: string; repo: string } | null
prRepo?: GitHubOwnerRepo | null
noCache?: boolean
}): Promise<unknown[]> => ipcRenderer.invoke('gh:prComments', args),
@@ -1397,7 +1398,7 @@ const api = {
repoId?: string
prNumber: number
title: string
prRepo?: { owner: string; repo: string } | null
prRepo?: GitHubOwnerRepo | null
}): Promise<boolean> => ipcRenderer.invoke('gh:updatePRTitle', args),
mergePR: (args: {
@@ -1406,7 +1407,7 @@ const api = {
sourceContext?: TaskSourceContext | null
prNumber: number
method?: 'merge' | 'squash' | 'rebase'
prRepo?: { owner: string; repo: string } | null
prRepo?: GitHubOwnerRepo | null
}): Promise<{ ok: true } | { ok: false; error: string }> =>
ipcRenderer.invoke('gh:mergePR', args),
@@ -1417,7 +1418,7 @@ const api = {
prNumber: number
enabled: boolean
method?: 'merge' | 'squash' | 'rebase'
prRepo?: { owner: string; repo: string } | null
prRepo?: GitHubOwnerRepo | null
}): Promise<{ ok: true } | { ok: false; error: string }> =>
ipcRenderer.invoke('gh:setPRAutoMerge', args),
@@ -1464,7 +1465,7 @@ const api = {
number: number
body: string
type?: 'issue' | 'pr'
prRepo?: { owner: string; repo: string } | null
prRepo?: GitHubOwnerRepo | null
}): Promise<GitHubCommentResult> => ipcRenderer.invoke('gh:addIssueComment', args),
addPRReviewCommentReply: (args: {
@@ -1477,7 +1478,7 @@ const api = {
threadId?: string
path?: string
line?: number
prRepo?: { owner: string; repo: string } | null
prRepo?: GitHubOwnerRepo | null
}): Promise<GitHubCommentResult> => ipcRenderer.invoke('gh:addPRReviewCommentReply', args),
addPRReviewComment: (args: {
@@ -27,6 +27,23 @@ describe('checksPanelAsyncResultKey', () => {
).toBe('repo-id::feature/test::acme/widgets::12::none')
})
it('includes non-default GitHub hosts in PR repo identity', () => {
const githubDotComKey = checksPanelAsyncResultKey('repo-id', 'feature/test', 12, {
owner: 'Acme',
repo: 'Widgets',
host: 'github.com'
})
const enterpriseKey = checksPanelAsyncResultKey('repo-id', 'feature/test', 12, {
owner: 'Acme',
repo: 'Widgets',
host: 'github.acme-corp.com'
})
expect(githubDotComKey).toBe('repo-id::feature/test::acme/widgets::12::none')
expect(enterpriseKey).toBe('repo-id::feature/test::github.acme-corp.com/acme/widgets::12::none')
expect(enterpriseKey).not.toBe(githubDotComKey)
})
it('includes PR head SHA so stale checks cannot commit after a new head is discovered', () => {
expect(checksPanelAsyncResultKey('repo-id', 'feature/test', 12, null, 'head-a')).toBe(
'repo-id::feature/test::none::12::head-a'
@@ -4,7 +4,9 @@ function normalizedPRRepoIdentity(prRepo?: GitHubOwnerRepo | null): string {
if (!prRepo) {
return 'none'
}
return `${prRepo.owner.toLowerCase()}/${prRepo.repo.toLowerCase()}`
const slug = `${prRepo.owner.toLowerCase()}/${prRepo.repo.toLowerCase()}`
const host = prRepo.host?.toLowerCase()
return host && host !== 'github.com' ? `${host}/${slug}` : slug
}
export function checksPanelAsyncResultKey(
@@ -91,4 +91,34 @@ describe('syncPRChecksStatus', () => {
expect(result?.prCache?.['repo-id::main']?.data?.checksStatus).toBe('success')
expect(result?.prCache?.['runtime:env-win::repo-id::main']?.data?.checksStatus).toBe('neutral')
})
it('rejects a checks result from the same slug on a different GitHub host', () => {
const state = {
prCache: {
'repo-id::main': {
fetchedAt: 0,
data: {
checksStatus: 'neutral' as const,
prRepo: {
owner: 'acme',
repo: 'widgets',
host: 'github.acme-corp.com'
}
}
}
}
} as unknown as AppState
const result = syncPRChecksStatus(
state,
'/repo',
'repo-id',
'main',
[{ name: 'build', status: 'completed', conclusion: 'success', url: null }],
undefined,
{ owner: 'acme', repo: 'widgets', host: 'github.com' }
)
expect(result).toBeNull()
})
})
@@ -100,7 +100,9 @@ function normalizedPRRepo(repo?: GitHubOwnerRepo | null): string | null {
if (!repo) {
return null
}
return `${repo.owner.toLowerCase()}/${repo.repo.toLowerCase()}`
const slug = `${repo.owner.toLowerCase()}/${repo.repo.toLowerCase()}`
const host = repo.host?.toLowerCase()
return host && host !== 'github.com' ? `${host}/${slug}` : slug
}
function samePRRepo(left?: GitHubOwnerRepo | null, right?: GitHubOwnerRepo | null): boolean {
@@ -966,6 +966,22 @@ describe('createGitHubSlice.fetchPRChecks', () => {
})
})
it('isolates PR detail caches by Enterprise host', () => {
const githubRepo = { owner: 'Acme', repo: 'Widgets', host: 'github.com' }
const enterpriseRepo = {
owner: 'Acme',
repo: 'Widgets',
host: 'github.acme-corp.com'
}
expect(prChecksCacheSuffix(12, enterpriseRepo, 'head')).not.toBe(
prChecksCacheSuffix(12, githubRepo, 'head')
)
expect(prCommentsCacheSuffix(12, enterpriseRepo)).not.toBe(
prCommentsCacheSuffix(12, githubRepo)
)
})
it('bounds checks cache entries across many repo and head combinations', async () => {
vi.useFakeTimers()
+3 -1
View File
@@ -902,7 +902,9 @@ function evictRepoCacheEntries<T>(
}
function normalizedRepoIdentity(repo: GitHubOwnerRepo): string {
return `${repo.owner.toLowerCase()}/${repo.repo.toLowerCase()}`
const slug = `${repo.owner.toLowerCase()}/${repo.repo.toLowerCase()}`
const host = repo.host?.toLowerCase()
return host && host !== 'github.com' ? `${host}/${slug}` : slug
}
function normalizedHeadSha(headSha?: string): string | null {
+3 -1
View File
@@ -1154,7 +1154,9 @@ export type PRConflictSummary = {
localMergeState?: 'clean'
}
export type GitHubRepositoryIdentity = { owner: string; repo: string }
// Why: host must survive renderer/RPC boundaries so Enterprise review actions
// cannot silently fall back to a same-named repository on github.com.
export type GitHubRepositoryIdentity = { owner: string; repo: string; host?: string }
export type GitHubPRMergeMethod = 'merge' | 'squash' | 'rebase'