From 84d26fc747ccc35d31b17bd2679e9bbd52b29d7b Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 15 May 2026 21:54:53 -0700 Subject: [PATCH] fix: require stage all for partially staged commits (#2063) --- .../CommitArea.chevron-spinner.test.tsx | 1 + .../CommitArea.generate.test.tsx | 1 + .../CommitArea.primary-icons.test.tsx | 5 ++- .../right-sidebar/CommitArea.test.tsx | 1 + .../right-sidebar/SourceControl.tsx | 11 ++++++ .../source-control-dropdown-items.test.ts | 20 ++++++++++ .../source-control-dropdown-items.ts | 4 ++ .../source-control-primary-action.test.ts | 37 +++++++++++++----- .../source-control-primary-action.ts | 38 +++++++++++++------ 9 files changed, 95 insertions(+), 23 deletions(-) diff --git a/src/renderer/src/components/right-sidebar/CommitArea.chevron-spinner.test.tsx b/src/renderer/src/components/right-sidebar/CommitArea.chevron-spinner.test.tsx index 6091979b4..0fdb06901 100644 --- a/src/renderer/src/components/right-sidebar/CommitArea.chevron-spinner.test.tsx +++ b/src/renderer/src/components/right-sidebar/CommitArea.chevron-spinner.test.tsx @@ -70,6 +70,7 @@ function buildInputs(overrides: Partial = {}): PrimaryActio return { stagedCount: 1, hasUnstagedChanges: false, + hasPartiallyStagedChanges: false, hasMessage: true, hasUnresolvedConflicts: false, isCommitting: false, diff --git a/src/renderer/src/components/right-sidebar/CommitArea.generate.test.tsx b/src/renderer/src/components/right-sidebar/CommitArea.generate.test.tsx index 90fdeca70..5111dea25 100644 --- a/src/renderer/src/components/right-sidebar/CommitArea.generate.test.tsx +++ b/src/renderer/src/components/right-sidebar/CommitArea.generate.test.tsx @@ -88,6 +88,7 @@ function buildInputs(overrides: Partial = {}): PrimaryActio return { stagedCount: 1, hasUnstagedChanges: false, + hasPartiallyStagedChanges: false, hasMessage: true, hasUnresolvedConflicts: false, isCommitting: false, diff --git a/src/renderer/src/components/right-sidebar/CommitArea.primary-icons.test.tsx b/src/renderer/src/components/right-sidebar/CommitArea.primary-icons.test.tsx index 3f0099934..60eb9a492 100644 --- a/src/renderer/src/components/right-sidebar/CommitArea.primary-icons.test.tsx +++ b/src/renderer/src/components/right-sidebar/CommitArea.primary-icons.test.tsx @@ -58,6 +58,7 @@ function buildInputs(overrides: Partial = {}): PrimaryActio return { stagedCount: 1, hasUnstagedChanges: false, + hasPartiallyStagedChanges: false, hasMessage: true, hasUnresolvedConflicts: false, isCommitting: false, @@ -138,9 +139,9 @@ describe('CommitArea primary action icons', () => { expect(primaryHasIcon(element, CloudUpload)).toBe(true) }) - // Why: a dirty tree with nothing staged surfaces 'Stage Files' as the + // Why: a dirty tree with nothing staged surfaces 'Stage All' as the // primary, anchored by a Plus icon to read as an additive bulk action. - it('renders a plus icon on a Stage Files primary', () => { + it('renders a plus icon on a Stage All primary', () => { const props = baseProps({ stagedCount: 0, hasUnstagedChanges: true, diff --git a/src/renderer/src/components/right-sidebar/CommitArea.test.tsx b/src/renderer/src/components/right-sidebar/CommitArea.test.tsx index c362b2d4e..5eb40a5d2 100644 --- a/src/renderer/src/components/right-sidebar/CommitArea.test.tsx +++ b/src/renderer/src/components/right-sidebar/CommitArea.test.tsx @@ -111,6 +111,7 @@ function buildInputs(overrides: Partial = {}): PrimaryActio return { stagedCount: 1, hasUnstagedChanges: false, + hasPartiallyStagedChanges: false, hasMessage: true, hasUnresolvedConflicts: false, isCommitting: false, diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 1a5c1c894..d603016f4 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -1243,12 +1243,20 @@ function SourceControlInner(): React.JSX.Element { ) const hasUnstagedChanges = grouped.unstaged.length > 0 || grouped.untracked.length > 0 + const hasPartiallyStagedChanges = useMemo(() => { + if (grouped.staged.length === 0 || grouped.unstaged.length === 0) { + return false + } + const unstagedPaths = new Set(grouped.unstaged.map((entry) => entry.path)) + return grouped.staged.some((entry) => unstagedPaths.has(entry.path)) + }, [grouped.staged, grouped.unstaged]) const primaryAction: PrimaryAction = useMemo( () => resolvePrimaryAction({ stagedCount: grouped.staged.length, hasUnstagedChanges, + hasPartiallyStagedChanges, hasMessage: commitMessage.trim().length > 0, hasUnresolvedConflicts: unresolvedConflicts.length > 0, isCommitting, @@ -1263,6 +1271,7 @@ function SourceControlInner(): React.JSX.Element { commitMessage, grouped.staged.length, hasUnstagedChanges, + hasPartiallyStagedChanges, isCommitting, isRemoteOperationActive, inFlightRemoteOpKind, @@ -1279,6 +1288,7 @@ function SourceControlInner(): React.JSX.Element { resolveDropdownItems({ stagedCount: grouped.staged.length, hasUnstagedChanges, + hasPartiallyStagedChanges, hasMessage: commitMessage.trim().length > 0, hasUnresolvedConflicts: unresolvedConflicts.length > 0, isCommitting, @@ -1293,6 +1303,7 @@ function SourceControlInner(): React.JSX.Element { commitMessage, grouped.staged.length, hasUnstagedChanges, + hasPartiallyStagedChanges, isCommitting, isRemoteOperationActive, inFlightRemoteOpKind, diff --git a/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts index 2f30edc60..9cfcca3ed 100644 --- a/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts @@ -8,6 +8,7 @@ function inputs(overrides: Partial = {}): PrimaryActionInpu return { stagedCount: 0, hasUnstagedChanges: false, + hasPartiallyStagedChanges: false, hasMessage: false, hasUnresolvedConflicts: false, isCommitting: false, @@ -54,6 +55,25 @@ describe('resolveDropdownItems', () => { expect(byKind.commit_sync.disabled).toBe(true) }) + it('disables commit actions when staged files also have unstaged changes', () => { + const items = resolveDropdownItems( + inputs({ + stagedCount: 1, + hasUnstagedChanges: true, + hasPartiallyStagedChanges: true, + hasMessage: true, + upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 } + }) + ) + const byKind = Object.fromEntries( + items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) + ) + expect(byKind.commit.disabled).toBe(true) + expect(byKind.commit_push.disabled).toBe(true) + expect(byKind.commit_sync.disabled).toBe(true) + expect(byKind.commit.title).toBe('Stage all changes before committing partially staged files') + }) + it('disables push actions but keeps Fetch enabled when branch has no upstream', () => { const items = resolveDropdownItems( inputs({ diff --git a/src/renderer/src/components/right-sidebar/source-control-dropdown-items.ts b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.ts index ed7fca8d8..eeb116216 100644 --- a/src/renderer/src/components/right-sidebar/source-control-dropdown-items.ts +++ b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.ts @@ -57,6 +57,7 @@ function formatSyncLabel(base: string, ahead: number, behind: number): string { export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry[] { const { stagedCount, + hasPartiallyStagedChanges, hasMessage, hasUnresolvedConflicts, isCommitting, @@ -94,6 +95,9 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry if (!hasStaged) { return 'Stage at least one file to commit' } + if (hasPartiallyStagedChanges) { + return 'Stage all changes before committing partially staged files' + } if (!hasMessage) { return 'Enter a commit message to commit' } diff --git a/src/renderer/src/components/right-sidebar/source-control-primary-action.test.ts b/src/renderer/src/components/right-sidebar/source-control-primary-action.test.ts index 8cfb44aff..dbec18d24 100644 --- a/src/renderer/src/components/right-sidebar/source-control-primary-action.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-primary-action.test.ts @@ -8,6 +8,7 @@ function inputs(overrides: Partial = {}): PrimaryActionInpu return { stagedCount: 0, hasUnstagedChanges: false, + hasPartiallyStagedChanges: false, hasMessage: false, hasUnresolvedConflicts: false, isCommitting: false, @@ -244,11 +245,11 @@ describe('resolvePrimaryAction', () => { }) // Why: dirty trees (no staged, has unstaged/untracked) must surface a - // 'Stage Files' primary regardless of upstream state. Pulling/syncing on + // 'Stage All' primary regardless of upstream state. Pulling/syncing on // a dirty tree fails ("Please commit or stash them"), and pushing skips // the immediate user need (prepare a commit), so the staging rung // intercepts before any remote rung fires. - it('returns Stage Files on a dirty tree that is behind upstream', () => { + it('returns Stage All on a dirty tree that is behind upstream', () => { const result = resolvePrimaryAction( inputs({ hasUnstagedChanges: true, @@ -257,13 +258,13 @@ describe('resolvePrimaryAction', () => { ) expect(result).toEqual({ kind: 'stage', - label: 'Stage Files', + label: 'Stage All', title: 'Stage all changes', disabled: false }) }) - it('returns Stage Files on a dirty tree that is ahead of upstream', () => { + it('returns Stage All on a dirty tree that is ahead of upstream', () => { const result = resolvePrimaryAction( inputs({ hasUnstagedChanges: true, @@ -271,11 +272,11 @@ describe('resolvePrimaryAction', () => { }) ) expect(result.kind).toBe('stage') - expect(result.label).toBe('Stage Files') + expect(result.label).toBe('Stage All') expect(result.disabled).toBe(false) }) - it('returns Stage Files on a dirty tree with no upstream branch', () => { + it('returns Stage All on a dirty tree with no upstream branch', () => { const result = resolvePrimaryAction( inputs({ hasUnstagedChanges: true, @@ -285,7 +286,7 @@ describe('resolvePrimaryAction', () => { expect(result.kind).toBe('stage') }) - it('returns Stage Files on a dirty tree while upstream status is still loading', () => { + it('returns Stage All on a dirty tree while upstream status is still loading', () => { const result = resolvePrimaryAction( inputs({ hasUnstagedChanges: true, upstreamStatus: undefined }) ) @@ -293,11 +294,27 @@ describe('resolvePrimaryAction', () => { expect(result.disabled).toBe(false) }) - it('still resolves to Commit when both staged and unstaged exist (staged wins)', () => { + it('returns Stage All when a staged file also has unstaged changes', () => { const result = resolvePrimaryAction( inputs({ stagedCount: 1, hasUnstagedChanges: true, + hasPartiallyStagedChanges: true, + hasMessage: true, + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 } + }) + ) + expect(result.kind).toBe('stage') + expect(result.label).toBe('Stage All') + expect(result.disabled).toBe(false) + }) + + it('still resolves to Commit when staged and unrelated unstaged files exist', () => { + const result = resolvePrimaryAction( + inputs({ + stagedCount: 1, + hasUnstagedChanges: true, + hasPartiallyStagedChanges: false, hasMessage: true, upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 } }) @@ -315,13 +332,13 @@ describe('resolvePrimaryAction', () => { expect(result.title).toBe('Enter a commit message to commit') }) - it('returns Stage Files when unstaged changes exist on an in-sync branch', () => { + it('returns Stage All when unstaged changes exist on an in-sync branch', () => { const result = resolvePrimaryAction( inputs({ hasUnstagedChanges: true, upstreamStatus: upstreamInSync }) ) expect(result).toEqual({ kind: 'stage', - label: 'Stage Files', + label: 'Stage All', title: 'Stage all changes', disabled: false }) diff --git a/src/renderer/src/components/right-sidebar/source-control-primary-action.ts b/src/renderer/src/components/right-sidebar/source-control-primary-action.ts index 022c0d383..b5266a5e1 100644 --- a/src/renderer/src/components/right-sidebar/source-control-primary-action.ts +++ b/src/renderer/src/components/right-sidebar/source-control-primary-action.ts @@ -42,6 +42,7 @@ export type PrimaryAction = { export type PrimaryActionInputs = { stagedCount: number hasUnstagedChanges: boolean + hasPartiallyStagedChanges: boolean hasMessage: boolean hasUnresolvedConflicts: boolean isCommitting: boolean @@ -58,7 +59,7 @@ export type PrimaryActionInputs = { } const PRIMARY_LABEL_BY_KIND: Record, string> = { - stage: 'Stage Files', + stage: 'Stage All', push: 'Push', pull: 'Pull', sync: 'Sync', @@ -85,11 +86,13 @@ function describeSyncCounts(ahead: number, behind: number): string { * 1. In-flight commit locks the primary to a disabled "Commit". * 2. In-flight remote operation keeps the current label but disables it. * 3. Unresolved conflicts block the commit path entirely. - * 4. Has staged files + message → plain "Commit" (compound flows live in - * the dropdown; after the commit lands, step 6 rotates the primary to + * 4. Has partially staged files → "Stage All" to avoid hook-time partial + * stash conflicts. + * 5. Has staged files + message → plain "Commit" (compound flows live in + * the dropdown; after the commit lands, step 7 rotates the primary to * the appropriate single remote action). - * 5. Has staged files + no message → disabled "Commit" with a reason. - * 6. Clean tree → adaptive remote action (or disabled "Commit" no-op). + * 6. Has staged files + no message → disabled "Commit" with a reason. + * 7. Clean tree → adaptive remote action (or disabled "Commit" no-op). * * An undefined upstream status means fetchUpstreamStatus has not resolved * yet for this worktree. We return a disabled Commit so the button has a @@ -100,6 +103,7 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction const { stagedCount, hasUnstagedChanges, + hasPartiallyStagedChanges, hasMessage, hasUnresolvedConflicts, isCommitting, @@ -180,10 +184,22 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction const hasStaged = stagedCount > 0 - // 4. Has staged files + message → plain Commit. The primary button never + // 4. A path with both staged and unstaged edits can make lint-staged's + // partial-stash restore fail after formatters rewrite the staged copy. Push + // the user through Stage All first so the index matches the worktree. + if (hasStaged && hasPartiallyStagedChanges) { + return { + kind: 'stage', + label: 'Stage All', + title: 'Stage all changes before committing partially staged files', + disabled: false + } + } + + // 5. Has staged files + message → plain Commit. The primary button never // compounds ("Commit & Push" etc.) — after the commit lands, the primary // naturally rotates to the appropriate remote action (Push / Sync / - // Publish Branch) via step 6 below. Users who want the one-click + // Publish Branch) via step 7 below. Users who want the one-click // compound flow can still reach it from the dropdown. if (hasStaged && hasMessage) { return { @@ -194,7 +210,7 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction } } - // 5. Has staged files but no message — user just needs to type something. + // 6. Has staged files but no message — user just needs to type something. if (hasStaged && !hasMessage) { return { kind: 'commit', @@ -204,7 +220,7 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction } } - // 5b. Nothing staged but local changes exist — surface staging as the + // 6b. Nothing staged but local changes exist — surface staging as the // primary so dirty trees don't invite a remote op (pull/sync would fail // with uncommitted changes; push/publish skips the actual user need). // Sits before the upstream-status checks so it works regardless of @@ -212,13 +228,13 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction if (!hasStaged && hasUnstagedChanges) { return { kind: 'stage', - label: 'Stage Files', + label: 'Stage All', title: 'Stage all changes', disabled: false } } - // 6. Clean tree + no staged files → adaptive remote action. + // 7. Clean tree + no staged files → adaptive remote action. if (!upstreamStatus) { return { kind: 'commit',