fix: pr-bug-scan validated finding from #2141 (#2154)

* fix: address pr-bug-scan validated finding from #2141

Narrowed LINT_PATTERN to lint-tool names only (eslint/oxlint/lint-staged/lint), dropping the loose '\d+ errors' alternative that misclassified non-lint hook failures.

* fix: keep commit failure details updated

---------

Co-authored-by: orca-bug-scan-bot <orca-bug-scan-bot@stably.ai>
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
This commit is contained in:
buf0-bot[bot]
2026-05-31 02:06:07 -07:00
committed by GitHub
co-authored by orca-bug-scan-bot Neil
parent f0125d2dbf
commit 0b829d021b
5 changed files with 79 additions and 44 deletions
@@ -186,11 +186,13 @@ import type { SourceControlAiOperation } from '../../../../shared/source-control
import { getCommitMessageModelDiscoveryHostKeyForScope } from '../../../../shared/commit-message-host-key'
import { getRuntimeGitScope } from '@/runtime/runtime-git-client'
import { getRepositorySourceControlAiSectionId } from '@/components/settings/repository-settings-targets'
import { hasExpandedCommitFailureDetails, summarizeCommitFailure } from './commit-failure-summary'
import {
resolveCommitFailureDialogState,
getCommitFailureDialogWorktreeKey,
shouldShowCommitFailureDialog,
syncCommitFailureDialogState,
type CommitFailureDialogState
} from './commit-failure-dialog-state'
import { hasExpandedCommitFailureDetails, summarizeCommitFailure } from './commit-failure-summary'
import {
isSourceControlSplitOpenModifier,
type SourceControlRowOpenEvent
@@ -5623,24 +5625,24 @@ export function CommitArea({
: false,
[commitError, commitFailureSummary]
)
const commitFailureIdentity = `${worktreeId ?? 'no-worktree'}:${commitError ?? ''}`
// Why: the details dialog is scoped to the worktree, not the exact stderr
// text, so a retried commit can refresh an open dialog with newer output.
const commitFailureWorktreeKey = getCommitFailureDialogWorktreeKey(worktreeId)
const [commitFailureDialogState, setCommitFailureDialogState] =
useState<CommitFailureDialogState>({ identity: commitFailureIdentity, open: false })
const resolvedCommitFailureDialogState = resolveCommitFailureDialogState(
useState<CommitFailureDialogState>({
worktreeKey: commitFailureWorktreeKey,
open: false
})
const isCommitFailureDialogOpen = shouldShowCommitFailureDialog(
commitFailureDialogState,
commitFailureIdentity
commitFailureWorktreeKey,
hasCommitFailureDetails
)
if (resolvedCommitFailureDialogState !== commitFailureDialogState) {
setCommitFailureDialogState(resolvedCommitFailureDialogState)
}
const isCommitFailureDialogOpen =
resolvedCommitFailureDialogState.open &&
resolvedCommitFailureDialogState.identity === commitFailureIdentity
const setCommitFailureDialogOpen = useCallback(
(open: boolean) => {
setCommitFailureDialogState({ identity: commitFailureIdentity, open })
setCommitFailureDialogState({ worktreeKey: commitFailureWorktreeKey, open })
},
[commitFailureIdentity]
[commitFailureWorktreeKey]
)
const handleFixCommitFailureWithAI = useCallback(
async (promptOverride?: string): Promise<boolean> => {
@@ -5656,6 +5658,12 @@ export function CommitArea({
setCommitFailureDialogOpen(false)
}, [setCommitFailureDialogOpen])
useEffect(() => {
setCommitFailureDialogState((current) =>
syncCommitFailureDialogState(current, commitFailureWorktreeKey, hasCommitFailureDetails)
)
}, [commitFailureWorktreeKey, hasCommitFailureDetails])
// Why: most primary-kind labels are anchored by a directional icon so
// the affirmative Commit (✓) reads distinctly from the remote-state
// labels sharing this slot — Push (↑), Sync (↕), Publish (☁︎↑). Pull is
@@ -5925,9 +5933,9 @@ export function CommitArea({
</div>
</div>
)}
{commitError && commitFailureSummary && (
{commitError && commitFailureSummary && hasCommitFailureDetails && (
<Dialog
key={commitFailureIdentity}
key={commitFailureWorktreeKey}
open={isCommitFailureDialogOpen}
onOpenChange={setCommitFailureDialogOpen}
>
@@ -1,31 +1,31 @@
import { describe, expect, it } from 'vitest'
import { resolveCommitFailureDialogState } from './commit-failure-dialog-state'
import {
shouldShowCommitFailureDialog,
syncCommitFailureDialogState,
type CommitFailureDialogState
} from './commit-failure-dialog-state'
describe('resolveCommitFailureDialogState', () => {
it('keeps the dialog state when the commit failure identity still matches', () => {
const state = { identity: 'wt-1:error-a', open: true }
describe('commit failure dialog state', () => {
it('keeps an open dialog visible when a new detailed error arrives for the same worktree', () => {
const state: CommitFailureDialogState = { worktreeKey: 'wt-1', open: true }
expect(resolveCommitFailureDialogState(state, 'wt-1:error-a')).toBe(state)
expect(syncCommitFailureDialogState(state, 'wt-1', true)).toBe(state)
expect(shouldShowCommitFailureDialog(state, 'wt-1', true)).toBe(true)
})
it('closes the dialog when the active commit failure identity changes', () => {
expect(
resolveCommitFailureDialogState({ identity: 'wt-1:error-a', open: true }, 'wt-1:')
).toEqual({
identity: 'wt-1:',
open: false
})
})
it('does not reopen an older failure when its identity comes back later', () => {
const cleared = resolveCommitFailureDialogState(
{ identity: 'wt-1:error-a', open: true },
'wt-1:error-b'
it('closes the dialog when the failure moves to another worktree', () => {
expect(syncCommitFailureDialogState({ worktreeKey: 'wt-1', open: true }, 'wt-2', true)).toEqual(
{
worktreeKey: 'wt-2',
open: false
}
)
})
expect(resolveCommitFailureDialogState(cleared, 'wt-1:error-a')).toEqual({
identity: 'wt-1:error-a',
open: false
})
it('closes the dialog when the latest failure no longer has expanded details', () => {
const next = syncCommitFailureDialogState({ worktreeKey: 'wt-1', open: true }, 'wt-1', false)
expect(next).toEqual({ worktreeKey: 'wt-1', open: false })
expect(shouldShowCommitFailureDialog(next, 'wt-1', false)).toBe(false)
})
})
@@ -1,11 +1,32 @@
export type CommitFailureDialogState = {
identity: string
worktreeKey: string
open: boolean
}
export function resolveCommitFailureDialogState(
export function getCommitFailureDialogWorktreeKey(worktreeId: string | null | undefined): string {
return worktreeId ?? 'no-worktree'
}
export function shouldShowCommitFailureDialog(
state: CommitFailureDialogState,
currentIdentity: string
worktreeKey: string,
hasDetails: boolean
): boolean {
return hasDetails && state.open && state.worktreeKey === worktreeKey
}
export function syncCommitFailureDialogState(
state: CommitFailureDialogState,
worktreeKey: string,
hasDetails: boolean
): CommitFailureDialogState {
return state.identity === currentIdentity ? state : { identity: currentIdentity, open: false }
if (state.worktreeKey === worktreeKey && hasDetails) {
return state
}
if (state.worktreeKey === worktreeKey && !state.open) {
return state
}
return { worktreeKey, open: false }
}
@@ -19,6 +19,13 @@ describe('commit failure summary', () => {
)
})
it('does not treat generic non-lint error counts as lint failures', () => {
expect(summarizeCommitFailure('tsc --noEmit\nFound 5 errors in 3 files.')).toBe('tsc --noEmit')
expect(summarizeCommitFailure('pre-commit hook failed\ntsc found 5 errors')).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'
@@ -11,8 +11,7 @@ const CONTROL_PATTERN =
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
const LINT_PATTERN = /\b(?:eslint|oxlint|lint-staged|lint)\b/i
function normalizeCommitFailure(raw: string): string {
return raw.replace(ANSI_PATTERN, '').replace(/\r\n?/g, '\n').replace(CONTROL_PATTERN, '').trim()