fix(source-control): keep push enabled for same-repo linked review with real upstream (#8202)

* fix(source-control): keep push enabled for same-repo linked review with real upstream

Push/Force Push (and Pull/Sync/Publish) were wrongly disabled when a branch
had a genuine git upstream but an open linked PR whose push target had not
yet hydrated. `hasUsableHostedReviewPushTarget` hard-returned false for any
resolvable review link without a hydrated `pushTarget`, so
`resolveHostedReviewActionUpstreamStatus` synthesized `hasUpstream:false`
and the whole remote menu treated the branch as unpublished.

A same-repo review's head IS the checked-out branch, so a real upstream
already tracking that branch is that head and is safe to use before the
resolver hydrates a target. Treat that upstream as usable; keep blocking the
fork-head/cross-repo case (upstream tracks a differently-named head) and the
no-upstream case until the resolver proves the real target. Provider-agnostic
(GitHub PR / GitLab MR) and covers the SSH linked-review path.

Adds regression tests to source-control-hosted-review-push-target.test.ts and
a composed pipeline test in source-control-dropdown-items.test.ts.

* docs(source-control): trim verbose comments on linked-review push-target fix

Co-authored-by: Orca <help@stably.ai>

* refactor(source-control): use canonical gitRefTargetsBranchName for linked-review upstream match

Replace the hand-rolled upstreamTracksBranch leaf-split with the shared
gitRefTargetsBranchName primitive, which trims and rejects malformed
remote-qualified refs. The remote stays unknown until the resolver hydrates
the push target, so this remains a branch-leaf match; the strict remote+branch
check takes over once the target is known.

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>

src/renderer/src/components/right-sidebar/SourceControl.tsx

@@ -1676,7 +1676,8 @@ function SourceControlInner(): React.JSX.Element {
   const canUseHostedReviewPushTarget = hasUsableHostedReviewPushTarget({
     pushTarget: activeWorktree?.pushTarget,
     upstreamStatus: remoteStatus,
-    hasResolvableHostedReviewPushTargetLink: hasResolvableReviewPushTargetLink
+    hasResolvableHostedReviewPushTargetLink: hasResolvableReviewPushTargetLink,
+    branchName
   })
   const hostedReviewStateForActions = resolveHostedReviewStateForActions({
     hostedReviewState: hostedReview?.state ?? null,

src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts

@@ -1,5 +1,13 @@
 import { describe, expect, it } from 'vitest'
-import { resolveDropdownItems, type DropdownActionInputs } from './source-control-dropdown-items'
+import {
+  resolveDropdownItems,
+  type DropdownActionInputs,
+  type DropdownItem
+} from './source-control-dropdown-items'
+import {
+  hasUsableHostedReviewPushTarget,
+  resolveHostedReviewActionUpstreamStatus
+} from './source-control-hosted-review-push-target'
 
 // Why: a shared defaults object keeps each case row terse while making the
 // "this is the one knob that differs from the baseline" intent obvious.
@@ -768,3 +776,73 @@ describe('resolveDropdownItems', () => {
     expect(byKind.create_pr.hint).toBe('Run glab auth login in this environment')
   })
 })
+
+// Why: PR #8196 — drive the real push-target resolution the component uses so
+// the whole chain stays regression-proof.
+describe('resolveDropdownItems with an unhydrated linked-review push target', () => {
+  function pipeline(args: {
+    branchName: string
+    upstreamStatus: DropdownActionInputs['upstreamStatus']
+  }): Record<string, DropdownItem> {
+    const canUseHostedReviewPushTarget = hasUsableHostedReviewPushTarget({
+      // pushTarget intentionally omitted: the resolver has not hydrated it yet.
+      hasResolvableHostedReviewPushTargetLink: true,
+      branchName: args.branchName,
+      upstreamStatus: args.upstreamStatus
+    })
+    const upstreamStatus = resolveHostedReviewActionUpstreamStatus({
+      hasHostedReviewLink: true,
+      hasResolvableHostedReviewPushTargetLink: true,
+      hostedReviewState: 'open',
+      isHostedReviewStateLoading: false,
+      canUseHostedReviewPushTarget,
+      upstreamStatus: args.upstreamStatus
+    })
+    const items = resolveDropdownItems(
+      inputs({
+        stagedCount: 1,
+        hasMessage: true,
+        branchCommitsAhead: 7,
+        prState: 'open',
+        upstreamStatus,
+        canPushLinkedReviewWithoutUpstream: canUseHostedReviewPushTarget
+      })
+    )
+    return Object.fromEntries(
+      items.filter((e): e is DropdownItem => e.kind !== 'separator').map((e) => [e.kind, e])
+    )
+  }
+
+  it('enables Push and Force Push when the real upstream is the same-repo review head', () => {
+    const byKind = pipeline({
+      branchName: 'mobile-resume-suspected-fixes',
+      upstreamStatus: {
+        hasUpstream: true,
+        upstreamName: 'origin/mobile-resume-suspected-fixes',
+        ahead: 7,
+        behind: 2
+      }
+    })
+    expect(byKind.push.disabled).toBe(false)
+    expect(byKind.push.title).not.toBe('Linked review branch target is unavailable')
+    expect(byKind.force_push.disabled).toBe(false)
+    expect(byKind.pull.disabled).toBe(false)
+    expect(byKind.sync.disabled).toBe(false)
+    expect(byKind.publish.disabled).toBe(true)
+  })
+
+  it('still blocks Push when the real upstream is an unrelated fork/helper head', () => {
+    const byKind = pipeline({
+      branchName: 'mobile-resume-suspected-fixes',
+      upstreamStatus: {
+        hasUpstream: true,
+        upstreamName: 'origin/helper-branch',
+        ahead: 1,
+        behind: 0
+      }
+    })
+    expect(byKind.push.disabled).toBe(true)
+    expect(byKind.push.title).toBe('Linked review branch target is unavailable')
+    expect(byKind.force_push.disabled).toBe(true)
+  })
+})

src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.test.ts

@@ -136,7 +136,12 @@ describe('hasPositiveHostedReviewNumberLink', () => {
     expect(hasPositiveHostedReviewNumberLink({ linkedBitbucketPR: 34 })).toBe(true)
     expect(hasPositiveHostedReviewNumberLink({ linkedAzureDevOpsPR: 56 })).toBe(true)
     expect(hasPositiveHostedReviewNumberLink({ linkedGiteaPR: 78 })).toBe(true)
-    expect(hasPositiveHostedReviewNumberLink({ linkedGitHubPR: 0, linkedGitLabMR: -1 })).toBe(false)
+    expect(
+      hasPositiveHostedReviewNumberLink({
+        linkedGitHubPR: 0,
+        linkedGitLabMR: -1
+      })
+    ).toBe(false)
     expect(hasPositiveHostedReviewNumberLink({ linkedGitHubPR: Number.NaN })).toBe(false)
     expect(hasPositiveHostedReviewNumberLink({})).toBe(false)
   })
@@ -187,18 +192,33 @@ describe('hasUsableHostedReviewPushTarget', () => {
     expect(
       hasUsableHostedReviewPushTarget({
         pushTarget: { remoteName: 'fork', branchName: 'feature' },
-        upstreamStatus: { hasUpstream: true, upstreamName: 'fork/feature', ahead: 1, behind: 0 }
+        upstreamStatus: {
+          hasUpstream: true,
+          upstreamName: 'fork/feature',
+          ahead: 1,
+          behind: 0
+        }
       })
     ).toBe(true)
     expect(
       hasUsableHostedReviewPushTarget({
-        upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0, hasConfiguredPushTarget: true }
+        upstreamStatus: {
+          hasUpstream: false,
+          ahead: 0,
+          behind: 0,
+          hasConfiguredPushTarget: true
+        }
       })
     ).toBe(true)
     expect(
       hasUsableHostedReviewPushTarget({
         hasResolvableHostedReviewPushTargetLink: true,
-        upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0, hasConfiguredPushTarget: true }
+        upstreamStatus: {
+          hasUpstream: false,
+          ahead: 0,
+          behind: 0,
+          hasConfiguredPushTarget: true
+        }
       })
     ).toBe(false)
     expect(
@@ -209,4 +229,68 @@ describe('hasUsableHostedReviewPushTarget', () => {
     ).toBe(false)
     expect(hasUsableHostedReviewPushTarget({ upstreamStatus: unrelatedUpstream })).toBe(false)
   })
+
+  it('treats a same-repo review upstream that already tracks the branch as usable', () => {
+    // Why: a same-repo review must not stay blocked while its push target is
+    // unhydrated — the real upstream already targets the review head.
+    expect(
+      hasUsableHostedReviewPushTarget({
+        hasResolvableHostedReviewPushTargetLink: true,
+        branchName: 'feature/foo',
+        upstreamStatus: {
+          hasUpstream: true,
+          upstreamName: 'origin/feature/foo',
+          ahead: 7,
+          behind: 2
+        }
+      })
+    ).toBe(true)
+  })
+
+  it('keeps blocking a review whose upstream tracks an unrelated fork/helper head', () => {
+    expect(
+      hasUsableHostedReviewPushTarget({
+        hasResolvableHostedReviewPushTargetLink: true,
+        branchName: 'feature',
+        upstreamStatus: unrelatedUpstream
+      })
+    ).toBe(false)
+  })
+
+  it('keeps blocking a resolvable review with no real upstream', () => {
+    expect(
+      hasUsableHostedReviewPushTarget({
+        hasResolvableHostedReviewPushTargetLink: true,
+        branchName: 'feature',
+        upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 }
+      })
+    ).toBe(false)
+  })
+})
+
+describe('resolveHostedReviewActionUpstreamStatus with a same-repo upstream', () => {
+  it('does not synthesize hasUpstream:false when the real upstream is the review head', () => {
+    const realUpstream = {
+      hasUpstream: true,
+      upstreamName: 'origin/mobile-resume-suspected-fixes',
+      ahead: 7,
+      behind: 2
+    }
+    const canUseHostedReviewPushTarget = hasUsableHostedReviewPushTarget({
+      hasResolvableHostedReviewPushTargetLink: true,
+      branchName: 'mobile-resume-suspected-fixes',
+      upstreamStatus: realUpstream
+    })
+    expect(canUseHostedReviewPushTarget).toBe(true)
+    expect(
+      resolveHostedReviewActionUpstreamStatus({
+        hasHostedReviewLink: true,
+        hasResolvableHostedReviewPushTargetLink: true,
+        hostedReviewState: 'open',
+        isHostedReviewStateLoading: false,
+        canUseHostedReviewPushTarget,
+        upstreamStatus: realUpstream
+      })
+    ).toBe(realUpstream)
+  })
 })

