Improve git remote selection (#2207)
This commit is contained in:
parent
29b8e36c39
commit
c7ddfd17d1
|
|
@ -174,4 +174,27 @@ describe('getPRChecks', () => {
|
|||
{ cwd: '/repo-root', env: { ...process.env, GH_PROMPT_DISABLED: '1' } }
|
||||
)
|
||||
})
|
||||
|
||||
it('uses explicit PR repo for check-runs and gh pr checks fallback', async () => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'widgets' })
|
||||
ghExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('gh: No commit found for SHA: stale-head (HTTP 422)'))
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([{ name: 'lint', state: 'PASS', link: 'https://example.com/lint' }])
|
||||
})
|
||||
|
||||
await getPRChecks('/repo-root', 42, 'stale-head', { owner: 'acme', repo: 'widgets' })
|
||||
|
||||
expect(getOwnerRepoMock).not.toHaveBeenCalled()
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
['api', '--cache', '60s', 'repos/acme/widgets/commits/stale-head/check-runs?per_page=100'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
['pr', 'checks', '42', '--json', 'name,state,link', '--repo', 'acme/widgets'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ const {
|
|||
getOwnerRepoMock,
|
||||
getIssueOwnerRepoMock,
|
||||
getOwnerRepoForRemoteMock,
|
||||
resolvePRRepositoryCandidatesMock,
|
||||
getRemoteUrlForRepoMock,
|
||||
gitExecFileAsyncMock,
|
||||
rateLimitGuardMock,
|
||||
|
|
@ -25,6 +26,7 @@ const {
|
|||
getOwnerRepoMock: vi.fn(),
|
||||
getIssueOwnerRepoMock: vi.fn(),
|
||||
getOwnerRepoForRemoteMock: vi.fn(),
|
||||
resolvePRRepositoryCandidatesMock: vi.fn(),
|
||||
getRemoteUrlForRepoMock: vi.fn(),
|
||||
gitExecFileAsyncMock: vi.fn(),
|
||||
rateLimitGuardMock: vi.fn<() => RateLimitGuardResult>(() => ({ blocked: false })),
|
||||
|
|
@ -46,6 +48,7 @@ vi.mock('./gh-utils', () => ({
|
|||
getOwnerRepo: getOwnerRepoMock,
|
||||
getIssueOwnerRepo: getIssueOwnerRepoMock,
|
||||
getOwnerRepoForRemote: getOwnerRepoForRemoteMock,
|
||||
resolvePRRepositoryCandidates: resolvePRRepositoryCandidatesMock,
|
||||
getRemoteUrlForRepo: getRemoteUrlForRepoMock,
|
||||
gitExecFileAsync: gitExecFileAsyncMock,
|
||||
ghRepoExecOptions: ghRepoExecOptionsMock,
|
||||
|
|
@ -82,7 +85,9 @@ import {
|
|||
getPRComments,
|
||||
getPRForBranch,
|
||||
getPullRequestPushTarget,
|
||||
mergePR,
|
||||
resolveReviewThread,
|
||||
updatePRTitle,
|
||||
_resetOwnerRepoCache
|
||||
} from './client'
|
||||
|
||||
|
|
@ -93,6 +98,11 @@ describe('getPRForBranch', () => {
|
|||
getOwnerRepoMock.mockReset()
|
||||
getIssueOwnerRepoMock.mockReset()
|
||||
getOwnerRepoForRemoteMock.mockReset()
|
||||
resolvePRRepositoryCandidatesMock.mockReset()
|
||||
resolvePRRepositoryCandidatesMock.mockImplementation(async (repoPath, connectionId) => {
|
||||
const origin = await getOwnerRepoMock(repoPath, connectionId)
|
||||
return { candidates: origin ? [origin] : [], headRepo: origin }
|
||||
})
|
||||
getRemoteUrlForRepoMock.mockReset()
|
||||
gitExecFileAsyncMock.mockReset()
|
||||
rateLimitGuardMock.mockReset()
|
||||
|
|
@ -113,16 +123,13 @@ describe('getPRForBranch', () => {
|
|||
{
|
||||
number: 42,
|
||||
title: 'Fix PR discovery',
|
||||
state: 'OPEN',
|
||||
url: 'https://github.com/acme/widgets/pull/42',
|
||||
statusCheckRollup: [],
|
||||
updatedAt: '2026-03-28T00:00:00Z',
|
||||
isDraft: false,
|
||||
mergeable: 'MERGEABLE',
|
||||
baseRefName: 'main',
|
||||
headRefName: 'feature/test',
|
||||
baseRefOid: 'base-oid',
|
||||
headRefOid: 'head-oid'
|
||||
state: 'open',
|
||||
html_url: 'https://github.com/acme/widgets/pull/42',
|
||||
updated_at: '2026-03-28T00:00:00Z',
|
||||
draft: false,
|
||||
mergeable: true,
|
||||
base: { ref: 'main', sha: 'base-oid' },
|
||||
head: { ref: 'feature/test', sha: 'head-oid' }
|
||||
}
|
||||
])
|
||||
})
|
||||
|
|
@ -131,25 +138,110 @@ describe('getPRForBranch', () => {
|
|||
|
||||
expect(getOwnerRepoMock).toHaveBeenCalledWith('/repo-root', undefined)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
[
|
||||
'pr',
|
||||
'list',
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--head',
|
||||
'feature/test',
|
||||
'--state',
|
||||
'all',
|
||||
'--limit',
|
||||
'1',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
],
|
||||
['api', 'repos/acme/widgets/pulls?head=acme%3Afeature%2Ftest&state=all&per_page=1'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(pr?.number).toBe(42)
|
||||
expect(pr?.state).toBe('open')
|
||||
expect(pr?.mergeable).toBe('MERGEABLE')
|
||||
expect(pr?.prRepo).toEqual({ owner: 'acme', repo: 'widgets' })
|
||||
expect(pr?.headRepo).toEqual({ owner: 'acme', repo: 'widgets' })
|
||||
})
|
||||
|
||||
it('resolves fork PRs from the upstream PR repo with the origin head owner', async () => {
|
||||
resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({
|
||||
candidates: [
|
||||
{ owner: 'stablyai', repo: 'orca' },
|
||||
{ owner: 'fork', repo: 'orca' }
|
||||
],
|
||||
headRepo: { owner: 'fork', repo: 'orca' }
|
||||
})
|
||||
ghExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
number: 1738,
|
||||
title: 'Fork PR',
|
||||
state: 'open',
|
||||
html_url: 'https://github.com/stablyai/orca/pull/1738',
|
||||
updated_at: '2026-03-28T00:00:00Z',
|
||||
draft: false,
|
||||
mergeable_state: 'clean',
|
||||
base: { ref: 'main', sha: 'base-oid' },
|
||||
head: { ref: 'feature/test', sha: 'head-oid' }
|
||||
}
|
||||
])
|
||||
})
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'feature/test')
|
||||
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
['api', 'repos/stablyai/orca/pulls?head=fork%3Afeature%2Ftest&state=all&per_page=1'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(pr).toMatchObject({
|
||||
number: 1738,
|
||||
prRepo: { owner: 'stablyai', repo: 'orca' },
|
||||
headRepo: { owner: 'fork', repo: 'orca' }
|
||||
})
|
||||
})
|
||||
|
||||
it('looks up a linked PR number across PR repo candidates', async () => {
|
||||
resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({
|
||||
candidates: [
|
||||
{ owner: 'stablyai', repo: 'orca' },
|
||||
{ owner: 'fork', repo: 'orca' }
|
||||
],
|
||||
headRepo: { owner: 'fork', repo: 'orca' }
|
||||
})
|
||||
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'linked-head-oid\n', stderr: '' })
|
||||
ghExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('HTTP 404: Not Found'))
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
number: 99,
|
||||
title: 'Linked fork PR',
|
||||
state: 'OPEN',
|
||||
url: 'https://github.com/fork/orca/pull/99',
|
||||
statusCheckRollup: [],
|
||||
updatedAt: '2026-03-28T00:00:00Z',
|
||||
isDraft: false,
|
||||
mergeable: 'MERGEABLE',
|
||||
baseRefName: 'main',
|
||||
headRefName: 'feature/test',
|
||||
baseRefOid: 'base-oid',
|
||||
headRefOid: 'linked-head-oid'
|
||||
})
|
||||
})
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'feature/test', 99)
|
||||
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
[
|
||||
'pr',
|
||||
'view',
|
||||
'99',
|
||||
'--repo',
|
||||
'stablyai/orca',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
[
|
||||
'pr',
|
||||
'view',
|
||||
'99',
|
||||
'--repo',
|
||||
'fork/orca',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(pr?.prRepo).toEqual({ owner: 'fork', repo: 'orca' })
|
||||
})
|
||||
|
||||
it('prefers exact linked PR lookup when the repo identity is known', async () => {
|
||||
|
|
@ -242,20 +334,7 @@ describe('getPRForBranch', () => {
|
|||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
[
|
||||
'pr',
|
||||
'list',
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--head',
|
||||
'feature/test',
|
||||
'--state',
|
||||
'all',
|
||||
'--limit',
|
||||
'1',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
],
|
||||
['api', 'repos/acme/widgets/pulls?head=acme%3Afeature%2Ftest&state=all&per_page=1'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(pr?.number).toBe(42)
|
||||
|
|
@ -301,20 +380,7 @@ describe('getPRForBranch', () => {
|
|||
)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
[
|
||||
'pr',
|
||||
'list',
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--head',
|
||||
'feature/test',
|
||||
'--state',
|
||||
'all',
|
||||
'--limit',
|
||||
'1',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
],
|
||||
['api', 'repos/acme/widgets/pulls?head=acme%3Afeature%2Ftest&state=all&per_page=1'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(pr?.number).toBe(42)
|
||||
|
|
@ -351,20 +417,7 @@ describe('getPRForBranch', () => {
|
|||
})
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
3,
|
||||
[
|
||||
'pr',
|
||||
'list',
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--head',
|
||||
'feature/test',
|
||||
'--state',
|
||||
'all',
|
||||
'--limit',
|
||||
'1',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
],
|
||||
['api', 'repos/acme/widgets/pulls?head=acme%3Afeature%2Ftest&state=all&per_page=1'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(pr?.number).toBe(42)
|
||||
|
|
@ -401,26 +454,13 @@ describe('getPRForBranch', () => {
|
|||
})
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
3,
|
||||
[
|
||||
'pr',
|
||||
'list',
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--head',
|
||||
'feature/test',
|
||||
'--state',
|
||||
'all',
|
||||
'--limit',
|
||||
'1',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
],
|
||||
['api', 'repos/acme/widgets/pulls?head=acme%3Afeature%2Ftest&state=all&per_page=1'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(pr?.number).toBe(42)
|
||||
})
|
||||
|
||||
it('does not spend branch discovery calls when exact linked PR REST fallback is rate limited', async () => {
|
||||
it('continues to branch discovery when exact linked PR REST fallback is rate limited', async () => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
|
||||
ghExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('GraphQL: API rate limit already exceeded'))
|
||||
|
|
@ -444,34 +484,37 @@ describe('getPRForBranch', () => {
|
|||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(2, ['api', 'repos/acme/widgets/pulls/99'], {
|
||||
cwd: '/repo-root'
|
||||
})
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
3,
|
||||
['api', 'repos/acme/widgets/pulls?head=acme%3Afeature%2Ftest&state=all&per_page=1'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(3)
|
||||
expect(pr).toBeNull()
|
||||
})
|
||||
|
||||
it('falls back to REST branch lookup when gh pr list is GraphQL rate limited', async () => {
|
||||
it('uses REST branch lookup directly when origin head repo is known', async () => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
|
||||
ghExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('GraphQL: API rate limit already exceeded'))
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
number: 43,
|
||||
title: 'REST branch lookup',
|
||||
state: 'open',
|
||||
html_url: 'https://github.com/acme/widgets/pull/43',
|
||||
updated_at: '2026-03-28T00:00:00Z',
|
||||
draft: false,
|
||||
mergeable: true,
|
||||
head: { ref: 'feature/test', sha: 'rest-head-oid' },
|
||||
base: { ref: 'main', sha: 'rest-base-oid' }
|
||||
}
|
||||
])
|
||||
})
|
||||
ghExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
number: 43,
|
||||
title: 'REST branch lookup',
|
||||
state: 'open',
|
||||
html_url: 'https://github.com/acme/widgets/pull/43',
|
||||
updated_at: '2026-03-28T00:00:00Z',
|
||||
draft: false,
|
||||
mergeable: true,
|
||||
head: { ref: 'feature/test', sha: 'rest-head-oid' },
|
||||
base: { ref: 'main', sha: 'rest-base-oid' }
|
||||
}
|
||||
])
|
||||
})
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'feature/test')
|
||||
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
1,
|
||||
['api', 'repos/acme/widgets/pulls?head=acme%3Afeature%2Ftest&state=all&per_page=1'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
|
|
@ -529,16 +572,13 @@ describe('getPRForBranch', () => {
|
|||
{
|
||||
number: 42,
|
||||
title: 'Fix PR discovery',
|
||||
state: 'OPEN',
|
||||
url: 'https://github.com/acme/widgets/pull/42',
|
||||
statusCheckRollup: [],
|
||||
updatedAt: '2026-03-28T00:00:00Z',
|
||||
isDraft: false,
|
||||
mergeable: 'CONFLICTING',
|
||||
baseRefName: 'main',
|
||||
headRefName: 'feature/test',
|
||||
baseRefOid: 'base-oid',
|
||||
headRefOid: 'head-oid'
|
||||
state: 'open',
|
||||
html_url: 'https://github.com/acme/widgets/pull/42',
|
||||
updated_at: '2026-03-28T00:00:00Z',
|
||||
draft: false,
|
||||
mergeable_state: 'dirty',
|
||||
base: { ref: 'main', sha: 'base-oid' },
|
||||
head: { ref: 'feature/test', sha: 'head-oid' }
|
||||
}
|
||||
])
|
||||
})
|
||||
|
|
@ -566,16 +606,13 @@ describe('getPRForBranch', () => {
|
|||
{
|
||||
number: 42,
|
||||
title: 'Fix PR discovery',
|
||||
state: 'OPEN',
|
||||
url: 'https://github.com/acme/widgets/pull/42',
|
||||
statusCheckRollup: [],
|
||||
updatedAt: '2026-03-28T00:00:00Z',
|
||||
isDraft: false,
|
||||
mergeable: 'CONFLICTING',
|
||||
baseRefName: 'main',
|
||||
headRefName: 'feature/test',
|
||||
baseRefOid: 'base-oid',
|
||||
headRefOid: 'head-oid'
|
||||
state: 'open',
|
||||
html_url: 'https://github.com/acme/widgets/pull/42',
|
||||
updated_at: '2026-03-28T00:00:00Z',
|
||||
draft: false,
|
||||
mergeable_state: 'dirty',
|
||||
base: { ref: 'main', sha: 'base-oid' },
|
||||
head: { ref: 'feature/test', sha: 'head-oid' }
|
||||
}
|
||||
])
|
||||
})
|
||||
|
|
@ -600,16 +637,13 @@ describe('getPRForBranch', () => {
|
|||
{
|
||||
number: 42,
|
||||
title: 'Fix PR discovery',
|
||||
state: 'OPEN',
|
||||
url: 'https://github.com/acme/widgets/pull/42',
|
||||
statusCheckRollup: [],
|
||||
updatedAt: '2026-03-28T00:00:00Z',
|
||||
isDraft: false,
|
||||
mergeable: 'CONFLICTING',
|
||||
baseRefName: 'main',
|
||||
headRefName: 'feature/test',
|
||||
baseRefOid: 'base-oid',
|
||||
headRefOid: 'head-oid'
|
||||
state: 'open',
|
||||
html_url: 'https://github.com/acme/widgets/pull/42',
|
||||
updated_at: '2026-03-28T00:00:00Z',
|
||||
draft: false,
|
||||
mergeable_state: 'dirty',
|
||||
base: { ref: 'main', sha: 'base-oid' },
|
||||
head: { ref: 'feature/test', sha: 'head-oid' }
|
||||
}
|
||||
])
|
||||
})
|
||||
|
|
@ -768,6 +802,11 @@ describe('GitHub GraphQL rate-limit guard', () => {
|
|||
getOwnerRepoMock.mockReset()
|
||||
getIssueOwnerRepoMock.mockReset()
|
||||
getOwnerRepoForRemoteMock.mockReset()
|
||||
resolvePRRepositoryCandidatesMock.mockReset()
|
||||
resolvePRRepositoryCandidatesMock.mockImplementation(async (repoPath, connectionId) => {
|
||||
const origin = await getOwnerRepoMock(repoPath, connectionId)
|
||||
return { candidates: origin ? [origin] : [], headRepo: origin }
|
||||
})
|
||||
getRemoteUrlForRepoMock.mockReset()
|
||||
gitExecFileAsyncMock.mockReset()
|
||||
rateLimitGuardMock.mockReset()
|
||||
|
|
@ -810,6 +849,68 @@ describe('GitHub GraphQL rate-limit guard', () => {
|
|||
expect(noteRateLimitSpendMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('uses explicit PR repo for comments when a fork PR is discovered', async () => {
|
||||
rateLimitGuardMock.mockReturnValue({
|
||||
blocked: true,
|
||||
remaining: 4,
|
||||
limit: 5000,
|
||||
resetAt: 1_800_000_000
|
||||
})
|
||||
ghExecFileAsyncMock
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
id: 10,
|
||||
user: { login: 'octo', avatar_url: 'https://avatar', type: 'User' },
|
||||
body: 'top-level',
|
||||
created_at: '2026-04-01T00:00:00Z',
|
||||
html_url: 'https://github.com/stablyai/orca/pull/7#issuecomment-10'
|
||||
}
|
||||
])
|
||||
})
|
||||
.mockResolvedValueOnce({ stdout: '[]' })
|
||||
|
||||
await getPRComments('/repo-root', 7, { prRepo: { owner: 'stablyai', repo: 'orca' } }, undefined)
|
||||
|
||||
expect(getOwnerRepoMock).not.toHaveBeenCalled()
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
['api', '--cache', '60s', 'repos/stablyai/orca/issues/7/comments?per_page=100'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
['api', '--cache', '60s', 'repos/stablyai/orca/pulls/7/reviews?per_page=100'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
})
|
||||
|
||||
it('uses explicit PR repo for merge and title mutations', async () => {
|
||||
ghExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' })
|
||||
|
||||
await expect(
|
||||
mergePR('/repo-root', 7, 'squash', undefined, { owner: 'stablyai', repo: 'orca' })
|
||||
).resolves.toEqual({ ok: true })
|
||||
await expect(
|
||||
updatePRTitle('/repo-root', 7, 'New title', undefined, { owner: 'stablyai', repo: 'orca' })
|
||||
).resolves.toBe(true)
|
||||
|
||||
expect(getOwnerRepoMock).not.toHaveBeenCalled()
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
['pr', 'merge', '7', '--squash', '--repo', 'stablyai/orca'],
|
||||
expect.objectContaining({
|
||||
cwd: '/repo-root',
|
||||
env: expect.objectContaining({ GH_PROMPT_DISABLED: '1' })
|
||||
})
|
||||
)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
['pr', 'edit', '7', '--title', 'New title', '--repo', 'stablyai/orca'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
})
|
||||
|
||||
it('blocks review-thread resolve mutations before spawning gh when GraphQL is low', async () => {
|
||||
rateLimitGuardMock.mockReturnValue({
|
||||
blocked: true,
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ import {
|
|||
getOwnerRepo,
|
||||
getIssueOwnerRepo,
|
||||
getOwnerRepoForRemote,
|
||||
resolvePRRepositoryCandidates,
|
||||
resolveIssueSource,
|
||||
classifyGhError,
|
||||
classifyListIssuesError,
|
||||
|
|
@ -1451,13 +1452,14 @@ function mapRestPullRequest(pr: RestPullRequest): PullRequestLookupData {
|
|||
}
|
||||
|
||||
async function getRestPRForBranch(
|
||||
ownerRepo: OwnerRepo,
|
||||
prRepo: OwnerRepo,
|
||||
headOwner: string,
|
||||
branchName: string,
|
||||
ghOptions: ReturnType<typeof ghRepoExecOptions>
|
||||
): Promise<PullRequestLookupData | null> {
|
||||
const head = encodeURIComponent(`${ownerRepo.owner}:${branchName}`)
|
||||
const head = encodeURIComponent(`${headOwner}:${branchName}`)
|
||||
const { stdout } = await ghExecFileAsync(
|
||||
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls?head=${head}&state=all&per_page=1`],
|
||||
['api', `repos/${prRepo.owner}/${prRepo.repo}/pulls?head=${head}&state=all&per_page=1`],
|
||||
ghOptions
|
||||
)
|
||||
const list = JSON.parse(stdout) as RestPullRequest[]
|
||||
|
|
@ -1465,6 +1467,32 @@ async function getRestPRForBranch(
|
|||
return pr ? mapRestPullRequest(pr) : null
|
||||
}
|
||||
|
||||
async function getFallbackPRListForBranch(
|
||||
prRepo: OwnerRepo,
|
||||
branchName: string,
|
||||
ghOptions: ReturnType<typeof ghRepoExecOptions>
|
||||
): Promise<PullRequestLookupData | null> {
|
||||
const { stdout } = await ghExecFileAsync(
|
||||
[
|
||||
'pr',
|
||||
'list',
|
||||
'--repo',
|
||||
`${prRepo.owner}/${prRepo.repo}`,
|
||||
'--head',
|
||||
branchName,
|
||||
'--state',
|
||||
'all',
|
||||
'--limit',
|
||||
'1',
|
||||
'--json',
|
||||
PR_LOOKUP_JSON_FIELDS
|
||||
],
|
||||
ghOptions
|
||||
)
|
||||
const list = JSON.parse(stdout) as PullRequestLookupData[]
|
||||
return list[0] ?? null
|
||||
}
|
||||
|
||||
async function getRestPRByNumber(
|
||||
ownerRepo: OwnerRepo,
|
||||
number: number,
|
||||
|
|
@ -1522,18 +1550,15 @@ async function exactPRMatchesWorktreeHead(
|
|||
data: PullRequestLookupData,
|
||||
connectionId?: string | null
|
||||
): Promise<boolean> {
|
||||
if (!branchName || data.headRefName === branchName) {
|
||||
return true
|
||||
}
|
||||
if (connectionId || !data.headRefOid) {
|
||||
return false
|
||||
}
|
||||
try {
|
||||
const { stdout } = await gitExecFileAsync(['rev-parse', 'HEAD'], { cwd: repoPath })
|
||||
return stdout.trim() === data.headRefOid
|
||||
} catch {
|
||||
return false
|
||||
if (!connectionId && data.headRefOid) {
|
||||
try {
|
||||
const { stdout } = await gitExecFileAsync(['rev-parse', 'HEAD'], { cwd: repoPath })
|
||||
return stdout.trim() === data.headRefOid
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return !branchName || data.headRefName === branchName
|
||||
}
|
||||
|
||||
function isNotFoundGhError(err: unknown): boolean {
|
||||
|
|
@ -1564,22 +1589,39 @@ export async function getPRForBranch(
|
|||
): Promise<PRInfo | null> {
|
||||
// Strip refs/heads/ prefix if present
|
||||
const branchName = branch.replace(/^refs\/heads\//, '')
|
||||
if (!branchName && typeof linkedPRNumber !== 'number') {
|
||||
return null
|
||||
}
|
||||
const context = githubRepoContext(repoPath, connectionId)
|
||||
const ghOptions = ghRepoExecOptions(context)
|
||||
|
||||
await acquire()
|
||||
try {
|
||||
const ownerRepo = await getOwnerRepo(repoPath, connectionId)
|
||||
const { candidates, headRepo } = await resolvePRRepositoryCandidates(repoPath, connectionId)
|
||||
let data: PullRequestLookupData | null = null
|
||||
let dataRepo: OwnerRepo | null = null
|
||||
let exactLinkedData: PullRequestLookupData | null = null
|
||||
let exactLinkedRepo: OwnerRepo | null = null
|
||||
|
||||
if (ownerRepo && typeof linkedPRNumber === 'number') {
|
||||
data = await getPRByNumber(ownerRepo, linkedPRNumber, ghOptions)
|
||||
if (data && !(await exactPRMatchesWorktreeHead(repoPath, branchName, data, connectionId))) {
|
||||
// Why: linked PR metadata is user-editable. If the stored number still
|
||||
// resolves but no longer matches this worktree, let branch lookup correct it.
|
||||
exactLinkedData = data
|
||||
data = null
|
||||
if (typeof linkedPRNumber === 'number') {
|
||||
for (const candidate of candidates) {
|
||||
try {
|
||||
const linkedData = await getPRByNumber(candidate, linkedPRNumber, ghOptions)
|
||||
if (!linkedData) {
|
||||
continue
|
||||
}
|
||||
if (await exactPRMatchesWorktreeHead(repoPath, branchName, linkedData, connectionId)) {
|
||||
data = linkedData
|
||||
dataRepo = candidate
|
||||
break
|
||||
}
|
||||
// Why: linked PR metadata is user-editable. If the stored number still
|
||||
// resolves but no longer matches this worktree, let branch lookup correct it.
|
||||
exactLinkedData ??= linkedData
|
||||
exactLinkedRepo ??= candidate
|
||||
} catch {
|
||||
// Candidate probing is best-effort; another repo may own the PR.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1587,33 +1629,28 @@ export async function getPRForBranch(
|
|||
// An empty --head filter causes gh to return an arbitrary PR — skip the
|
||||
// branch lookup and rely on the linkedPR fallback below if available.
|
||||
if (!data && branchName) {
|
||||
if (ownerRepo) {
|
||||
try {
|
||||
const { stdout } = await ghExecFileAsync(
|
||||
[
|
||||
'pr',
|
||||
'list',
|
||||
'--repo',
|
||||
`${ownerRepo.owner}/${ownerRepo.repo}`,
|
||||
'--head',
|
||||
branchName,
|
||||
'--state',
|
||||
'all',
|
||||
'--limit',
|
||||
'1',
|
||||
'--json',
|
||||
PR_LOOKUP_JSON_FIELDS
|
||||
],
|
||||
ghOptions
|
||||
)
|
||||
const list = JSON.parse(stdout) as PullRequestLookupData[]
|
||||
data = list[0] ?? null
|
||||
} catch (err) {
|
||||
// Why: `gh pr list/view` uses GraphQL and can hit that quota while
|
||||
// REST is still available. Falling back prevents a real PR from
|
||||
// rendering as "No pull request found" during GraphQL outages.
|
||||
if (!isNotFoundGhError(err)) {
|
||||
data = await getRestPRForBranch(ownerRepo, branchName, ghOptions)
|
||||
if (candidates.length > 0) {
|
||||
for (const candidate of candidates) {
|
||||
try {
|
||||
data = headRepo
|
||||
? await getRestPRForBranch(candidate, headRepo.owner, branchName, ghOptions)
|
||||
: await getFallbackPRListForBranch(candidate, branchName, ghOptions)
|
||||
if (data) {
|
||||
dataRepo = candidate
|
||||
break
|
||||
}
|
||||
} catch (err) {
|
||||
if (!headRepo && !isNotFoundGhError(err)) {
|
||||
try {
|
||||
data = await getRestPRForBranch(candidate, candidate.owner, branchName, ghOptions)
|
||||
if (data) {
|
||||
dataRepo = candidate
|
||||
break
|
||||
}
|
||||
} catch {
|
||||
// Continue to the next candidate below.
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
|
|
@ -1625,7 +1662,7 @@ export async function getPRForBranch(
|
|||
}
|
||||
}
|
||||
|
||||
if (!data && !ownerRepo && typeof linkedPRNumber === 'number') {
|
||||
if (!data && candidates.length === 0 && typeof linkedPRNumber === 'number') {
|
||||
const args = ['pr', 'view', String(linkedPRNumber), '--json', PR_LOOKUP_JSON_FIELDS]
|
||||
try {
|
||||
const { stdout } = await ghExecFileAsync(args, ghOptions)
|
||||
|
|
@ -1641,6 +1678,7 @@ export async function getPRForBranch(
|
|||
|
||||
if (!data && exactLinkedData) {
|
||||
data = exactLinkedData
|
||||
dataRepo = exactLinkedRepo
|
||||
}
|
||||
|
||||
if (!data) {
|
||||
|
|
@ -1665,6 +1703,8 @@ export async function getPRForBranch(
|
|||
updatedAt: data.updatedAt,
|
||||
mergeable: (data.mergeable as PRMergeableState) ?? 'UNKNOWN',
|
||||
headSha: data.headRefOid,
|
||||
prRepo: dataRepo ?? undefined,
|
||||
headRepo: headRepo ?? undefined,
|
||||
conflictSummary
|
||||
}
|
||||
} catch {
|
||||
|
|
@ -1683,11 +1723,12 @@ export async function getPRChecks(
|
|||
repoPath: string,
|
||||
prNumber: number,
|
||||
headSha?: string,
|
||||
prRepo?: OwnerRepo | null,
|
||||
options?: { noCache?: boolean },
|
||||
connectionId?: string | null
|
||||
): Promise<PRCheckDetail[]> {
|
||||
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId))
|
||||
const ownerRepo = await getOwnerRepo(repoPath, connectionId)
|
||||
const ownerRepo = prRepo ?? (await getOwnerRepo(repoPath, connectionId))
|
||||
const fallbackToPRChecks = async (): Promise<PRCheckDetail[]> => {
|
||||
const fallbackArgs = ['pr', 'checks', String(prNumber), '--json', 'name,state,link']
|
||||
if (ownerRepo) {
|
||||
|
|
@ -1784,6 +1825,7 @@ export async function rerunPRChecks(
|
|||
repoPath,
|
||||
prNumber,
|
||||
options.headSha,
|
||||
ownerRepo,
|
||||
{ noCache: true },
|
||||
connectionId
|
||||
)
|
||||
|
|
@ -1908,11 +1950,11 @@ query($owner: String!, $repo: String!, $pr: Int!) {
|
|||
export async function getPRComments(
|
||||
repoPath: string,
|
||||
prNumber: number,
|
||||
options?: { noCache?: boolean },
|
||||
options?: { noCache?: boolean; prRepo?: OwnerRepo | null },
|
||||
connectionId?: string | null
|
||||
): Promise<PRComment[]> {
|
||||
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId))
|
||||
const ownerRepo = await getOwnerRepo(repoPath, connectionId)
|
||||
const ownerRepo = options?.prRepo ?? (await getOwnerRepo(repoPath, connectionId))
|
||||
await acquire()
|
||||
try {
|
||||
if (ownerRepo) {
|
||||
|
|
@ -2345,10 +2387,11 @@ export async function mergePR(
|
|||
repoPath: string,
|
||||
prNumber: number,
|
||||
method: 'merge' | 'squash' | 'rebase' = 'squash',
|
||||
connectionId?: string | null
|
||||
connectionId?: string | null,
|
||||
prRepo?: OwnerRepo | null
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> {
|
||||
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId))
|
||||
const ownerRepo = await getOwnerRepo(repoPath, connectionId)
|
||||
const ownerRepo = prRepo ?? (await getOwnerRepo(repoPath, connectionId))
|
||||
await acquire()
|
||||
try {
|
||||
// Don't use --delete-branch: it tries to delete the local branch which
|
||||
|
|
@ -2447,10 +2490,11 @@ export async function updatePRTitle(
|
|||
repoPath: string,
|
||||
prNumber: number,
|
||||
title: string,
|
||||
connectionId?: string | null
|
||||
connectionId?: string | null,
|
||||
prRepo?: OwnerRepo | null
|
||||
): Promise<boolean> {
|
||||
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId))
|
||||
const ownerRepo = await getOwnerRepo(repoPath, connectionId)
|
||||
const ownerRepo = prRepo ?? (await getOwnerRepo(repoPath, connectionId))
|
||||
await acquire()
|
||||
try {
|
||||
const args = ['pr', 'edit', String(prNumber), '--title', title]
|
||||
|
|
|
|||
|
|
@ -20,8 +20,10 @@ import {
|
|||
classifyListIssuesError,
|
||||
getIssueOwnerRepo,
|
||||
getOwnerRepo,
|
||||
getOwnerRepoForRemote,
|
||||
parseGitHubRemoteIdentity,
|
||||
parseGitHubOwnerRepo,
|
||||
resolvePRRepositoryCandidates,
|
||||
resolveIssueSource
|
||||
} from './gh-utils'
|
||||
|
||||
|
|
@ -136,6 +138,57 @@ describe('github owner/repo resolution', () => {
|
|||
await expect(getOwnerRepo('/repo')).resolves.toEqual({ owner: 'local', repo: 'orca' })
|
||||
await expect(getOwnerRepo('/repo', 'ssh-1')).resolves.toEqual({ owner: 'remote', repo: 'orca' })
|
||||
})
|
||||
|
||||
it('resolves PR candidates as upstream then origin and de-dupes matching slugs', async () => {
|
||||
gitExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: 'git@github.com:Acme/Orca.git\n' })
|
||||
.mockResolvedValueOnce({ stdout: 'git@github.com:acme/orca.git\n' })
|
||||
|
||||
await expect(resolvePRRepositoryCandidates('/repo')).resolves.toEqual({
|
||||
candidates: [{ owner: 'Acme', repo: 'Orca' }],
|
||||
headRepo: { owner: 'acme', repo: 'orca' }
|
||||
})
|
||||
})
|
||||
|
||||
it('ignores non-GitHub upstream while keeping origin as the head repo', async () => {
|
||||
gitExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: 'git@example.com:Acme/Orca.git\n' })
|
||||
.mockResolvedValueOnce({ stdout: 'git@github.com:fork/orca.git\n' })
|
||||
|
||||
await expect(resolvePRRepositoryCandidates('/repo')).resolves.toEqual({
|
||||
candidates: [{ owner: 'fork', repo: 'orca' }],
|
||||
headRepo: { owner: 'fork', repo: 'orca' }
|
||||
})
|
||||
})
|
||||
|
||||
it('expires cached remote owner/repo entries after the TTL', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
gitExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: 'git@github.com:old/orca.git\n' })
|
||||
.mockResolvedValueOnce({ stdout: 'git@github.com:new/orca.git\n' })
|
||||
|
||||
await expect(getOwnerRepoForRemote('/repo', 'origin')).resolves.toEqual({
|
||||
owner: 'old',
|
||||
repo: 'orca'
|
||||
})
|
||||
await expect(getOwnerRepoForRemote('/repo', 'origin')).resolves.toEqual({
|
||||
owner: 'old',
|
||||
repo: 'orca'
|
||||
})
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1)
|
||||
|
||||
await vi.advanceTimersByTimeAsync(30_001)
|
||||
|
||||
await expect(getOwnerRepoForRemote('/repo', 'origin')).resolves.toEqual({
|
||||
owner: 'new',
|
||||
repo: 'orca'
|
||||
})
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveIssueSource', () => {
|
||||
|
|
|
|||
|
|
@ -137,7 +137,14 @@ export function ghRepoExecOptions(context: GitHubRepoContext): {
|
|||
return context.connectionId ? {} : { cwd: context.repoPath }
|
||||
}
|
||||
|
||||
const ownerRepoCache = new Map<string, OwnerRepo | null>()
|
||||
const OWNER_REPO_CACHE_TTL_MS = 30_000
|
||||
|
||||
type OwnerRepoCacheEntry = {
|
||||
value: OwnerRepo | null
|
||||
expiresAt: number
|
||||
}
|
||||
|
||||
const ownerRepoCache = new Map<string, OwnerRepoCacheEntry>()
|
||||
|
||||
/** @internal — exposed for tests only */
|
||||
export function _resetOwnerRepoCache(): void {
|
||||
|
|
@ -190,20 +197,27 @@ export async function getOwnerRepoForRemote(
|
|||
): Promise<OwnerRepo | null> {
|
||||
const context = githubRepoContext(repoPath, connectionId)
|
||||
const cacheKey = `${context.connectionId ?? 'local'}\0${context.repoPath}\0${remoteName}`
|
||||
if (ownerRepoCache.has(cacheKey)) {
|
||||
return ownerRepoCache.get(cacheKey)!
|
||||
const cached = ownerRepoCache.get(cacheKey)
|
||||
if (cached && cached.expiresAt > Date.now()) {
|
||||
return cached.value
|
||||
}
|
||||
if (cached) {
|
||||
ownerRepoCache.delete(cacheKey)
|
||||
}
|
||||
try {
|
||||
const remoteUrl = await getRemoteUrlForRepo(context, remoteName)
|
||||
const result = remoteUrl ? parseGitHubOwnerRepo(remoteUrl) : null
|
||||
if (result) {
|
||||
ownerRepoCache.set(cacheKey, result)
|
||||
ownerRepoCache.set(cacheKey, {
|
||||
value: result,
|
||||
expiresAt: Date.now() + OWNER_REPO_CACHE_TTL_MS
|
||||
})
|
||||
return result
|
||||
}
|
||||
} catch {
|
||||
// ignore — non-GitHub remote or no remote
|
||||
}
|
||||
ownerRepoCache.set(cacheKey, null)
|
||||
ownerRepoCache.set(cacheKey, { value: null, expiresAt: Date.now() + OWNER_REPO_CACHE_TTL_MS })
|
||||
return null
|
||||
}
|
||||
|
||||
|
|
@ -225,6 +239,39 @@ export async function getIssueOwnerRepo(
|
|||
return getOwnerRepoForRemote(repoPath, 'origin', connectionId)
|
||||
}
|
||||
|
||||
export type PRRepositoryCandidates = {
|
||||
candidates: OwnerRepo[]
|
||||
headRepo: OwnerRepo | null
|
||||
}
|
||||
|
||||
function ownerRepoKey(ownerRepo: OwnerRepo): string {
|
||||
return `${ownerRepo.owner.toLowerCase()}/${ownerRepo.repo.toLowerCase()}`
|
||||
}
|
||||
|
||||
export async function resolvePRRepositoryCandidates(
|
||||
repoPath: string,
|
||||
connectionId?: string | null
|
||||
): Promise<PRRepositoryCandidates> {
|
||||
const upstream = await getOwnerRepoForRemote(repoPath, 'upstream', connectionId)
|
||||
const origin = await getOwnerRepoForRemote(repoPath, 'origin', connectionId)
|
||||
const seen = new Set<string>()
|
||||
const candidates: OwnerRepo[] = []
|
||||
|
||||
for (const candidate of [upstream, origin]) {
|
||||
if (!candidate) {
|
||||
continue
|
||||
}
|
||||
const key = ownerRepoKey(candidate)
|
||||
if (seen.has(key)) {
|
||||
continue
|
||||
}
|
||||
seen.add(key)
|
||||
candidates.push(candidate)
|
||||
}
|
||||
|
||||
return { candidates, headRepo: origin }
|
||||
}
|
||||
|
||||
export type ResolvedIssueSource = {
|
||||
source: OwnerRepo | null
|
||||
/** True when the user preferred `upstream` but the upstream remote is no
|
||||
|
|
|
|||
|
|
@ -158,6 +158,13 @@ describe('getWorkItemDetails PR file viewed state', () => {
|
|||
['src/viewed.ts', 'VIEWED'],
|
||||
['src/changed.ts', 'DISMISSED']
|
||||
])
|
||||
expect(getPRChecksMock).toHaveBeenCalledWith('/repo-root', 42, 'head-sha', undefined, undefined)
|
||||
expect(getPRChecksMock).toHaveBeenCalledWith(
|
||||
'/repo-root',
|
||||
42,
|
||||
'head-sha',
|
||||
null,
|
||||
undefined,
|
||||
undefined
|
||||
)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -741,8 +741,8 @@ export async function getWorkItemDetails(
|
|||
const [mentionParticipants, checks] = await Promise.all([
|
||||
getMentionParticipants(repoPath, item, comments, participants, connectionId),
|
||||
shas?.headSha
|
||||
? getPRChecks(repoPath, item.number, shas.headSha, undefined, connectionId)
|
||||
: getPRChecks(repoPath, item.number, undefined, undefined, connectionId)
|
||||
? getPRChecks(repoPath, item.number, shas.headSha, null, undefined, connectionId)
|
||||
: getPRChecks(repoPath, item.number, undefined, null, undefined, connectionId)
|
||||
])
|
||||
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -4,7 +4,12 @@ reviewable as one surface. Splitting by feature area would risk drifting
|
|||
validation/gate conventions across handler files. */
|
||||
import { ipcMain, webContents } from 'electron'
|
||||
import { resolve } from 'path'
|
||||
import type { Repo, GitHubIssueUpdate, GitHubPullRequestStateUpdate } from '../../shared/types'
|
||||
import type {
|
||||
Repo,
|
||||
GitHubIssueUpdate,
|
||||
GitHubOwnerRepo,
|
||||
GitHubPullRequestStateUpdate
|
||||
} from '../../shared/types'
|
||||
import type { Store } from '../persistence'
|
||||
import type { StatsCollector } from '../stats/collector'
|
||||
import {
|
||||
|
|
@ -278,6 +283,7 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi
|
|||
repoPath: string
|
||||
prNumber: number
|
||||
headSha?: string
|
||||
prRepo?: GitHubOwnerRepo | null
|
||||
noCache?: boolean
|
||||
}
|
||||
) => {
|
||||
|
|
@ -286,6 +292,7 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi
|
|||
repo.path,
|
||||
args.prNumber,
|
||||
args.headSha,
|
||||
args.prRepo ?? null,
|
||||
{
|
||||
noCache: args.noCache
|
||||
},
|
||||
|
|
@ -296,12 +303,20 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi
|
|||
|
||||
ipcMain.handle(
|
||||
'gh:prComments',
|
||||
(_event, args: { repoPath: string; prNumber: number; noCache?: boolean }) => {
|
||||
(
|
||||
_event,
|
||||
args: {
|
||||
repoPath: string
|
||||
prNumber: number
|
||||
prRepo?: GitHubOwnerRepo | null
|
||||
noCache?: boolean
|
||||
}
|
||||
) => {
|
||||
const repo = assertRegisteredRepo(args, store)
|
||||
return getPRComments(
|
||||
repo.path,
|
||||
args.prNumber,
|
||||
{ noCache: args.noCache },
|
||||
{ noCache: args.noCache, prRepo: args.prRepo ?? null },
|
||||
repoConnectionId(repo)
|
||||
)
|
||||
}
|
||||
|
|
@ -478,9 +493,18 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi
|
|||
|
||||
ipcMain.handle(
|
||||
'gh:updatePRTitle',
|
||||
async (event, args: { repoPath: string; prNumber: number; title: string }) => {
|
||||
async (
|
||||
event,
|
||||
args: { repoPath: string; prNumber: number; title: string; prRepo?: GitHubOwnerRepo | null }
|
||||
) => {
|
||||
const repo = assertRegisteredRepo(args, store)
|
||||
const ok = await updatePRTitle(repo.path, args.prNumber, args.title, repoConnectionId(repo))
|
||||
const ok = await updatePRTitle(
|
||||
repo.path,
|
||||
args.prNumber,
|
||||
args.title,
|
||||
repoConnectionId(repo),
|
||||
args.prRepo ?? null
|
||||
)
|
||||
if (ok) {
|
||||
broadcastWorkItemMutated(
|
||||
{ repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber },
|
||||
|
|
@ -495,10 +519,21 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi
|
|||
'gh:mergePR',
|
||||
async (
|
||||
event,
|
||||
args: { repoPath: string; prNumber: number; method?: 'merge' | 'squash' | 'rebase' }
|
||||
args: {
|
||||
repoPath: string
|
||||
prNumber: number
|
||||
method?: 'merge' | 'squash' | 'rebase'
|
||||
prRepo?: GitHubOwnerRepo | null
|
||||
}
|
||||
) => {
|
||||
const repo = assertRegisteredRepo(args, store)
|
||||
const result = await mergePR(repo.path, args.prNumber, args.method, repoConnectionId(repo))
|
||||
const result = await mergePR(
|
||||
repo.path,
|
||||
args.prNumber,
|
||||
args.method,
|
||||
repoConnectionId(repo),
|
||||
args.prRepo ?? null
|
||||
)
|
||||
if (result.ok) {
|
||||
broadcastWorkItemMutated(
|
||||
{ repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber },
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import type {
|
|||
CreateWorktreeResult,
|
||||
GitPushTarget,
|
||||
GitWorktreeInfo,
|
||||
GitHubOwnerRepo,
|
||||
GlobalSettings,
|
||||
Repo,
|
||||
StatsSummary,
|
||||
|
|
@ -4711,11 +4712,12 @@ export class OrcaRuntimeService {
|
|||
repoSelector: string,
|
||||
prNumber: number,
|
||||
headSha?: string,
|
||||
prRepo?: GitHubOwnerRepo | null,
|
||||
options?: { noCache?: boolean }
|
||||
): Promise<Awaited<ReturnType<typeof getPRChecks>>> {
|
||||
const repo = await this.resolveRepoSelector(repoSelector)
|
||||
this.assertHostIntegrationRepoIsLocal(repo, 'repo_pr_checks')
|
||||
return getPRChecks(repo.path, prNumber, headSha, options)
|
||||
return getPRChecks(repo.path, prNumber, headSha, prRepo ?? null, options)
|
||||
}
|
||||
|
||||
async rerunRepoPRChecks(
|
||||
|
|
@ -4731,11 +4733,12 @@ export class OrcaRuntimeService {
|
|||
async getRepoPRComments(
|
||||
repoSelector: string,
|
||||
prNumber: number,
|
||||
prRepo?: GitHubOwnerRepo | null,
|
||||
options?: { noCache?: boolean }
|
||||
): Promise<Awaited<ReturnType<typeof getPRComments>>> {
|
||||
const repo = await this.resolveRepoSelector(repoSelector)
|
||||
this.assertHostIntegrationRepoIsLocal(repo, 'repo_pr_comments')
|
||||
return getPRComments(repo.path, prNumber, options)
|
||||
return getPRComments(repo.path, prNumber, { ...options, prRepo: prRepo ?? null })
|
||||
}
|
||||
|
||||
async getRepoPRFileContents(
|
||||
|
|
@ -4780,21 +4783,23 @@ export class OrcaRuntimeService {
|
|||
async updateRepoPRTitle(
|
||||
repoSelector: string,
|
||||
prNumber: number,
|
||||
title: string
|
||||
title: string,
|
||||
prRepo?: GitHubOwnerRepo | null
|
||||
): Promise<Awaited<ReturnType<typeof updatePRTitle>>> {
|
||||
const repo = await this.resolveRepoSelector(repoSelector)
|
||||
this.assertHostIntegrationRepoIsLocal(repo, 'repo_pr_title')
|
||||
return updatePRTitle(repo.path, prNumber, title)
|
||||
return updatePRTitle(repo.path, prNumber, title, undefined, prRepo ?? null)
|
||||
}
|
||||
|
||||
async mergeRepoPR(
|
||||
repoSelector: string,
|
||||
prNumber: number,
|
||||
method?: 'merge' | 'squash' | 'rebase'
|
||||
method?: 'merge' | 'squash' | 'rebase',
|
||||
prRepo?: GitHubOwnerRepo | null
|
||||
): Promise<Awaited<ReturnType<typeof mergePR>>> {
|
||||
const repo = await this.resolveRepoSelector(repoSelector)
|
||||
this.assertHostIntegrationRepoIsLocal(repo, 'repo_pr_merge')
|
||||
return mergePR(repo.path, prNumber, method)
|
||||
return mergePR(repo.path, prNumber, method, undefined, prRepo ?? null)
|
||||
}
|
||||
|
||||
async updateRepoPRState(
|
||||
|
|
|
|||
|
|
@ -156,13 +156,47 @@ describe('github RPC methods', () => {
|
|||
repo: 'repo-1',
|
||||
prNumber: 7,
|
||||
headSha: 'abc123',
|
||||
prRepo: { owner: 'acme', repo: 'widgets' },
|
||||
noCache: true
|
||||
})
|
||||
)
|
||||
|
||||
expect(runtime.getRepoPRChecks).toHaveBeenCalledWith('repo-1', 7, 'abc123', {
|
||||
noCache: true
|
||||
})
|
||||
expect(runtime.getRepoPRChecks).toHaveBeenCalledWith(
|
||||
'repo-1',
|
||||
7,
|
||||
'abc123',
|
||||
{ owner: 'acme', repo: 'widgets' },
|
||||
{
|
||||
noCache: true
|
||||
}
|
||||
)
|
||||
expect(response).toMatchObject({ ok: true, result: [] })
|
||||
})
|
||||
|
||||
it('fetches PR comments on the runtime server with explicit PR repo', async () => {
|
||||
const runtime = {
|
||||
getRuntimeId: () => 'test-runtime',
|
||||
getRepoPRComments: vi.fn().mockResolvedValue([])
|
||||
} as unknown as OrcaRuntimeService
|
||||
const dispatcher = new RpcDispatcher({ runtime, methods: GITHUB_METHODS })
|
||||
|
||||
const response = await dispatcher.dispatch(
|
||||
makeRequest('github.prComments', {
|
||||
repo: 'repo-1',
|
||||
prNumber: 7,
|
||||
prRepo: { owner: 'acme', repo: 'widgets' },
|
||||
noCache: true
|
||||
})
|
||||
)
|
||||
|
||||
expect(runtime.getRepoPRComments).toHaveBeenCalledWith(
|
||||
'repo-1',
|
||||
7,
|
||||
{ owner: 'acme', repo: 'widgets' },
|
||||
{
|
||||
noCache: true
|
||||
}
|
||||
)
|
||||
expect(response).toMatchObject({ ok: true, result: [] })
|
||||
})
|
||||
|
||||
|
|
@ -254,11 +288,15 @@ describe('github RPC methods', () => {
|
|||
makeRequest('github.updatePRTitle', {
|
||||
repo: 'repo-1',
|
||||
prNumber: 7,
|
||||
title: 'New title'
|
||||
title: 'New title',
|
||||
prRepo: { owner: 'acme', repo: 'widgets' }
|
||||
})
|
||||
)
|
||||
|
||||
expect(runtime.updateRepoPRTitle).toHaveBeenCalledWith('repo-1', 7, 'New title')
|
||||
expect(runtime.updateRepoPRTitle).toHaveBeenCalledWith('repo-1', 7, 'New title', {
|
||||
owner: 'acme',
|
||||
repo: 'widgets'
|
||||
})
|
||||
expect(response).toMatchObject({ ok: true, result: true })
|
||||
})
|
||||
|
||||
|
|
@ -273,11 +311,15 @@ describe('github RPC methods', () => {
|
|||
makeRequest('github.mergePR', {
|
||||
repo: 'repo-1',
|
||||
prNumber: 7,
|
||||
method: 'squash'
|
||||
method: 'squash',
|
||||
prRepo: { owner: 'acme', repo: 'widgets' }
|
||||
})
|
||||
)
|
||||
|
||||
expect(runtime.mergeRepoPR).toHaveBeenCalledWith('repo-1', 7, 'squash')
|
||||
expect(runtime.mergeRepoPR).toHaveBeenCalledWith('repo-1', 7, 'squash', {
|
||||
owner: 'acme',
|
||||
repo: 'widgets'
|
||||
})
|
||||
expect(response).toMatchObject({ ok: true, result: { ok: true } })
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -55,7 +55,8 @@ const Issue = RepoSelector.extend({
|
|||
|
||||
const PullRequest = RepoSelector.extend({
|
||||
prNumber: z.number().int().positive(),
|
||||
noCache: z.boolean().optional()
|
||||
noCache: z.boolean().optional(),
|
||||
prRepo: SlugRepo.nullable().optional()
|
||||
})
|
||||
|
||||
const PullRequestChecks = PullRequest.extend({
|
||||
|
|
@ -89,12 +90,14 @@ const ReviewThread = RepoSelector.extend({
|
|||
|
||||
const UpdatePrTitle = RepoSelector.extend({
|
||||
prNumber: z.number().int().positive(),
|
||||
title: requiredString('Missing title')
|
||||
title: requiredString('Missing title'),
|
||||
prRepo: SlugRepo.nullable().optional()
|
||||
})
|
||||
|
||||
const MergePr = RepoSelector.extend({
|
||||
prNumber: z.number().int().positive(),
|
||||
method: z.enum(['merge', 'squash', 'rebase']).optional()
|
||||
method: z.enum(['merge', 'squash', 'rebase']).optional(),
|
||||
prRepo: SlugRepo.nullable().optional()
|
||||
})
|
||||
|
||||
const UpdatePrState = RepoSelector.extend({
|
||||
|
|
@ -308,7 +311,7 @@ export const GITHUB_METHODS: RpcMethod[] = [
|
|||
name: 'github.prChecks',
|
||||
params: PullRequestChecks,
|
||||
handler: async (params, { runtime }) =>
|
||||
runtime.getRepoPRChecks(params.repo, params.prNumber, params.headSha, {
|
||||
runtime.getRepoPRChecks(params.repo, params.prNumber, params.headSha, params.prRepo ?? null, {
|
||||
noCache: params.noCache
|
||||
})
|
||||
}),
|
||||
|
|
@ -325,7 +328,9 @@ export const GITHUB_METHODS: RpcMethod[] = [
|
|||
name: 'github.prComments',
|
||||
params: PullRequest,
|
||||
handler: async (params, { runtime }) =>
|
||||
runtime.getRepoPRComments(params.repo, params.prNumber, { noCache: params.noCache })
|
||||
runtime.getRepoPRComments(params.repo, params.prNumber, params.prRepo ?? null, {
|
||||
noCache: params.noCache
|
||||
})
|
||||
}),
|
||||
defineMethod({
|
||||
name: 'github.prFileContents',
|
||||
|
|
@ -360,13 +365,13 @@ export const GITHUB_METHODS: RpcMethod[] = [
|
|||
name: 'github.updatePRTitle',
|
||||
params: UpdatePrTitle,
|
||||
handler: async (params, { runtime }) =>
|
||||
runtime.updateRepoPRTitle(params.repo, params.prNumber, params.title)
|
||||
runtime.updateRepoPRTitle(params.repo, params.prNumber, params.title, params.prRepo ?? null)
|
||||
}),
|
||||
defineMethod({
|
||||
name: 'github.mergePR',
|
||||
params: MergePr,
|
||||
handler: async (params, { runtime }) =>
|
||||
runtime.mergeRepoPR(params.repo, params.prNumber, params.method)
|
||||
runtime.mergeRepoPR(params.repo, params.prNumber, params.method, params.prRepo ?? null)
|
||||
}),
|
||||
defineMethod({
|
||||
name: 'github.updatePRState',
|
||||
|
|
|
|||
|
|
@ -38,6 +38,7 @@ import type {
|
|||
GitHubPRFileContents,
|
||||
GitHubPRReviewCommentInput,
|
||||
GitHubCommentResult,
|
||||
GitHubOwnerRepo,
|
||||
GitHubWorkItem,
|
||||
GitHubWorkItemDetails,
|
||||
GitHubViewer,
|
||||
|
|
@ -798,6 +799,7 @@ export type PreloadApi = {
|
|||
repoId?: string
|
||||
prNumber: number
|
||||
headSha?: string
|
||||
prRepo?: GitHubOwnerRepo | null
|
||||
noCache?: boolean
|
||||
}) => Promise<PRCheckDetail[]>
|
||||
rerunPRChecks: (args: {
|
||||
|
|
@ -811,6 +813,7 @@ export type PreloadApi = {
|
|||
repoPath: string
|
||||
repoId?: string
|
||||
prNumber: number
|
||||
prRepo?: GitHubOwnerRepo | null
|
||||
noCache?: boolean
|
||||
}) => Promise<PRComment[]>
|
||||
resolveReviewThread: (args: {
|
||||
|
|
@ -832,12 +835,14 @@ export type PreloadApi = {
|
|||
repoId?: string
|
||||
prNumber: number
|
||||
title: string
|
||||
prRepo?: GitHubOwnerRepo | null
|
||||
}) => Promise<boolean>
|
||||
mergePR: (args: {
|
||||
repoPath: string
|
||||
repoId?: string
|
||||
prNumber: number
|
||||
method?: 'merge' | 'squash' | 'rebase'
|
||||
prRepo?: GitHubOwnerRepo | null
|
||||
}) => Promise<{ ok: true } | { ok: false; error: string }>
|
||||
updatePRState: (args: {
|
||||
repoPath: string
|
||||
|
|
|
|||
|
|
@ -814,6 +814,7 @@ const api = {
|
|||
repoId?: string
|
||||
prNumber: number
|
||||
headSha?: string
|
||||
prRepo?: { owner: string; repo: string } | null
|
||||
noCache?: boolean
|
||||
}): Promise<unknown[]> => ipcRenderer.invoke('gh:prChecks', args),
|
||||
|
||||
|
|
@ -830,6 +831,7 @@ const api = {
|
|||
repoPath: string
|
||||
repoId?: string
|
||||
prNumber: number
|
||||
prRepo?: { owner: string; repo: string } | null
|
||||
noCache?: boolean
|
||||
}): Promise<unknown[]> => ipcRenderer.invoke('gh:prComments', args),
|
||||
|
||||
|
|
@ -854,6 +856,7 @@ const api = {
|
|||
repoId?: string
|
||||
prNumber: number
|
||||
title: string
|
||||
prRepo?: { owner: string; repo: string } | null
|
||||
}): Promise<boolean> => ipcRenderer.invoke('gh:updatePRTitle', args),
|
||||
|
||||
mergePR: (args: {
|
||||
|
|
@ -861,6 +864,7 @@ const api = {
|
|||
repoId?: string
|
||||
prNumber: number
|
||||
method?: 'merge' | 'squash' | 'rebase'
|
||||
prRepo?: { owner: string; repo: string } | null
|
||||
}): Promise<{ ok: true } | { ok: false; error: string }> =>
|
||||
ipcRenderer.invoke('gh:mergePR', args),
|
||||
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ merge actions, and conflict state in one component to keep the data flow straigh
|
|||
import React, { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import { LoaderCircle, ExternalLink, RefreshCw, Check, X, Pencil } from 'lucide-react'
|
||||
import { useAppStore } from '@/store'
|
||||
import { prChecksCacheSuffix, prCommentsCacheSuffix } from '@/store/slices/github'
|
||||
import { useActiveWorktree, useRepoById } from '@/store/selectors'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { Button } from '@/components/ui/button'
|
||||
|
|
@ -124,8 +125,10 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
const prFetchedAt = useAppStore((s) =>
|
||||
prCacheKey ? s.prCache[prCacheKey]?.fetchedAt : undefined
|
||||
)
|
||||
const checksCacheKey = repo && prNumber ? `${repo.id}::pr-checks::${prNumber}` : ''
|
||||
const commentsCacheKey = repo && prNumber ? `${repo.id}::pr-comments::${prNumber}` : ''
|
||||
const checksCacheKey =
|
||||
repo && prNumber ? `${repo.id}::${prChecksCacheSuffix(prNumber, pr?.prRepo)}` : ''
|
||||
const commentsCacheKey =
|
||||
repo && prNumber ? `${repo.id}::${prCommentsCacheSuffix(prNumber, pr?.prRepo)}` : ''
|
||||
const checksFetchedAt = useAppStore((s) =>
|
||||
checksCacheKey ? s.checksCache[checksCacheKey]?.fetchedAt : undefined
|
||||
)
|
||||
|
|
@ -139,7 +142,8 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
const linkedPR = activeWorktree?.linkedPR ?? null
|
||||
const linkedGitLabMR = activeWorktree?.linkedGitLabMR ?? null
|
||||
const activeWorktreePath = activeWorktree?.path ?? null
|
||||
const stateRequestKey = repo && branch ? checksPanelAsyncResultKey(repo.id, branch, prNumber) : ''
|
||||
const stateRequestKey =
|
||||
repo && branch ? checksPanelAsyncResultKey(repo.id, branch, prNumber, pr?.prRepo) : ''
|
||||
asyncResultKeyRef.current = stateRequestKey
|
||||
|
||||
const isCurrentAsyncResult = useCallback(
|
||||
|
|
@ -241,11 +245,18 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
}
|
||||
setChecksLoading(true)
|
||||
try {
|
||||
const requestKey = checksPanelAsyncResultKey(repo.id, branch, targetPRNumber)
|
||||
const result = await fetchPRChecks(repo.path, targetPRNumber, branch, pr?.headSha, {
|
||||
force,
|
||||
repoId: repo.id
|
||||
})
|
||||
const requestKey = checksPanelAsyncResultKey(repo.id, branch, targetPRNumber, pr?.prRepo)
|
||||
const result = await fetchPRChecks(
|
||||
repo.path,
|
||||
targetPRNumber,
|
||||
branch,
|
||||
pr?.headSha,
|
||||
pr?.prRepo,
|
||||
{
|
||||
force,
|
||||
repoId: repo.id
|
||||
}
|
||||
)
|
||||
if (!isCurrentAsyncResult(requestKey)) {
|
||||
return
|
||||
}
|
||||
|
|
@ -260,18 +271,26 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
: 30_000
|
||||
prevChecksRef.current = signature
|
||||
} catch (err) {
|
||||
if (!isCurrentAsyncResult(checksPanelAsyncResultKey(repo.id, branch, targetPRNumber))) {
|
||||
if (
|
||||
!isCurrentAsyncResult(
|
||||
checksPanelAsyncResultKey(repo.id, branch, targetPRNumber, pr?.prRepo)
|
||||
)
|
||||
) {
|
||||
return
|
||||
}
|
||||
console.warn('Failed to fetch PR checks:', err)
|
||||
setChecks([])
|
||||
} finally {
|
||||
if (isCurrentAsyncResult(checksPanelAsyncResultKey(repo.id, branch, targetPRNumber))) {
|
||||
if (
|
||||
isCurrentAsyncResult(
|
||||
checksPanelAsyncResultKey(repo.id, branch, targetPRNumber, pr?.prRepo)
|
||||
)
|
||||
) {
|
||||
setChecksLoading(false)
|
||||
}
|
||||
}
|
||||
},
|
||||
[repo, prNumber, branch, pr?.headSha, fetchPRChecks, isCurrentAsyncResult]
|
||||
[repo, prNumber, branch, pr?.headSha, pr?.prRepo, fetchPRChecks, isCurrentAsyncResult]
|
||||
)
|
||||
|
||||
// Fetch checks on mount + poll with exponential backoff
|
||||
|
|
@ -312,33 +331,51 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
const fetchComments = useCallback(
|
||||
async ({
|
||||
force = false,
|
||||
prNumberOverride
|
||||
}: { force?: boolean; prNumberOverride?: number | null } = {}) => {
|
||||
prNumberOverride,
|
||||
prRepoOverride
|
||||
}: {
|
||||
force?: boolean
|
||||
prNumberOverride?: number | null
|
||||
prRepoOverride?: PRInfo['prRepo'] | null
|
||||
} = {}) => {
|
||||
const targetPRNumber = prNumberOverride ?? prNumber
|
||||
const targetPRRepo = prRepoOverride ?? pr?.prRepo
|
||||
if (!repo || !targetPRNumber) {
|
||||
return
|
||||
}
|
||||
setCommentsLoading(true)
|
||||
try {
|
||||
const requestKey = checksPanelAsyncResultKey(repo.id, branch, targetPRNumber)
|
||||
const result = await fetchPRComments(repo.path, targetPRNumber, { force, repoId: repo.id })
|
||||
const requestKey = checksPanelAsyncResultKey(repo.id, branch, targetPRNumber, targetPRRepo)
|
||||
const result = await fetchPRComments(repo.path, targetPRNumber, {
|
||||
force,
|
||||
repoId: repo.id,
|
||||
prRepo: targetPRRepo
|
||||
})
|
||||
if (!isCurrentAsyncResult(requestKey)) {
|
||||
return
|
||||
}
|
||||
setComments(result)
|
||||
} catch (err) {
|
||||
if (!isCurrentAsyncResult(checksPanelAsyncResultKey(repo.id, branch, targetPRNumber))) {
|
||||
if (
|
||||
!isCurrentAsyncResult(
|
||||
checksPanelAsyncResultKey(repo.id, branch, targetPRNumber, targetPRRepo)
|
||||
)
|
||||
) {
|
||||
return
|
||||
}
|
||||
console.warn('Failed to fetch PR comments:', err)
|
||||
setComments([])
|
||||
} finally {
|
||||
if (isCurrentAsyncResult(checksPanelAsyncResultKey(repo.id, branch, targetPRNumber))) {
|
||||
if (
|
||||
isCurrentAsyncResult(
|
||||
checksPanelAsyncResultKey(repo.id, branch, targetPRNumber, targetPRRepo)
|
||||
)
|
||||
) {
|
||||
setCommentsLoading(false)
|
||||
}
|
||||
}
|
||||
},
|
||||
[repo, prNumber, fetchPRComments, branch, isCurrentAsyncResult]
|
||||
[repo, prNumber, pr?.prRepo, fetchPRComments, branch, isCurrentAsyncResult]
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
|
|
@ -350,7 +387,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
// state after the user switches worktrees, showing the wrong PR's comments.
|
||||
let cancelled = false
|
||||
setCommentsLoading(true)
|
||||
void fetchPRComments(repo.path, prNumber, { repoId: repo.id }).then(
|
||||
void fetchPRComments(repo.path, prNumber, { repoId: repo.id, prRepo: pr?.prRepo }).then(
|
||||
(result) => {
|
||||
if (!cancelled) {
|
||||
setComments(result)
|
||||
|
|
@ -367,13 +404,13 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
return () => {
|
||||
cancelled = true
|
||||
}
|
||||
}, [repo, prNumber, isPanelVisible, fetchPRComments])
|
||||
}, [repo, prNumber, pr?.prRepo, isPanelVisible, fetchPRComments])
|
||||
|
||||
const handleRefresh = useCallback(async () => {
|
||||
if (!repo || !branch) {
|
||||
return
|
||||
}
|
||||
const initialRequestKey = checksPanelAsyncResultKey(repo.id, branch, prNumber)
|
||||
const initialRequestKey = checksPanelAsyncResultKey(repo.id, branch, prNumber, pr?.prRepo)
|
||||
let activeRefreshKey = initialRequestKey
|
||||
setIsRefreshing(true)
|
||||
try {
|
||||
|
|
@ -390,7 +427,12 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
linkedGitLabMR
|
||||
})
|
||||
if (refreshedPR) {
|
||||
const requestKey = checksPanelAsyncResultKey(repo.id, branch, refreshedPR.number)
|
||||
const requestKey = checksPanelAsyncResultKey(
|
||||
repo.id,
|
||||
branch,
|
||||
refreshedPR.number,
|
||||
refreshedPR.prRepo
|
||||
)
|
||||
if (!isCurrentAsyncResult(initialRequestKey) && !isCurrentAsyncResult(requestKey)) {
|
||||
return
|
||||
}
|
||||
|
|
@ -407,6 +449,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
refreshedPR.number,
|
||||
branch,
|
||||
refreshedPR.headSha,
|
||||
refreshedPR.prRepo,
|
||||
{ force: true, repoId: repo.id }
|
||||
).then(
|
||||
(result) => {
|
||||
|
|
@ -434,7 +477,8 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
setChecksLoading(true)
|
||||
const refreshedComments = fetchComments({
|
||||
force: true,
|
||||
prNumberOverride: refreshedPR.number
|
||||
prNumberOverride: refreshedPR.number,
|
||||
prRepoOverride: refreshedPR.prRepo
|
||||
})
|
||||
await Promise.all([
|
||||
refreshedChecks.finally(() => {
|
||||
|
|
@ -457,6 +501,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
repo,
|
||||
branch,
|
||||
prNumber,
|
||||
pr?.prRepo,
|
||||
linkedPR,
|
||||
linkedGitLabMR,
|
||||
fetchPRForBranch,
|
||||
|
|
@ -533,7 +578,8 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
repoPath: repo.path,
|
||||
repoId: repo.id,
|
||||
prNumber: pr.number,
|
||||
title: titleDraft.trim()
|
||||
title: titleDraft.trim(),
|
||||
prRepo: pr.prRepo ?? null
|
||||
})
|
||||
if (ok) {
|
||||
// Re-fetch PR to get updated title
|
||||
|
|
@ -566,20 +612,21 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
if (!repo || !prNumber) {
|
||||
return
|
||||
}
|
||||
void resolveReviewThread(repo.path, prNumber, threadId, resolve, { repoId: repo.id }).then(
|
||||
(ok) => {
|
||||
if (ok) {
|
||||
// Update local state to match the optimistic store update
|
||||
setComments((prev) =>
|
||||
prev.map((c) => (c.threadId === threadId ? { ...c, isResolved: resolve } : c))
|
||||
)
|
||||
} else {
|
||||
toast.error('Could not update review thread. Check the GitHub API budget.')
|
||||
}
|
||||
void resolveReviewThread(repo.path, prNumber, threadId, resolve, {
|
||||
repoId: repo.id,
|
||||
prRepo: pr?.prRepo
|
||||
}).then((ok) => {
|
||||
if (ok) {
|
||||
// Update local state to match the optimistic store update
|
||||
setComments((prev) =>
|
||||
prev.map((c) => (c.threadId === threadId ? { ...c, isResolved: resolve } : c))
|
||||
)
|
||||
} else {
|
||||
toast.error('Could not update review thread. Check the GitHub API budget.')
|
||||
}
|
||||
)
|
||||
})
|
||||
},
|
||||
[repo, prNumber, resolveReviewThread]
|
||||
[repo, prNumber, pr?.prRepo, resolveReviewThread]
|
||||
)
|
||||
|
||||
// Refresh PR (passed to PRActions)
|
||||
|
|
@ -632,7 +679,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
if (!repo || !branch) {
|
||||
return
|
||||
}
|
||||
const initialRequestKey = checksPanelAsyncResultKey(repo.id, branch, prNumber)
|
||||
const initialRequestKey = checksPanelAsyncResultKey(repo.id, branch, prNumber, pr?.prRepo)
|
||||
setRightSidebarOpen(true)
|
||||
setRightSidebarTab('checks')
|
||||
try {
|
||||
|
|
@ -649,23 +696,36 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
linkedGitLabMR
|
||||
})
|
||||
if (refreshedPR) {
|
||||
const requestKey = checksPanelAsyncResultKey(repo.id, branch, refreshedPR.number)
|
||||
const requestKey = checksPanelAsyncResultKey(
|
||||
repo.id,
|
||||
branch,
|
||||
refreshedPR.number,
|
||||
refreshedPR.prRepo
|
||||
)
|
||||
if (!isCurrentAsyncResult(initialRequestKey) && !isCurrentAsyncResult(requestKey)) {
|
||||
return
|
||||
}
|
||||
asyncResultKeyRef.current = requestKey
|
||||
await Promise.all([
|
||||
fetchPRChecks(repo.path, refreshedPR.number, branch, refreshedPR.headSha, {
|
||||
force: true,
|
||||
repoId: repo.id
|
||||
}).then((result) => {
|
||||
fetchPRChecks(
|
||||
repo.path,
|
||||
refreshedPR.number,
|
||||
branch,
|
||||
refreshedPR.headSha,
|
||||
refreshedPR.prRepo,
|
||||
{
|
||||
force: true,
|
||||
repoId: repo.id
|
||||
}
|
||||
).then((result) => {
|
||||
if (isCurrentAsyncResult(requestKey)) {
|
||||
setChecks(result)
|
||||
}
|
||||
}),
|
||||
fetchPRComments(repo.path, refreshedPR.number, {
|
||||
force: true,
|
||||
repoId: repo.id
|
||||
repoId: repo.id,
|
||||
prRepo: refreshedPR.prRepo
|
||||
}).then((result) => {
|
||||
if (isCurrentAsyncResult(requestKey)) {
|
||||
setComments(result)
|
||||
|
|
@ -686,6 +746,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
isCurrentAsyncResult,
|
||||
linkedGitLabMR,
|
||||
prNumber,
|
||||
pr?.prRepo,
|
||||
repo,
|
||||
setRightSidebarOpen,
|
||||
setRightSidebarTab
|
||||
|
|
|
|||
|
|
@ -46,7 +46,8 @@ export default function PRActions({
|
|||
repoPath: repo.path,
|
||||
repoId: repo.id,
|
||||
prNumber: pr.number,
|
||||
method
|
||||
method,
|
||||
prRepo: pr.prRepo ?? null
|
||||
})
|
||||
if (!result.ok) {
|
||||
setMergeError(result.error)
|
||||
|
|
@ -59,7 +60,7 @@ export default function PRActions({
|
|||
setMerging(false)
|
||||
}
|
||||
},
|
||||
[repo.id, repo.path, pr.number, onRefreshPR]
|
||||
[repo.id, repo.path, pr.number, pr.prRepo, onRefreshPR]
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
|
|
|
|||
|
|
@ -7,15 +7,24 @@ import {
|
|||
describe('checksPanelAsyncResultKey', () => {
|
||||
it('builds a stable repo-scoped key', () => {
|
||||
expect(checksPanelAsyncResultKey('repo-id', 'feature/test', 12)).toBe(
|
||||
'repo-id::feature/test::12'
|
||||
'repo-id::feature/test::none::12'
|
||||
)
|
||||
})
|
||||
|
||||
it('uses explicit none marker when PR is absent', () => {
|
||||
expect(checksPanelAsyncResultKey('repo-id', 'feature/test', null)).toBe(
|
||||
'repo-id::feature/test::none'
|
||||
'repo-id::feature/test::none::none'
|
||||
)
|
||||
})
|
||||
|
||||
it('normalizes PR repo identity', () => {
|
||||
expect(
|
||||
checksPanelAsyncResultKey('repo-id', 'feature/test', 12, {
|
||||
owner: 'Acme',
|
||||
repo: 'Widgets'
|
||||
})
|
||||
).toBe('repo-id::feature/test::acme/widgets::12')
|
||||
})
|
||||
})
|
||||
|
||||
describe('shouldCommitChecksPanelAsyncResult', () => {
|
||||
|
|
@ -27,4 +36,19 @@ describe('shouldCommitChecksPanelAsyncResult', () => {
|
|||
)
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('suppresses stale completions when the PR repo changes without a PR number change', () => {
|
||||
expect(
|
||||
shouldCommitChecksPanelAsyncResult(
|
||||
checksPanelAsyncResultKey('repo-id', 'feature/test', 12, {
|
||||
owner: 'upstream',
|
||||
repo: 'orca'
|
||||
}),
|
||||
checksPanelAsyncResultKey('repo-id', 'feature/test', 12, {
|
||||
owner: 'fork',
|
||||
repo: 'orca'
|
||||
})
|
||||
)
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,9 +1,19 @@
|
|||
import type { GitHubOwnerRepo } from '../../../../shared/types'
|
||||
|
||||
function normalizedPRRepoIdentity(prRepo?: GitHubOwnerRepo | null): string {
|
||||
if (!prRepo) {
|
||||
return 'none'
|
||||
}
|
||||
return `${prRepo.owner.toLowerCase()}/${prRepo.repo.toLowerCase()}`
|
||||
}
|
||||
|
||||
export function checksPanelAsyncResultKey(
|
||||
repoId: string,
|
||||
branch: string,
|
||||
prNumber: number | null
|
||||
prNumber: number | null,
|
||||
prRepo?: GitHubOwnerRepo | null
|
||||
): string {
|
||||
return `${repoId}::${branch}::${prNumber ?? 'none'}`
|
||||
return `${repoId}::${branch}::${normalizedPRRepoIdentity(prRepo)}::${prNumber ?? 'none'}`
|
||||
}
|
||||
|
||||
export function shouldCommitChecksPanelAsyncResult(
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import type { AppState } from '../types'
|
||||
import type { PRCheckDetail, CheckStatus } from '../../../../shared/types'
|
||||
import type { PRCheckDetail, CheckStatus, GitHubOwnerRepo } from '../../../../shared/types'
|
||||
|
||||
export function normalizeBranchName(branch: string): string {
|
||||
return branch.replace(/^refs\/heads\//, '')
|
||||
|
|
@ -38,7 +38,8 @@ export function syncPRChecksStatus(
|
|||
repoPath: string,
|
||||
repoId: string | undefined,
|
||||
branch: string | undefined,
|
||||
checks: PRCheckDetail[]
|
||||
checks: PRCheckDetail[],
|
||||
prRepo?: GitHubOwnerRepo | null
|
||||
): Partial<AppState> | null {
|
||||
const normalized = branch ? normalizeBranchName(branch) : ''
|
||||
if (!normalized) {
|
||||
|
|
@ -50,6 +51,11 @@ export function syncPRChecksStatus(
|
|||
if (!prEntry?.data) {
|
||||
return null
|
||||
}
|
||||
// Why: fork PR rediscovery can retarget the branch cache while an older
|
||||
// checks request is still in flight; only the matching PR repo may update it.
|
||||
if (!samePRRepo(prEntry.data.prRepo, prRepo)) {
|
||||
return null
|
||||
}
|
||||
|
||||
const nextStatus = deriveCheckStatusFromChecks(checks)
|
||||
if (prEntry.data.checksStatus === nextStatus) {
|
||||
|
|
@ -69,3 +75,14 @@ export function syncPRChecksStatus(
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
function normalizedPRRepo(repo?: GitHubOwnerRepo | null): string | null {
|
||||
if (!repo) {
|
||||
return null
|
||||
}
|
||||
return `${repo.owner.toLowerCase()}/${repo.repo.toLowerCase()}`
|
||||
}
|
||||
|
||||
function samePRRepo(left?: GitHubOwnerRepo | null, right?: GitHubOwnerRepo | null): boolean {
|
||||
return normalizedPRRepo(left) === normalizedPRRepo(right)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ const mockApi = {
|
|||
prForBranch: vi.fn().mockResolvedValue(null),
|
||||
issue: vi.fn().mockResolvedValue(null),
|
||||
prChecks: vi.fn().mockResolvedValue([]),
|
||||
prComments: vi.fn().mockResolvedValue([]),
|
||||
listWorkItems: vi.fn(),
|
||||
getProjectViewTable: vi.fn()
|
||||
},
|
||||
|
|
@ -194,7 +195,9 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
|||
{ name: 'lint', status: 'completed', conclusion: 'success', url: null }
|
||||
])
|
||||
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, undefined, { force: true, repoId })
|
||||
await store
|
||||
.getState()
|
||||
.fetchPRChecks(repoPath, 12, branch, undefined, null, { force: true, repoId })
|
||||
|
||||
expect(store.getState().prCache[prCacheKey]?.data?.checksStatus).toBe('success')
|
||||
})
|
||||
|
|
@ -220,7 +223,9 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
|||
{ name: 'integration', status: 'completed', conclusion: 'failure', url: null }
|
||||
])
|
||||
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, undefined, { force: true, repoId })
|
||||
await store
|
||||
.getState()
|
||||
.fetchPRChecks(repoPath, 12, branch, undefined, null, { force: true, repoId })
|
||||
|
||||
expect(store.getState().prCache[prCacheKey]?.data?.checksStatus).toBe('failure')
|
||||
})
|
||||
|
|
@ -247,7 +252,7 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
|||
|
||||
await store
|
||||
.getState()
|
||||
.fetchPRChecks(repoPath, 12, `refs/heads/${branch}`, undefined, { force: true, repoId })
|
||||
.fetchPRChecks(repoPath, 12, `refs/heads/${branch}`, undefined, null, { force: true, repoId })
|
||||
|
||||
expect(store.getState().prCache[prCacheKey]?.data?.checksStatus).toBe('success')
|
||||
})
|
||||
|
|
@ -274,7 +279,9 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
|||
{ name: 'build', status: 'completed', conclusion: 'success', url: null }
|
||||
])
|
||||
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, undefined, { force: true, repoId })
|
||||
await store
|
||||
.getState()
|
||||
.fetchPRChecks(repoPath, 12, branch, undefined, null, { force: true, repoId })
|
||||
await vi.advanceTimersByTimeAsync(1000)
|
||||
|
||||
expect(mockApi.cache.setGitHub).toHaveBeenCalledWith({
|
||||
|
|
@ -310,7 +317,7 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
|||
}
|
||||
})
|
||||
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, undefined, { repoId })
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, undefined, null, { repoId })
|
||||
await vi.advanceTimersByTimeAsync(1000)
|
||||
|
||||
expect(mockApi.gh.prChecks).not.toHaveBeenCalled()
|
||||
|
|
@ -341,17 +348,109 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
|||
|
||||
await store
|
||||
.getState()
|
||||
.fetchPRChecks(repoPath, 12, branch, 'abc123head', { force: true, repoId })
|
||||
.fetchPRChecks(repoPath, 12, branch, 'abc123head', null, { force: true, repoId })
|
||||
|
||||
expect(mockApi.gh.prChecks).toHaveBeenCalledWith({
|
||||
repoPath,
|
||||
repoId,
|
||||
prNumber: 12,
|
||||
headSha: 'abc123head',
|
||||
prRepo: null,
|
||||
noCache: true
|
||||
})
|
||||
})
|
||||
|
||||
it('keys PR checks by normalized PR repo identity', async () => {
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
const repoId = 'repo-id'
|
||||
const branch = 'feature/test'
|
||||
|
||||
mockApi.gh.prChecks
|
||||
.mockResolvedValueOnce([
|
||||
{ name: 'upstream', status: 'completed', conclusion: 'success', url: null }
|
||||
])
|
||||
.mockResolvedValueOnce([
|
||||
{ name: 'fork', status: 'completed', conclusion: 'failure', url: null }
|
||||
])
|
||||
|
||||
await store
|
||||
.getState()
|
||||
.fetchPRChecks(
|
||||
repoPath,
|
||||
12,
|
||||
branch,
|
||||
'head-a',
|
||||
{ owner: 'Acme', repo: 'Widgets' },
|
||||
{ force: true, repoId }
|
||||
)
|
||||
await store
|
||||
.getState()
|
||||
.fetchPRChecks(
|
||||
repoPath,
|
||||
12,
|
||||
branch,
|
||||
'head-b',
|
||||
{ owner: 'Fork', repo: 'Widgets' },
|
||||
{ force: true, repoId }
|
||||
)
|
||||
|
||||
expect(
|
||||
store.getState().checksCache[`${repoId}::pr-checks::acme/widgets::12`]?.data?.[0].name
|
||||
).toBe('upstream')
|
||||
expect(
|
||||
store.getState().checksCache[`${repoId}::pr-checks::fork/widgets::12`]?.data?.[0].name
|
||||
).toBe('fork')
|
||||
expect(mockApi.gh.prChecks).toHaveBeenNthCalledWith(1, {
|
||||
repoPath,
|
||||
repoId,
|
||||
prNumber: 12,
|
||||
headSha: 'head-a',
|
||||
prRepo: { owner: 'Acme', repo: 'Widgets' },
|
||||
noCache: true
|
||||
})
|
||||
})
|
||||
|
||||
it('does not sync stale checks into a PR cache entry for a different PR repo', async () => {
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
const repoId = 'repo-id'
|
||||
const branch = 'feature/test'
|
||||
const prCacheKey = `${repoId}::${branch}`
|
||||
|
||||
store.setState({
|
||||
prCache: {
|
||||
[prCacheKey]: {
|
||||
data: makePR({
|
||||
checksStatus: 'pending',
|
||||
prRepo: { owner: 'Fork', repo: 'Widgets' }
|
||||
}),
|
||||
fetchedAt: 1
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
mockApi.gh.prChecks.mockResolvedValue([
|
||||
{ name: 'build', status: 'completed', conclusion: 'success', url: null }
|
||||
])
|
||||
|
||||
await store
|
||||
.getState()
|
||||
.fetchPRChecks(
|
||||
repoPath,
|
||||
12,
|
||||
branch,
|
||||
'head-a',
|
||||
{ owner: 'Acme', repo: 'Widgets' },
|
||||
{ force: true, repoId }
|
||||
)
|
||||
|
||||
expect(store.getState().prCache[prCacheKey]?.data?.checksStatus).toBe('pending')
|
||||
expect(
|
||||
store.getState().checksCache[`${repoId}::pr-checks::acme/widgets::12`]?.data?.[0].name
|
||||
).toBe('build')
|
||||
})
|
||||
|
||||
it('updates repo-scoped PR cache entry instead of repoPath fallback key', async () => {
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
|
|
@ -371,13 +470,62 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
|||
{ name: 'build', status: 'completed', conclusion: 'success', url: null }
|
||||
])
|
||||
|
||||
await store.getState().fetchPRChecks(repoPath, 12, branch, undefined, { force: true, repoId })
|
||||
await store
|
||||
.getState()
|
||||
.fetchPRChecks(repoPath, 12, branch, undefined, null, { force: true, repoId })
|
||||
|
||||
expect(store.getState().prCache[repoScopedKey]?.data?.checksStatus).toBe('success')
|
||||
expect(store.getState().prCache[pathScopedKey]?.data?.checksStatus).toBe('pending')
|
||||
})
|
||||
})
|
||||
|
||||
describe('createGitHubSlice.fetchPRComments', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
resetRemoteRuntimeMocks()
|
||||
mockApi.gh.prComments.mockResolvedValue([])
|
||||
})
|
||||
|
||||
it('keys PR comments by normalized PR repo identity', async () => {
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
const repoId = 'repo-id'
|
||||
|
||||
mockApi.gh.prComments
|
||||
.mockResolvedValueOnce([
|
||||
{ id: 1, author: 'upstream', authorAvatarUrl: '', body: '', createdAt: '', url: '' }
|
||||
])
|
||||
.mockResolvedValueOnce([
|
||||
{ id: 2, author: 'fork', authorAvatarUrl: '', body: '', createdAt: '', url: '' }
|
||||
])
|
||||
|
||||
await store.getState().fetchPRComments(repoPath, 12, {
|
||||
force: true,
|
||||
repoId,
|
||||
prRepo: { owner: 'Acme', repo: 'Widgets' }
|
||||
})
|
||||
await store.getState().fetchPRComments(repoPath, 12, {
|
||||
force: true,
|
||||
repoId,
|
||||
prRepo: { owner: 'Fork', repo: 'Widgets' }
|
||||
})
|
||||
|
||||
expect(
|
||||
store.getState().commentsCache[`${repoId}::pr-comments::acme/widgets::12`]?.data?.[0].author
|
||||
).toBe('upstream')
|
||||
expect(
|
||||
store.getState().commentsCache[`${repoId}::pr-comments::fork/widgets::12`]?.data?.[0].author
|
||||
).toBe('fork')
|
||||
expect(mockApi.gh.prComments).toHaveBeenNthCalledWith(1, {
|
||||
repoPath,
|
||||
repoId,
|
||||
prNumber: 12,
|
||||
prRepo: { owner: 'Acme', repo: 'Widgets' },
|
||||
noCache: true
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('createGitHubSlice.fetchPRForBranch', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
|
|
|
|||
|
|
@ -393,6 +393,24 @@ function evictRepoCacheEntries<T>(
|
|||
return next ? { cache: next, evicted: true } : { cache, evicted: false }
|
||||
}
|
||||
|
||||
function normalizedRepoIdentity(repo: GitHubOwnerRepo): string {
|
||||
return `${repo.owner.toLowerCase()}/${repo.repo.toLowerCase()}`
|
||||
}
|
||||
|
||||
export function prChecksCacheSuffix(prNumber: number, prRepo?: GitHubOwnerRepo | null): string {
|
||||
if (!prRepo) {
|
||||
return `pr-checks::${prNumber}`
|
||||
}
|
||||
return `pr-checks::${normalizedRepoIdentity(prRepo)}::${prNumber}`
|
||||
}
|
||||
|
||||
export function prCommentsCacheSuffix(prNumber: number, prRepo?: GitHubOwnerRepo | null): string {
|
||||
if (!prRepo) {
|
||||
return `pr-comments::${prNumber}`
|
||||
}
|
||||
return `pr-comments::${normalizedRepoIdentity(prRepo)}::${prNumber}`
|
||||
}
|
||||
|
||||
// Why: 500 entries is generous enough that active developers will never hit it
|
||||
// during normal use, but prevents the cache from growing without bound across
|
||||
// many repos and branches over a long-running session.
|
||||
|
|
@ -467,19 +485,20 @@ export type GitHubSlice = {
|
|||
prNumber: number,
|
||||
branch?: string,
|
||||
headSha?: string,
|
||||
prRepo?: GitHubOwnerRepo | null,
|
||||
options?: RepoScopedFetchOptions
|
||||
) => Promise<PRCheckDetail[]>
|
||||
fetchPRComments: (
|
||||
repoPath: string,
|
||||
prNumber: number,
|
||||
options?: RepoScopedFetchOptions
|
||||
options?: RepoScopedFetchOptions & { prRepo?: GitHubOwnerRepo | null }
|
||||
) => Promise<PRComment[]>
|
||||
resolveReviewThread: (
|
||||
repoPath: string,
|
||||
prNumber: number,
|
||||
threadId: string,
|
||||
resolve: boolean,
|
||||
options?: RepoScopedFetchOptions
|
||||
options?: RepoScopedFetchOptions & { prRepo?: GitHubOwnerRepo | null }
|
||||
) => Promise<boolean>
|
||||
initGitHubCache: () => Promise<void>
|
||||
refreshAllGitHub: () => void
|
||||
|
|
@ -1397,13 +1416,27 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
|||
return request
|
||||
},
|
||||
|
||||
fetchPRChecks: async (repoPath, prNumber, branch, headSha, options): Promise<PRCheckDetail[]> => {
|
||||
fetchPRChecks: async (
|
||||
repoPath,
|
||||
prNumber,
|
||||
branch,
|
||||
headSha,
|
||||
prRepo,
|
||||
options
|
||||
): Promise<PRCheckDetail[]> => {
|
||||
const repoId = options?.repoId ?? get().repos?.find((repo) => repo.path === repoPath)?.id
|
||||
const cacheKey = repoScopedCacheKey(repoPath, repoId, `pr-checks::${prNumber}`)
|
||||
const cacheKey = repoScopedCacheKey(repoPath, repoId, prChecksCacheSuffix(prNumber, prRepo))
|
||||
const cached = get().checksCache[cacheKey]
|
||||
if (!options?.force && isFresh(cached, CHECKS_CACHE_TTL)) {
|
||||
const cachedChecks = cached.data ?? []
|
||||
const prStatusUpdate = syncPRChecksStatus(get(), repoPath, repoId, branch, cachedChecks)
|
||||
const prStatusUpdate = syncPRChecksStatus(
|
||||
get(),
|
||||
repoPath,
|
||||
repoId,
|
||||
branch,
|
||||
cachedChecks,
|
||||
prRepo
|
||||
)
|
||||
if (prStatusUpdate) {
|
||||
set(prStatusUpdate)
|
||||
debouncedSaveCache(get())
|
||||
|
|
@ -1423,6 +1456,7 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
|||
repoId,
|
||||
prNumber,
|
||||
headSha,
|
||||
prRepo: prRepo ?? null,
|
||||
noCache: options?.force
|
||||
})) as PRCheckDetail[]
|
||||
set((s) => {
|
||||
|
|
@ -1430,7 +1464,7 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
|||
checksCache: { ...s.checksCache, [cacheKey]: { data: checks, fetchedAt: Date.now() } }
|
||||
}
|
||||
|
||||
const prStatusUpdate = syncPRChecksStatus(s, repoPath, repoId, branch, checks)
|
||||
const prStatusUpdate = syncPRChecksStatus(s, repoPath, repoId, branch, checks, prRepo)
|
||||
if (prStatusUpdate?.prCache) {
|
||||
nextState.prCache = prStatusUpdate.prCache
|
||||
}
|
||||
|
|
@ -1453,7 +1487,11 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
|||
|
||||
fetchPRComments: async (repoPath, prNumber, options): Promise<PRComment[]> => {
|
||||
const repoId = options?.repoId ?? get().repos?.find((repo) => repo.path === repoPath)?.id
|
||||
const cacheKey = repoScopedCacheKey(repoPath, repoId, `pr-comments::${prNumber}`)
|
||||
const cacheKey = repoScopedCacheKey(
|
||||
repoPath,
|
||||
repoId,
|
||||
prCommentsCacheSuffix(prNumber, options?.prRepo)
|
||||
)
|
||||
const cached = get().commentsCache[cacheKey]
|
||||
if (!options?.force && isFresh(cached)) {
|
||||
return cached.data ?? []
|
||||
|
|
@ -1470,6 +1508,7 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
|||
repoPath,
|
||||
repoId,
|
||||
prNumber,
|
||||
prRepo: options?.prRepo ?? null,
|
||||
noCache: options?.force
|
||||
})) as PRComment[]
|
||||
set((s) => ({
|
||||
|
|
@ -1493,7 +1532,11 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
|||
|
||||
resolveReviewThread: async (repoPath, prNumber, threadId, resolve, options) => {
|
||||
const repoId = options?.repoId ?? get().repos?.find((repo) => repo.path === repoPath)?.id
|
||||
const cacheKey = repoScopedCacheKey(repoPath, repoId, `pr-comments::${prNumber}`)
|
||||
const cacheKey = repoScopedCacheKey(
|
||||
repoPath,
|
||||
repoId,
|
||||
prCommentsCacheSuffix(prNumber, options?.prRepo)
|
||||
)
|
||||
|
||||
// Optimistic update: toggle isResolved on all comments in this thread immediately
|
||||
// so the UI feels instant. Reverts if the API call fails.
|
||||
|
|
|
|||
|
|
@ -585,6 +585,8 @@ export type PRConflictSummary = {
|
|||
files: string[]
|
||||
}
|
||||
|
||||
export type GitHubRepositoryIdentity = { owner: string; repo: string }
|
||||
|
||||
export type PRInfo = {
|
||||
number: number
|
||||
title: string
|
||||
|
|
@ -597,6 +599,8 @@ export type PRInfo = {
|
|||
// Keeping the head SHA in cached PR metadata lets the checks panel poll the
|
||||
// correct commit without re-querying GitHub or guessing from local branch refs.
|
||||
headSha?: string
|
||||
prRepo?: GitHubRepositoryIdentity
|
||||
headRepo?: GitHubRepositoryIdentity
|
||||
conflictSummary?: PRConflictSummary
|
||||
}
|
||||
|
||||
|
|
@ -904,7 +908,7 @@ export type ClassifiedError = {
|
|||
// slices can reference the same structural type without importing from main.
|
||||
// Aliased as `OwnerRepo` in `src/main/github/gh-utils.ts` so main call sites
|
||||
// can continue using the short local name.
|
||||
export type GitHubOwnerRepo = { owner: string; repo: string }
|
||||
export type GitHubOwnerRepo = GitHubRepositoryIdentity
|
||||
|
||||
// Why: GitLab-specific types live in `./gitlab-types` so they can grow
|
||||
// independently from the central types file (which is touched by every
|
||||
|
|
|
|||
Loading…
Reference in New Issue