Keep opened issue details and edits in the selected repository (#24729)

Keeps a fork issue’s details, metadata and edits bound to the repository the user opened, including same-number issues in fork and upstream. Repairs selected-assignee leakage and delayed failed edits repainting another issue.

Fixes #24378

Incorporates and cross-reviews contributor PR #24379, including its source-resolver correction and regression material. Covers the contributor PR’s Project-row identity and retained-dialog mutation findings. The final published head passes focused tests, hidden macOS rendering and current CI; the callback-timing bot thread has an evidence-based response.

Co-authored-by: Katsuma Takehisa <k.takehisa@nissogr.com>
This commit is contained in:
Neil
2026-10-03 23:19:55 -07:00
committed by GitHub
co-authored by Katsuma Takehisa
parent 8267b578b2
commit ce07786266
58 changed files with 3045 additions and 372 deletions
+5 -1
View File
@@ -23,7 +23,11 @@ export {
type PullRequestPushTarget
} from './client/lookup/pull-request-push-target'
export { getRepoSlug, getRepoUpstream } from './client/fetch/repo-slug-upstream'
export { getWorkItem, getWorkItemByOwnerRepo } from './client/fetch/get-work-item'
export {
getWorkItem,
getWorkItemByOwnerRepo,
getWorkItemWithRepository
} from './client/fetch/get-work-item'
export { listWorkItems } from './client/list/list-work-items'
export { mergePR } from './client/merge/merge-pr'
export { removePRReviewers, requestPRReviewers } from './client/update/pr-reviewers'
+51 -18
View File
@@ -21,6 +21,30 @@ export async function getWorkItem(
localGitOptions: LocalGitExecOptions = {},
preference?: IssueSourcePreference
): Promise<MainWorkItem | null> {
const { item } = await getWorkItemWithRepository(
repoPath,
number,
type,
connectionId,
localGitOptions,
preference
)
return item
}
export type WorkItemRepositoryLookup = {
item: MainWorkItem | null
repository: GitHubApiRepository | null
}
export async function getWorkItemWithRepository(
repoPath: string,
number: number,
type?: 'issue' | 'pr',
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {},
preference?: IssueSourcePreference
): Promise<WorkItemRepositoryLookup> {
await acquire()
try {
// Why: listWorkItems uses resolveIssueGitHubApiRepositorySource; open-by-number
@@ -35,18 +59,24 @@ export async function getWorkItem(
// 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 { item: null, repository: null }
}
return {
item: await fetchIssueWorkItem(repoPath, source, number, connectionId, localGitOptions),
repository: source
}
return await fetchIssueWorkItem(repoPath, source, number, connectionId, localGitOptions)
}
if (type === 'pr') {
return await fetchPullRequestWorkItemFromCandidates(
repoPath,
number,
connectionId,
localGitOptions,
preference
)
return {
item: await fetchPullRequestWorkItemFromCandidates(
repoPath,
number,
connectionId,
localGitOptions,
preference
),
repository: null
}
}
try {
@@ -65,7 +95,7 @@ export async function getWorkItem(
localGitOptions
)
if (issue) {
return issue
return { item: issue, repository: source }
}
}
} catch (err) {
@@ -75,15 +105,18 @@ export async function getWorkItem(
throw err
}
}
return await fetchPullRequestWorkItemFromCandidates(
repoPath,
number,
connectionId,
localGitOptions,
preference
)
return {
item: await fetchPullRequestWorkItemFromCandidates(
repoPath,
number,
connectionId,
localGitOptions,
preference
),
repository: null
}
} catch {
return null
return { item: null, repository: null }
} finally {
release()
}
@@ -22,4 +22,25 @@ describe('isValidGitHubApiRepository', () => {
expect(isValidGitHubApiRepository({ owner: 'acme', repo: '..' })).toBe(false)
expect(isValidGitHubApiRepository({ owner: 'acme', repo: 'a/b' })).toBe(false)
})
it.each([42, false, null, {}])('rejects a non-string repository host: %j', (host) => {
expect(isValidGitHubApiRepository({ owner: 'acme', repo: 'orca', host })).toBe(false)
})
it.each([undefined, 'github.com', 'github.enterprise.test:8443'])(
'accepts supported legacy or explicit repository host types: %j',
(host) => {
expect(isValidGitHubApiRepository({ owner: 'acme', repo: 'orca', host })).toBe(true)
}
)
it.each([
null,
'acme/orca',
{ owner: 42, repo: 'orca' },
{ owner: 'acme', repo: 42 },
{ owner: 'acme' }
])('rejects malformed runtime repository identities: %j', (repository) => {
expect(isValidGitHubApiRepository(repository)).toBe(false)
})
})
@@ -11,8 +11,21 @@ export type GitHubApiRepositoryResolution =
const OWNER_SLUG_RE = GITHUB_OWNER_SLUG_RE
const REPOSITORY_SLUG_RE = /^[A-Za-z0-9._-]+$/
export function isValidGitHubApiRepository(repository: GitHubOwnerRepo): boolean {
export function isValidGitHubApiRepository(repository: unknown): repository is GitHubOwnerRepo {
if (
!repository ||
typeof repository !== 'object' ||
!('owner' in repository) ||
!('repo' in repository)
) {
return false
}
return (
typeof repository.owner === 'string' &&
typeof repository.repo === 'string' &&
(!('host' in repository) ||
repository.host === undefined ||
typeof repository.host === 'string') &&
OWNER_SLUG_RE.test(repository.owner) &&
REPOSITORY_SLUG_RE.test(repository.repo) &&
repository.repo !== '.' &&
+17 -15
View File
@@ -1,32 +1,34 @@
import type { GitHubCommentResult, PRComment } from '../../shared/github/comment-types'
import type { IssueSourcePreference } from '../../shared/repo-types'
import type { LocalGitExecOptions, OwnerRepo } from './gh-utils'
import { getIssueGitHubApiRepository, resolveGitHubRepoExecution } from './github-api-repository'
import {
resolveGitHubRepoExecution,
resolveIssueGitHubApiRepositorySource
} from './github-api-repository'
import { acquire, classifyGhError, ghExecFileAsync, release } from './gh-utils'
/**
* Add a comment to an existing GitHub issue.
*
* Why this path doesn't take a preference (mirrors `getIssue` / `updateIssue`):
* a comment is posted against an issue number already bound to a worktree or
* surfaced from a prior read. Routing through the live per-repo preference
* would let a user read upstream#N, toggle the selector to origin, and have
* their reply silently post on origin#N — a different issue entirely. That
* is the same silent-source-switch class of wrongness #1186 / the parent
* design doc guard against. List and create paths honor preference;
* mutations stay on the heuristic `getIssueOwnerRepo`.
*/
// An explicit target keeps replies bound to the conversation; otherwise use the selected source.
export async function addIssueComment(
repoPath: string,
issueNumber: number,
body: string,
connectionId?: string | null,
ownerRepoOverride?: OwnerRepo | null,
localGitOptions: LocalGitExecOptions = {}
localGitOptions: LocalGitExecOptions = {},
preference?: IssueSourcePreference
): Promise<GitHubCommentResult> {
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
ownerRepoOverride ??
(() => getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions)),
(async () =>
(
await resolveIssueGitHubApiRepositorySource(
repoPath,
preference,
connectionId,
localGitOptions
)
).source),
connectionId,
localGitOptions
)
+25 -21
View File
@@ -1,4 +1,4 @@
import type { GitHubAssignableUser } from '../../shared/github/pull-request-types'
import type { GitHubAssignableUser, GitHubOwnerRepo } from '../../shared/github/pull-request-types'
import type { IssueSourcePreference } from '../../shared/repo-types'
import type { LocalGitExecOptions } from './gh-utils'
import {
@@ -11,19 +11,21 @@ export async function listLabels(
repoPath: string,
preference?: IssueSourcePreference,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
localGitOptions: LocalGitExecOptions = {},
repositoryOverride?: GitHubOwnerRepo | null
): Promise<string[]> {
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
async () =>
(
await resolveIssueGitHubApiRepositorySource(
repoPath,
preference,
connectionId,
localGitOptions
)
).source,
repositoryOverride ??
(async () =>
(
await resolveIssueGitHubApiRepositorySource(
repoPath,
preference,
connectionId,
localGitOptions
)
).source),
connectionId,
localGitOptions
)
@@ -57,19 +59,21 @@ export async function listAssignableUsers(
repoPath: string,
preference?: IssueSourcePreference,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
localGitOptions: LocalGitExecOptions = {},
repositoryOverride?: GitHubOwnerRepo | null
): Promise<GitHubAssignableUser[]> {
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
async () =>
(
await resolveIssueGitHubApiRepositorySource(
repoPath,
preference,
connectionId,
localGitOptions
)
).source,
repositoryOverride ??
(async () =>
(
await resolveIssueGitHubApiRepositorySource(
repoPath,
preference,
connectionId,
localGitOptions
)
).source),
connectionId,
localGitOptions
)
@@ -0,0 +1,178 @@
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, resolveIssueSourceMock, acquireMock, releaseMock } = vi.hoisted(
() => ({
ghExecFileAsyncMock: vi.fn(),
resolveIssueSourceMock: vi.fn(),
acquireMock: vi.fn(),
releaseMock: vi.fn()
})
)
vi.mock('./gh-utils', async () => {
const actual = await vi.importActual<typeof GhUtils>('./gh-utils')
return {
...actual,
ghExecFileAsync: ghExecFileAsyncMock,
acquire: acquireMock,
release: releaseMock
}
})
vi.mock('./github-api-repository', async (importOriginal) => ({
...(await importOriginal<typeof GithubApiRepositoryModule>()),
resolveIssueGitHubApiRepositorySource: resolveIssueSourceMock
}))
import { addIssueComment } from './issue-comment'
import { updateIssue } from './issue-update'
const ORIGIN = { owner: 'fork-owner', repo: 'widgets', host: 'github.com' }
const UPSTREAM = { owner: 'upstream-owner', repo: 'widgets', host: 'github.com' }
describe('issue mutations follow the issue source preference', () => {
beforeEach(() => {
ghExecFileAsyncMock.mockReset()
acquireMock.mockReset()
acquireMock.mockResolvedValue(undefined)
releaseMock.mockReset()
resolveIssueSourceMock.mockReset()
resolveIssueSourceMock.mockImplementation(async (_repoPath: string, preference: unknown) => ({
source: preference === 'origin' ? ORIGIN : UPSTREAM,
fellBack: false
}))
})
it('posts issue comments to origin when origin is selected', async () => {
ghExecFileAsyncMock.mockResolvedValueOnce({
stdout: JSON.stringify({ id: 1, user: { login: 'octo', avatar_url: '' }, body: 'hi' })
})
await expect(
addIssueComment('/repo-root', 5, 'hi', null, null, {}, 'origin')
).resolves.toMatchObject({ ok: true })
expect(resolveIssueSourceMock).toHaveBeenCalledWith('/repo-root', 'origin', null, {})
expect(ghExecFileAsyncMock.mock.calls[0][0]).toContain(
'repos/fork-owner/widgets/issues/5/comments'
)
})
it('keeps an explicit owner/repo override ahead of the preference', async () => {
ghExecFileAsyncMock.mockResolvedValueOnce({
stdout: JSON.stringify({ id: 2, user: { login: 'octo', avatar_url: '' }, body: 'hi' })
})
await addIssueComment('/repo-root', 7, 'hi', null, UPSTREAM, {}, 'origin')
expect(resolveIssueSourceMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock.mock.calls[0][0]).toContain(
'repos/upstream-owner/widgets/issues/7/comments'
)
})
it('edits issues on origin when origin is selected', async () => {
ghExecFileAsyncMock.mockResolvedValue({ stdout: '' })
await expect(
updateIssue('/repo-root', 5, { body: 'Updated', addAssignees: ['octo'] }, null, {}, 'origin')
).resolves.toEqual({ ok: true })
expect(resolveIssueSourceMock).toHaveBeenCalledWith('/repo-root', 'origin', null, {})
expect(ghExecFileAsyncMock.mock.calls[0][0]).toContain('repos/fork-owner/widgets/issues/5')
expect(ghExecFileAsyncMock.mock.calls[1][0]).toEqual([
'issue',
'edit',
'5',
'--repo',
'fork-owner/widgets',
'--add-assignee',
'octo'
])
})
it('keeps the upstream-first default when no preference is set', async () => {
ghExecFileAsyncMock.mockResolvedValue({ stdout: '' })
await updateIssue('/repo-root', 5, { body: 'Updated' })
expect(resolveIssueSourceMock).toHaveBeenCalledWith('/repo-root', undefined, undefined, {})
expect(ghExecFileAsyncMock.mock.calls[0][0]).toContain('repos/upstream-owner/widgets/issues/5')
})
it('keeps edits on the opened issue when the saved preference changes', async () => {
ghExecFileAsyncMock.mockResolvedValue({ stdout: '' })
await updateIssue(
'/repo-root',
5,
{ body: 'Origin edit', state: 'closed', addLabels: ['bug'], addAssignees: ['octo'] },
null,
{},
'upstream',
ORIGIN
)
expect(resolveIssueSourceMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock.mock.calls.map(([args]) => args)).toEqual([
['issue', 'close', '5', '--repo', 'fork-owner/widgets'],
[
'api',
'-X',
'PATCH',
'repos/fork-owner/widgets/issues/5',
'--raw-field',
'body=Origin edit'
],
[
'issue',
'edit',
'5',
'--repo',
'fork-owner/widgets',
'--add-label',
'bug',
'--add-assignee',
'octo'
]
])
})
it('preserves local WSL execution with an explicit issue repository', async () => {
ghExecFileAsyncMock.mockResolvedValue({ stdout: '' })
await updateIssue(
'/home/fixture/widgets',
5,
{ body: 'Origin edit' },
null,
{ wslDistro: 'Ubuntu' },
'upstream',
ORIGIN
)
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
expect.arrayContaining(['repos/fork-owner/widgets/issues/5']),
expect.objectContaining({
cwd: '/home/fixture/widgets',
wslDistro: 'Ubuntu',
host: 'github.com'
})
)
})
it('refuses unresolved Origin mutations instead of using ambient gh', async () => {
resolveIssueSourceMock.mockResolvedValue({ source: null, fellBack: false })
await expect(
updateIssue('/repo-root', 5, { body: 'Origin edit' }, null, {}, 'origin')
).resolves.toMatchObject({ ok: false })
await expect(
addIssueComment('/repo-root', 5, 'Origin reply', null, null, {}, 'origin')
).resolves.toMatchObject({ ok: false })
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
})
})
+20 -16
View File
@@ -1,31 +1,35 @@
import type { GitHubIssueUpdate } from '../../shared/issue-mutation-types'
import type { IssueSourcePreference } from '../../shared/repo-types'
import type { LocalGitExecOptions } from './gh-utils'
import { getIssueGitHubApiRepository, resolveGitHubRepoExecution } from './github-api-repository'
import {
resolveGitHubRepoExecution,
resolveIssueGitHubApiRepositorySource,
type GitHubApiRepository
} from './github-api-repository'
import { acquire, classifyGhError, ghExecFileAsync, release } from './gh-utils'
/**
* Update an existing GitHub issue. Fans out to separate gh commands for
* state changes vs field edits since `gh issue edit` does not support state.
*
* Why this path doesn't take a preference (mirrors `getIssue`): mutations
* target an issue number already bound to a worktree / linked elsewhere in
* the UI. Routing an update through the live per-repo preference would let
* a user open upstream#N, toggle the selector to origin, save, and silently
* write to origin#N — a different issue (or 404). That is the exact
* silent-source-switch class of wrongness #1186 / the parent design doc
* guard against. List and create paths honor preference; mutations stay on
* the heuristic `getIssueOwnerRepo`.
*/
// Use the selected source for issue-number edits.
export async function updateIssue(
repoPath: string,
issueNumber: number,
updates: GitHubIssueUpdate,
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
localGitOptions: LocalGitExecOptions = {},
preference?: IssueSourcePreference,
ownerRepoOverride?: GitHubApiRepository | null
): Promise<{ ok: true } | { ok: false; error: string }> {
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
() => getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions),
ownerRepoOverride ??
(async () =>
(
await resolveIssueGitHubApiRepositorySource(
repoPath,
preference,
connectionId,
localGitOptions
)
).source),
connectionId,
localGitOptions
)
@@ -1,3 +1,4 @@
import type * as WorkItemDetailsTestLookup from './work-item-details-test-lookup'
import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
@@ -27,8 +28,11 @@ vi.mock('./gh-utils', () => ({
})
}))
vi.mock('./client', () => ({
vi.mock('./client', async () => ({
getWorkItem: getWorkItemMock,
getWorkItemWithRepository: (
await vi.importActual<typeof WorkItemDetailsTestLookup>('./work-item-details-test-lookup')
).makeWorkItemDetailsLookupMock(getWorkItemMock),
getPRComments: getPRCommentsMock,
getPRChecks: getPRChecksMock
}))
@@ -0,0 +1,207 @@
import { afterEach, beforeEach, expect, it, vi } from 'vitest'
import { mkdir, mkdtemp, rm } from 'node:fs/promises'
import { join } from 'node:path'
import { tmpdir } from 'node:os'
import type * as NodeOs from 'node:os'
import type * as GitExecError from '../git/exec-error'
import type * as WorkItemLookup from './client/fetch/get-work-item'
import { runProcess } from '../../shared/child-process/run-process'
const { ghMock, fixture } = vi.hoisted(() => {
const fixture: {
path: string
initialHost: string
replacementUrl: string
requests: { args: string[]; host?: string }[]
} = { path: '', initialHost: '', replacementUrl: '', requests: [] }
return { ghMock: vi.fn(), fixture }
})
vi.mock('node:os', async (original) => ({
...(await original<typeof NodeOs>()),
homedir: () => join(fixture.path, 'isolated-home')
}))
vi.mock('../git/runner', async () => {
const errors = await vi.importActual<typeof GitExecError>('../git/exec-error')
return {
...errors,
ghExecFileAsync: ghMock,
gitExecFileAsync: async (args: string[], options: { cwd?: string }) => {
const result = await runProcess({
program: 'git',
args,
cwd: options.cwd,
env: {
...process.env,
GIT_CONFIG_NOSYSTEM: '1',
GIT_CONFIG_GLOBAL: join(fixture.path, 'isolated-home', '.gitconfig')
}
})
if (result.code !== 0) {
throw new Error(result.stderr)
}
return { stdout: result.stdout, stderr: result.stderr }
}
}
})
vi.mock('./client', async () => ({
...(await vi.importActual<typeof WorkItemLookup>('./client/fetch/get-work-item')),
getPRChecks: vi.fn(),
getPRComments: vi.fn()
}))
vi.mock('./rate-limit', () => ({
repositoryRateLimitGuard: () => ({ blocked: false }),
noteRepositoryRateLimitSpend: vi.fn()
}))
import { getWorkItemDetails } from './work-item-details'
import { _resetOwnerRepoCache } from './github-repository-identity'
import { _resetOriginGitHubApiRepositoryCache } from './github-api-repository'
import { _resetGitHubHostAuthCache } from './github-enterprise-repository'
async function fixtureGit(args: string[]): Promise<void> {
const result = await runProcess({
program: 'git',
args,
cwd: fixture.path,
env: {
...process.env,
GIT_CONFIG_NOSYSTEM: '1',
GIT_CONFIG_GLOBAL: join(fixture.path, 'isolated-home', '.gitconfig')
}
})
expect(result.code, result.stderr).toBe(0)
}
beforeEach(async () => {
fixture.path = await mkdtemp(join(tmpdir(), 'orca-issue-config-change-'))
await mkdir(join(fixture.path, 'isolated-home'))
await fixtureGit(['init', '--quiet'])
await fixtureGit(['remote', 'add', 'origin', 'https://github.com/fork-owner/widgets.git'])
_resetOwnerRepoCache()
_resetOriginGitHubApiRepositoryCache()
_resetGitHubHostAuthCache()
fixture.initialHost = 'github.com'
fixture.replacementUrl = 'https://github.com/replacement-owner/widgets.git'
fixture.requests = []
ghMock.mockReset()
ghMock.mockImplementation(async (args: string[], options: { host?: string }) => {
fixture.requests.push({ args, host: options.host })
if (args[0] === 'auth' && args[1] === 'status') {
return {
stdout: ['github.com', 'github.enterprise.test']
.map((host) => `${host}\n Logged in to ${host} account fixture (keyring)`)
.join('\n'),
stderr: ''
}
}
if (args.includes('repos/fork-owner/widgets/issues/5')) {
await fixtureGit(['remote', 'set-url', 'origin', fixture.replacementUrl])
return {
stdout: JSON.stringify({
number: 5,
title: 'ORIGIN title',
state: 'open',
html_url: `https://${fixture.initialHost}/fork-owner/widgets/issues/5`,
labels: [],
updated_at: '2026-10-02T00:00:00Z',
user: { login: 'fork-author' }
}),
stderr: ''
}
}
if (args.includes('graphql')) {
const original = args.includes('owner=fork-owner') && options.host === fixture.initialHost
const marker = original ? 'ORIGIN' : 'REPLACEMENT'
return {
stdout: JSON.stringify({
data: {
repository: {
issue: {
body: `${marker} body`,
assignees: { nodes: [{ login: `${marker}-assignee` }] },
participants: { nodes: [] },
comments: {
nodes: [
{
databaseId: 1,
body: `${marker} comment`,
createdAt: '2026-10-02T00:00:00Z',
url: `https://${options.host}/fork-owner/widgets/issues/5#issuecomment-1`,
author: { login: `${marker}-author`, avatarUrl: '' }
}
]
}
}
}
}
}),
stderr: ''
}
}
if (args.some((arg) => arg.includes('/timeline?'))) {
return { stdout: '[]', stderr: '' }
}
throw new Error(`Unexpected fixture gh request: ${args.join(' ')}`)
})
})
afterEach(async () => {
await rm(fixture.path, { recursive: true, force: true })
})
it.each(['issue', undefined] as const)(
'keeps implicit %s conversation on the repository used for the item fetch',
async (type) => {
const details = await getWorkItemDetails(fixture.path, 5, type, null, {}, 'origin')
expect(details?.item.title).toBe('ORIGIN title')
expect(details?.body).toBe('ORIGIN body')
expect(details?.assignees).toEqual(['ORIGIN-assignee'])
expect(details?.comments.map((comment) => comment.body)).toEqual(['ORIGIN comment'])
expect(fixture.requests[1].args).toContain('owner=fork-owner')
}
)
it('keeps an explicit local Tasks identity through the same real config change', async () => {
const details = await getWorkItemDetails(fixture.path, 5, 'issue', null, {}, 'origin', {
owner: 'fork-owner',
repo: 'widgets',
host: 'github.com'
})
expect(details?.item.title).toBe('ORIGIN title')
expect(details?.body).toBe('ORIGIN body')
})
it.each([
{ from: 'github.com', to: 'github.enterprise.test', qualified: false },
{ from: 'github.enterprise.test', to: 'github.com', qualified: false },
{ from: 'github.com', to: 'github.enterprise.test', qualified: true },
{ from: 'github.enterprise.test', to: 'github.com', qualified: true }
])(
'keeps explicit host from $from to $to (qualified=$qualified)',
async ({ from, to, qualified }) => {
fixture.initialHost = from
fixture.replacementUrl = `https://${to}/fork-owner/widgets.git`
await fixtureGit(['remote', 'set-url', 'origin', `https://${from}/fork-owner/widgets.git`])
const details = await getWorkItemDetails(fixture.path, 5, 'issue', null, {}, 'origin', {
owner: 'fork-owner',
repo: 'widgets',
...(qualified ? { host: from } : {})
})
expect(details?.item.url).toBe(`https://${from}/fork-owner/widgets/issues/5`)
expect(details?.body).toBe('ORIGIN body')
expect(details?.assignees).toEqual(['ORIGIN-assignee'])
expect(details?.comments.map((comment) => comment.body)).toEqual(['ORIGIN comment'])
const conversationRequests = fixture.requests.filter(
({ args }) => args.includes('graphql') || args.some((arg) => arg.includes('/timeline?'))
)
expect(conversationRequests.some(({ args }) => args.includes('graphql'))).toBe(true)
expect(
conversationRequests.some(({ args }) => args.some((arg) => arg.includes('/timeline?')))
).toBe(true)
expect(conversationRequests.map(({ host }) => host)).toEqual([from, from])
}
)
@@ -1,3 +1,4 @@
import type * as WorkItemDetailsTestLookup from './work-item-details-test-lookup'
import { beforeEach, describe, expect, it, vi } from 'vitest'
type RateLimitGuardResult =
@@ -47,8 +48,11 @@ vi.mock('./gh-utils', () => ({
release: vi.fn()
}))
vi.mock('./client', () => ({
vi.mock('./client', async () => ({
getWorkItem: getWorkItemMock,
getWorkItemWithRepository: (
await vi.importActual<typeof WorkItemDetailsTestLookup>('./work-item-details-test-lookup')
).makeWorkItemDetailsLookupMock(getWorkItemMock),
getWorkItemByOwnerRepo: getWorkItemByOwnerRepoMock,
getPRChecks: getPRChecksMock,
getPRComments: getPRCommentsMock
@@ -1,3 +1,4 @@
import type * as WorkItemDetailsTestLookup from './work-item-details-test-lookup'
import { beforeEach, describe, expect, it, vi } from 'vitest'
type RateLimitGuardResult =
@@ -59,8 +60,11 @@ vi.mock('./gh-utils', () => ({
release: releaseMock
}))
vi.mock('./client', () => ({
vi.mock('./client', async () => ({
getWorkItem: getWorkItemMock,
getWorkItemWithRepository: (
await vi.importActual<typeof WorkItemDetailsTestLookup>('./work-item-details-test-lookup')
).makeWorkItemDetailsLookupMock(getWorkItemMock),
getWorkItemByOwnerRepo: vi.fn(),
getPRChecks: getPRChecksMock,
getPRComments: getPRCommentsMock
@@ -0,0 +1,239 @@
import type * as WorkItemDetailsTestLookup from './work-item-details-test-lookup'
import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
ghExecFileAsyncMock,
getOwnerRepoMock,
getOwnerRepoForRemoteMock,
getWorkItemMock,
getWorkItemByOwnerRepoMock,
acquireMock,
releaseMock
} = vi.hoisted(() => ({
ghExecFileAsyncMock: vi.fn(),
getOwnerRepoMock: vi.fn(),
getOwnerRepoForRemoteMock: vi.fn(),
getWorkItemMock: vi.fn(),
getWorkItemByOwnerRepoMock: vi.fn(),
acquireMock: vi.fn(),
releaseMock: vi.fn()
}))
vi.mock('./gh-utils', () => ({
ghExecFileAsync: ghExecFileAsyncMock,
getOwnerRepo: getOwnerRepoMock,
getOwnerRepoForRemote: getOwnerRepoForRemoteMock,
ghRepoExecOptions: vi.fn((context) => ({ cwd: context.repoPath })),
githubRepoContext: vi.fn((repoPath, connectionId, localGitOptions) => ({
repoPath,
connectionId: connectionId ?? null,
...localGitOptions
})),
acquire: acquireMock,
release: releaseMock
}))
vi.mock('./client', async () => ({
getWorkItem: getWorkItemMock,
getWorkItemWithRepository: (
await vi.importActual<typeof WorkItemDetailsTestLookup>('./work-item-details-test-lookup')
).makeWorkItemDetailsLookupMock(getWorkItemMock),
getWorkItemByOwnerRepo: getWorkItemByOwnerRepoMock,
getPRChecks: vi.fn(),
getPRComments: vi.fn()
}))
vi.mock('./github-enterprise-repository', () => ({
getEnterpriseGitHubRepoSlug: vi.fn().mockResolvedValue(null),
getEnterpriseGitHubRepoSlugForRemote: vi.fn().mockResolvedValue(null),
isGitHubHostAuthenticated: vi.fn().mockResolvedValue(true)
}))
vi.mock('./rate-limit', () => ({
repositoryRateLimitGuard: vi.fn(() => ({ blocked: false })),
noteRepositoryRateLimitSpend: vi.fn()
}))
vi.mock('../git/remote-name-listing', () => ({
shouldProbeGitRemote: vi.fn().mockResolvedValue(true)
}))
import { getWorkItemDetails } from './work-item-details'
import { _resetOriginGitHubApiRepositoryCache } from './github-api-repository'
const REMOTES: Record<string, { owner: string; repo: string }> = {
origin: { owner: 'fork-owner', repo: 'widgets' },
upstream: { owner: 'upstream-owner', repo: 'widgets' }
}
function graphQLIssueResponse(assignee: string): { stdout: string } {
return {
stdout: JSON.stringify({
data: {
repository: {
issue: {
body: `${assignee} body`,
assignees: { nodes: [{ login: assignee }] },
participants: { nodes: [] },
comments: { nodes: [] }
}
}
}
})
}
}
function graphQLOwnerArg(): string | undefined {
const args: string[] = ghExecFileAsyncMock.mock.calls[0][0]
return args.find((arg) => arg.startsWith('owner='))
}
describe('getWorkItemDetails issue source', () => {
beforeEach(() => {
_resetOriginGitHubApiRepositoryCache()
ghExecFileAsyncMock.mockReset()
getOwnerRepoMock.mockReset()
getOwnerRepoMock.mockResolvedValue(REMOTES.origin)
getOwnerRepoForRemoteMock.mockReset()
getOwnerRepoForRemoteMock.mockImplementation(
async (_repoPath: string, remoteName: string) => REMOTES[remoteName] ?? null
)
getWorkItemMock.mockReset()
getWorkItemByOwnerRepoMock.mockReset()
getWorkItemMock.mockResolvedValue({
id: 'issue:5',
type: 'issue',
number: 5,
title: 'Origin issue',
state: 'open',
url: 'https://github.com/fork-owner/widgets/issues/5',
labels: [],
updatedAt: '2026-04-01T00:00:00Z',
author: 'fork-author'
})
acquireMock.mockReset()
acquireMock.mockResolvedValue(undefined)
releaseMock.mockReset()
})
it('reads body, assignees and comments from origin when origin is selected', async () => {
ghExecFileAsyncMock
.mockResolvedValueOnce(graphQLIssueResponse('fork-assignee'))
.mockResolvedValueOnce({ stdout: '' })
const details = await getWorkItemDetails('/repo-root', 5, 'issue', null, {}, 'origin')
expect(getWorkItemMock).toHaveBeenCalledWith('/repo-root', 5, 'issue', null, {}, 'origin')
expect(graphQLOwnerArg()).toBe('owner=fork-owner')
expect(ghExecFileAsyncMock.mock.calls[1][0]).toContain(
'repos/fork-owner/widgets/issues/5/timeline?per_page=100&page=1'
)
expect(details?.assignees).toEqual(['fork-assignee'])
})
it('keeps the upstream-first default when no preference is set', async () => {
ghExecFileAsyncMock
.mockResolvedValueOnce(graphQLIssueResponse('upstream-assignee'))
.mockResolvedValueOnce({ stdout: '' })
const details = await getWorkItemDetails('/repo-root', 5, 'issue', null, {})
expect(graphQLOwnerArg()).toBe('owner=upstream-owner')
expect(details?.assignees).toEqual(['upstream-assignee'])
})
it('keeps a Project row on upstream even when Origin is selected', async () => {
const upstream = { ...REMOTES.upstream, host: 'github.com' }
getWorkItemByOwnerRepoMock.mockResolvedValue({
id: 'issue:5',
type: 'issue',
number: 5,
title: 'Upstream project issue',
url: 'https://github.com/upstream-owner/widgets/issues/5',
state: 'open',
labels: [],
updatedAt: '',
author: 'upstream-author'
})
ghExecFileAsyncMock
.mockResolvedValueOnce(graphQLIssueResponse('upstream-assignee'))
.mockResolvedValueOnce({ stdout: '' })
const details = await getWorkItemDetails('/repo-root', 5, 'issue', null, {}, 'origin', upstream)
expect(getWorkItemMock).not.toHaveBeenCalled()
expect(getWorkItemByOwnerRepoMock).toHaveBeenCalledWith(
'/repo-root',
upstream,
5,
'issue',
null,
{}
)
expect(graphQLOwnerArg()).toBe('owner=upstream-owner')
expect(details?.item.title).toBe('Upstream project issue')
expect(details?.assignees).toEqual(['upstream-assignee'])
})
it('does not fall back when the selected issue repository is unavailable', async () => {
getWorkItemByOwnerRepoMock.mockResolvedValue(null)
await expect(
getWorkItemDetails('/repo-root', 5, 'issue', null, {}, 'upstream', {
...REMOTES.origin,
host: 'github.com'
})
).resolves.toBeNull()
expect(getWorkItemMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
})
it('rejects an invalid runtime issue host before calling the repository lookup', async () => {
await expect(
getWorkItemDetails('/repo-root', 5, 'issue', null, {}, 'upstream', {
...REMOTES.origin,
host: 42
})
).resolves.toBeNull()
expect(getWorkItemByOwnerRepoMock).not.toHaveBeenCalled()
expect(getWorkItemMock).not.toHaveBeenCalled()
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
})
it('ignores an issue repository override on an untyped lookup', async () => {
ghExecFileAsyncMock
.mockResolvedValueOnce(graphQLIssueResponse('fork-assignee'))
.mockResolvedValueOnce({ stdout: '' })
await getWorkItemDetails('/repo-root', 5, undefined, null, {}, 'origin', {
owner: 'untrusted',
repo: 'other',
host: 'untrusted.example'
})
expect(getWorkItemByOwnerRepoMock).not.toHaveBeenCalled()
expect(graphQLOwnerArg()).toBe('owner=fork-owner')
expect(ghExecFileAsyncMock.mock.calls[0][1]).toMatchObject({ host: 'github.com' })
})
it('normalizes host-less issue targets before loading the conversation', async () => {
getWorkItemByOwnerRepoMock.mockResolvedValue({
type: 'issue',
number: 5,
title: 'Origin issue',
url: 'https://github.com/fork-owner/widgets/issues/5',
author: null
})
ghExecFileAsyncMock
.mockResolvedValueOnce(graphQLIssueResponse('fork-assignee'))
.mockResolvedValueOnce({ stdout: '' })
await getWorkItemDetails('/repo-root', 5, 'issue', null, {}, 'upstream', REMOTES.origin)
expect(graphQLOwnerArg()).toBe('owner=fork-owner')
expect(ghExecFileAsyncMock.mock.calls[0][1]).toMatchObject({ host: 'github.com' })
expect(ghExecFileAsyncMock.mock.calls[1][1]).toMatchObject({ host: 'github.com' })
})
})
@@ -1,3 +1,4 @@
import type * as WorkItemDetailsTestLookup from './work-item-details-test-lookup'
import { beforeEach, describe, expect, it, vi } from 'vitest'
type RateLimitGuardResult =
@@ -52,8 +53,11 @@ vi.mock('./gh-utils', () => ({
release: vi.fn()
}))
vi.mock('./client', () => ({
vi.mock('./client', async () => ({
getWorkItem: getWorkItemMock,
getWorkItemWithRepository: (
await vi.importActual<typeof WorkItemDetailsTestLookup>('./work-item-details-test-lookup')
).makeWorkItemDetailsLookupMock(getWorkItemMock),
getWorkItemByOwnerRepo: vi.fn(),
getPRChecks: getPRChecksMock,
getPRComments: getPRCommentsMock
@@ -0,0 +1,17 @@
import { resolveIssueGitHubApiRepositorySource } from './github-api-repository'
import type { getWorkItem } from './client'
import type { WorkItemRepositoryLookup } from './client/fetch/get-work-item'
// Why: detail tests replace item lookup; retain its selected-source metadata.
export function makeWorkItemDetailsLookupMock(lookup: typeof getWorkItem) {
return async (...args: Parameters<typeof getWorkItem>): Promise<WorkItemRepositoryLookup> => {
const item = await lookup(...args)
return {
item,
repository:
item?.type === 'issue'
? (await resolveIssueGitHubApiRepositorySource(args[0], args[5], args[3], args[4])).source
: null
}
}
}
+5 -1
View File
@@ -1,3 +1,4 @@
import type * as WorkItemDetailsTestLookup from './work-item-details-test-lookup'
import { beforeEach, describe, expect, it, vi } from 'vitest'
type RateLimitGuardResult =
@@ -62,8 +63,11 @@ vi.mock('./gh-utils', () => ({
release: releaseMock
}))
vi.mock('./client', () => ({
vi.mock('./client', async () => ({
getWorkItem: getWorkItemMock,
getWorkItemWithRepository: (
await vi.importActual<typeof WorkItemDetailsTestLookup>('./work-item-details-test-lookup')
).makeWorkItemDetailsLookupMock(getWorkItemMock),
getWorkItemByOwnerRepo: getWorkItemByOwnerRepoMock,
getPRChecks: getPRChecksMock,
getPRComments: getPRCommentsMock
+45 -17
View File
@@ -1,14 +1,16 @@
import type { PRCheckDetail } from '../../shared/github/check-types'
import type { GitHubPRFile, GitHubPRFileContents } from '../../shared/github/pull-request-types'
import type { GitHubWorkItem, GitHubWorkItemDetails } from '../../shared/github/work-item-types'
import type { GitHubWorkItemDetails } from '../../shared/github/work-item-types'
import type { IssueSourcePreference } from '../../shared/repo-types'
import { getPRChecks, getPRComments, getWorkItem } from './client'
import { acquire, release, type LocalGitExecOptions } from './gh-utils'
import {
getIssueGitHubApiRepository,
resolveGitHubRepoExecution,
type GitHubApiRepository
} from './github-api-repository'
getPRChecks,
getPRComments,
getWorkItemWithRepository,
getWorkItemByOwnerRepo
} from './client'
import { acquire, release, type LocalGitExecOptions } from './gh-utils'
import { resolveGitHubRepoExecution, type GitHubApiRepository } from './github-api-repository'
import { isValidGitHubApiRepository } from './github-api-repository-validation'
import { getIssueBodyAndComments, getIssueDetailsViaGraphQL } from './issue-work-item-details'
import {
getPRFiles,
@@ -69,23 +71,49 @@ export async function getWorkItemDetails(
type?: 'issue' | 'pr',
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {},
preference?: IssueSourcePreference
preference?: IssueSourcePreference,
repositoryOverride?: unknown
): Promise<GitHubWorkItemDetails | null> {
const item: Omit<GitHubWorkItem, 'repoId'> | null = await getWorkItem(
repoPath,
number,
type,
connectionId,
localGitOptions,
preference
)
let issueRepositoryOverride: GitHubApiRepository | null = null
if (type === 'issue' && repositoryOverride != null) {
if (!isValidGitHubApiRepository(repositoryOverride)) {
return null
}
issueRepositoryOverride = (
await resolveGitHubRepoExecution(repoPath, repositoryOverride, connectionId, localGitOptions)
).ownerRepo
if (!issueRepositoryOverride) {
return null
}
}
const { item, repository: selectedRepository } = issueRepositoryOverride
? {
item: await getWorkItemByOwnerRepo(
repoPath,
issueRepositoryOverride,
number,
'issue',
connectionId,
localGitOptions
),
repository: issueRepositoryOverride
}
: await getWorkItemWithRepository(
repoPath,
number,
type,
connectionId,
localGitOptions,
preference
)
if (!item) {
return null
}
// Keep the conversation in the same repository as the selected issue.
const resolvedRepository =
item.type === 'issue'
? await getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions)
? selectedRepository
: (await resolveGitHubRepoExecution(repoPath, item.prRepo, connectionId, localGitOptions))
.ownerRepo