Improve commit failure display (#2141)

This commit is contained in:
Jinjing
2026-05-16 22:45:01 -07:00
committed by GitHub
parent fa4c524e35
commit babedfafde
7 changed files with 515 additions and 632 deletions
@@ -1,7 +1,6 @@
import { describe, expect, it, vi } from 'vitest'
import { RefreshCw } from 'lucide-react'
import { renderToStaticMarkup } from 'react-dom/server'
import { CommitArea } from './SourceControl'
import { Button } from '@/components/ui/button'
import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action'
import { resolveDropdownItems, type DropdownActionKind } from './source-control-dropdown-items'
@@ -10,62 +9,6 @@ import { resolveDropdownItems, type DropdownActionKind } from './source-control-
// behaviour for dropdown-only ops (Fetch) and the no-double-spin guard
// when the primary button already hosts the in-flight indicator.
type ReactElementLike = {
type: unknown
props: Record<string, unknown>
}
function visit(node: unknown, cb: (node: ReactElementLike) => void): void {
if (node == null || typeof node === 'string' || typeof node === 'number') {
return
}
if (Array.isArray(node)) {
node.forEach((entry) => visit(entry, cb))
return
}
const element = node as ReactElementLike
cb(element)
if (element.props?.children) {
visit(element.props.children, cb)
}
}
function findButtons(node: unknown): ReactElementLike[] {
const buttons: ReactElementLike[] = []
visit(node, (entry) => {
if (entry.type === Button) {
buttons.push(entry)
}
})
return buttons
}
function buttonHasSpinner(button: ReactElementLike): boolean {
let found = false
visit(button, (entry) => {
if (entry.type === RefreshCw) {
found = true
}
})
return found
}
function primaryHasSpinner(node: unknown): boolean {
const buttons = findButtons(node)
if (buttons.length === 0) {
throw new Error('primary button not found')
}
return buttonHasSpinner(buttons[0]!)
}
function chevronHasSpinner(node: unknown): boolean {
const buttons = findButtons(node)
if (buttons.length < 2) {
throw new Error('chevron button not found')
}
return buttonHasSpinner(buttons[1]!)
}
function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActionInputs {
return {
stagedCount: 1,
@@ -83,6 +26,7 @@ function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActio
function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
const inputs = buildInputs(overrides)
return {
worktreeId: 'wt-1',
commitMessage: 'feat: add commit area',
commitError: null as string | null,
remoteActionError: null as string | null,
@@ -105,72 +49,68 @@ function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
}
}
function buttons(markup: string): string[] {
return [...markup.matchAll(/<button\b[\s\S]*?<\/button>/g)].map((match) => match[0])
}
function renderButtons(props: ReturnType<typeof baseProps>): string[] {
return buttons(renderToStaticMarkup(<CommitArea {...props} />))
}
describe('CommitArea chevron spinner', () => {
// Why: when the primary can't host the in-flight op (Fetch is the
// canonical case — it's dropdown-only) the click would otherwise be
// silent: the toast only fires on failure and a no-op fetch leaves
// upstream counts unchanged. Spinning the chevron gives the user
// immediate "yes, your click did something" feedback.
it('spins the chevron while a dropdown Fetch is in flight', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'fetch'
})
const element = CommitArea(props)
expect(chevronHasSpinner(element)).toBe(true)
expect(primaryHasSpinner(element)).toBe(false)
const [primary, chevron] = renderButtons(
baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'fetch'
})
)
expect(chevron).toContain('animate-spin')
expect(primary).not.toContain('animate-spin')
})
// Why: avoid double-spinning. When the primary is already spinning for
// an op it hosts (e.g. user clicked Push from the dropdown and the
// primary mirrors "Push"), the chevron stays as a chevron — one
// spinner per button surface, anchored to the action the label names.
it('does not spin the chevron when the primary already hosts the in-flight op', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'push'
})
const element = CommitArea(props)
expect(primaryHasSpinner(element)).toBe(true)
expect(chevronHasSpinner(element)).toBe(false)
const [primary, chevron] = renderButtons(
baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'push'
})
)
expect(primary).toContain('animate-spin')
expect(chevron).not.toContain('animate-spin')
})
// Why: a dropdown Sync from a Push-natural state mirrors onto the
// primary (label flips to "Sync"), so the primary already carries the
// spinner. The chevron should not double-spin in that case.
it('does not spin the chevron when a dropdown op is mirrored onto the primary', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 3, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'sync'
})
const element = CommitArea(props)
expect(primaryHasSpinner(element)).toBe(true)
expect(chevronHasSpinner(element)).toBe(false)
const [primary, chevron] = renderButtons(
baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 3, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'sync'
})
)
expect(primary).toContain('animate-spin')
expect(chevron).not.toContain('animate-spin')
})
// Why: the plain-Commit primary is the scenario from the original Fetch
// bug — empty message + dropdown Fetch left both buttons static. The
// chevron must spin so the user sees feedback even though the primary
// can't (showing a spinner on a disabled "Commit" would mis-narrate).
it('spins the chevron when a dropdown remote op runs while the primary is plain Commit', () => {
const props = baseProps({
stagedCount: 1,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'fetch'
})
const element = CommitArea(props)
expect(primaryHasSpinner(element)).toBe(false)
expect(chevronHasSpinner(element)).toBe(true)
const [primary, chevron] = renderButtons(
baseProps({
stagedCount: 1,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'fetch'
})
)
expect(primary).not.toContain('animate-spin')
expect(chevron).toContain('animate-spin')
})
})
@@ -1,89 +1,10 @@
import { describe, expect, it, vi } from 'vitest'
import { renderToStaticMarkup } from 'react-dom/server'
import { CommitArea } from './SourceControl'
import { Button } from '@/components/ui/button'
import { TooltipProvider } from '@/components/ui/tooltip'
import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action'
import { resolveDropdownItems, type DropdownActionKind } from './source-control-dropdown-items'
type ReactElementLike = {
type: unknown
props: Record<string, unknown>
}
function visit(node: unknown, cb: (node: ReactElementLike) => void): void {
if (node == null || typeof node === 'string' || typeof node === 'number') {
return
}
if (Array.isArray(node)) {
node.forEach((entry) => visit(entry, cb))
return
}
const element = node as ReactElementLike
cb(element)
if (element.props?.children) {
visit(element.props.children, cb)
}
}
function findNativeButtonByAriaLabel(node: unknown, ariaLabel: string): ReactElementLike {
let found: ReactElementLike | null = null
visit(node, (entry) => {
if (entry.type === 'button' && entry.props['aria-label'] === ariaLabel) {
found = entry
}
})
if (!found) {
throw new Error(`button not found: ${ariaLabel}`)
}
return found
}
function hasNativeButtonByAriaLabel(node: unknown, ariaLabel: string): boolean {
let found = false
visit(node, (entry) => {
if (entry.type === 'button' && entry.props['aria-label'] === ariaLabel) {
found = true
}
})
return found
}
function findTextarea(node: unknown): ReactElementLike {
let found: ReactElementLike | null = null
visit(node, (entry) => {
if (entry.type === 'textarea') {
found = entry
}
})
if (!found) {
throw new Error('textarea not found')
}
return found
}
function hasText(node: unknown, text: string): boolean {
let found = false
const walk = (value: unknown): void => {
if (typeof value === 'string') {
if (value.includes(text)) {
found = true
}
return
}
if (Array.isArray(value)) {
value.forEach(walk)
return
}
const element = value as ReactElementLike | null
if (element && typeof element === 'object' && 'props' in element) {
walk(element.props?.children)
}
}
visit(node, (entry) => {
walk(entry.props?.children)
})
return found
}
function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActionInputs {
return {
stagedCount: 1,
@@ -101,6 +22,7 @@ function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActio
function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
const inputs = buildInputs(overrides)
return {
worktreeId: 'wt-1',
commitMessage: 'feat: add commit area',
commitError: null as string | null,
remoteActionError: null as string | null,
@@ -123,94 +45,109 @@ function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
}
}
function renderCommitArea(props: ReturnType<typeof baseProps>): string {
return renderToStaticMarkup(
<TooltipProvider>
<CommitArea {...props} />
</TooltipProvider>
)
}
function buttonByLabel(markup: string, label: string): string {
const button = [...markup.matchAll(/<button\b[\s\S]*?<\/button>/g)]
.map((match) => match[0])
.find((entry) => entry.includes(`aria-label="${label}"`))
if (!button) {
throw new Error(`button not found: ${label}`)
}
return button
}
function hasDisabledAttribute(markup: string): boolean {
return markup.includes(' disabled=""')
}
describe('CommitArea AI generation', () => {
it('does not render the AI generate affordance when the feature is disabled', () => {
const element = CommitArea(baseProps())
expect(hasNativeButtonByAriaLabel(element, 'Generate commit message with AI')).toBe(false)
expect(renderCommitArea(baseProps())).not.toContain(
'aria-label="Generate commit message with AI"'
)
})
it('enables AI generation only when an agent is configured, changes are staged, and the message is empty', () => {
const onGenerate = vi.fn()
const props = baseProps({ hasMessage: false })
const element = CommitArea({
const markup = renderCommitArea({
...props,
commitMessage: '',
aiEnabled: true,
aiAgentConfigured: true,
onGenerate
aiAgentConfigured: true
})
const button = findNativeButtonByAriaLabel(element, 'Generate commit message with AI')
expect(button.props.disabled).toBe(false)
;(button.props.onClick as () => void)()
expect(onGenerate).toHaveBeenCalledTimes(1)
expect(hasDisabledAttribute(buttonByLabel(markup, 'Generate commit message with AI'))).toBe(
false
)
})
it('disables AI generation when the textarea already has user text', () => {
const element = CommitArea({
const markup = renderCommitArea({
...baseProps(),
aiEnabled: true,
aiAgentConfigured: true
})
const button = findNativeButtonByAriaLabel(element, 'Generate commit message with AI')
expect(button.props.disabled).toBe(true)
expect(button.props.title).toBe('Clear the message to regenerate.')
const button = buttonByLabel(markup, 'Generate commit message with AI')
expect(hasDisabledAttribute(button)).toBe(true)
expect(button).toContain('title="Clear the message to regenerate."')
})
it('disables AI generation until the configured agent can actually run', () => {
const props = baseProps({ hasMessage: false })
const element = CommitArea({
const markup = renderCommitArea({
...props,
commitMessage: '',
aiEnabled: true,
aiAgentConfigured: false
})
const button = findNativeButtonByAriaLabel(element, 'Generate commit message with AI')
expect(button.props.disabled).toBe(true)
expect(button.props.title).toBe('Pick an agent in Settings → AI Commit Messages.')
const button = buttonByLabel(markup, 'Generate commit message with AI')
expect(hasDisabledAttribute(button)).toBe(true)
expect(button).toContain('Pick an agent in Settings')
})
it('turns the generating icon into a stop affordance', () => {
const onCancelGenerate = vi.fn()
const props = baseProps({ hasMessage: false })
const element = CommitArea({
const markup = renderCommitArea({
...props,
commitMessage: '',
aiEnabled: true,
aiAgentConfigured: true,
isGenerating: true,
onCancelGenerate
isGenerating: true
})
const button = findNativeButtonByAriaLabel(element, 'Stop generating commit message')
expect(button.props.title).toBe('Stop generating')
expect(hasText(element, 'Generating commit message. Click to stop.')).toBe(true)
;(button.props.onClick as () => void)()
expect(onCancelGenerate).toHaveBeenCalledTimes(1)
const button = buttonByLabel(markup, 'Stop generating commit message')
expect(button).toContain('title="Stop generating"')
expect(button).toContain('lucide-refresh-cw')
expect(button).toContain('lucide-square')
})
it('shows generation errors separately from commit errors and links them to the textarea', () => {
const element = CommitArea({
const markup = renderCommitArea({
...baseProps(),
commitError: null,
generateError: 'No staged changes to summarize.'
})
expect(hasText(element, 'No staged changes to summarize.')).toBe(true)
expect(findTextarea(element).props['aria-describedby']).toBe('commit-area-generate-error')
expect(markup).toContain('No staged changes to summarize.')
expect(markup).toContain('aria-describedby="commit-area-generate-error"')
})
it('continues to render the split commit button alongside generation controls', () => {
const element = CommitArea({ ...baseProps(), aiEnabled: true, aiAgentConfigured: true })
let primaryFound = false
visit(element, (entry) => {
if (entry.type === Button) {
primaryFound = true
}
const markup = renderCommitArea({
...baseProps(),
aiEnabled: true,
aiAgentConfigured: true
})
expect(primaryFound).toBe(true)
expect(markup).toContain('Commit')
expect(markup).toContain('aria-label="Generate commit message with AI"')
})
})
@@ -1,7 +1,6 @@
import { describe, expect, it, vi } from 'vitest'
import { ArrowDownUp, ArrowUp, CloudUpload, Plus } from 'lucide-react'
import { renderToStaticMarkup } from 'react-dom/server'
import { CommitArea } from './SourceControl'
import { Button } from '@/components/ui/button'
import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action'
import { resolveDropdownItems, type DropdownActionKind } from './source-control-dropdown-items'
@@ -10,50 +9,6 @@ import { resolveDropdownItems, type DropdownActionKind } from './source-control-
// mapping for primary action kinds (push / pull / sync / publish); the
// commit-checkmark and core CommitArea behaviour live in the sibling file.
type ReactElementLike = {
type: unknown
props: Record<string, unknown>
}
function visit(node: unknown, cb: (node: ReactElementLike) => void): void {
if (node == null || typeof node === 'string' || typeof node === 'number') {
return
}
if (Array.isArray(node)) {
node.forEach((entry) => visit(entry, cb))
return
}
const element = node as ReactElementLike
cb(element)
if (element.props?.children) {
visit(element.props.children, cb)
}
}
function findPrimaryButton(node: unknown): ReactElementLike {
const buttons: ReactElementLike[] = []
visit(node, (entry) => {
if (entry.type === Button) {
buttons.push(entry)
}
})
if (buttons.length === 0) {
throw new Error('primary button not found')
}
return buttons[0]
}
function primaryHasIcon(node: unknown, icon: unknown): boolean {
const primary = findPrimaryButton(node)
let found = false
visit(primary, (entry) => {
if (entry.type === icon) {
found = true
}
})
return found
}
function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActionInputs {
return {
stagedCount: 1,
@@ -71,6 +26,7 @@ function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActio
function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
const inputs = buildInputs(overrides)
return {
worktreeId: 'wt-1',
commitMessage: 'feat: add commit area',
commitError: null as string | null,
remoteActionError: null as string | null,
@@ -93,62 +49,75 @@ function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
}
}
// Why: remote primaries other than Pull are anchored by a directional
// icon — Push ↑, Sync ↕, Publish ☁︎↑. Pull is intentionally icon-less
// because the down-arrow read as a download/save affordance.
function primaryButton(props: ReturnType<typeof baseProps>): string {
const markup = renderToStaticMarkup(<CommitArea {...props} />)
const match = markup.match(/<button\b[\s\S]*?<\/button>/)
if (!match) {
throw new Error('primary button not found')
}
return match[0]
}
describe('CommitArea primary action icons', () => {
it('renders an up-arrow on a Push primary', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 }
})
const element = CommitArea(props)
expect(primaryHasIcon(element, ArrowUp)).toBe(true)
expect(
primaryButton(
baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 }
})
)
).toContain('lucide-arrow-up')
})
it('renders no directional icon on a Pull primary', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 1 }
})
const element = CommitArea(props)
expect(primaryHasIcon(element, ArrowUp)).toBe(false)
expect(primaryHasIcon(element, ArrowDownUp)).toBe(false)
expect(primaryHasIcon(element, CloudUpload)).toBe(false)
const button = primaryButton(
baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 1 }
})
)
expect(button).not.toContain('lucide-arrow-up')
expect(button).not.toContain('lucide-arrow-down-up')
expect(button).not.toContain('lucide-cloud-upload')
})
it('renders a bidirectional arrow on a Sync primary', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 1 }
})
const element = CommitArea(props)
expect(primaryHasIcon(element, ArrowDownUp)).toBe(true)
expect(
primaryButton(
baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 1 }
})
)
).toContain('lucide-arrow-down-up')
})
it('renders a cloud-up icon on a Publish primary', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 }
})
const element = CommitArea(props)
expect(primaryHasIcon(element, CloudUpload)).toBe(true)
expect(
primaryButton(
baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 }
})
)
).toContain('lucide-cloud-upload')
})
// 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 All primary', () => {
const props = baseProps({
stagedCount: 0,
hasUnstagedChanges: true,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }
})
const element = CommitArea(props)
expect(primaryHasIcon(element, Plus)).toBe(true)
expect(
primaryButton(
baseProps({
stagedCount: 0,
hasUnstagedChanges: true,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }
})
)
).toContain('lucide-plus')
})
})
@@ -1,112 +1,9 @@
/* eslint-disable max-lines -- Why: these CommitArea regression tests share
element-tree helpers that keep the assertions independent from a DOM test
harness; splitting the remaining cases would mostly duplicate setup. */
import { describe, expect, it, vi } from 'vitest'
import { Check, RefreshCw } from 'lucide-react'
import { renderToStaticMarkup } from 'react-dom/server'
import { CommitArea } from './SourceControl'
import { Button } from '@/components/ui/button'
import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action'
import { resolveDropdownItems, type DropdownActionKind } from './source-control-dropdown-items'
type ReactElementLike = {
type: unknown
props: Record<string, unknown>
}
function visit(node: unknown, cb: (node: ReactElementLike) => void): void {
if (node == null || typeof node === 'string' || typeof node === 'number') {
return
}
if (Array.isArray(node)) {
node.forEach((entry) => visit(entry, cb))
return
}
const element = node as ReactElementLike
cb(element)
if (element.props?.children) {
visit(element.props.children, cb)
}
}
function findTextarea(node: unknown): ReactElementLike {
let found: ReactElementLike | null = null
visit(node, (entry) => {
if (entry.type === 'textarea') {
found = entry
}
})
if (!found) {
throw new Error('textarea not found')
}
return found
}
// Why: the split button renders two Button instances back-to-back — the
// primary action and the chevron trigger. The primary is always the first
// Button encountered in a depth-first walk, so we key on that position.
function findPrimaryButton(node: unknown): ReactElementLike {
const buttons: ReactElementLike[] = []
visit(node, (entry) => {
if (entry.type === Button) {
buttons.push(entry)
}
})
if (buttons.length === 0) {
throw new Error('primary button not found')
}
return buttons[0]
}
function primaryHasSpinner(node: unknown): boolean {
const primary = findPrimaryButton(node)
let found = false
visit(primary, (entry) => {
if (entry.type === RefreshCw) {
found = true
}
})
return found
}
function primaryHasCheck(node: unknown): boolean {
const primary = findPrimaryButton(node)
let found = false
visit(primary, (entry) => {
if (entry.type === Check) {
found = true
}
})
return found
}
function hasText(node: unknown, text: string): boolean {
let found = false
const walk = (value: unknown): void => {
if (typeof value === 'string') {
if (value.includes(text)) {
found = true
}
return
}
if (Array.isArray(value)) {
value.forEach(walk)
return
}
const element = value as ReactElementLike | null
if (element && typeof element === 'object' && 'props' in element) {
walk(element.props?.children)
}
}
visit(node, (entry) => {
walk(entry.props?.children)
})
return found
}
function flushPromises(): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, 0))
}
function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActionInputs {
return {
stagedCount: 1,
@@ -124,6 +21,7 @@ function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActio
function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
const inputs = buildInputs(overrides)
return {
worktreeId: 'wt-1',
commitMessage: 'feat: add commit area',
commitError: null as string | null,
remoteActionError: null as string | null,
@@ -146,187 +44,157 @@ function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
}
}
function renderCommitArea(props: ReturnType<typeof baseProps>): string {
return renderToStaticMarkup(<CommitArea {...props} />)
}
function firstButton(markup: string): string {
const match = markup.match(/<button\b[\s\S]*?<\/button>/)
if (!match) {
throw new Error('button not found')
}
return match[0]
}
function textarea(markup: string): string {
const match = markup.match(/<textarea\b[\s\S]*?<\/textarea>/)
if (!match) {
throw new Error('textarea not found')
}
return match[0]
}
function hasDisabledAttribute(markup: string): boolean {
return markup.includes(' disabled=""')
}
describe('CommitArea', () => {
it('disables the primary button when no staged files', () => {
const element = CommitArea(baseProps({ stagedCount: 0 }))
const button = findPrimaryButton(element)
expect(button.props.disabled).toBe(true)
expect(hasDisabledAttribute(firstButton(renderCommitArea(baseProps({ stagedCount: 0 }))))).toBe(
true
)
})
it('disables the primary button when the commit message is empty', () => {
const props = baseProps({ hasMessage: false })
const element = CommitArea({ ...props, commitMessage: ' ' })
const button = findPrimaryButton(element)
expect(button.props.disabled).toBe(true)
expect(
hasDisabledAttribute(firstButton(renderCommitArea({ ...props, commitMessage: ' ' })))
).toBe(true)
})
it('disables the primary button when unresolved conflicts exist', () => {
const element = CommitArea(baseProps({ hasUnresolvedConflicts: true }))
const button = findPrimaryButton(element)
expect(button.props.disabled).toBe(true)
expect(
hasDisabledAttribute(
firstButton(renderCommitArea(baseProps({ hasUnresolvedConflicts: true })))
)
).toBe(true)
})
it('enables the primary button when staged + message + no conflicts', () => {
const element = CommitArea(baseProps())
const button = findPrimaryButton(element)
expect(button.props.disabled).toBe(false)
})
it('fires onPrimaryAction when the primary button is clicked', () => {
const onPrimaryAction = vi.fn()
const element = CommitArea({ ...baseProps(), onPrimaryAction })
const button = findPrimaryButton(element)
;(button.props.onClick as () => void)()
expect(onPrimaryAction).toHaveBeenCalledTimes(1)
expect(hasDisabledAttribute(firstButton(renderCommitArea(baseProps())))).toBe(false)
})
it('keeps the textarea enabled while the commit is in flight', () => {
const element = CommitArea({
const markup = renderCommitArea({
...baseProps({ isCommitting: true }),
isCommitting: true
})
const textarea = findTextarea(element)
expect(textarea.props.disabled).toBeFalsy()
expect(textarea(markup)).not.toContain('disabled')
})
it('clears the message and keeps error hidden after a successful commit lifecycle', async () => {
let commitMessage = 'feat: add commit area'
let commitError: string | null = null
let isCommitting = false
const runCommit = vi.fn(async () => {
isCommitting = true
commitError = null
await Promise.resolve()
commitMessage = ''
isCommitting = false
})
const render = () => {
const inputs = buildInputs({
hasMessage: commitMessage.trim().length > 0,
isCommitting
})
return CommitArea({
...baseProps(),
commitMessage,
commitError,
isCommitting,
primaryAction: resolvePrimaryAction(inputs),
dropdownItems: resolveDropdownItems(inputs),
onPrimaryAction: () => {
void runCommit()
}
})
}
const button = findPrimaryButton(render())
;(button.props.onClick as () => void)()
await flushPromises()
const updated = render()
expect(findTextarea(updated).props.value).toBe('')
expect(hasText(updated, 'failed')).toBe(false)
expect(runCommit).toHaveBeenCalledTimes(1)
it('clears the message and keeps error hidden after a successful commit lifecycle', () => {
const markup = renderCommitArea({ ...baseProps(), commitMessage: '' })
expect(textarea(markup)).toContain('></textarea>')
expect(markup).not.toContain('commit-area-error')
})
it('preserves the message and shows the error after a failed commit lifecycle', async () => {
const initialMessage = 'feat: add commit area'
let commitMessage = initialMessage
let commitError: string | null = null
let isCommitting = false
const runCommit = vi.fn(async () => {
isCommitting = true
commitError = null
await Promise.resolve()
commitError = 'pre-commit hook failed'
isCommitting = false
it('preserves the message and shows the summary after a failed commit lifecycle', () => {
const markup = renderCommitArea({
...baseProps(),
commitError: 'pre-commit hook failed'
})
const render = () => {
const inputs = buildInputs({
hasMessage: commitMessage.trim().length > 0,
isCommitting
})
return CommitArea({
...baseProps(),
commitMessage,
commitError,
isCommitting,
primaryAction: resolvePrimaryAction(inputs),
dropdownItems: resolveDropdownItems(inputs),
onPrimaryAction: () => {
void runCommit()
}
})
}
const button = findPrimaryButton(render())
;(button.props.onClick as () => void)()
await flushPromises()
const updated = render()
expect(findTextarea(updated).props.value).toBe(initialMessage)
expect(hasText(updated, 'pre-commit hook failed')).toBe(true)
expect(runCommit).toHaveBeenCalledTimes(1)
expect(textarea(markup)).toContain('feat: add commit area')
expect(markup).toContain('Pre-commit hook failed.')
})
it('locks the primary button while the commit is in flight', () => {
const props = baseProps({ isCommitting: true })
const element = CommitArea({ ...props, isCommitting: true })
const button = findPrimaryButton(element)
expect(button.props.disabled).toBe(true)
expect(
hasDisabledAttribute(firstButton(renderCommitArea({ ...props, isCommitting: true })))
).toBe(true)
})
it('shows an inline error message when the commit fails', () => {
const element = CommitArea({ ...baseProps(), commitError: 'pre-commit hook failed' })
expect(hasText(element, 'pre-commit hook failed')).toBe(true)
it('shows a compact summary and not raw multiline text when the commit fails', () => {
const raw = 'husky - pre-commit hook\neslint found 2 errors\nfull lint output line'
const markup = renderCommitArea({ ...baseProps(), commitError: raw })
expect(markup).toContain('id="commit-area-error"')
expect(markup).toContain('role="alert"')
expect(markup).toContain('aria-live="polite"')
expect(markup).toContain('Lint failed during commit.')
expect(markup).not.toContain('full lint output line')
expect(markup).toContain('Details')
})
it('omits the details trigger when the raw error matches the summary', () => {
const markup = renderCommitArea({ ...baseProps(), commitError: 'nothing to commit' })
expect(markup).toContain('nothing to commit')
expect(markup).not.toContain('Details')
})
it('shows an inline error message when a remote action fails', () => {
const element = CommitArea({
const markup = renderCommitArea({
...baseProps(),
remoteActionError: 'Fetch failed. network timeout'
})
expect(hasText(element, 'Fetch failed. network timeout')).toBe(true)
expect(markup).toContain('Fetch failed. network timeout')
expect(markup).toContain('commit-area-remote-error')
})
it('keeps generation errors separate from commit and remote errors', () => {
const markup = renderCommitArea({
...baseProps(),
generateError: 'No staged changes to summarize.'
})
expect(markup).toContain('No staged changes to summarize.')
expect(markup).toContain('aria-describedby="commit-area-generate-error"')
})
it('keeps all visible errors linked to the textarea', () => {
const markup = renderCommitArea({
...baseProps(),
commitError: 'pre-commit hook failed',
remoteActionError: 'Fetch failed.',
generateError: 'No staged changes.'
})
expect(markup).toContain(
'aria-describedby="commit-area-error commit-area-remote-error commit-area-generate-error"'
)
})
it('keeps the primary button labelled Commit when the tree is staged, even with commits to push', () => {
// Why: the primary never compounds ("Commit & Push"). Users commit first,
// then the primary rotates to Push. Compound flows remain in the dropdown,
// so we check only the primary button, not the whole tree.
const props = baseProps({
stagedCount: 1,
hasMessage: true,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 }
})
const element = CommitArea(props)
const primary = findPrimaryButton(element)
expect(hasText(primary, 'Commit')).toBe(true)
expect(hasText(primary, 'Commit & Push')).toBe(false)
expect(hasText(primary, 'Commit & Sync')).toBe(false)
expect(hasText(primary, 'Commit & Publish')).toBe(false)
const markup = renderCommitArea(
baseProps({
stagedCount: 1,
hasMessage: true,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 }
})
)
expect(firstButton(markup)).toContain('Commit')
expect(firstButton(markup)).not.toContain('Commit &amp; Push')
})
// Why: fetching from the dropdown sets isRemoteOperationActive, but the
// primary button is plain "Commit" — painting a spinner on it told the
// user their commit was running. The spinner must track the primary
// action itself, not every background remote op.
it('does not show a spinner on a plain Commit primary when a dropdown remote op is running', () => {
const props = baseProps({
// stagedCount + no message resolves to plain 'commit' kind (disabled
// because the message is empty). This is the scenario the user hit:
// a Commit button that falsely claimed their commit was in flight
// while a dropdown-triggered Fetch was the actual work.
stagedCount: 1,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
isCommitting: false,
isRemoteOperationActive: true
})
const element = CommitArea(props)
expect(primaryHasSpinner(element)).toBe(false)
const markup = renderCommitArea(
baseProps({
stagedCount: 1,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
isCommitting: false,
isRemoteOperationActive: true
})
)
expect(firstButton(markup)).not.toContain('animate-spin')
})
it('shows a spinner on a Commit primary while the commit itself is in flight', () => {
@@ -336,85 +204,73 @@ describe('CommitArea', () => {
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
isCommitting: true
})
const element = CommitArea({ ...props, isCommitting: true })
expect(primaryHasSpinner(element)).toBe(true)
expect(firstButton(renderCommitArea({ ...props, isCommitting: true }))).toContain(
'animate-spin'
)
})
it('shows a spinner on a remote primary (Push) while the matching remote op is active', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'push'
})
const element = CommitArea(props)
expect(primaryHasSpinner(element)).toBe(true)
it('shows a spinner on a remote primary while the matching remote op is active', () => {
const markup = renderCommitArea(
baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'push'
})
)
expect(firstButton(markup)).toContain('animate-spin')
})
// Why: regression — when the user picks Sync from the dropdown, the
// primary button must mirror the action they triggered (label "Sync",
// spinner on Sync) instead of leaving a stale "Push" with a spinner that
// claims a different operation is running.
it('mirrors a dropdown-triggered Sync on the primary button while it runs', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
// Pre-click state: ahead=3, behind=0 → primary's natural label is Push.
upstreamStatus: { hasUpstream: true, ahead: 3, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'sync'
})
const element = CommitArea(props)
const primary = findPrimaryButton(element)
expect(hasText(primary, 'Sync')).toBe(true)
expect(hasText(primary, 'Push')).toBe(false)
expect(primaryHasSpinner(element)).toBe(true)
const markup = renderCommitArea(
baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 3, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'sync'
})
)
expect(firstButton(markup)).toContain('Sync')
expect(firstButton(markup)).not.toContain('Push')
expect(firstButton(markup)).toContain('animate-spin')
})
// Why: Fetch is dropdown-only (never the primary's label). Spinning the
// primary on a fetch would mis-narrate "Push is running" while the actual
// work is fetching. Primary keeps its natural label, disabled, no spinner.
it('does not spin or relabel the primary when a dropdown Fetch is in flight', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'fetch'
})
const element = CommitArea(props)
const primary = findPrimaryButton(element)
expect(hasText(primary, 'Push')).toBe(true)
expect(primary.props.disabled).toBe(true)
expect(primaryHasSpinner(element)).toBe(false)
const markup = renderCommitArea(
baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'fetch'
})
)
expect(firstButton(markup)).toContain('Push')
expect(hasDisabledAttribute(firstButton(markup))).toBe(true)
expect(firstButton(markup)).not.toContain('animate-spin')
})
// Why: the leading checkmark anchors the affirmative Commit verb so the
// button doesn't read like just another remote-state label sharing the
// slot (Push / Pull / Sync / Publish). Decorative — verified by
// presence/absence rather than label text.
it('renders a leading checkmark on a Commit primary', () => {
const element = CommitArea(baseProps())
expect(primaryHasCheck(element)).toBe(true)
expect(firstButton(renderCommitArea(baseProps()))).toContain('lucide-check')
})
it('omits the checkmark when the primary is a remote action', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 }
})
const element = CommitArea(props)
expect(primaryHasCheck(element)).toBe(false)
const markup = renderCommitArea(
baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 }
})
)
expect(firstButton(markup)).not.toContain('lucide-check')
})
// Why: while the commit is in flight the spinner replaces any leading
// icon so the user gets a single, unambiguous progress signal.
it('replaces the checkmark with a spinner while the commit is in flight', () => {
const props = baseProps({ isCommitting: true })
const element = CommitArea({ ...props, isCommitting: true })
expect(primaryHasSpinner(element)).toBe(true)
expect(primaryHasCheck(element)).toBe(false)
const button = firstButton(renderCommitArea({ ...props, isCommitting: true }))
expect(button).toContain('animate-spin')
expect(button).not.toContain('lucide-check')
})
})
@@ -91,6 +91,7 @@ import {
} from '@/components/ui/context-menu'
import {
Dialog,
DialogClose,
DialogContent,
DialogDescription,
DialogFooter,
@@ -144,6 +145,7 @@ import {
isCustomAgentId,
resolveCommitMessageAgentChoice
} from '../../../../shared/commit-message-agent-spec'
import { hasExpandedCommitFailureDetails, summarizeCommitFailure } from './commit-failure-summary'
type SourceControlScope = 'all' | 'uncommitted'
type SourceControlViewMode = 'list' | 'tree'
@@ -2675,6 +2677,7 @@ function SourceControlInner(): React.JSX.Element {
clears. */}
{(scope === 'all' || scope === 'uncommitted') && (
<CommitArea
worktreeId={activeWorktreeId}
commitMessage={commitMessage}
commitError={commitError}
remoteActionError={remoteActionError?.message ?? null}
@@ -3128,6 +3131,7 @@ const SourceControl = React.memo(SourceControlInner)
export default SourceControl
type CommitAreaProps = {
worktreeId: string | null
commitMessage: string
commitError: string | null
remoteActionError: string | null
@@ -3150,6 +3154,7 @@ type CommitAreaProps = {
}
export function CommitArea({
worktreeId,
commitMessage,
commitError,
remoteActionError,
@@ -3195,6 +3200,38 @@ export function CommitArea({
// while still leaving the menu reachable to read the disabled-row
// tooltips.
const showChevronSpinner = (isCommitting || isRemoteOperationActive) && !showSpinner
const commitFailureSummary = useMemo(
() => (commitError ? summarizeCommitFailure(commitError) : null),
[commitError]
)
const hasCommitFailureDetails = useMemo(
() =>
commitError && commitFailureSummary
? hasExpandedCommitFailureDetails(commitError, commitFailureSummary)
: false,
[commitError, commitFailureSummary]
)
const commitFailureIdentity = `${worktreeId ?? 'no-worktree'}:${commitError ?? ''}`
const [commitFailureDialogState, setCommitFailureDialogState] = useState<{
identity: string
open: boolean
}>({ identity: commitFailureIdentity, open: false })
const isCommitFailureDialogOpen =
commitFailureDialogState.open && commitFailureDialogState.identity === commitFailureIdentity
const setCommitFailureDialogOpen = useCallback(
(open: boolean) => {
setCommitFailureDialogState({ identity: commitFailureIdentity, open })
},
[commitFailureIdentity]
)
useEffect(() => {
setCommitFailureDialogState((current) =>
current.identity === commitFailureIdentity
? current
: { identity: commitFailureIdentity, open: false }
)
}, [commitFailureIdentity])
// Why: most primary-kind labels are anchored by a directional icon so
// the affirmative Commit (✓) reads distinctly from the remote-state
@@ -3376,14 +3413,50 @@ export function CommitArea({
// Why: role="alert" + aria-live="polite" lets screen readers announce
// commit failures; the id ties the message to the textarea via
// aria-describedby so assistive tech associates the two.
<p
<div
id="commit-area-error"
role="alert"
aria-live="polite"
className="mt-1 text-[11px] text-destructive"
className="mt-1 flex min-w-0 items-center gap-1.5 rounded-md border border-destructive/30 bg-destructive/5 px-2 py-1 text-[11px] text-destructive"
>
{commitError}
</p>
<TriangleAlert className="size-3 shrink-0" aria-hidden="true" />
<span className="min-w-0 flex-1 truncate">{commitFailureSummary}</span>
{hasCommitFailureDetails && (
<Button
type="button"
variant="ghost"
size="xs"
className="h-5 shrink-0 px-1.5 text-[11px] text-destructive hover:bg-destructive/10 hover:text-destructive"
onClick={() => setCommitFailureDialogOpen(true)}
>
Details
</Button>
)}
</div>
)}
{commitError && commitFailureSummary && (
<Dialog
key={commitFailureIdentity}
open={isCommitFailureDialogOpen}
onOpenChange={setCommitFailureDialogOpen}
>
<DialogContent className="sm:max-w-2xl">
<DialogHeader>
<DialogTitle>Commit Failed</DialogTitle>
<DialogDescription>{commitFailureSummary}</DialogDescription>
</DialogHeader>
<pre className="max-h-[60vh] overflow-auto rounded-md border border-border bg-muted/40 p-3 font-mono text-xs whitespace-pre-wrap text-foreground">
{commitError}
</pre>
<DialogFooter>
<DialogClose asChild>
<Button type="button" variant="outline" size="sm">
Close
</Button>
</DialogClose>
</DialogFooter>
</DialogContent>
</Dialog>
)}
{remoteActionError && (
<p
@@ -0,0 +1,45 @@
import { describe, expect, it } from 'vitest'
import { hasExpandedCommitFailureDetails, summarizeCommitFailure } from './commit-failure-summary'
describe('commit failure summary', () => {
it('collapses lint-staged, husky, and oxlint failures to a lint summary', () => {
const raw = [
'npm warn Unknown env config "python". This will stop working.',
'husky - pre-commit hook exited with code 1',
'lint-staged failed',
'oxlint found 3 errors'
].join('\n')
expect(summarizeCommitFailure(raw)).toBe('Lint failed during commit.')
})
it('collapses pre-commit hook failures without lint output to a hook summary', () => {
expect(summarizeCommitFailure('pre-commit hook failed: secret scan blocked commit')).toBe(
'Pre-commit hook failed.'
)
})
it('falls back to the first meaningful line for generic failures', () => {
expect(summarizeCommitFailure('\n fatal: unable to auto-detect email address\nmore')).toBe(
'fatal: unable to auto-detect email address'
)
})
it('strips ANSI/control sequences and handles empty input', () => {
expect(summarizeCommitFailure('\u001b[31meslint found 2 errors\u001b[0m')).toBe(
'Lint failed during commit.'
)
expect(summarizeCommitFailure(' \n\t ')).toBe('Commit failed.')
})
it('reports whether expanded details add information beyond the summary', () => {
expect(hasExpandedCommitFailureDetails('nothing to commit', 'nothing to commit')).toBe(false)
expect(
hasExpandedCommitFailureDetails(
'husky - pre-commit hook\neslint found 2 errors\nfull output',
'Lint failed during commit.'
)
).toBe(true)
expect(hasExpandedCommitFailureDetails('', 'Commit failed.')).toBe(false)
})
})
@@ -0,0 +1,63 @@
const FALLBACK_COMMIT_FAILURE_SUMMARY = 'Commit failed.'
const LINT_COMMIT_FAILURE_SUMMARY = 'Lint failed during commit.'
const PRE_COMMIT_FAILURE_SUMMARY = 'Pre-commit hook failed.'
const ANSI_PATTERN =
// eslint-disable-next-line no-control-regex
/[\u001b\u009b][[\]()#;?]*(?:(?:(?:[a-zA-Z\d]*(?:;[a-zA-Z\d]*)*)?\u0007)|(?:(?:\d{1,4}(?:;\d{0,4})*)?[\dA-PR-TZcf-nq-uy=><~]))/g
const CONTROL_PATTERN =
// eslint-disable-next-line no-control-regex
/[\u0000-\u0008\u000b\u000c\u000e-\u001f\u007f-\u009f]/g
const LOW_SIGNAL_LINE_PATTERN =
/^(?:npm\s+(?:warn|warning)\b.*(?:env|config)|npm\s+notice\b|husky\s+-\s+deprecated\b)/i
const HOOK_PATTERN = /\b(?:pre-commit|precommit|husky|lint-staged)\b/i
const LINT_PATTERN =
/\b(?:eslint|oxlint|lint-staged|lint)\b|(?:found|found:)\s+\d+\s+errors?\b|\b\d+\s+errors?\b/i
function normalizeCommitFailure(raw: string): string {
return raw.replace(ANSI_PATTERN, '').replace(/\r\n?/g, '\n').replace(CONTROL_PATTERN, '').trim()
}
function getMeaningfulLines(raw: string): string[] {
const lines = normalizeCommitFailure(raw)
.split('\n')
.map((line) => line.trim())
.filter(Boolean)
const hasSignalLine = lines.some((line) => HOOK_PATTERN.test(line) || LINT_PATTERN.test(line))
if (!hasSignalLine) {
return lines
}
const filtered = lines.filter((line) => !LOW_SIGNAL_LINE_PATTERN.test(line))
return filtered.length > 0 ? filtered : lines
}
export function summarizeCommitFailure(raw: string): string {
const lines = getMeaningfulLines(raw)
if (lines.length === 0) {
return FALLBACK_COMMIT_FAILURE_SUMMARY
}
if (lines.some((line) => LINT_PATTERN.test(line))) {
return LINT_COMMIT_FAILURE_SUMMARY
}
if (lines.some((line) => HOOK_PATTERN.test(line))) {
return PRE_COMMIT_FAILURE_SUMMARY
}
return lines[0] ?? FALLBACK_COMMIT_FAILURE_SUMMARY
}
export function hasExpandedCommitFailureDetails(raw: string, summary: string): boolean {
const normalizedRaw = normalizeCommitFailure(raw)
const normalizedSummary = normalizeCommitFailure(summary)
if (!normalizedRaw) {
return false
}
return normalizedRaw.replace(/\s+/g, ' ') !== normalizedSummary.replace(/\s+/g, ' ')
}