fix(github): enforce origin preference in issue open-by-number lookup

listWorkItems and getWorkItem must share preference so origin/upstream
toggles cannot disagree. Explicit origin preference now fail-closes when
origin identity is unresolved (no bare-lookup fallback), matching the
PR candidate resolution rule.
This commit is contained in:
Jinjing
2026-07-25 22:48:15 -07:00
parent 5e22697608
commit bc72dcb5d1
2 changed files with 204 additions and 9 deletions
@@ -0,0 +1,181 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import type * as GithubApiRepositoryModule from './github-api-repository'
import type * as GhUtils from './gh-utils'
const {
ghExecFileAsyncMock,
getOwnerRepoMock,
getIssueOwnerRepoMock,
getOwnerRepoForRemoteMock,
resolvePRRepositoryCandidatesMock,
resolveIssueSourceMock,
rateLimitGuardMock,
noteRateLimitSpendMock,
acquireMock,
releaseMock
} = vi.hoisted(() => ({
ghExecFileAsyncMock: vi.fn(),
getOwnerRepoMock: vi.fn(),
getIssueOwnerRepoMock: vi.fn(),
getOwnerRepoForRemoteMock: vi.fn(),
resolvePRRepositoryCandidatesMock: vi.fn(),
resolveIssueSourceMock: vi.fn(),
rateLimitGuardMock: vi.fn(() => ({ blocked: false })),
noteRateLimitSpendMock: vi.fn(),
acquireMock: vi.fn(),
releaseMock: vi.fn()
}))
vi.mock('./gh-utils', async () => {
const actual = await vi.importActual<typeof GhUtils>('./gh-utils')
return {
...actual,
execFileAsync: vi.fn(),
ghExecFileAsync: ghExecFileAsyncMock,
getOwnerRepo: getOwnerRepoMock,
getIssueOwnerRepo: getIssueOwnerRepoMock,
getOwnerRepoForRemote: getOwnerRepoForRemoteMock,
resolveIssueSource: resolveIssueSourceMock,
acquire: acquireMock,
release: releaseMock,
_resetOwnerRepoCache: vi.fn()
}
})
vi.mock('./rate-limit', () => ({
rateLimitGuard: rateLimitGuardMock,
noteRateLimitSpend: noteRateLimitSpendMock,
getRateLimit: vi.fn(async () => ({ ok: false, error: 'not probed in tests' })),
repositoryRateLimitGuard: vi.fn(() => ({ blocked: false })),
noteRepositoryRateLimitSpend: vi.fn(),
spendsSharedGitHubComQuota: () => true
}))
vi.mock('./github-api-repository', async (importOriginal) => {
const actual = await importOriginal<typeof GithubApiRepositoryModule>()
return {
...actual,
resolveIssueGitHubApiRepositorySource: (
repoPath: string,
preference: unknown,
connectionId?: string | null,
localGitOptions?: unknown
) => resolveIssueSourceMock(repoPath, preference, connectionId, localGitOptions),
getIssueGitHubApiRepository: (repoPath: string, connectionId?: string | null) =>
getIssueOwnerRepoMock(repoPath, connectionId),
getOriginGitHubApiRepository: (
repoPath: string,
connectionId?: string | null,
localGitOptions?: unknown
) => getOwnerRepoMock(repoPath, connectionId, localGitOptions),
getGitHubApiRepositoryForRemote: (
repoPath: string,
remoteName: string,
connectionId?: string | null,
localGitOptions?: unknown
) =>
remoteName === 'origin'
? getOwnerRepoMock(repoPath, connectionId, localGitOptions)
: getOwnerRepoForRemoteMock(repoPath, remoteName, connectionId, localGitOptions),
resolveGitHubApiRepositoryCandidates: (
repoPath: string,
connectionId?: string | null,
localGitOptions?: unknown
) => resolvePRRepositoryCandidatesMock(repoPath, connectionId, localGitOptions)
}
})
import { getWorkItem, _resetOwnerRepoCache } from './client'
describe('GitHub issue open-by-number origin preference', () => {
beforeEach(() => {
ghExecFileAsyncMock.mockReset()
getOwnerRepoMock.mockReset()
getIssueOwnerRepoMock.mockReset()
getOwnerRepoForRemoteMock.mockReset()
resolvePRRepositoryCandidatesMock.mockReset()
resolveIssueSourceMock.mockReset()
rateLimitGuardMock.mockReset()
rateLimitGuardMock.mockReturnValue({ blocked: false })
noteRateLimitSpendMock.mockReset()
acquireMock.mockReset()
releaseMock.mockReset()
acquireMock.mockResolvedValue(undefined)
getOwnerRepoForRemoteMock.mockImplementation(
async (repoPath: string, remoteName: string, connectionId?: string | null, opts = {}) =>
remoteName === 'origin' ? getOwnerRepoMock(repoPath, connectionId, opts) : null
)
resolvePRRepositoryCandidatesMock.mockImplementation(async (repoPath, connectionId) => {
const origin = await getOwnerRepoMock(repoPath, connectionId)
const repository = origin ? { host: 'github.com', ...origin } : null
return { candidates: repository ? [repository] : [], headRepo: repository }
})
_resetOwnerRepoCache()
})
it('pins typed issue metadata to explicit origin preference', async () => {
const source = { owner: 'fork', repo: 'orca', host: 'github.com' }
resolveIssueSourceMock.mockResolvedValueOnce({ source, fellBack: false })
ghExecFileAsyncMock.mockResolvedValueOnce({
stdout: JSON.stringify({
number: 7,
title: 'Origin issue',
state: 'open',
labels: [],
url: 'https://github.com/fork/orca/issues/7',
updatedAt: '2026-04-02T00:00:00Z',
author: { login: 'octocat' }
})
})
const item = await getWorkItem('/repo-root', 7, 'issue', null, {}, 'origin')
expect(resolveIssueSourceMock).toHaveBeenCalledWith('/repo-root', 'origin', null, {})
expect(getIssueOwnerRepoMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
['api', 'repos/fork/orca/issues/7'],
expect.objectContaining({ cwd: '/repo-root', host: 'github.com' })
)
expect(item).toMatchObject({ number: 7, title: 'Origin issue', type: 'issue' })
})
it('does not run a bare issue lookup when explicit origin identity is unresolved', async () => {
resolveIssueSourceMock.mockResolvedValueOnce({ source: null, fellBack: false })
await expect(getWorkItem('/repo-root', 7, 'issue', null, {}, 'origin')).resolves.toBeNull()
expect(resolveIssueSourceMock).toHaveBeenCalledWith('/repo-root', 'origin', null, {})
expect(getIssueOwnerRepoMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
})
it('skips the issue probe on untyped open when origin identity is unresolved', async () => {
const origin = { owner: 'fork', repo: 'orca', host: 'github.com' }
resolveIssueSourceMock.mockResolvedValueOnce({ source: null, fellBack: false })
getOwnerRepoMock.mockResolvedValue(origin)
resolvePRRepositoryCandidatesMock.mockResolvedValue({ candidates: [origin], headRepo: origin })
ghExecFileAsyncMock.mockResolvedValueOnce({
stdout: JSON.stringify({
number: 7,
title: 'Origin PR',
state: 'open',
labels: [],
isDraft: false,
url: 'https://github.com/fork/orca/pull/7',
baseRefName: 'main',
headRefName: 'origin/fix',
updatedAt: '2026-04-02T00:00:00Z',
author: { login: 'octocat' }
})
})
const item = await getWorkItem('/repo-root', 7, undefined, null, {}, 'origin')
expect(resolveIssueSourceMock).toHaveBeenCalledWith('/repo-root', 'origin', null, {})
expect(getIssueOwnerRepoMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock.mock.calls[0]?.[0]).toEqual(
expect.arrayContaining(['pr', 'view', '--repo', 'fork/orca'])
)
expect(item).toMatchObject({ number: 7, type: 'pr' })
})
})
+23 -9
View File
@@ -76,7 +76,6 @@ import { shouldHideNonOpenReviewOnDefaultBranch } from '../source-control/repo-d
import { readLocalGitConfigSignature } from './local-git-config-signature'
import {
getGitHubApiRepositoryForRemote,
getIssueGitHubApiRepository,
getOriginGitHubApiRepository,
githubHostExecOptions,
githubRepositorySlugArg,
@@ -1982,14 +1981,21 @@ export async function getWorkItem(
): Promise<MainWorkItem | null> {
await acquire()
try {
// Why: listWorkItems uses resolveIssueGitHubApiRepositorySource; open-by-number
// must share that preference so origin/upstream toggles cannot disagree.
if (type === 'issue') {
return await fetchIssueWorkItem(
const { source } = await resolveIssueGitHubApiRepositorySource(
repoPath,
await getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions),
number,
preference,
connectionId,
localGitOptions
)
// Why: explicit origin with no origin identity must not bare-lookup ambient gh
// (same fail-closed rule as origin-pinned PR candidate resolution).
if (!source && preference === 'origin') {
return null
}
return await fetchIssueWorkItem(repoPath, source, number, connectionId, localGitOptions)
}
if (type === 'pr') {
return await fetchPullRequestWorkItemFromCandidates(
@@ -2002,15 +2008,23 @@ export async function getWorkItem(
}
try {
const issue = await fetchIssueWorkItem(
const { source } = await resolveIssueGitHubApiRepositorySource(
repoPath,
await getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions),
number,
preference,
connectionId,
localGitOptions
)
if (issue) {
return issue
if (source || preference !== 'origin') {
const issue = await fetchIssueWorkItem(
repoPath,
source,
number,
connectionId,
localGitOptions
)
if (issue) {
return issue
}
}
} catch (err) {
// Why: only fall through to PR #N on a genuine 404; re-throw transient errors so a flake can't surface an unrelated PR.