Keep explicit push and force push actions available without upstream

- Keep the explicit "Push" and "Force Push" dropdown options enabled when
  there is no upstream or the upstream status is loading, letting Git
  resolve the target at execution time.
- Stop auto-upgrading regular push operations to force push in the background,
  ensuring normal push actions stay non-force.
- Explicitly route the primary action and commit-and-push flows to the
  "force_push" action when a lease force push is required.
This commit is contained in:
Jinjing
2026-07-08 19:01:50 -07:00
parent 01d21cdce1
commit c762b06d13
3 changed files with 171 additions and 76 deletions
@@ -2429,7 +2429,6 @@ function SourceControlInner(): React.JSX.Element {
| 'rebase',
options?: {
target?: SourceControlOperationTarget
remoteStatus?: GitUpstreamStatus
baseRef?: string | null
}
): Promise<boolean> => {
@@ -2461,18 +2460,17 @@ function SourceControlInner(): React.JSX.Element {
return true
}
if (kind === 'push') {
const forceWithLease = shouldForcePushWithLeaseForUpstream(
options?.remoteStatus ?? remoteStatus
)
// Why: kind 'push' must stay a regular push. Force-with-lease is only
// for kind 'force_push' (and callers that choose it). Auto-upgrading
// here made the always-enabled dropdown Push row silently force-push
// while its tooltip claimed a normal push might merely fail.
await pushBranch(
target.worktreeId,
target.worktreePath,
false,
target.connectionId,
target.pushTarget,
forceWithLease
? { forceWithLease: true, runtimeTargetSettings: target.settings }
: { runtimeTargetSettings: target.settings }
{ runtimeTargetSettings: target.settings }
)
return true
}
@@ -2581,7 +2579,6 @@ function SourceControlInner(): React.JSX.Element {
pushBranch,
rebaseFromBase,
refreshActiveGitStatusAfterMutation,
remoteStatus,
syncBranch,
worktreePath
]
@@ -2695,9 +2692,20 @@ function SourceControlInner(): React.JSX.Element {
if (!ok) {
return
}
// Why: "Commit & Force Push" still invokes remoteKind 'push' from the
// dropdown kind map. Route to force_push when the upstream shape requires
// lease force so compound commit does not silently fall back to a regular
// push after we stopped auto-upgrading kind 'push'.
if (
remoteKind === 'push' &&
shouldForcePushWithLeaseForUpstream(remoteStatusForActions ?? remoteStatus)
) {
await runRemoteAction('force_push')
return
}
await runRemoteAction(remoteKind)
},
[handleCommit, runRemoteAction]
[handleCommit, remoteStatus, remoteStatusForActions, runRemoteAction]
)
const handlePullRequestCreated = useCallback(
@@ -3951,7 +3959,6 @@ function SourceControlInner(): React.JSX.Element {
})
const remoteOk = await runRemoteAction(remoteStep, {
target: operationTarget,
remoteStatus: latestUpstreamStatus,
baseRef: token.baseRef
})
if (abortIfStale()) {
@@ -4642,8 +4649,17 @@ function SourceControlInner(): React.JSX.Element {
case 'stage':
void handleStageAllPrimary()
return
case 'commit':
case 'push':
// Why: primary labels "Force Push" while keeping kind 'push'. Invoke the
// explicit force path when lease force is required so regular push stays
// non-force for the dropdown.
handleActionInvoke(
shouldForcePushWithLeaseForUpstream(remoteStatusForActions ?? remoteStatus)
? 'force_push'
: 'push'
)
return
case 'commit':
case 'pull':
case 'sync':
case 'publish':
@@ -4653,7 +4669,14 @@ function SourceControlInner(): React.JSX.Element {
case 'create_pr_intent':
void runCreatePrIntent()
}
}, [handleActionInvoke, handleStageAllPrimary, primaryAction.kind, runCreatePrIntent])
}, [
handleActionInvoke,
handleStageAllPrimary,
primaryAction.kind,
remoteStatus,
remoteStatusForActions,
runCreatePrIntent
])
const handleCreatePrHeaderClick = useCallback((): void => {
if (!createPrHeaderAction || createPrHeaderAction.disabled) {
@@ -77,7 +77,7 @@ describe('resolveDropdownItems', () => {
expect(byKind.commit.title).toBe('Commit staged changes')
})
it('disables push actions but keeps Fetch enabled when branch has no upstream', () => {
it('enables explicit push actions and keeps Fetch enabled when branch has no upstream', () => {
const items = resolveDropdownItems(
inputs({
stagedCount: 1,
@@ -88,7 +88,8 @@ describe('resolveDropdownItems', () => {
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.push.disabled).toBe(true)
expect(byKind.push.disabled).toBe(false)
expect(byKind.force_push.disabled).toBe(false)
expect(byKind.commit_push.disabled).toBe(true)
expect(byKind.publish.disabled).toBe(false)
expect(byKind.fetch.disabled).toBe(false)
@@ -183,8 +184,8 @@ describe('resolveDropdownItems', () => {
)
expect(byKind.push.label).toBe('Push (14)')
expect(byKind.push.disabled).toBe(true)
expect(byKind.push.title).toBe('Use Force Push — remote only has older copies of local commits')
expect(byKind.push.disabled).toBe(false)
expect(byKind.push.title).toBe('Try a regular push; git may require force push')
expect(byKind.force_push.label).toBe('Force Push (4)')
expect(byKind.force_push.disabled).toBe(false)
expect(byKind.force_push.title).toBe(
@@ -364,10 +365,11 @@ describe('resolveDropdownItems', () => {
}
})
it('disables remote rows with a loading tooltip when upstreamStatus is undefined', () => {
it('keeps explicit push rows available while upstreamStatus is undefined', () => {
// Why: mirrors the primary-action guard — while fetchUpstreamStatus is in
// flight we must not let the user click Publish on an already-tracked
// branch (which would re-run `git push -u` and clobber the upstream).
// branch, but explicit Push/Force Push resolve their git target at click
// time and should remain available.
const items = resolveDropdownItems(
inputs({ stagedCount: 1, hasMessage: true, upstreamStatus: undefined })
)
@@ -377,8 +379,6 @@ describe('resolveDropdownItems', () => {
const loadingBlocked = [
'commit_push',
'commit_sync',
'push',
'force_push',
'pull',
'fast_forward',
'sync',
@@ -391,21 +391,36 @@ describe('resolveDropdownItems', () => {
// Commit itself does not depend on upstream — it remains enabled when
// staged + message are present and no commit is in flight.
expect(byKind.commit.disabled).toBe(false)
expect(byKind.push.disabled).toBe(false)
expect(byKind.push.title).toBe('Push this branch and set an upstream if needed')
expect(byKind.force_push.disabled).toBe(false)
expect(byKind.force_push.title).toBe(
'Force push this branch with lease and set an upstream if needed.'
)
expect(byKind.fetch.disabled).toBe(false)
expect(byKind.fetch.title).toBe('Fetch from remote without merging')
})
it('keeps Fetch enabled and surfaces publish-first tooltips when upstream is absent', () => {
it('keeps explicit push rows enabled and surfaces publish-first tooltips elsewhere', () => {
// Why: sibling to the upstreamStatus=undefined test above. Once the fetch
// resolves to hasUpstream=false, the dropdown should explain that the
// user needs to publish first (rather than leaving the loading copy).
// resolves to hasUpstream=false, only pull/sync rows need publish-first
// copy; explicit push rows can set the upstream themselves.
const items = resolveDropdownItems(
inputs({ upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 } })
inputs({
upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 },
branchCommitsAhead: 2
})
)
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.push.title).toBe('Publish the branch first to push commits')
expect(byKind.push.title).toBe('Push this branch and set an upstream if needed')
expect(byKind.push.disabled).toBe(false)
expect(byKind.force_push.label).toBe('Force Push (2)')
expect(byKind.force_push.title).toBe(
'Force push 2 branch commits with lease and set an upstream if needed.'
)
expect(byKind.force_push.disabled).toBe(false)
expect(byKind.pull.title).toBe('Publish the branch first to pull commits')
expect(byKind.fast_forward.title).toBe('Publish the branch first to fast-forward')
expect(byKind.sync.title).toBe('Publish the branch first to sync commits')
@@ -482,7 +497,7 @@ describe('resolveDropdownItems', () => {
expect(byKind.publish.disabled).toBe(false)
})
it('does not mention Publish Branch when the linked PR is already merged', () => {
it('keeps explicit push rows available when the linked PR is already merged', () => {
const items = resolveDropdownItems(
inputs({
upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 },
@@ -492,7 +507,12 @@ describe('resolveDropdownItems', () => {
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.push.title).toBe('PR is already merged')
expect(byKind.push.title).toBe('Push this branch and set an upstream if needed')
expect(byKind.push.disabled).toBe(false)
expect(byKind.force_push.title).toBe(
'Force push this branch with lease and set an upstream if needed.'
)
expect(byKind.force_push.disabled).toBe(false)
expect(byKind.pull.title).toBe('PR is already merged')
expect(byKind.fast_forward.title).toBe('PR is already merged')
expect(byKind.sync.title).toBe('PR is already merged')
@@ -518,12 +538,40 @@ describe('resolveDropdownItems', () => {
expect(byKind.commit_push.disabled).toBe(false)
expect(byKind.push.title).toBe('Push updates to the linked review branch')
expect(byKind.push.disabled).toBe(false)
expect(byKind.force_push.label).toBe('Force Push (1)')
expect(byKind.force_push.disabled).toBe(false)
expect(byKind.publish.label).toBe('Linked Review')
expect(byKind.publish.title).toBe('Linked review branch already exists')
expect(byKind.publish.disabled).toBe(true)
})
it('blocks Push when an open linked review has no upstream and no branch target', () => {
it('keeps explicit Push available for a linked review with a usable target and no commits ahead', () => {
// Why: branchCommitsAhead === 0 must not be confused with a missing review
// head. Primary Push stays enabled; dropdown Push/Force Push match that.
const items = resolveDropdownItems(
inputs({
stagedCount: 1,
hasMessage: true,
upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 },
branchCommitsAhead: 0,
prState: 'open',
canPushLinkedReviewWithoutUpstream: true
})
)
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.push.disabled).toBe(false)
expect(byKind.push.title).toBe('Push this branch and set an upstream if needed')
expect(byKind.force_push.disabled).toBe(false)
expect(byKind.commit_push.disabled).toBe(false)
expect(byKind.publish.disabled).toBe(true)
})
it('blocks Push when an open linked review has no branch target', () => {
// Why: faked hasUpstream=false for an unusable review head must not re-open
// push against an unrelated configured upstream. Primary already blocks
// this; the always-allow Push rows keep the same target-safety gate.
const items = resolveDropdownItems(
inputs({
stagedCount: 1,
@@ -540,6 +588,8 @@ describe('resolveDropdownItems', () => {
expect(byKind.commit_push.title).toBe('Linked review branch target is unavailable')
expect(byKind.push.title).toBe('Linked review branch target is unavailable')
expect(byKind.push.disabled).toBe(true)
expect(byKind.force_push.title).toBe('Linked review branch target is unavailable')
expect(byKind.force_push.disabled).toBe(true)
expect(byKind.publish.label).toBe('Linked Review')
expect(byKind.publish.title).toBe('Linked review branch target is unavailable')
expect(byKind.publish.disabled).toBe(true)
@@ -95,6 +95,14 @@ function formatManualForcePushTitle(ahead: number, behind: number, upstreamName?
return `Force push ${commitText} with lease to update ${upstreamName ?? 'the remote branch'}.`
}
function formatUnpublishedForcePushTitle(branchCommitsAhead: number | undefined): string {
const countText =
branchCommitsAhead && branchCommitsAhead > 0
? `${branchCommitsAhead} branch commit${branchCommitsAhead === 1 ? '' : 's'}`
: 'this branch'
return `Force push ${countText} with lease and set an upstream if needed.`
}
function formatRebaseBaseRef(baseRef: string): string {
return baseRef.replace(/^refs\/remotes\//, '').replace(/^remotes\//, '')
}
@@ -143,7 +151,11 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
// branch during the post-worktree-switch transient window, and a click there
// would re-run `git push -u` and clobber the real upstream. Every
// upstream-dependent row disables itself while loading so the primary
// button's stable-frame guarantee extends to the dropdown.
// button's stable-frame guarantee extends to the dropdown. Explicit
// Push/Force Push are the exception for loading/unpublished branches: the
// git command resolves its target itself. Linked-review rows without a
// usable push target still block — otherwise we would push an unrelated
// configured upstream while the menu claimed first-publish semantics.
const upstreamLoading = upstreamStatus === undefined
const hasUpstream = upstreamStatus?.hasUpstream ?? false
const hasOpenHostedReview = prState === 'open' || prState === 'draft'
@@ -153,8 +165,11 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
hasCurrentBranch &&
branchCommitsAhead !== 0 &&
canPushLinkedReviewWithoutUpstream
// Why: only the missing review head is a hard block. branchCommitsAhead === 0
// still means the target is known — primary Push stays available, and
// explicit Push must match that rather than claiming "target unavailable".
const pushBlockedByOpenHostedReviewTarget =
!hasUpstream && hasOpenHostedReview && !canPushUntrackedHostedReview
!hasUpstream && hasOpenHostedReview && !canPushLinkedReviewWithoutUpstream
const publishBlockedByMergedPR = !hasUpstream && prState === 'merged'
const publishBlockedByPRLoading = !hasUpstream && !!isPRStateLoading
const publishBlockedByOpenHostedReview = !hasUpstream && hasOpenHostedReview
@@ -162,8 +177,16 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
const ahead = upstreamStatus?.ahead ?? 0
const behind = upstreamStatus?.behind ?? 0
const shouldForcePushWithLease = shouldForcePushWithLeaseForUpstream(upstreamStatus)
// Why: force-push counts prefer branch-compare when upstream ahead is missing
// or misleading — unpublished/loading branches report ahead=0, and
// patch-equivalent rewrites inflate ahead far above real branch commits.
// On a normal tracked branch, upstream ahead is the correct force-push size.
const pushLabelCount =
shouldForcePushWithLease && branchCommitsAhead !== undefined ? branchCommitsAhead : ahead
branchCommitsAhead !== undefined &&
branchCommitsAhead > 0 &&
(shouldForcePushWithLease || !hasUpstream)
? branchCommitsAhead
: ahead
const forcePushTitle = formatForcePushTitle(branchCommitsAhead, upstreamStatus?.upstreamName)
const createReviewCopy = reviewCopy(hostedReviewCreation?.provider)
@@ -216,7 +239,7 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
? 'Check out a branch before pushing commits'
: pushBlockedByOpenHostedReviewTarget
? 'Linked review branch target is unavailable'
: !hasUpstream && !canPushUntrackedHostedReview
: !hasUpstream && !(hasOpenHostedReview && canPushLinkedReviewWithoutUpstream)
? 'Publish the branch first to push commits'
: (commitDisabledReason ??
(shouldForcePushWithLease
@@ -228,10 +251,13 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
kind: 'commit_push',
label: shouldForcePushWithLease ? 'Commit & Force Push' : 'Commit & Push',
title: commitPushTitle,
// Why: match explicit Push — only an open linked review with a known head
// can commit+push without a git upstream. Missing heads still block; plain
// unpublished branches still need Publish first.
disabled:
globalBusy ||
upstreamLoading ||
(!hasUpstream && !canPushUntrackedHostedReview) ||
(!hasUpstream && !(hasOpenHostedReview && canPushLinkedReviewWithoutUpstream)) ||
publishBlockedByDetachedHead ||
publishBlockedByPRLoading ||
publishBlockedByMergedPR ||
@@ -284,54 +310,50 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
const pushItem: DropdownItem = {
kind: 'push',
label: formatCountLabel('Push', ahead),
title: upstreamLoading
? 'Checking branch status…'
: publishBlockedByPRLoading
? 'Checking PR status…'
: publishBlockedByMergedPR
? 'PR is already merged'
: publishBlockedByDetachedHead
? 'Check out a branch before pushing commits'
: canPushUntrackedHostedReview
? 'Push updates to the linked review branch'
: pushBlockedByOpenHostedReviewTarget
? 'Linked review branch target is unavailable'
: !hasUpstream
? 'Publish the branch first to push commits'
: shouldForcePushWithLease
? 'Use Force Push — remote only has older copies of local commits'
: behind > 0 && ahead > 0
? 'Push local commits; git may require syncing first'
: ahead === 0
? `Nothing to push${upstreamStatus?.upstreamName ? ` to ${upstreamStatus.upstreamName}` : ''}`
: describePushCount(ahead),
disabled:
globalBusy ||
upstreamLoading ||
(!hasUpstream && !canPushUntrackedHostedReview) ||
publishBlockedByDetachedHead ||
shouldForcePushWithLease
title: publishBlockedByDetachedHead
? 'Check out a branch before pushing commits'
: pushBlockedByOpenHostedReviewTarget
? 'Linked review branch target is unavailable'
: upstreamLoading
? 'Push this branch and set an upstream if needed'
: canPushUntrackedHostedReview
? 'Push updates to the linked review branch'
: !hasUpstream
? 'Push this branch and set an upstream if needed'
: shouldForcePushWithLease
? 'Try a regular push; git may require force push'
: behind > 0 && ahead > 0
? 'Push local commits; git may require syncing first'
: ahead === 0
? `Nothing to push${upstreamStatus?.upstreamName ? ` to ${upstreamStatus.upstreamName}` : ''}`
: describePushCount(ahead),
// Why: keep Push available without an upstream (git resolves --set-upstream
// itself) and even when force-with-lease is recommended — regular Push must
// stay a non-force path. Only block detached HEAD and linked reviews whose
// push target is still unknown (otherwise we would push an unrelated upstream).
disabled: globalBusy || publishBlockedByDetachedHead || pushBlockedByOpenHostedReviewTarget
}
const forcePushItem: DropdownItem = {
kind: 'force_push',
label: formatCountLabel('Force Push', pushLabelCount),
title: upstreamLoading
? 'Checking branch status…'
: publishBlockedByPRLoading
? 'Checking PR status…'
: publishBlockedByMergedPR
? 'PR is already merged'
: publishBlockedByDetachedHead
? 'Check out a branch before force pushing commits'
: !hasUpstream
? 'Publish the branch first to force push commits'
: ahead === 0
? `Nothing to force push${upstreamStatus?.upstreamName ? ` to ${upstreamStatus.upstreamName}` : ''}`
: shouldForcePushWithLease
? forcePushTitle
: formatManualForcePushTitle(ahead, behind, upstreamStatus?.upstreamName),
disabled: globalBusy || upstreamLoading || !hasUpstream || publishBlockedByDetachedHead
title: publishBlockedByDetachedHead
? 'Check out a branch before force pushing commits'
: pushBlockedByOpenHostedReviewTarget
? 'Linked review branch target is unavailable'
: upstreamLoading
? formatUnpublishedForcePushTitle(branchCommitsAhead)
: !hasUpstream
? formatUnpublishedForcePushTitle(branchCommitsAhead)
: pushLabelCount === 0
? `Nothing to force push${upstreamStatus?.upstreamName ? ` to ${upstreamStatus.upstreamName}` : ''}`
: shouldForcePushWithLease
? forcePushTitle
: formatManualForcePushTitle(pushLabelCount, behind, upstreamStatus?.upstreamName),
// Why: same target-safety gate as Push — force-with-lease to a wrong or
// unresolved review head is worse than blocking the row until the target
// is known. Explicit Force Push stays available without an upstream.
disabled: globalBusy || publishBlockedByDetachedHead || pushBlockedByOpenHostedReviewTarget
}
const pullItem: DropdownItem = {