Fix detached HEAD publish actions (#5173)

This commit is contained in:
Jinjing
2026-06-11 12:52:49 -07:00
committed by Jinwoo-H
parent 72a15d0894
commit 38e55de26f
14 changed files with 194 additions and 70 deletions
@@ -2414,13 +2414,15 @@ export default function ChecksPanel(): React.JSX.Element {
(!publishActionHasUncommittedChanges &&
shouldShowChecksPanelPublishBranchAction({
hostedReviewBlockedReason: hostedReviewCreation?.blockedReason,
hasUpstream: publishActionRemoteStatus?.hasUpstream
hasUpstream: publishActionRemoteStatus?.hasUpstream,
hasCurrentBranch: Boolean(branch)
}))
const emptyStateCopy = getChecksPanelEmptyStateCopy({
operationLabel,
prRefreshStatus: emptyReviewIsGitLab ? undefined : prRefreshState?.status,
hostedReviewBlockedReason: hostedReviewCreation?.blockedReason,
hasUpstream: publishActionRemoteStatus?.hasUpstream,
hasCurrentBranch: Boolean(branch),
reviewLabel: emptyReviewLabel,
reviewShortLabel: emptyReviewShortLabel
})
@@ -2576,7 +2576,8 @@ function SourceControlInner(): React.JSX.Element {
inFlightRemoteOpKind,
hostedReviewCreation,
branchCommitsAhead:
branchSummary?.status === 'ready' ? (branchSummary.commitsAhead ?? 0) : undefined
branchSummary?.status === 'ready' ? (branchSummary.commitsAhead ?? 0) : undefined,
hasCurrentBranch: Boolean(branchName)
})
return isCreatingPr && action.kind === 'create_pr'
? {
@@ -2606,6 +2607,7 @@ function SourceControlInner(): React.JSX.Element {
isCreatingPr,
branchSummary?.commitsAhead,
branchSummary?.status,
branchName,
remoteStatus,
unresolvedConflicts.length
])
@@ -2630,6 +2632,7 @@ function SourceControlInner(): React.JSX.Element {
isPullRequestOperationActive: prGenerating || isCreatingPr,
branchCommitsAhead:
branchSummary?.status === 'ready' ? (branchSummary.commitsAhead ?? 0) : undefined,
hasCurrentBranch: Boolean(branchName),
rebaseBaseRef: effectiveBaseRef
}),
[
@@ -2650,6 +2653,7 @@ function SourceControlInner(): React.JSX.Element {
prGenerating,
branchSummary?.commitsAhead,
branchSummary?.status,
branchName,
effectiveBaseRef,
remoteStatus,
unresolvedConflicts.length
@@ -30,6 +30,18 @@ describe('getChecksPanelEmptyStateCopy', () => {
).toBe('Branch not published')
})
it('does not show unpublished branch copy when HEAD is detached', () => {
expect(
getChecksPanelEmptyStateCopy({
operationLabel: null,
prRefreshStatus: 'error',
hostedReviewBlockedReason: 'no_upstream',
hasUpstream: false,
hasCurrentBranch: false
}).title
).toBe('Could not refresh pull request')
})
it('uses remote status as a fallback when eligibility has no concrete blocker', () => {
expect(
getChecksPanelEmptyStateCopy({
@@ -118,4 +130,14 @@ describe('shouldShowChecksPanelPublishBranchAction', () => {
})
).toBe(true)
})
it('does not show publish when HEAD is detached', () => {
expect(
shouldShowChecksPanelPublishBranchAction({
hostedReviewBlockedReason: 'no_upstream',
hasUpstream: false,
hasCurrentBranch: false
})
).toBe(false)
})
})
@@ -8,6 +8,7 @@ type ChecksPanelEmptyStateInput = {
prRefreshStatus: PRRefreshStatus
hostedReviewBlockedReason: HostedReviewCreationBlockedReason | undefined
hasUpstream: boolean | undefined
hasCurrentBranch?: boolean
reviewLabel?: 'pull request' | 'merge request'
reviewShortLabel?: 'PR' | 'MR'
}
@@ -41,7 +42,8 @@ export function getChecksPanelEmptyStateCopy(
if (
shouldShowChecksPanelPublishBranchAction({
hostedReviewBlockedReason: blockedReason,
hasUpstream: input.hasUpstream
hasUpstream: input.hasUpstream,
hasCurrentBranch: input.hasCurrentBranch
})
) {
// Why: a local-only branch cannot have GitHub PR status yet; surfacing a
@@ -138,7 +140,11 @@ export function getChecksPanelEmptyStateCopy(
export function shouldShowChecksPanelPublishBranchAction(input: {
hostedReviewBlockedReason: HostedReviewCreationBlockedReason | undefined
hasUpstream: boolean | undefined
hasCurrentBranch?: boolean
}): boolean {
if (input.hasCurrentBranch === false) {
return false
}
const blockedReason = input.hostedReviewBlockedReason
return input.hasUpstream === false || blockedReason === 'no_upstream'
}
@@ -95,6 +95,23 @@ describe('resolveDropdownItems', () => {
expect(byKind.fetch.disabled).toBe(false)
})
it('does not offer Publish Branch when HEAD is detached', () => {
const items = resolveDropdownItems(
inputs({
upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 },
branchCommitsAhead: 4,
hasCurrentBranch: false
})
)
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.push.title).toBe('Check out a branch before pushing commits')
expect(byKind.publish.label).toBe('No Branch')
expect(byKind.publish.title).toBe('Check out a branch before publishing commits')
expect(byKind.publish.disabled).toBe(true)
})
it('disables Publish Branch when branch already has an upstream', () => {
const items = resolveDropdownItems(
inputs({
@@ -123,6 +123,7 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
hostedReviewCreation,
conflictOperation = 'unknown',
branchCommitsAhead,
hasCurrentBranch = true,
rebaseBaseRef,
isPullRequestOperationActive = false
} = inputs
@@ -140,6 +141,7 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
const hasUpstream = upstreamStatus?.hasUpstream ?? false
const publishBlockedByMergedPR = !hasUpstream && prState === 'merged'
const publishBlockedByPRLoading = !hasUpstream && !!isPRStateLoading
const publishBlockedByDetachedHead = !hasUpstream && !hasCurrentBranch
const publishBlockedByNoBranchCommits = !hasUpstream && branchCommitsAhead === 0
const publishBlockedByUncommittedChanges = publishBlockedByNoBranchCommits && hasDirtyLocalChanges
const ahead = upstreamStatus?.ahead ?? 0
@@ -194,14 +196,16 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
? 'Checking PR status…'
: publishBlockedByMergedPR
? 'PR is already merged'
: !hasUpstream
? 'Publish the branch first to push commits'
: (commitDisabledReason ??
(shouldForcePushWithLease
? 'Commit staged changes and force push with lease'
: behind > 0
? 'Use Commit & Sync to pull remote changes before pushing'
: 'Commit staged changes and push'))
: publishBlockedByDetachedHead
? 'Check out a branch before pushing commits'
: !hasUpstream
? 'Publish the branch first to push commits'
: (commitDisabledReason ??
(shouldForcePushWithLease
? 'Commit staged changes and force push with lease'
: behind > 0
? 'Use Commit & Sync to pull remote changes before pushing'
: 'Commit staged changes and push'))
const commitPushItem: DropdownItem = {
kind: 'commit_push',
label: shouldForcePushWithLease ? 'Commit & Force Push' : 'Commit & Push',
@@ -210,6 +214,7 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
globalBusy ||
upstreamLoading ||
!hasUpstream ||
publishBlockedByDetachedHead ||
(behind > 0 && !shouldForcePushWithLease) ||
publishBlockedByPRLoading ||
publishBlockedByMergedPR ||
@@ -226,6 +231,9 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
if (publishBlockedByMergedPR) {
return 'PR is already merged'
}
if (publishBlockedByDetachedHead) {
return 'Check out a branch before syncing commits'
}
if (!hasUpstream) {
// Why: mirror pushItem/syncItem — direct the user to Publish Branch
// (the primary action on an unpublished branch) rather than naming a
@@ -254,6 +262,7 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
globalBusy ||
upstreamLoading ||
!hasUpstream ||
publishBlockedByDetachedHead ||
shouldForcePushWithLease ||
behind === 0 ||
commitDisabledReason !== null
@@ -268,19 +277,22 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
? 'Checking PR status…'
: publishBlockedByMergedPR
? 'PR is already merged'
: !hasUpstream
? 'Publish the branch first to push commits'
: shouldForcePushWithLease
? 'Use Force Push — remote only has older copies of local commits'
: behind > 0 && ahead > 0
? 'Sync first to pull remote changes before pushing'
: ahead === 0
? `Nothing to push${upstreamStatus?.upstreamName ? ` to ${upstreamStatus.upstreamName}` : ''}`
: describePushCount(ahead),
: publishBlockedByDetachedHead
? 'Check out a branch before pushing commits'
: !hasUpstream
? 'Publish the branch first to push commits'
: shouldForcePushWithLease
? 'Use Force Push — remote only has older copies of local commits'
: behind > 0 && ahead > 0
? 'Sync first to pull remote changes before pushing'
: ahead === 0
? `Nothing to push${upstreamStatus?.upstreamName ? ` to ${upstreamStatus.upstreamName}` : ''}`
: describePushCount(ahead),
disabled:
globalBusy ||
upstreamLoading ||
!hasUpstream ||
publishBlockedByDetachedHead ||
ahead === 0 ||
shouldForcePushWithLease ||
(behind > 0 && !shouldForcePushWithLease)
@@ -295,14 +307,17 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
? 'Checking PR status…'
: publishBlockedByMergedPR
? 'PR is already merged'
: !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 || ahead === 0
: 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 || ahead === 0
}
const pullItem: DropdownItem = {
@@ -314,15 +329,22 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
? 'Checking PR status…'
: publishBlockedByMergedPR
? 'PR is already merged'
: !hasUpstream
? 'Publish the branch first to pull commits'
: shouldForcePushWithLease
? 'Nothing new to pull — remote only has older copies of local commits'
: behind === 0
? 'Nothing to pull'
: describePullCount(behind),
: publishBlockedByDetachedHead
? 'Check out a branch before pulling commits'
: !hasUpstream
? 'Publish the branch first to pull commits'
: shouldForcePushWithLease
? 'Nothing new to pull — remote only has older copies of local commits'
: behind === 0
? 'Nothing to pull'
: describePullCount(behind),
disabled:
globalBusy || upstreamLoading || !hasUpstream || behind === 0 || shouldForcePushWithLease
globalBusy ||
upstreamLoading ||
!hasUpstream ||
publishBlockedByDetachedHead ||
behind === 0 ||
shouldForcePushWithLease
}
const fastForwardItem: DropdownItem = {
@@ -334,19 +356,22 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
? 'Checking PR status…'
: publishBlockedByMergedPR
? 'PR is already merged'
: !hasUpstream
? 'Publish the branch first to fast-forward'
: shouldForcePushWithLease
? 'Nothing new to fast-forward — remote only has older copies of local commits'
: behind === 0
? 'Nothing to fast-forward'
: ahead > 0
? 'Local commits prevent a fast-forward pull'
: describeFastForwardCount(behind),
: publishBlockedByDetachedHead
? 'Check out a branch before fast-forwarding'
: !hasUpstream
? 'Publish the branch first to fast-forward'
: shouldForcePushWithLease
? 'Nothing new to fast-forward — remote only has older copies of local commits'
: behind === 0
? 'Nothing to fast-forward'
: ahead > 0
? 'Local commits prevent a fast-forward pull'
: describeFastForwardCount(behind),
disabled:
globalBusy ||
upstreamLoading ||
!hasUpstream ||
publishBlockedByDetachedHead ||
behind === 0 ||
ahead > 0 ||
shouldForcePushWithLease
@@ -361,17 +386,20 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
? 'Checking PR status…'
: publishBlockedByMergedPR
? 'PR is already merged'
: !hasUpstream
? 'Publish the branch first to sync commits'
: shouldForcePushWithLease
? 'Use Force Push — remote only has older copies of local commits'
: ahead === 0 && behind === 0
? 'Branch is up to date'
: describeSyncCounts(ahead, behind),
: publishBlockedByDetachedHead
? 'Check out a branch before syncing commits'
: !hasUpstream
? 'Publish the branch first to sync commits'
: shouldForcePushWithLease
? 'Use Force Push — remote only has older copies of local commits'
: ahead === 0 && behind === 0
? 'Branch is up to date'
: describeSyncCounts(ahead, behind),
disabled:
globalBusy ||
upstreamLoading ||
!hasUpstream ||
publishBlockedByDetachedHead ||
shouldForcePushWithLease ||
(ahead === 0 && behind === 0)
}
@@ -416,30 +444,35 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr
label:
publishBlockedByMergedPR || publishBlockedByPRLoading
? 'PR Status'
: publishBlockedByUncommittedChanges
? 'Commit Changes First'
: publishBlockedByNoBranchCommits
? 'No Branch Changes'
: 'Publish Branch',
: publishBlockedByDetachedHead
? 'No Branch'
: publishBlockedByUncommittedChanges
? 'Commit Changes First'
: publishBlockedByNoBranchCommits
? 'No Branch Changes'
: 'Publish Branch',
title: upstreamLoading
? 'Checking branch status…'
: publishBlockedByPRLoading
? 'Checking PR status…'
: publishBlockedByMergedPR
? 'PR is already merged'
: publishBlockedByUncommittedChanges
? 'Commit changes before publishing the branch'
: publishBlockedByNoBranchCommits
? 'Nothing to publish'
: hasUpstream
? 'Branch is already published'
: 'Publish this branch to origin',
: publishBlockedByDetachedHead
? 'Check out a branch before publishing commits'
: publishBlockedByUncommittedChanges
? 'Commit changes before publishing the branch'
: publishBlockedByNoBranchCommits
? 'Nothing to publish'
: hasUpstream
? 'Branch is already published'
: 'Publish this branch to origin',
disabled:
globalBusy ||
upstreamLoading ||
hasUpstream ||
publishBlockedByPRLoading ||
publishBlockedByMergedPR ||
publishBlockedByDetachedHead ||
publishBlockedByNoBranchCommits
}
@@ -62,6 +62,9 @@ export type PrimaryActionInputs = {
// carries commits beyond the compare base. Undefined preserves the old
// behavior while the branch compare request is still unavailable/loading.
branchCommitsAhead?: number
// Why: detached HEAD can look like an unpublished branch from upstream
// status alone, but it has no branch ref that Publish Branch can push.
hasCurrentBranch?: boolean
}
export const PRIMARY_LABEL_BY_KIND: Record<Exclude<PrimaryActionKind, 'commit'>, string> = {
@@ -201,6 +201,22 @@ describe('resolvePrimaryAction', () => {
})
})
it('does not offer Publish Branch when HEAD is detached', () => {
const result = resolvePrimaryAction(
inputs({
upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 },
branchCommitsAhead: 4,
hasCurrentBranch: false
})
)
expect(result).toEqual({
kind: 'commit',
label: 'Commit',
title: 'Check out a branch before publishing commits.',
disabled: true
})
})
it('does not offer Publish Branch when an unpublished branch has no commits ahead', () => {
const result = resolvePrimaryAction(
inputs({ upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 }, branchCommitsAhead: 0 })
@@ -61,7 +61,8 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction
prState,
isPRStateLoading,
hostedReviewCreation,
branchCommitsAhead
branchCommitsAhead,
hasCurrentBranch = true
} = inputs
// 1. Commit in flight — lock the primary no matter what else is true.
@@ -193,6 +194,21 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction
}
if (!upstreamStatus.hasUpstream) {
if (!hasCurrentBranch) {
return {
kind: 'commit',
label: translate(
'auto.components.right.sidebar.source.control.primary.action.ed93b4f14f',
'Commit'
),
title: translate(
'auto.components.right.sidebar.source.control.primary.action.e61b0d7a3c',
'Check out a branch before publishing commits.'
),
disabled: true
}
}
if (branchCommitsAhead === 0) {
return {
kind: 'commit',
+2 -1
View File
@@ -8218,7 +8218,8 @@
"a6457b46a7": "Resolve conflicts before committing",
"484f45c439": "{{value0}} in progress…",
"74fc171e99": "Force Push in progress…",
"16aee3a5c1": "Commit in progress…"
"16aee3a5c1": "Commit in progress…",
"e61b0d7a3c": "Check out a branch before publishing commits."
}
}
}
+2 -1
View File
@@ -8091,7 +8091,8 @@
"a6457b46a7": "Resolver conflictos antes de comprometerse",
"484f45c439": "{{value0}} en progreso…",
"74fc171e99": "Empuje forzado en progreso...",
"16aee3a5c1": "Compromiso en progreso..."
"16aee3a5c1": "Compromiso en progreso...",
"e61b0d7a3c": "Check out a branch before publishing commits."
}
}
}
+2 -1
View File
@@ -8199,7 +8199,8 @@
"a6457b46a7": "コミットする前に競合を解決する",
"484f45c439": "{{value0}} が進行中です…",
"74fc171e99": "強制プッシュ中です…",
"16aee3a5c1": "Commit 中です…"
"16aee3a5c1": "Commit 中です…",
"e61b0d7a3c": "Check out a branch before publishing commits."
}
}
}
+2 -1
View File
@@ -8199,7 +8199,8 @@
"a6457b46a7": "커밋하기 전에 충돌을 해결하세요.",
"484f45c439": "{{value0}} 진행 중…",
"74fc171e99": "강제 푸시 진행 중…",
"16aee3a5c1": "Commit 진행 중…"
"16aee3a5c1": "Commit 진행 중…",
"e61b0d7a3c": "Check out a branch before publishing commits."
}
}
}
+2 -1
View File
@@ -8199,7 +8199,8 @@
"a6457b46a7": "提交前解决冲突",
"484f45c439": "{{value0}} 正在进行中...",
"74fc171e99": "强制推送正在进行中...",
"16aee3a5c1": "正在进行中……"
"16aee3a5c1": "正在进行中……",
"e61b0d7a3c": "Check out a branch before publishing commits."
}
}
}