Fix push-target resolution missing queue-discovered PRs (#8351)
Include fallbackGitHubPR alongside linkedGitHubPR/linkedGitLabMR when determining whether a hosted review link resolves to a push target. Worktrees without persisted linkedPR metadata (e.g. child worktrees) were incorrectly blocked with "target unavailable" despite having a real matching upstream, since their PR was only known via the queue fallback. Also splits hasPositiveHostedReviewNumberLink to build on the resolvable subset so the two helpers can't drift.
This commit is contained in:
parent
969341bb30
commit
6883052a8f
|
|
@ -1669,6 +1669,7 @@ function SourceControlInner(): React.JSX.Element {
|
|||
!activeRepo?.connectionId && hasHostedReviewLink && hostedReviewEntry === undefined
|
||||
const hasResolvableReviewPushTargetLink = hasResolvableHostedReviewPushTargetLink({
|
||||
linkedGitHubPR,
|
||||
fallbackGitHubPR: fallbackGitHubPRNumber,
|
||||
linkedGitLabMR
|
||||
})
|
||||
useEffect(() => {
|
||||
|
|
@ -1687,9 +1688,7 @@ function SourceControlInner(): React.JSX.Element {
|
|||
ensureHostedReviewPushTarget,
|
||||
hasResolvableReviewPushTargetLink,
|
||||
isBranchVisible,
|
||||
isFolder,
|
||||
linkedGitHubPR,
|
||||
linkedGitLabMR
|
||||
isFolder
|
||||
])
|
||||
const canUseHostedReviewPushTarget = hasUsableHostedReviewPushTarget({
|
||||
pushTarget: activeWorktree?.pushTarget,
|
||||
|
|
|
|||
|
|
@ -121,6 +121,12 @@ describe('hasResolvableHostedReviewPushTargetLink', () => {
|
|||
it('accepts only hosted-review links with supported target lookup APIs', () => {
|
||||
expect(hasResolvableHostedReviewPushTargetLink({ linkedGitHubPR: 12 })).toBe(true)
|
||||
expect(hasResolvableHostedReviewPushTargetLink({ linkedGitLabMR: 34 })).toBe(true)
|
||||
// Why: a queue-discovered same-repo PR (no persisted linkedPR) is resolvable.
|
||||
expect(hasResolvableHostedReviewPushTargetLink({ fallbackGitHubPR: 8333 })).toBe(true)
|
||||
expect(
|
||||
hasResolvableHostedReviewPushTargetLink({ linkedGitHubPR: null, fallbackGitHubPR: 8333 })
|
||||
).toBe(true)
|
||||
expect(hasResolvableHostedReviewPushTargetLink({ fallbackGitHubPR: 0 })).toBe(false)
|
||||
expect(hasResolvableHostedReviewPushTargetLink({ linkedGitHubPR: null })).toBe(false)
|
||||
expect(hasResolvableHostedReviewPushTargetLink({ linkedGitHubPR: 0 })).toBe(false)
|
||||
expect(hasResolvableHostedReviewPushTargetLink({ linkedGitLabMR: -1 })).toBe(false)
|
||||
|
|
@ -145,6 +151,17 @@ describe('hasPositiveHostedReviewNumberLink', () => {
|
|||
expect(hasPositiveHostedReviewNumberLink({ linkedGitHubPR: Number.NaN })).toBe(false)
|
||||
expect(hasPositiveHostedReviewNumberLink({})).toBe(false)
|
||||
})
|
||||
|
||||
it('blocks resolver-less providers without treating them as resolvable', () => {
|
||||
// Bitbucket/Azure/Gitea have no push-target resolver yet, so they must block
|
||||
// unsafe pushes but stay out of the resolvable subset. Locks the intended
|
||||
// relationship: resolvable ⊂ positive, so the two helpers cannot drift.
|
||||
for (const provider of ['linkedBitbucketPR', 'linkedAzureDevOpsPR', 'linkedGiteaPR'] as const) {
|
||||
const args = { [provider]: 42 }
|
||||
expect(hasPositiveHostedReviewNumberLink(args)).toBe(true)
|
||||
expect(hasResolvableHostedReviewPushTargetLink(args)).toBe(false)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveHostedReviewStateForActions', () => {
|
||||
|
|
@ -293,4 +310,39 @@ describe('resolveHostedReviewActionUpstreamStatus with a same-repo upstream', ()
|
|||
})
|
||||
).toBe(realUpstream)
|
||||
})
|
||||
|
||||
it('does not block push for a queue-discovered open PR whose upstream tracks the branch', () => {
|
||||
// Why: a child worktree with no persisted linkedPR discovers its open PR via
|
||||
// the queue (fallbackGitHubPR). Before the fix, that PR counted as a hosted
|
||||
// review link but not a resolvable target, so the real matching upstream was
|
||||
// ignored and Push was wrongly disabled as "target unavailable".
|
||||
const realUpstream = {
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/fix-f1-codex-wsl-path-trust',
|
||||
ahead: 1,
|
||||
behind: 0
|
||||
}
|
||||
const hasResolvable = hasResolvableHostedReviewPushTargetLink({
|
||||
linkedGitHubPR: null,
|
||||
fallbackGitHubPR: 8333,
|
||||
linkedGitLabMR: null
|
||||
})
|
||||
expect(hasResolvable).toBe(true)
|
||||
const canUseHostedReviewPushTarget = hasUsableHostedReviewPushTarget({
|
||||
hasResolvableHostedReviewPushTargetLink: hasResolvable,
|
||||
branchName: 'fix-f1-codex-wsl-path-trust',
|
||||
upstreamStatus: realUpstream
|
||||
})
|
||||
expect(canUseHostedReviewPushTarget).toBe(true)
|
||||
expect(
|
||||
resolveHostedReviewActionUpstreamStatus({
|
||||
hasHostedReviewLink: true,
|
||||
hasResolvableHostedReviewPushTargetLink: hasResolvable,
|
||||
hostedReviewState: 'open',
|
||||
isHostedReviewStateLoading: false,
|
||||
canUseHostedReviewPushTarget,
|
||||
upstreamStatus: realUpstream
|
||||
})
|
||||
).toBe(realUpstream)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import type { GitPushTarget, GitUpstreamStatus } from '../../../../shared/types'
|
||||
import type { HostedReviewState } from '../../../../shared/hosted-review'
|
||||
import { isPositiveHostedReviewNumber } from '../../../../shared/hosted-review'
|
||||
import { getPublishTargetDisplayName } from '../../../../shared/git-publish-target-status'
|
||||
import { gitRefTargetsBranchName } from '../../../../shared/git-remote-branch-name'
|
||||
|
||||
|
|
@ -30,8 +31,21 @@ export function hasUsableHostedReviewPushTarget(args: {
|
|||
return args.upstreamStatus?.hasConfiguredPushTarget === true
|
||||
}
|
||||
|
||||
function isResolvableHostedReviewNumber(value: unknown): value is number {
|
||||
return typeof value === 'number' && Number.isInteger(value) && value > 0
|
||||
export function hasResolvableHostedReviewPushTargetLink(args: {
|
||||
linkedGitHubPR?: number | null
|
||||
fallbackGitHubPR?: number | null
|
||||
linkedGitLabMR?: number | null
|
||||
}): boolean {
|
||||
// Why: only GitHub (including a queue-discovered same-repo fallbackGitHubPR,
|
||||
// whose head IS the checked-out branch) and GitLab links resolve to a push
|
||||
// target — this mirrors getHostedReviewPushTargetLookup in the worktrees store.
|
||||
// Omitting fallbackGitHubPR left worktrees without persisted linkedPR metadata
|
||||
// (e.g. child worktrees) blocked as "target unavailable" despite a real upstream.
|
||||
return (
|
||||
isPositiveHostedReviewNumber(args.linkedGitHubPR) ||
|
||||
isPositiveHostedReviewNumber(args.fallbackGitHubPR) ||
|
||||
isPositiveHostedReviewNumber(args.linkedGitLabMR)
|
||||
)
|
||||
}
|
||||
|
||||
export function hasPositiveHostedReviewNumberLink(args: {
|
||||
|
|
@ -42,23 +56,14 @@ export function hasPositiveHostedReviewNumberLink(args: {
|
|||
linkedAzureDevOpsPR?: number | null
|
||||
linkedGiteaPR?: number | null
|
||||
}): boolean {
|
||||
// Why: a linked review from any provider blocks unsafe pushes. Build on the
|
||||
// resolvable subset so the two helpers cannot drift — a resolvable link is by
|
||||
// definition also a blocking link; only the resolver-less providers are added.
|
||||
return (
|
||||
isResolvableHostedReviewNumber(args.linkedGitHubPR) ||
|
||||
isResolvableHostedReviewNumber(args.fallbackGitHubPR) ||
|
||||
isResolvableHostedReviewNumber(args.linkedGitLabMR) ||
|
||||
isResolvableHostedReviewNumber(args.linkedBitbucketPR) ||
|
||||
isResolvableHostedReviewNumber(args.linkedAzureDevOpsPR) ||
|
||||
isResolvableHostedReviewNumber(args.linkedGiteaPR)
|
||||
)
|
||||
}
|
||||
|
||||
export function hasResolvableHostedReviewPushTargetLink(args: {
|
||||
linkedGitHubPR?: number | null
|
||||
linkedGitLabMR?: number | null
|
||||
}): boolean {
|
||||
return (
|
||||
isResolvableHostedReviewNumber(args.linkedGitHubPR) ||
|
||||
isResolvableHostedReviewNumber(args.linkedGitLabMR)
|
||||
hasResolvableHostedReviewPushTargetLink(args) ||
|
||||
isPositiveHostedReviewNumber(args.linkedBitbucketPR) ||
|
||||
isPositiveHostedReviewNumber(args.linkedAzureDevOpsPR) ||
|
||||
isPositiveHostedReviewNumber(args.linkedGiteaPR)
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ import {
|
|||
} from '../../runtime/runtime-rpc-client'
|
||||
import { toRuntimeWorktreeSelector } from '../../runtime/runtime-worktree-selector'
|
||||
import { getHostedReviewCacheKey, refreshHostedReviewCard } from './hosted-review'
|
||||
import { isPositiveHostedReviewNumber } from '../../../../shared/hosted-review'
|
||||
import { getGitHubPRCacheKey, getLegacyGitHubPRCacheKey } from './github-cache-key'
|
||||
import { moveFocusToRendererBeforeFocusedWebviewHidden } from './browser-webview-cleanup'
|
||||
import { toast } from 'sonner'
|
||||
|
|
@ -1384,10 +1385,6 @@ function getHostedReviewPushTargetLookup(worktree: Worktree): {
|
|||
return null
|
||||
}
|
||||
|
||||
function isPositiveHostedReviewNumber(value: unknown): value is number {
|
||||
return typeof value === 'number' && Number.isInteger(value) && value > 0
|
||||
}
|
||||
|
||||
type HostedReviewLinkKey =
|
||||
| 'linkedPR'
|
||||
| 'linkedGitLabMR'
|
||||
|
|
|
|||
|
|
@ -10,6 +10,11 @@ export type HostedReviewProvider =
|
|||
|
||||
export type HostedReviewState = 'open' | 'closed' | 'merged' | 'draft'
|
||||
|
||||
/** A linked review is identified by a positive integer PR/MR number. */
|
||||
export function isPositiveHostedReviewNumber(value: unknown): value is number {
|
||||
return typeof value === 'number' && Number.isInteger(value) && value > 0
|
||||
}
|
||||
|
||||
export type HostedReviewInfo = {
|
||||
provider: HostedReviewProvider
|
||||
number: number
|
||||
|
|
|
|||
Loading…
Reference in New Issue