Refactor GitHub repository execution setup

This commit is contained in:
Jinjing
2026-07-16 22:32:05 -07:00
parent 5ae1c99902
commit 75398ed56e
4 changed files with 181 additions and 110 deletions
+52 -78
View File
@@ -81,9 +81,10 @@ import {
githubHostExecOptions,
githubRepositorySlugArg,
githubRepositoryWebHost,
resolveGitHubApiRepository,
resolveGitHubApiRepositoryCandidates,
resolveGitHubRepoExecution,
resolveIssueGitHubApiRepositorySource,
type GitHubRepoExecOptions,
type GitHubApiRepository
} from './github-api-repository'
import { githubRepoIdentityKey } from '../../shared/github-repository-identity-key'
@@ -116,7 +117,7 @@ import {
type RateLimitBucketKind
} from './rate-limit'
type GhExecOptions = ReturnType<typeof ghRepoExecOptions> & { host?: string }
type GhExecOptions = GitHubRepoExecOptions
type HostedReviewLocalGitOptions = ReturnType<typeof getHostedReviewLocalGitOptions>
const ORCA_REPO = 'stablyai/orca'
@@ -1742,7 +1743,12 @@ export async function getRepoUpstream(
): Promise<OwnerRepo | null> {
const localGitArgs = hostedReviewLocalGitOptionArgs(options)
const localGitOptions = localGitArgs[0] ?? {}
const origin = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
const { ownerRepo: origin, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
undefined,
connectionId,
localGitOptions
)
if (!origin) {
return null
}
@@ -1764,8 +1770,7 @@ export async function getRepoUpstream(
// Why: best-effort fork lookup runs at add-time; cap latency so a stalled
// gh process can't hold up repo creation.
{
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(origin),
...ghOptions,
timeout: 10_000
}
)
@@ -3691,16 +3696,12 @@ export async function getPRChecks(
localGitOptions: LocalGitExecOptions = {}
): Promise<PRCheckDetail[]> {
void headSha
const ownerRepo = await resolveGitHubApiRepository(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
prRepo,
connectionId,
localGitOptions
)
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
if (connectionId && !ownerRepo) {
throw new Error(GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE)
}
@@ -3976,7 +3977,7 @@ export async function getPRCheckDetails(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<PRCheckRunDetails | null> {
const ownerRepo = await resolveGitHubApiRepository(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
args.prRepo,
connectionId,
@@ -3985,10 +3986,6 @@ export async function getPRCheckDetails(
if (!ownerRepo) {
return null
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
@@ -4077,14 +4074,15 @@ export async function rerunPRChecks(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubRerunPRChecksResult> {
const ownerRepo = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
undefined,
connectionId,
localGitOptions
)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
const checks = await getPRChecks(
repoPath,
@@ -4220,16 +4218,12 @@ export async function getPRComments(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<PRComment[]> {
const ownerRepo = await resolveGitHubApiRepository(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
options?.prRepo,
connectionId,
localGitOptions
)
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
if (connectionId && !ownerRepo) {
throw new Error(GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE)
}
@@ -4474,18 +4468,15 @@ export async function setPRFileViewed(args: {
path: string
viewed: boolean
}): Promise<boolean> {
const ownerRepo = await getOriginGitHubApiRepository(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
args.repoPath,
undefined,
args.connectionId,
args.localGitOptions
)
if (args.connectionId && !ownerRepo) {
return false
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(args.repoPath, args.connectionId, args.localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
const mutation = args.viewed ? 'markFileAsViewed' : 'unmarkFileAsViewed'
const query = `mutation($pullRequestId: ID!, $path: String!) {
${mutation}(input: { pullRequestId: $pullRequestId, path: $path }) {
@@ -4528,14 +4519,15 @@ export async function resolveReviewThread(
): Promise<boolean> {
const mutation = resolve ? 'resolveReviewThread' : 'unresolveReviewThread'
const query = `mutation($threadId: ID!) { ${mutation}(input: { threadId: $threadId }) { thread { isResolved } } }`
const ownerRepo = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
undefined,
connectionId,
localGitOptions
)
if (connectionId && !ownerRepo) {
return false
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
const guard = repositoryRateLimitGuard(ownerRepo, 'graphql', ghOptions)
if (guard.blocked) {
console.warn(
@@ -4602,7 +4594,7 @@ export async function addPRReviewCommentReply(
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubCommentResult> {
const ownerRepo = await resolveGitHubApiRepository(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
prRepo,
connectionId,
@@ -4611,10 +4603,6 @@ export async function addPRReviewCommentReply(
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const { stdout } = await ghExecFileAsync(
@@ -4650,18 +4638,15 @@ export async function addPRReviewComment(
localGitOptions?: LocalGitExecOptions
}
): Promise<GitHubCommentResult> {
const ownerRepo = await getOriginGitHubApiRepository(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
args.repoPath,
undefined,
args.connectionId,
args.localGitOptions
)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(args.repoPath, args.connectionId, args.localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const fields = [
@@ -4719,7 +4704,7 @@ export async function mergePR(
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const ownerRepo = await resolveGitHubApiRepository(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
prRepo,
connectionId,
@@ -4728,10 +4713,6 @@ export async function mergePR(
if (connectionId && !ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const mergeBlocker = await getPRMergeBlocker(
@@ -4776,7 +4757,7 @@ export async function setPRAutoMerge(
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const ownerRepo = await resolveGitHubApiRepository(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
prRepo,
connectionId,
@@ -4785,10 +4766,6 @@ export async function setPRAutoMerge(
if (connectionId && !ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
if (enabled) {
@@ -4993,12 +4970,15 @@ export async function updatePRState(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const ownerRepo = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
undefined,
connectionId,
localGitOptions
)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = { ...ghRepoExecOptions(context), ...githubHostExecOptions(ownerRepo) }
await acquire()
try {
@@ -5032,14 +5012,15 @@ export async function requestPRReviewers(
if (logins.length === 0) {
return { ok: false, error: 'Enter at least one reviewer' }
}
const ownerRepo = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
undefined,
connectionId,
localGitOptions
)
if (connectionId && !ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const args = ['pr', 'edit', String(prNumber), '--add-reviewer', logins.join(',')]
@@ -5071,14 +5052,15 @@ export async function removePRReviewers(
if (logins.length === 0) {
return { ok: false, error: 'Enter at least one reviewer' }
}
const ownerRepo = await getOriginGitHubApiRepository(repoPath, connectionId, localGitOptions)
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
undefined,
connectionId,
localGitOptions
)
if (connectionId && !ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const args = ['pr', 'edit', String(prNumber), '--remove-reviewer', logins.join(',')]
@@ -5110,7 +5092,7 @@ export async function updatePRTitle(
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<boolean> {
const ownerRepo = await resolveGitHubApiRepository(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
prRepo,
connectionId,
@@ -5119,10 +5101,6 @@ export async function updatePRTitle(
if (connectionId && !ownerRepo) {
return false
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
await acquire()
try {
const args = ['pr', 'edit', String(prNumber), '--title', title]
@@ -5149,7 +5127,7 @@ export async function updatePRDetails(
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const ownerRepo = await resolveGitHubApiRepository(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
prRepo,
connectionId,
@@ -5158,10 +5136,6 @@ export async function updatePRDetails(
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
const fields: string[] = []
if (updates.title !== undefined) {
+29 -1
View File
@@ -1,6 +1,6 @@
import { describe, expect, it } from 'vitest'
import { githubHostExecOptions } from './github-api-repository'
import { githubHostExecOptions, resolveGitHubRepoExecution } from './github-api-repository'
describe('githubHostExecOptions', () => {
it('pins every known repository host', () => {
@@ -17,3 +17,31 @@ describe('githubHostExecOptions', () => {
expect(githubHostExecOptions(null)).toEqual({})
})
})
describe('resolveGitHubRepoExecution', () => {
it('combines local repository and GitHub host execution options', async () => {
const ownerRepo = { owner: 'acme', repo: 'widgets', host: 'github.acme-corp.com' }
await expect(
resolveGitHubRepoExecution('/repo', ownerRepo, null, { wslDistro: 'Ubuntu' })
).resolves.toEqual({
ownerRepo,
ghOptions: {
cwd: '/repo',
wslDistro: 'Ubuntu',
host: 'github.acme-corp.com'
}
})
})
it('uses a caller-specific repository resolver without changing its identity', async () => {
const ownerRepo = { owner: 'upstream', repo: 'widgets' }
await expect(
resolveGitHubRepoExecution('/remote/repo', async () => ownerRepo, 'ssh-1')
).resolves.toEqual({
ownerRepo,
ghOptions: {}
})
})
})
+39 -1
View File
@@ -3,13 +3,30 @@ import {
githubRepoIdentityKey,
isDefaultGitHubHost
} from '../../shared/github-repository-identity-key'
import { getOwnerRepo, getOwnerRepoForRemote, type LocalGitExecOptions } from './gh-utils'
import {
getOwnerRepo,
getOwnerRepoForRemote,
ghRepoExecOptions,
githubRepoContext,
type LocalGitExecOptions
} from './gh-utils'
import {
getEnterpriseGitHubRepoSlug,
getEnterpriseGitHubRepoSlugForRemote
} from './github-enterprise-repository'
export type GitHubApiRepository = GitHubOwnerRepo
export type GitHubRepoExecOptions = ReturnType<typeof ghRepoExecOptions> & { host?: string }
export type GitHubRepoExecution = {
ownerRepo: GitHubApiRepository | null
ghOptions: GitHubRepoExecOptions
}
type GitHubApiRepositoryResolution =
| GitHubApiRepository
| null
| undefined
| (() => Promise<GitHubApiRepository | null>)
// Why: the enterprise branch spawns an uncached `git remote get-url` (an SSH
// round trip on connection-backed repos) — hot paths like per-file contents
@@ -256,6 +273,27 @@ export function githubHostExecOptions(repository: GitHubApiRepository | null | u
return repository?.host ? { host: repository.host } : {}
}
export async function resolveGitHubRepoExecution(
repoPath: string,
repository?: GitHubApiRepositoryResolution,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubRepoExecution> {
// Why: issue-scoped paths retain their upstream-first resolver while sharing
// the same repo-scoped and host-scoped gh execution option construction.
const ownerRepo =
typeof repository === 'function'
? await repository()
: await resolveGitHubApiRepository(repoPath, repository, connectionId, localGitOptions)
return {
ownerRepo,
ghOptions: {
...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)),
...githubHostExecOptions(ownerRepo)
}
}
}
export function githubRepositoryWebHost(repository: GitHubApiRepository): string {
return repository.host ?? 'github.com'
}
+61 -30
View File
@@ -17,12 +17,11 @@ import { mapIssueInfo } from './mappers'
import type { LocalGitExecOptions, OwnerRepo } from './gh-utils'
import {
getIssueGitHubApiRepository,
githubHostExecOptions,
resolveGitHubApiRepository,
resolveGitHubRepoExecution,
resolveIssueGitHubApiRepositorySource
} from './github-api-repository'
// prettier-ignore
import { ghExecFileAsync, acquire, release, classifyGhError, classifyListIssuesError, ghRepoExecOptions, githubRepoContext, extractExecError } from './gh-utils'
import { ghExecFileAsync, acquire, release, classifyGhError, classifyListIssuesError, extractExecError } from './gh-utils'
// Why: distinguishes a successful-empty listing from a failed fetch. The
// previous `catch { return [] }` conflated a 403 on a private upstream with an
@@ -60,9 +59,12 @@ export async function getIssue(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<IssueInfo | null> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const ownerRepo = await getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions)
const ghOptions = { ...ghRepoExecOptions(context), ...githubHostExecOptions(ownerRepo) }
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
() => getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions),
connectionId,
localGitOptions
)
await acquire()
try {
if (ownerRepo) {
@@ -110,14 +112,20 @@ export async function listIssues(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<IssueListResult> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const { source: ownerRepo } = await resolveIssueGitHubApiRepositorySource(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
preference,
async () =>
(
await resolveIssueGitHubApiRepositorySource(
repoPath,
preference,
connectionId,
localGitOptions
)
).source,
connectionId,
localGitOptions
)
const ghOptions = { ...ghRepoExecOptions(context), ...githubHostExecOptions(ownerRepo) }
await acquire()
try {
if (ownerRepo) {
@@ -179,14 +187,20 @@ export async function createIssue(
if (!trimmedTitle) {
return { ok: false, error: 'Title is required' }
}
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const { source: ownerRepo } = await resolveIssueGitHubApiRepositorySource(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
preference,
async () =>
(
await resolveIssueGitHubApiRepositorySource(
repoPath,
preference,
connectionId,
localGitOptions
)
).source,
connectionId,
localGitOptions
)
const ghOptions = { ...ghRepoExecOptions(context), ...githubHostExecOptions(ownerRepo) }
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
@@ -292,12 +306,15 @@ export async function updateIssue(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const ownerRepo = await getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions)
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
() => getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions),
connectionId,
localGitOptions
)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = { ...ghRepoExecOptions(context), ...githubHostExecOptions(ownerRepo) }
const repo = `${ownerRepo.owner}/${ownerRepo.repo}`
const errors: string[] = []
@@ -415,14 +432,16 @@ export async function addIssueComment(
ownerRepoOverride?: OwnerRepo | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubCommentResult> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const ownerRepo = ownerRepoOverride
? await resolveGitHubApiRepository(repoPath, ownerRepoOverride, connectionId, localGitOptions)
: await getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions)
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
ownerRepoOverride ??
(() => getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions)),
connectionId,
localGitOptions
)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
const ghOptions = { ...ghRepoExecOptions(context), ...githubHostExecOptions(ownerRepo) }
await acquire()
try {
const { stdout } = await ghExecFileAsync(
@@ -470,14 +489,20 @@ export async function listLabels(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<string[]> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const { source: ownerRepo } = await resolveIssueGitHubApiRepositorySource(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
preference,
async () =>
(
await resolveIssueGitHubApiRepositorySource(
repoPath,
preference,
connectionId,
localGitOptions
)
).source,
connectionId,
localGitOptions
)
const ghOptions = { ...ghRepoExecOptions(context), ...githubHostExecOptions(ownerRepo) }
if (!ownerRepo) {
return []
}
@@ -510,14 +535,20 @@ export async function listAssignableUsers(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<GitHubAssignableUser[]> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const { source: ownerRepo } = await resolveIssueGitHubApiRepositorySource(
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
preference,
async () =>
(
await resolveIssueGitHubApiRepositorySource(
repoPath,
preference,
connectionId,
localGitOptions
)
).source,
connectionId,
localGitOptions
)
const ghOptions = { ...ghRepoExecOptions(context), ...githubHostExecOptions(ownerRepo) }
if (!ownerRepo) {
return []
}