mirror of
https://github.com/stablyai/orca.git
synced 2026-11-04 02:57:36 +00:00
fix(github): validate implicit default-branch PR repository identity
This commit is contained in:
@@ -110,7 +110,7 @@ describe('GitHub GraphQL rate-limit guard', () => {
|
||||
'--repo',
|
||||
'stablyai/orca',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root', host: 'github.com' }
|
||||
)
|
||||
|
||||
@@ -188,7 +188,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
|
||||
@@ -126,7 +126,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
@@ -162,7 +162,7 @@ describe('getPRForBranch', () => {
|
||||
'--limit',
|
||||
'1',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
@@ -308,7 +308,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'stablyai/orca',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
@@ -381,7 +381,7 @@ describe('getPRForBranch', () => {
|
||||
'--limit',
|
||||
'1',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
@@ -507,7 +507,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
|
||||
@@ -80,7 +80,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'stablyai/orca',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
@@ -93,7 +93,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'fork/orca',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
@@ -132,7 +132,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
@@ -411,7 +411,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
@@ -502,7 +502,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
@@ -543,7 +543,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
@@ -579,7 +579,7 @@ describe('getPRForBranch', () => {
|
||||
'view',
|
||||
'77',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/non-github-repo' }
|
||||
)
|
||||
@@ -618,7 +618,7 @@ describe('getPRForBranch', () => {
|
||||
'view',
|
||||
'feature/test',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/non-github-repo' }
|
||||
)
|
||||
@@ -658,7 +658,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
|
||||
@@ -1,9 +1,4 @@
|
||||
/*
|
||||
* Issue #26948: a branch created with `git switch -c feature/x origin/develop`
|
||||
* tracks the default branch. Its PR lookup misses, falls back to the tracked
|
||||
* upstream, and attaches the newest PR whose head is `develop` (an unrelated
|
||||
* develop -> release PR), then caches that number so the link survives restarts.
|
||||
*/
|
||||
// #26948: tracking a base branch must not attach its integration PR to a feature branch.
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type * as GithubApiRepositoryModule from './github-api-repository'
|
||||
import type * as GitHubEnterpriseRepositoryModule from './github-enterprise-repository'
|
||||
@@ -35,6 +30,8 @@ vi.mock('./github-api-repository', async (importOriginal) =>
|
||||
import { getPRForBranch } from './client/lookup/get-pr-for-branch'
|
||||
import { getPRForBranchOutcome } from './client/lookup/pr-for-branch-outcome'
|
||||
import { resetPRForBranchMocks } from './client-test-harness'
|
||||
import type { RestPullRequest } from './client/lookup/pull-request-lookup-data'
|
||||
import type { OwnerRepo } from './gh-utils'
|
||||
|
||||
const {
|
||||
ghExecFileAsyncMock,
|
||||
@@ -45,6 +42,8 @@ const {
|
||||
} = clientMocks
|
||||
|
||||
const ACME = { owner: 'acme', repo: 'widgets' }
|
||||
const UPSTREAM = { owner: 'stablyai', repo: 'widgets' }
|
||||
const CONTRIBUTOR = { owner: 'contributor', repo: 'widgets' }
|
||||
|
||||
/** remoteHeads maps a remote to the branch its `refs/remotes/<remote>/HEAD` points at. */
|
||||
function primeGit(
|
||||
@@ -71,43 +70,59 @@ function primeGit(
|
||||
})
|
||||
}
|
||||
|
||||
function restPR(headOwnerRef: string): Record<string, unknown> {
|
||||
function restPR(
|
||||
headRefName = 'develop',
|
||||
headRepo: OwnerRepo = ACME,
|
||||
prRepo: OwnerRepo = ACME
|
||||
): RestPullRequest {
|
||||
return {
|
||||
number: 7,
|
||||
title: 'Release develop',
|
||||
state: 'closed',
|
||||
merged_at: null,
|
||||
html_url: 'https://github.com/acme/widgets/pull/7',
|
||||
html_url: `https://github.com/${prRepo.owner}/${prRepo.repo}/pull/7`,
|
||||
updated_at: '2025-01-01T00:00:00Z',
|
||||
draft: false,
|
||||
mergeable: null,
|
||||
base: { ref: 'release', sha: 'release-oid' },
|
||||
head: { ref: headOwnerRef, sha: 'old-develop-oid' }
|
||||
head: {
|
||||
ref: headRefName,
|
||||
sha: 'old-develop-oid',
|
||||
repo: { name: headRepo.repo, owner: { login: headRepo.owner } }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Branch-list responses keyed by the `head=` filter; every other gh call fails. */
|
||||
function primeGh(listsByHead: Record<string, Record<string, unknown>[]>): void {
|
||||
function primeGh(
|
||||
listsByHead: Record<string, RestPullRequest[]>,
|
||||
exactPR: RestPullRequest = Object.values(listsByHead).flat()[0] ?? restPR()
|
||||
): void {
|
||||
const prsByNumber = new Map(
|
||||
[...Object.values(listsByHead).flat(), exactPR].map((pr) => [pr.number, pr])
|
||||
)
|
||||
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
const head = args[1]?.match(/pulls\?head=([^&]+)/)?.[1]
|
||||
if (args[0] === 'api' && head) {
|
||||
return { stdout: JSON.stringify(listsByHead[decodeURIComponent(head)] ?? []) }
|
||||
}
|
||||
if (args[0] === 'pr' && args[1] === 'view' && args[2] === '7') {
|
||||
const exact = prsByNumber.get(Number(args[2]))
|
||||
if (args[0] === 'pr' && args[1] === 'view' && exact) {
|
||||
return {
|
||||
stdout: JSON.stringify({
|
||||
number: 7,
|
||||
title: 'Release develop',
|
||||
state: 'CLOSED',
|
||||
url: 'https://github.com/acme/widgets/pull/7',
|
||||
number: exact.number,
|
||||
title: exact.title,
|
||||
state: exact.merged_at ? 'MERGED' : exact.state.toUpperCase(),
|
||||
url: exact.html_url,
|
||||
statusCheckRollup: [],
|
||||
updatedAt: '2025-01-01T00:00:00Z',
|
||||
isDraft: false,
|
||||
updatedAt: exact.updated_at,
|
||||
isDraft: exact.draft,
|
||||
mergeable: 'UNKNOWN',
|
||||
baseRefName: 'release',
|
||||
headRefName: 'develop',
|
||||
baseRefOid: 'release-oid',
|
||||
headRefOid: 'old-develop-oid'
|
||||
baseRefName: exact.base?.ref,
|
||||
headRefName: exact.head?.ref,
|
||||
baseRefOid: exact.base?.sha,
|
||||
headRefOid: exact.head?.sha,
|
||||
headRepositoryOwner: exact.head?.repo?.owner,
|
||||
headRepository: exact.head?.repo ? { name: exact.head.repo.name } : null
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -117,7 +132,6 @@ function primeGh(listsByHead: Record<string, Record<string, unknown>[]>): void {
|
||||
|
||||
/** A fork clone: `origin` is the fork, `upstream` is the repo PRs target. */
|
||||
function primeForkWithUpstream(): void {
|
||||
const UPSTREAM = { owner: 'stablyai', repo: 'widgets' }
|
||||
resolvePRRepositoryCandidatesMock.mockResolvedValue({
|
||||
candidates: [UPSTREAM, ACME],
|
||||
headRepo: ACME
|
||||
@@ -153,8 +167,8 @@ describe('issue #26948: a branch tracking the default branch', () => {
|
||||
|
||||
it('skips the default branch on a second remote that PRs target (fork checkout off upstream)', async () => {
|
||||
primeForkWithUpstream()
|
||||
primeGit('refs/remotes/upstream/develop')
|
||||
primeGh({ 'stablyai:develop': [restPR('develop')] })
|
||||
primeGit('refs/remotes/upstream/develop', { origin: 'develop', upstream: 'develop' })
|
||||
primeGh({ 'stablyai:develop': [restPR('develop', UPSTREAM, UPSTREAM)] })
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'feature/my-change')
|
||||
|
||||
@@ -164,7 +178,7 @@ describe('issue #26948: a branch tracking the default branch', () => {
|
||||
it("uses the tracked remote's own default branch when it differs from origin's", async () => {
|
||||
primeForkWithUpstream()
|
||||
primeGit('refs/remotes/upstream/develop', { origin: 'main', upstream: 'develop' })
|
||||
primeGh({ 'stablyai:develop': [restPR('develop')] })
|
||||
primeGh({ 'stablyai:develop': [restPR('develop', UPSTREAM, UPSTREAM)] })
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'feature/my-change')
|
||||
|
||||
@@ -174,7 +188,7 @@ describe('issue #26948: a branch tracking the default branch', () => {
|
||||
it("keeps a PR headed by a tracked branch that is only origin's default", async () => {
|
||||
primeForkWithUpstream()
|
||||
primeGit('refs/remotes/upstream/develop', { origin: 'develop', upstream: 'main' })
|
||||
primeGh({ 'stablyai:develop': [restPR('develop')] })
|
||||
primeGh({ 'stablyai:develop': [restPR('develop', UPSTREAM, UPSTREAM)] })
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'feature/my-change')
|
||||
|
||||
@@ -186,10 +200,100 @@ describe('issue #26948: a branch tracking the default branch', () => {
|
||||
remoteName === 'contributor' ? { owner: 'contributor', repo: 'widgets' } : ACME
|
||||
)
|
||||
primeGit('refs/remotes/contributor/develop')
|
||||
primeGh({ 'contributor:develop': [{ ...restPR('develop'), state: 'open' }] })
|
||||
primeGh({ 'contributor:develop': [{ ...restPR('develop', CONTRIBUTOR), state: 'open' }] })
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'feature/my-change')
|
||||
|
||||
expect(pr).toMatchObject({ number: 7, headRepo: { owner: 'contributor', repo: 'widgets' } })
|
||||
})
|
||||
|
||||
it('preserves a cached contributor PR sharing the tracked default branch name', async () => {
|
||||
primeGit('refs/remotes/origin/develop')
|
||||
primeGh({}, { ...restPR('develop', CONTRIBUTOR), state: 'open' })
|
||||
const pr = await getPRForBranch('/repo-root', 'feature/my-change', null, null, 7)
|
||||
expect(pr).toMatchObject({ number: 7, headRepo: CONTRIBUTOR })
|
||||
})
|
||||
|
||||
it('preserves a PR from the origin fork default branch into upstream', async () => {
|
||||
primeForkWithUpstream()
|
||||
primeGit('refs/remotes/origin/develop')
|
||||
primeGh({ 'acme:develop': [{ ...restPR('develop', ACME, UPSTREAM), state: 'open' }] })
|
||||
const pr = await getPRForBranch('/repo-root', 'feature/my-change')
|
||||
expect(pr).toMatchObject({ number: 7, headRepo: ACME, prRepo: UPSTREAM })
|
||||
})
|
||||
|
||||
it('clears an old cached integration PR after upstream tracking is removed', async () => {
|
||||
primeGit('')
|
||||
primeGh({})
|
||||
expect(
|
||||
(await getPRForBranchOutcome('/repo-root', 'feature/my-change', null, null, 7)).kind
|
||||
).toBe('no-pr')
|
||||
})
|
||||
|
||||
it('tries the cached feature PR after rejecting an unrelated upstream result', async () => {
|
||||
primeGit('refs/remotes/origin/develop')
|
||||
primeGh(
|
||||
{ 'acme:develop': [restPR()] },
|
||||
{ ...restPR('feature/my-change'), number: 8, state: 'open' }
|
||||
)
|
||||
const pr = await getPRForBranch('/repo-root', 'feature/my-change', null, null, 8)
|
||||
expect(pr).toMatchObject({ number: 8, headRefName: 'feature/my-change' })
|
||||
})
|
||||
|
||||
it('does not borrow origin default when upstream HEAD is unknown', async () => {
|
||||
primeForkWithUpstream()
|
||||
primeGit('refs/remotes/upstream/develop', { origin: 'develop' })
|
||||
primeGh({ 'stablyai:develop': [restPR('develop', UPSTREAM, UPSTREAM)] })
|
||||
expect(await getPRForBranch('/repo-root', 'feature/my-change')).toMatchObject({ number: 7 })
|
||||
})
|
||||
|
||||
it('uses PR head default metadata when upstream has no recorded HEAD', async () => {
|
||||
primeForkWithUpstream()
|
||||
primeGit('refs/remotes/upstream/develop', { origin: 'main' })
|
||||
const pr = restPR('develop', UPSTREAM, UPSTREAM)
|
||||
primeGh({
|
||||
'stablyai:develop': [
|
||||
{
|
||||
...pr,
|
||||
head: {
|
||||
...pr.head,
|
||||
repo: { name: 'widgets', owner: { login: 'stablyai' }, default_branch: 'develop' }
|
||||
}
|
||||
}
|
||||
]
|
||||
})
|
||||
expect(await getPRForBranch('/repo-root', 'feature/my-change')).toBeNull()
|
||||
})
|
||||
|
||||
it('preserves explicit links to integration PRs', async () => {
|
||||
primeGit('refs/remotes/origin/develop')
|
||||
primeGh({})
|
||||
expect(await getPRForBranch('/repo-root', 'feature/my-change', 7)).toMatchObject({ number: 7 })
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('preserves cached review context while detached', async () => {
|
||||
primeGh({})
|
||||
expect(await getPRForBranch('/repo-root', '', null, null, 7)).toMatchObject({ number: 7 })
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not infer a head repository for a cached PR with unavailable fork metadata', async () => {
|
||||
primeGit('refs/remotes/origin/develop')
|
||||
const pr = restPR()
|
||||
primeGh({}, { ...pr, head: { ref: 'develop', sha: 'deleted-fork-oid', repo: null } })
|
||||
expect(await getPRForBranch('/repo-root', 'feature/my-change', null, null, 7)).toMatchObject({
|
||||
number: 7
|
||||
})
|
||||
})
|
||||
|
||||
it('does not fetch the rejected integration PR twice when its number is cached', async () => {
|
||||
primeGit('refs/remotes/origin/develop')
|
||||
primeGh({ 'acme:develop': [restPR()] })
|
||||
expect(await getPRForBranch('/repo-root', 'feature/my-change', null, null, 7)).toBeNull()
|
||||
const exactCalls = ghExecFileAsyncMock.mock.calls.filter(
|
||||
([args]) => args[0] === 'pr' && args[1] === 'view'
|
||||
)
|
||||
expect(exactCalls).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -113,7 +113,7 @@ describe('getPRForBranch', () => {
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--json',
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
|
||||
@@ -17,6 +17,8 @@ import {
|
||||
isMergedImplicitPR,
|
||||
shouldHideMergedImplicitPR,
|
||||
getCurrentHeadOid,
|
||||
pullRequestHeadRepository,
|
||||
linkedMergedPRDivergedHeadOid,
|
||||
type PullRequestLookupData,
|
||||
type GitHubPRBranchLookupOptions
|
||||
} from './pull-request-lookup-data'
|
||||
@@ -29,7 +31,7 @@ import {
|
||||
type TrackedUpstreamBranch
|
||||
} from './tracked-upstream-cache'
|
||||
import { getTrackedUpstreamBranch } from './tracked-upstream-branch'
|
||||
import { isTrackedUpstreamDefaultBranch } from './tracked-upstream-default-branch'
|
||||
import { shouldHideImplicitDefaultBranchPR } from './implicit-default-branch-pr'
|
||||
import { PR_BRANCH_LOOKUP_BUCKETS } from './pr-lookup-rate-limit'
|
||||
import type { HostedReviewLocalGitOptions } from './../github-exec-scope'
|
||||
export async function resolvePRForBranchOutcome(input: {
|
||||
@@ -105,29 +107,6 @@ export async function resolvePRForBranchOutcome(input: {
|
||||
}
|
||||
return membership
|
||||
}
|
||||
const recordLinkedMergedPRDivergence = async (
|
||||
candidate: PullRequestLookupData | null,
|
||||
candidateRepo: OwnerRepo | null
|
||||
): Promise<void> => {
|
||||
if (
|
||||
typeof linkedPRNumber !== 'number' ||
|
||||
!candidate ||
|
||||
mapPRState(candidate.state, candidate.isDraft) !== 'merged' ||
|
||||
explicitCurrentHeadOid === null ||
|
||||
candidate.headRefOid === explicitCurrentHeadOid
|
||||
) {
|
||||
return
|
||||
}
|
||||
const membership = await mergedPRContainsHead(
|
||||
candidate,
|
||||
candidateRepo ?? ownerRepoFromPullRequestUrl(candidate.url),
|
||||
explicitCurrentHeadOid
|
||||
)
|
||||
if (membership === 'not-contained') {
|
||||
// explicitCurrentHeadOid is non-null here (guarded above); record the exact diverged head so consumers clear only that worktree.
|
||||
headDivergedFromMergedPRAtOid = explicitCurrentHeadOid
|
||||
}
|
||||
}
|
||||
const hideMergedImplicitPR = async (
|
||||
candidate: PullRequestLookupData | null,
|
||||
candidateRepo: OwnerRepo | null
|
||||
@@ -216,6 +195,29 @@ export async function resolvePRForBranchOutcome(input: {
|
||||
}
|
||||
}
|
||||
}
|
||||
let rejectedDefaultPRNumber: number | null = null
|
||||
const hideUnrelatedDefaultPR = async (): Promise<void> => {
|
||||
if (
|
||||
data &&
|
||||
(await shouldHideImplicitDefaultBranchPR({
|
||||
data,
|
||||
prRepo: dataRepo,
|
||||
originHeadRepo: headRepo,
|
||||
trackedUpstream: retriedUpstream,
|
||||
branchName,
|
||||
linkedPRNumber,
|
||||
repoPath,
|
||||
connectionId,
|
||||
localGitOptions
|
||||
}))
|
||||
) {
|
||||
rejectedDefaultPRNumber = data.number
|
||||
data = null
|
||||
dataRepo = null
|
||||
dataHeadRepo = headRepo
|
||||
}
|
||||
}
|
||||
await hideUnrelatedDefaultPR()
|
||||
let mergedBranchLookupNumber: number | null = null
|
||||
if (await hideMergedImplicitPR(data, dataRepo)) {
|
||||
mergedBranchLookupNumber = data?.number ?? null
|
||||
@@ -223,7 +225,12 @@ export async function resolvePRForBranchOutcome(input: {
|
||||
dataRepo = null
|
||||
dataHeadRepo = headRepo
|
||||
}
|
||||
if (!data && typeof linkedPRNumber !== 'number' && typeof fallbackPRNumber === 'number') {
|
||||
if (
|
||||
!data &&
|
||||
typeof linkedPRNumber !== 'number' &&
|
||||
typeof fallbackPRNumber === 'number' &&
|
||||
fallbackPRNumber !== rejectedDefaultPRNumber
|
||||
) {
|
||||
usedExactNumberLookup = true
|
||||
const fallbackLookup = await lookupPRByNumber({
|
||||
candidates,
|
||||
@@ -234,30 +241,26 @@ export async function resolvePRForBranchOutcome(input: {
|
||||
data = fallbackLookup.data
|
||||
dataRepo = fallbackLookup.dataRepo
|
||||
}
|
||||
// Why: covers the cached fallback number too, so a link stored before #26948 heals.
|
||||
if (
|
||||
data &&
|
||||
retriedUpstream &&
|
||||
data.headRefName === retriedUpstream.branch.branchName &&
|
||||
(await isTrackedUpstreamDefaultBranch({
|
||||
upstreamBranch: retriedUpstream.branch,
|
||||
upstreamHeadRepo: retriedUpstream.headRepo,
|
||||
candidates,
|
||||
repoPath,
|
||||
connectionId,
|
||||
localGitOptions
|
||||
}))
|
||||
) {
|
||||
data = null
|
||||
dataRepo = null
|
||||
}
|
||||
// Recheck exact fallbacks so links cached before #26948 heal without upstream tracking.
|
||||
await hideUnrelatedDefaultPR()
|
||||
if (!data) {
|
||||
if (hasPendingBranchLookupError) {
|
||||
return prRefreshUpstreamError(pendingBranchLookupError)
|
||||
}
|
||||
return { kind: 'no-pr', fetchedAt: Date.now() }
|
||||
}
|
||||
await recordLinkedMergedPRDivergence(data, dataRepo)
|
||||
dataHeadRepo = pullRequestHeadRepository(data, dataRepo) ?? dataHeadRepo
|
||||
headDivergedFromMergedPRAtOid = await linkedMergedPRDivergedHeadOid(
|
||||
data,
|
||||
linkedPRNumber,
|
||||
explicitCurrentHeadOid,
|
||||
(candidate, headOid) =>
|
||||
mergedPRContainsHead(
|
||||
candidate,
|
||||
dataRepo ?? ownerRepoFromPullRequestUrl(candidate.url),
|
||||
headOid
|
||||
)
|
||||
)
|
||||
const fallbackConfirmedMergedBranch =
|
||||
typeof fallbackPRNumber === 'number' &&
|
||||
mergedBranchLookupNumber === fallbackPRNumber &&
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
import type { OwnerRepo } from '../../gh-utils'
|
||||
import { githubRepoIdentityKey } from '../../../../shared/github/repository-identity-key'
|
||||
import {
|
||||
getRemoteHeadBranchName,
|
||||
getRepoDefaultBranchName
|
||||
} from '../../../source-control/repo-default-branch'
|
||||
import type { HostedReviewLocalGitOptions } from './../github-exec-scope'
|
||||
import type { TrackedUpstreamBranch } from './tracked-upstream-cache'
|
||||
import { pullRequestHeadRepository, type PullRequestLookupData } from './pull-request-lookup-data'
|
||||
|
||||
export async function shouldHideImplicitDefaultBranchPR(input: {
|
||||
data: PullRequestLookupData
|
||||
prRepo: OwnerRepo | null
|
||||
originHeadRepo: OwnerRepo | null
|
||||
trackedUpstream: { branch: TrackedUpstreamBranch; headRepo: OwnerRepo } | null
|
||||
branchName: string
|
||||
linkedPRNumber?: number | null
|
||||
repoPath: string
|
||||
connectionId?: string | null
|
||||
localGitOptions: HostedReviewLocalGitOptions
|
||||
}): Promise<boolean> {
|
||||
const { data, branchName, prRepo } = input
|
||||
if (
|
||||
!branchName ||
|
||||
!data.headRefName ||
|
||||
data.headRefName === branchName ||
|
||||
typeof input.linkedPRNumber === 'number'
|
||||
) {
|
||||
return false
|
||||
}
|
||||
const headRepo = pullRequestHeadRepository(data, prRepo)
|
||||
// Fork heads are real review branches even when their name is a default branch elsewhere.
|
||||
if (!headRepo || !prRepo || githubRepoIdentityKey(headRepo) !== githubRepoIdentityKey(prRepo)) {
|
||||
return false
|
||||
}
|
||||
if (data.headDefaultBranchName) {
|
||||
return data.headRefName === data.headDefaultBranchName
|
||||
}
|
||||
const headRepoKey = githubRepoIdentityKey(headRepo)
|
||||
const remoteName =
|
||||
input.trackedUpstream && githubRepoIdentityKey(input.trackedUpstream.headRepo) === headRepoKey
|
||||
? input.trackedUpstream.branch.remoteName
|
||||
: input.originHeadRepo && githubRepoIdentityKey(input.originHeadRepo) === headRepoKey
|
||||
? 'origin'
|
||||
: null
|
||||
if (!remoteName) {
|
||||
return false
|
||||
}
|
||||
// An unknown upstream HEAD cannot borrow a different repository's origin default.
|
||||
const defaultBranchName =
|
||||
remoteName === 'origin'
|
||||
? await getRepoDefaultBranchName(input.repoPath, input.connectionId, input.localGitOptions)
|
||||
: await getRemoteHeadBranchName(
|
||||
input.repoPath,
|
||||
remoteName,
|
||||
input.connectionId,
|
||||
input.localGitOptions
|
||||
)
|
||||
return defaultBranchName !== null && data.headRefName === defaultBranchName
|
||||
}
|
||||
@@ -4,10 +4,11 @@ import type {
|
||||
PRMergeableState,
|
||||
PRReviewDecision
|
||||
} from '../../../../shared/github/pull-request-types'
|
||||
import { gitExecFileAsync } from '../../gh-utils'
|
||||
import { gitExecFileAsync, type OwnerRepo } from '../../gh-utils'
|
||||
import type { GitAdmissionTier } from '../../../git/command-runner/git-exec-options'
|
||||
import type { HostedReviewExecutionOptions } from '../../../source-control/hosted-review-git-options'
|
||||
import { mapPRState } from '../../mappers'
|
||||
import type { MergedPRCommitMembership } from '../../merged-pr-commit-membership'
|
||||
import {
|
||||
normalizePRMergeable,
|
||||
normalizeReviewDecision,
|
||||
@@ -35,6 +36,9 @@ export type PullRequestLookupData = {
|
||||
headRefName?: string
|
||||
baseRefOid?: string
|
||||
headRefOid?: string
|
||||
headRepositoryOwner?: { login?: string } | null
|
||||
headRepository?: { name?: string } | null
|
||||
headDefaultBranchName?: string
|
||||
stack?: GitHubPRStack
|
||||
stackMetadataChecked?: boolean
|
||||
}
|
||||
@@ -51,7 +55,12 @@ export type RestPullRequest = {
|
||||
mergeable?: boolean | null
|
||||
mergeable_state?: string | null
|
||||
base?: { ref?: string; sha?: string }
|
||||
head?: { ref?: string; sha?: string }
|
||||
head?: {
|
||||
ref?: string
|
||||
sha?: string
|
||||
repo?: { name?: string; owner?: { login?: string }; default_branch?: string } | null
|
||||
user?: { login?: string }
|
||||
}
|
||||
stack?: {
|
||||
number?: number
|
||||
position?: number
|
||||
@@ -61,10 +70,10 @@ export type RestPullRequest = {
|
||||
}
|
||||
|
||||
export const PR_LOOKUP_JSON_FIELDS =
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
|
||||
export const PR_BRANCH_LIST_JSON_FIELDS =
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
|
||||
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid,headRepositoryOwner,headRepository'
|
||||
|
||||
export type GitHubPRBranchLookupOptions = HostedReviewExecutionOptions & {
|
||||
acceptMergedFallbackPR?: boolean
|
||||
@@ -118,11 +127,23 @@ export function mapRestPullRequest(pr: RestPullRequest): PullRequestLookupData {
|
||||
headRefName: pr.head?.ref,
|
||||
baseRefOid: pr.base?.sha,
|
||||
headRefOid: pr.head?.sha,
|
||||
headRepositoryOwner: pr.head?.repo?.owner ?? pr.head?.user,
|
||||
headRepository: pr.head?.repo,
|
||||
headDefaultBranchName: pr.head?.repo?.default_branch,
|
||||
stackMetadataChecked: true,
|
||||
...(stack ? { stack } : {})
|
||||
}
|
||||
}
|
||||
|
||||
export function pullRequestHeadRepository(
|
||||
data: PullRequestLookupData,
|
||||
prRepo: OwnerRepo | null
|
||||
): OwnerRepo | null {
|
||||
const owner = data.headRepositoryOwner?.login?.trim()
|
||||
const repo = data.headRepository?.name?.trim()
|
||||
return owner && repo ? { owner, repo, ...(prRepo?.host ? { host: prRepo.host } : {}) } : null
|
||||
}
|
||||
|
||||
export function isMergedImplicitPR(
|
||||
data: PullRequestLookupData,
|
||||
linkedPRNumber?: number | null
|
||||
@@ -143,6 +164,24 @@ export function shouldHideMergedImplicitPR(
|
||||
return !currentHeadOid || data.headRefOid !== currentHeadOid
|
||||
}
|
||||
|
||||
export async function linkedMergedPRDivergedHeadOid(
|
||||
data: PullRequestLookupData | null,
|
||||
linkedPRNumber: number | null | undefined,
|
||||
currentHeadOid: string | null,
|
||||
containsHead: (data: PullRequestLookupData, headOid: string) => Promise<MergedPRCommitMembership>
|
||||
): Promise<string | null> {
|
||||
if (
|
||||
typeof linkedPRNumber !== 'number' ||
|
||||
!data ||
|
||||
mapPRState(data.state, data.isDraft) !== 'merged' ||
|
||||
currentHeadOid === null ||
|
||||
data.headRefOid === currentHeadOid
|
||||
) {
|
||||
return null
|
||||
}
|
||||
return (await containsHead(data, currentHeadOid)) === 'not-contained' ? currentHeadOid : null
|
||||
}
|
||||
|
||||
export function normalizePullRequestLookupData(data: PullRequestLookupData): PullRequestLookupData {
|
||||
return {
|
||||
...data,
|
||||
|
||||
@@ -1,42 +0,0 @@
|
||||
import type { OwnerRepo } from '../../gh-utils'
|
||||
import { githubRepoIdentityKey } from '../../../../shared/github/repository-identity-key'
|
||||
import {
|
||||
getRemoteHeadBranchName,
|
||||
getRepoDefaultBranchName
|
||||
} from '../../../source-control/repo-default-branch'
|
||||
import type { HostedReviewLocalGitOptions } from './../github-exec-scope'
|
||||
import type { TrackedUpstreamBranch } from './tracked-upstream-cache'
|
||||
|
||||
/**
|
||||
* #26948: `git switch -c x origin/develop` makes a branch track the default
|
||||
* branch, whose PRs (e.g. develop -> release) belong to that branch, not this one.
|
||||
* A same-named branch on a fork outside the PR candidates is a real head, so it stays.
|
||||
*/
|
||||
export async function isTrackedUpstreamDefaultBranch(input: {
|
||||
upstreamBranch: TrackedUpstreamBranch
|
||||
upstreamHeadRepo: OwnerRepo
|
||||
candidates: OwnerRepo[]
|
||||
repoPath: string
|
||||
connectionId?: string | null
|
||||
localGitOptions: HostedReviewLocalGitOptions
|
||||
}): Promise<boolean> {
|
||||
const upstreamRepoKey = githubRepoIdentityKey(input.upstreamHeadRepo)
|
||||
if (!input.candidates.some((candidate) => githubRepoIdentityKey(candidate) === upstreamRepoKey)) {
|
||||
return false
|
||||
}
|
||||
const { remoteName } = input.upstreamBranch
|
||||
// Why: a fork's `upstream` can default to a different branch than `origin`; use origin's only when the remote never recorded its HEAD.
|
||||
const remoteDefaultBranchName =
|
||||
remoteName === 'origin'
|
||||
? null
|
||||
: await getRemoteHeadBranchName(
|
||||
input.repoPath,
|
||||
remoteName,
|
||||
input.connectionId,
|
||||
input.localGitOptions
|
||||
)
|
||||
const defaultBranchName =
|
||||
remoteDefaultBranchName ??
|
||||
(await getRepoDefaultBranchName(input.repoPath, input.connectionId, input.localGitOptions))
|
||||
return defaultBranchName === input.upstreamBranch.branchName
|
||||
}
|
||||
Reference in New Issue
Block a user