src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.ts

@@ -1,11 +1,13 @@
 import type { GitPushTarget, GitUpstreamStatus } from '../../../../shared/types'
 import type { HostedReviewState } from '../../../../shared/hosted-review'
 import { getPublishTargetDisplayName } from '../../../../shared/git-publish-target-status'
+import { gitRefTargetsBranchName } from '../../../../shared/git-remote-branch-name'
 
 export function hasUsableHostedReviewPushTarget(args: {
   pushTarget?: GitPushTarget
   upstreamStatus?: GitUpstreamStatus
   hasResolvableHostedReviewPushTargetLink?: boolean
+  branchName?: string
 }): boolean {
   if (args.pushTarget) {
     return (
@@ -14,9 +16,16 @@ export function hasUsableHostedReviewPushTarget(args: {
     )
   }
   if (args.hasResolvableHostedReviewPushTargetLink) {
-    // Why: a bare branch-config flag does not identify which review head it will
-    // push to; resolver-backed links need hydrated target metadata to prove it.
-    return false
+    // Why: a same-repo review's head is the checked-out branch, so a real
+    // upstream tracking it is safe before the resolver hydrates. Fork/cross-repo
+    // heads differ, so a mismatched or missing upstream stays blocked. The
+    // review's remote is unknown pre-hydration, so this can only match the
+    // branch leaf; the strict remote+branch check above takes over once known.
+    return (
+      args.upstreamStatus?.hasUpstream === true &&
+      args.branchName !== undefined &&
+      gitRefTargetsBranchName(args.upstreamStatus.upstreamName, args.branchName)
+    )
   }
   return args.upstreamStatus?.hasConfiguredPushTarget === true
 }