From 6883052a8f27bdcf617e44727ccaf930cb512657 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 11 Jul 2026 20:55:23 -0700 Subject: [PATCH] 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. --- .../right-sidebar/SourceControl.tsx | 5 +- ...-control-hosted-review-push-target.test.ts | 52 +++++++++++++++++++ ...ource-control-hosted-review-push-target.ts | 41 ++++++++------- src/renderer/src/store/slices/worktrees.ts | 5 +- src/shared/hosted-review.ts | 5 ++ 5 files changed, 83 insertions(+), 25 deletions(-) diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index a12f53050..45a9e212c 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -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, diff --git a/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.test.ts b/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.test.ts index b65a6f8e3..19084837f 100644 --- a/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.test.ts @@ -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) + }) }) diff --git a/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.ts b/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.ts index 99be32e88..bd87f24c4 100644 --- a/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.ts +++ b/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.ts @@ -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) ) } diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 17b3444fe..021139af7 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -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' diff --git a/src/shared/hosted-review.ts b/src/shared/hosted-review.ts index 7bf3dd241..864baceba 100644 --- a/src/shared/hosted-review.ts +++ b/src/shared/hosted-review.ts @@ -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