fix(worktree-palette): guard undefined review title in matcher (crash c5d87873) (#10024)

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil 2026-07-22 17:43:14 -07:00 committed by GitHub
parent c8381f3ea7
commit bc301de7be
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 108 additions and 3 deletions

View File

@ -0,0 +1,102 @@
import { describe, expect, it } from 'vitest'
import { matchWorktreePaletteReview } from './worktree-palette-review-match'
import { searchWorktrees } from './worktree-palette-search'
import type { Repo, Worktree } from '../../../shared/types'
import type { HostedReviewInfo } from '../../../shared/hosted-review'
// Regression tests for the production crash (report c5d87873, macOS, Orca 1.4.147):
// TypeError: Cannot read properties of undefined (reading 'toLowerCase')
// at matchWorktreePaletteReview -> searchWorktrees -> WorktreeJumpPalette useMemo
// A rehydrated review/PR cache entry can carry an undefined `title` even though the
// type declares it non-optional, so the Cmd+J worktree palette crashed on any text query.
type MatcherReview = Parameters<typeof matchWorktreePaletteReview>[0]
function makeWorktree(overrides: Partial<Worktree> = {}): Worktree {
return {
id: 'wt-1',
repoId: 'repo-1',
path: '/tmp/wt-1',
head: 'abc123',
branch: 'refs/heads/feature/worktree-jump',
isBare: false,
isMainWorktree: false,
displayName: 'Jump Palette',
comment: '',
linkedIssue: null,
linkedPR: null,
linkedLinearIssue: null,
isArchived: false,
isUnread: false,
isPinned: false,
sortOrder: 0,
lastActivityAt: 0,
...overrides
}
}
const repoMap = new Map<string, Repo>([
[
'repo-1',
{
id: 'repo-1',
path: '/repo/orca',
displayName: 'stablyai/orca',
badgeColor: '#22c55e',
addedAt: 0
}
]
])
describe('matchWorktreePaletteReview title null-safety', () => {
it('returns null instead of throwing when a cached review has no title', () => {
const review = { number: 42, provider: 'github' } as unknown as MatcherReview
expect(() => matchWorktreePaletteReview(review, 'feature', 'feature')).not.toThrow()
expect(matchWorktreePaletteReview(review, 'feature', 'feature')).toBeNull()
})
it('still matches on the title text when a title is present', () => {
const review = {
number: 42,
title: 'Fix the thing',
provider: 'github'
} as unknown as MatcherReview
const match = matchWorktreePaletteReview(review, 'thing', 'thing')
expect(match).not.toBeNull()
expect(match?.text).toBe('Fix the thing')
expect(match?.matchRange).toEqual({ start: 8, end: 13 })
})
it('still matches on the PR number even when the title is missing', () => {
const review = { number: 42, provider: 'github' } as unknown as MatcherReview
const match = matchWorktreePaletteReview(review, '42', '42')
expect(match?.text).toBe('PR #42')
})
})
describe('searchWorktrees with a titleless cached review (Cmd+J palette crash path)', () => {
it('does not throw when the checks-review cache entry has no title', () => {
const worktree = makeWorktree()
const titlelessReview = {
number: 7,
provider: 'github',
state: 'open',
url: 'https://example.test/pr/7'
} as unknown as HostedReviewInfo
const checksReviewByWorktree = new Map<Worktree, HostedReviewInfo | null>([
[worktree, titlelessReview]
])
expect(() =>
searchWorktrees(
[worktree],
'nonmatchingtext',
repoMap,
null,
null,
undefined,
checksReviewByWorktree
)
).not.toThrow()
})
})

View File

@ -1,6 +1,7 @@
import type { HostedReviewInfo } from '../../../shared/hosted-review'
type SearchableReview = Pick<HostedReviewInfo, 'number' | 'title' | 'provider'>
// title is intentionally optional: rehydrated review/PR caches can hold entries without one.
type SearchableReview = Pick<HostedReviewInfo, 'number' | 'provider'> & { title?: string }
type WorktreePaletteReviewMatch = {
labelKind: 'pr' | 'mr'
text: string
@ -35,13 +36,15 @@ export function matchWorktreePaletteReview(
}
}
const titleIndex = review.title.toLowerCase().indexOf(query)
// Null-safe: a cached review may have no title, so fall back to '' (query is non-empty, so it won't match).
const title = review.title ?? ''
const titleIndex = title.toLowerCase().indexOf(query)
if (titleIndex === -1) {
return null
}
return {
labelKind: isMergeRequest ? 'mr' : 'pr',
text: review.title,
text: title,
matchRange: { start: titleIndex, end: titleIndex + query.length }
}
}