diff --git a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte index c1a70f17..ef342828 100644 --- a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte +++ b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte @@ -33,7 +33,7 @@ type CompletedPushOutcome, } from './branchCardHelpers'; import { buildPrButtonTitle } from './prButtonTooltip'; - import { shouldShowPushChanges } from './prButtonGitState'; + import { derivePrPushAction } from './prButtonGitState'; import { getPreferredAgent } from '../settings/preferences.svelte'; import { agentState, REMOTE_AGENTS } from '../agents/agent.svelte'; import { prStateStore, type PrState } from '../../stores/prState.svelte'; @@ -107,17 +107,18 @@ let prStatusDraft = $state(null); let failedChecks = $state([]); - // Derive hasUnpushed from the git-state upstream relation when available, + // Derive push action from the git-state upstream relation when available, // falling back to a HEAD/PR SHA comparison only when upstream is missing // (e.g. fork PRs without an `origin/` ref). - let hasUnpushed = $derived( - shouldShowPushChanges({ + let pushAction = $derived( + derivePrPushAction({ prNumber: branch.prNumber, prState: branch.prState, prHeadSha, gitState: timeline?.gitState ?? null, }) ); + let hasUnpushed = $derived(pushAction !== 'none'); // Sync local PR status state when branch prop changes let syncedBranchId = $state(null); @@ -357,7 +358,9 @@ if (pushState === 'pushing') return 'Pushing… (click to view)'; if (pushState === 'error') return 'Push failed — click for details'; if (prState === 'created' && hasUnpushed) { - return optionHeld ? 'Force push to remote' : 'Push changes to remote'; + return pushAction === 'forcePush' || optionHeld + ? 'Force push to remote' + : 'Push changes to remote'; } if (prState === 'created') { if (prStatusState === 'MERGED') return 'Merged'; @@ -596,7 +599,13 @@ return; } if (prState === 'created' && hasUnpushed && pushState === 'idle') { - handlePush(optionHeld); + const wantsForcePush = pushAction === 'forcePush' || optionHeld; + if (wantsForcePush && !optionHeld) { + // Force push without an explicit alt override — confirm first. + showForcePushDialog = true; + } else { + handlePush(wantsForcePush); + } } else if (prState === 'created') { const url = prUrl ?? cachedPrUrl; if (url) { @@ -683,7 +692,7 @@ {:else if pushState === 'error'} Push failed {:else if prState === 'created' && hasUnpushed} - {optionHeld ? 'Force push' : 'Push changes'} + {pushAction === 'forcePush' || optionHeld ? 'Force push' : 'Push changes'} {:else if prState === 'created'} {#if prStatusText} {prStatusText} diff --git a/apps/staged/src/lib/features/branches/prButtonGitState.test.ts b/apps/staged/src/lib/features/branches/prButtonGitState.test.ts index aeb5a3d8..22128c43 100644 --- a/apps/staged/src/lib/features/branches/prButtonGitState.test.ts +++ b/apps/staged/src/lib/features/branches/prButtonGitState.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { shouldShowPushChanges } from './prButtonGitState'; +import { derivePrPushAction, shouldShowPushChanges } from './prButtonGitState'; import type { BranchGitState, UpstreamRelation } from '../../types'; function makeGitState( @@ -179,3 +179,132 @@ describe('shouldShowPushChanges', () => { ).toBe(false); }); }); + +describe('derivePrPushAction', () => { + it("returns 'push' when local is ahead of upstream", () => { + expect( + derivePrPushAction({ + prNumber: 42, + prState: 'OPEN', + prHeadSha: 'prsha', + gitState: makeGitState('localAhead'), + }) + ).toBe('push'); + }); + + it("returns 'forcePush' when local has diverged from upstream", () => { + expect( + derivePrPushAction({ + prNumber: 42, + prState: 'OPEN', + prHeadSha: 'prsha', + gitState: makeGitState('diverged'), + }) + ).toBe('forcePush'); + }); + + it("returns 'none' when local is in sync with upstream", () => { + expect( + derivePrPushAction({ + prNumber: 42, + prState: 'OPEN', + prHeadSha: 'prsha', + gitState: makeGitState('inSync'), + }) + ).toBe('none'); + }); + + it("returns 'none' when origin is ahead of local", () => { + expect( + derivePrPushAction({ + prNumber: 42, + prState: 'OPEN', + prHeadSha: 'prsha', + gitState: makeGitState('originAhead'), + }) + ).toBe('none'); + }); + + it("returns 'push' for missing upstream with differing SHAs (fork PR fallback)", () => { + // We deliberately don't classify fork PRs as 'forcePush' up-front — + // the backend's non-FF rejection still drives that path through + // showForcePushDialog. + expect( + derivePrPushAction({ + prNumber: 42, + prState: 'OPEN', + prHeadSha: 'prsha', + gitState: makeGitState('missing', { headSha: 'localsha' }), + }) + ).toBe('push'); + }); + + it("returns 'none' for missing upstream when SHAs match", () => { + expect( + derivePrPushAction({ + prNumber: 42, + prState: 'OPEN', + prHeadSha: 'samesha', + gitState: makeGitState('missing', { headSha: 'samesha' }), + }) + ).toBe('none'); + }); + + it("returns 'none' for missing upstream when prHeadSha is null", () => { + expect( + derivePrPushAction({ + prNumber: 42, + prState: 'OPEN', + prHeadSha: null, + gitState: makeGitState('missing'), + }) + ).toBe('none'); + }); + + it("returns 'none' for merged PRs even when local has diverged", () => { + expect( + derivePrPushAction({ + prNumber: 42, + prState: 'MERGED', + prHeadSha: 'prsha', + gitState: makeGitState('diverged'), + }) + ).toBe('none'); + }); + + it("returns 'none' when the branch has no PR yet", () => { + expect( + derivePrPushAction({ + prNumber: null, + prState: null, + prHeadSha: null, + gitState: makeGitState('diverged'), + }) + ).toBe('none'); + }); + + it("returns 'none' when git state has not loaded yet", () => { + expect( + derivePrPushAction({ + prNumber: 42, + prState: 'OPEN', + prHeadSha: 'prsha', + gitState: null, + }) + ).toBe('none'); + }); + + it("returns 'none' when a different branch is checked out (diverged would otherwise apply)", () => { + expect( + derivePrPushAction({ + prNumber: 42, + prState: 'OPEN', + prHeadSha: 'prsha', + gitState: makeGitState('diverged', { + expectedBranchMatches: false, + currentBranch: 'main', + }), + }) + ).toBe('none'); + }); +}); diff --git a/apps/staged/src/lib/features/branches/prButtonGitState.ts b/apps/staged/src/lib/features/branches/prButtonGitState.ts index a06d70a2..f73b88a6 100644 --- a/apps/staged/src/lib/features/branches/prButtonGitState.ts +++ b/apps/staged/src/lib/features/branches/prButtonGitState.ts @@ -7,33 +7,43 @@ export interface ShouldShowPushChangesInput { gitState: BranchGitState | null | undefined; } -export function shouldShowPushChanges(input: ShouldShowPushChangesInput): boolean { +export type PrPushAction = 'none' | 'push' | 'forcePush'; + +export function derivePrPushAction(input: ShouldShowPushChangesInput): PrPushAction { const { prNumber, prState, prHeadSha, gitState } = input; - if (!prNumber) return false; - if (prState === 'MERGED') return false; - if (!gitState) return false; + if (!prNumber) return 'none'; + if (prState === 'MERGED') return 'none'; + if (!gitState) return 'none'; // When a different branch is checked out, neither `upstream.relation` // (computed against HEAD's upstream) nor `headSha` reflect this PR's // branch, so the comparison would be meaningless. - if (!gitState.expectedBranchMatches) return false; + if (!gitState.expectedBranchMatches) return 'none'; switch (gitState.upstream.relation) { case 'localAhead': + return 'push'; case 'diverged': - return true; + return 'forcePush'; case 'inSync': case 'originAhead': - return false; + return 'none'; case 'missing': { // Fork PRs may not have origin/. Fall back to comparing local - // HEAD with the PR head SHA reported by GitHub. - if (!gitState.headSha || !prHeadSha) return false; - return gitState.headSha !== prHeadSha; + // HEAD with the PR head SHA reported by GitHub. We can't tell from + // this data alone whether the remote diverged, so classify as 'push' + // and rely on the backend's non-fast-forward rejection to surface + // the force-push dialog if needed. + if (!gitState.headSha || !prHeadSha) return 'none'; + return gitState.headSha !== prHeadSha ? 'push' : 'none'; } default: { const _exhaustive: never = gitState.upstream.relation; void _exhaustive; - return false; + return 'none'; } } } + +export function shouldShowPushChanges(input: ShouldShowPushChangesInput): boolean { + return derivePrPushAction(input) !== 'none'; +}