fix(worktree-palette): stop blanked display names from crashing Cmd+J (#11323)
* fix(worktree-palette): stop blanked display names from crashing Cmd+J
Blanking the "Display Name" field made buildWorktreeMetaUpdates emit
`displayName: undefined` as a present key. The store's `{ ...worktree,
...updates }` spread then erased the live name, so the next palette
keystroke threw "Cannot read properties of undefined (reading
'toLowerCase')" in searchWorktrees (crash a1f81ea1, build 1.4.159).
Fixed at three layers so no single guard is load-bearing:
- Producer: persist the blanking intent as '' instead of undefined, and
let WorktreeSet accept '' so remote/SSH hosts stop dropping the clear.
- Store: applyWorktreeUpdates and applyDetectedWorktreeUpdates drop
present-but-undefined keys for fields Worktree declares required.
- Readers: resolveWorktreeDisplayName/resolveWorktreeBranchLabel mirror
the main-side mergeWorktree fallback (custom -> branch -> folder) for
all four Cmd+J searches, the checks/review index, and the render site.
Co-authored-by: Orca <help@stably.ai>
* test(worktree): assert omitted display name shape
---------
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
4517088c42
commit
fa449bc0ef
|
|
@ -1,5 +1,5 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { WorktreeActivate, WorktreeCreate } from './worktree-schemas'
|
||||
import { WorktreeActivate, WorktreeCreate, WorktreeSet } from './worktree-schemas'
|
||||
|
||||
describe('worktree RPC schemas', () => {
|
||||
it('validates additive navigation intent', () => {
|
||||
|
|
@ -31,4 +31,27 @@ describe('worktree RPC schemas', () => {
|
|||
|
||||
expect(parsed.success).toBe(false)
|
||||
})
|
||||
|
||||
it('keeps a blanked display name on remote hosts instead of dropping the clear', () => {
|
||||
// Blanking sends displayName:'' meaning "fall back to the branch/folder name".
|
||||
// Coercing it to undefined made updateManagedWorktreeMeta's omitUndefinedProperties
|
||||
// drop the key, so an SSH/paired-web rename-to-blank silently kept the old name.
|
||||
const parsed = WorktreeSet.parse({ worktree: 'id:r1::/repos/wt', displayName: '' })
|
||||
|
||||
expect(parsed.displayName).toBe('')
|
||||
expect(Object.prototype.hasOwnProperty.call(parsed, 'displayName')).toBe(true)
|
||||
})
|
||||
|
||||
it('still omits a display name that was never sent', () => {
|
||||
const parsed = WorktreeSet.parse({ worktree: 'id:r1::/repos/wt', comment: 'note' })
|
||||
|
||||
expect(parsed.displayName).toBeUndefined()
|
||||
expect(Object.prototype.hasOwnProperty.call(parsed, 'displayName')).toBe(false)
|
||||
})
|
||||
|
||||
it('ignores a non-string display name rather than persisting it', () => {
|
||||
const parsed = WorktreeSet.parse({ worktree: 'id:r1::/repos/wt', displayName: 42 })
|
||||
|
||||
expect(parsed.displayName).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -189,7 +189,10 @@ export const WorktreePrefetchCreateBase = z.object({
|
|||
})
|
||||
|
||||
export const WorktreeSet = WorktreeSelector.extend({
|
||||
displayName: OptionalString,
|
||||
// Why: '' is the blanking contract — "fall back to the branch/folder name".
|
||||
// OptionalString coerced it to undefined, so on remote/SSH hosts clearing the
|
||||
// name was dropped here and the old name came back on the next refresh.
|
||||
displayName: OptionalPlainString,
|
||||
// Why: empty comments are meaningful metadata updates, so use the plain
|
||||
// string parser instead of OptionalString's empty-as-undefined behavior.
|
||||
comment: OptionalPlainString,
|
||||
|
|
|
|||
|
|
@ -23,7 +23,6 @@ import {
|
|||
CommandEmpty,
|
||||
CommandItem
|
||||
} from '@/components/ui/command'
|
||||
import { branchName } from '@/lib/git-utils'
|
||||
import { parseGitHubIssueOrPRNumber, parseGitHubIssueOrPRLink } from '@/lib/github-links'
|
||||
import { getLinkedWorkItemSuggestedName, getLinkedWorkItemWorkspaceName } from '@/lib/new-workspace'
|
||||
import type { LinkedWorkItemSummary } from '@/lib/new-workspace'
|
||||
|
|
@ -48,6 +47,10 @@ import {
|
|||
type MatchRange,
|
||||
type PaletteSearchResult
|
||||
} from '@/lib/worktree-palette-search'
|
||||
import {
|
||||
resolveWorktreeBranchLabel,
|
||||
resolveWorktreeDisplayName
|
||||
} from '@/lib/worktree-default-display-name'
|
||||
import {
|
||||
CREATE_WORKTREE_ITEM_ID,
|
||||
createWorktreePaletteRequestGuard,
|
||||
|
|
@ -1794,7 +1797,10 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
|||
const worktree = entry.worktree
|
||||
const repo = repoMap.get(worktree.repoId)
|
||||
const repoName = repo?.displayName ?? ''
|
||||
const branch = branchName(worktree.branch)
|
||||
// Why: both must match searchWorktrees' resolution, or highlight ranges land on
|
||||
// the wrong text — and a branch-less row would throw here before search ever ran.
|
||||
const branch = resolveWorktreeBranchLabel(worktree)
|
||||
const worktreeLabel = resolveWorktreeDisplayName(worktree)
|
||||
const status = getWorktreeStatus(
|
||||
tabsByWorktree[worktree.id] ?? [],
|
||||
browserTabsByWorktree[worktree.id] ?? [],
|
||||
|
|
@ -1862,11 +1868,11 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
|||
<span className="truncate text-[14px] font-semibold text-foreground">
|
||||
{entry.match.displayNameRange ? (
|
||||
<HighlightedText
|
||||
text={worktree.displayName}
|
||||
text={worktreeLabel}
|
||||
matchRange={entry.match.displayNameRange}
|
||||
/>
|
||||
) : (
|
||||
worktree.displayName
|
||||
worktreeLabel
|
||||
)}
|
||||
</span>
|
||||
{isCurrentWorktree && (
|
||||
|
|
|
|||
|
|
@ -177,4 +177,25 @@ describe('buildWorktreeChecksReviewIndex', () => {
|
|||
expect(reviews.has(localWorktree)).toBe(false)
|
||||
expect(reviews.get(sshWorktree)).toMatchObject({ provider: 'github', number: 42 })
|
||||
})
|
||||
|
||||
it('skips a branch-less worktree instead of throwing', () => {
|
||||
// Why: Cmd+J builds this index for every worktree before any query is typed,
|
||||
// so a folder workspace (empty branch) or a partially hydrated row reaches it.
|
||||
const branchless: Worktree = {
|
||||
...worktree,
|
||||
id: 'worktree-folder',
|
||||
branch: undefined as unknown as string,
|
||||
displayName: undefined as unknown as string
|
||||
}
|
||||
|
||||
const reviews = buildWorktreeChecksReviewIndex({
|
||||
worktrees: [branchless],
|
||||
repoByHostIdentity: new Map([[getRepoHostIdentity(repo), repo]]),
|
||||
prCache: {},
|
||||
hostedReviewCache: {},
|
||||
settings: null
|
||||
})
|
||||
|
||||
expect(reviews.has(branchless)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { branchName } from '@/lib/git-utils'
|
||||
import { resolveWorktreeBranchLabel } from '@/lib/worktree-default-display-name'
|
||||
import { getGitHubPRCacheKey } from '@/store/slices/github-cache-key'
|
||||
import { getHostedReviewCacheKey } from '@/store/slices/hosted-review-cache-identity'
|
||||
import { getRepoHostIdentityForParts } from '@/store/slices/repo-host-identity'
|
||||
|
|
@ -35,7 +35,9 @@ export function buildWorktreeChecksReviewIndex({
|
|||
if (!repo) {
|
||||
continue
|
||||
}
|
||||
const branch = branchName(worktree.branch)
|
||||
// Why: Cmd+J builds this index for every worktree before search runs, so a
|
||||
// branch-less folder workspace or partially hydrated row must not throw here.
|
||||
const branch = resolveWorktreeBranchLabel(worktree)
|
||||
const prKey = getGitHubPRCacheKey(
|
||||
repo.path,
|
||||
repo.id,
|
||||
|
|
|
|||
|
|
@ -49,6 +49,19 @@ describe('buildWorktreeMetaUpdates', () => {
|
|||
})
|
||||
})
|
||||
|
||||
it('clears a display name with empty string, never a present-undefined key', () => {
|
||||
const updates = buildWorktreeMetaUpdates({
|
||||
displayNameInput: ' ',
|
||||
currentDisplayName: 'Custom Name',
|
||||
issueInput: '',
|
||||
prInput: '',
|
||||
commentInput: ''
|
||||
})
|
||||
|
||||
expect(updates.displayName).toBe('')
|
||||
expect(Object.values(updates).every((value) => value !== undefined)).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects PR URLs in the issue input', () => {
|
||||
expect(
|
||||
buildWorktreeMetaUpdates({
|
||||
|
|
|
|||
|
|
@ -48,12 +48,14 @@ export function buildWorktreeMetaUpdates(args: {
|
|||
const finalLinkedPR =
|
||||
trimmedPR === '' ? null : linkedPRNumber !== null ? linkedPRNumber : undefined
|
||||
|
||||
// Why: blanking the field means "fall back to the branch/folder name", and the
|
||||
// empty string is how that intent is persisted. Emitting `undefined` instead
|
||||
// put a present-but-undefined key into the store spread, wiping the live name
|
||||
// and crashing the worktree palette (crash a1f81ea1).
|
||||
const trimmedDisplayName = args.displayNameInput.trim()
|
||||
const updates: Partial<WorktreeMeta> = {
|
||||
comment: args.commentInput.trim(),
|
||||
...(trimmedDisplayName !== args.currentDisplayName && {
|
||||
displayName: trimmedDisplayName || undefined
|
||||
})
|
||||
...(trimmedDisplayName !== args.currentDisplayName && { displayName: trimmedDisplayName })
|
||||
}
|
||||
if (finalLinkedIssue !== undefined) {
|
||||
updates.linkedIssue = finalLinkedIssue
|
||||
|
|
|
|||
|
|
@ -331,4 +331,54 @@ describe('browser-palette-search', () => {
|
|||
it('rejects oversized whitespace before trimming', () => {
|
||||
expect(searchBrowserPages([], ' '.repeat(BROWSER_PALETTE_QUERY_MAX_BYTES + 1))).toEqual([])
|
||||
})
|
||||
|
||||
it('falls back to the branch label when a cleared display name left it undefined', () => {
|
||||
// Why: Cmd+J runs this search over the same worktree objects as searchWorktrees,
|
||||
// so the store-level display-name corruption reaches here too.
|
||||
const cleared = makeWorktree({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: 'refs/heads/feature/browser-search'
|
||||
})
|
||||
const entries: SearchableBrowserPage[] = [
|
||||
{
|
||||
page: makePage(),
|
||||
workspace: makeWorkspace(),
|
||||
worktree: cleared,
|
||||
repoName: 'orca',
|
||||
worktreeSortIndex: 0,
|
||||
isCurrentPage: false,
|
||||
isCurrentWorktree: false
|
||||
}
|
||||
]
|
||||
|
||||
const results = searchBrowserPages(entries, 'browser-search')
|
||||
expect(results[0]).toMatchObject({
|
||||
worktreeName: 'feature/browser-search',
|
||||
worktreeRange: { start: 'feature/'.length, end: 'feature/browser-search'.length }
|
||||
})
|
||||
})
|
||||
|
||||
it('lists a branch-less row on the empty query without throwing', () => {
|
||||
const cleared = makeWorktree({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: undefined as unknown as string,
|
||||
path: '/repos/design-review'
|
||||
})
|
||||
const entries: SearchableBrowserPage[] = [
|
||||
{
|
||||
page: makePage(),
|
||||
workspace: makeWorkspace(),
|
||||
worktree: cleared,
|
||||
repoName: 'orca',
|
||||
worktreeSortIndex: 0,
|
||||
isCurrentPage: false,
|
||||
isCurrentWorktree: false
|
||||
}
|
||||
]
|
||||
|
||||
expect(searchBrowserPages(entries, '')[0]).toMatchObject({
|
||||
worktreeName: 'design-review',
|
||||
worktreeRange: null
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { ORCA_BROWSER_BLANK_URL } from '../../../shared/constants'
|
||||
import type { BrowserPage, BrowserWorkspace, Worktree } from '../../../shared/types'
|
||||
import { isClipboardTextByteLengthOverLimit } from '../../../shared/clipboard-text'
|
||||
import { resolveWorktreeDisplayName } from './worktree-default-display-name'
|
||||
import type { MatchRange } from './worktree-palette-search'
|
||||
|
||||
export type SearchableBrowserPage = {
|
||||
|
|
@ -125,6 +126,8 @@ export function searchBrowserPages(
|
|||
const formattedUrl = formatBrowserPaletteUrl(entry.page.url)
|
||||
const title = entry.page.title || formattedUrl
|
||||
const fallbackSecondaryText = formattedUrl
|
||||
// Why: a cleared display name leaves this undefined at runtime; findRange would throw.
|
||||
const worktreeName = resolveWorktreeDisplayName(entry.worktree)
|
||||
const baseResult = {
|
||||
pageId: entry.page.id,
|
||||
workspaceId: entry.workspace.id,
|
||||
|
|
@ -132,7 +135,7 @@ export function searchBrowserPages(
|
|||
title,
|
||||
workspaceLabel: entry.workspace.label ?? null,
|
||||
repoName: entry.repoName,
|
||||
worktreeName: entry.worktree.displayName,
|
||||
worktreeName,
|
||||
isCurrentPage: entry.isCurrentPage,
|
||||
isCurrentWorktree: entry.isCurrentWorktree
|
||||
}
|
||||
|
|
@ -234,7 +237,7 @@ export function searchBrowserPages(
|
|||
continue
|
||||
}
|
||||
|
||||
const worktreeRange = findRange(entry.worktree.displayName, trimmedQuery)
|
||||
const worktreeRange = findRange(worktreeName, trimmedQuery)
|
||||
if (worktreeRange) {
|
||||
results.push({
|
||||
...baseResult,
|
||||
|
|
|
|||
|
|
@ -174,4 +174,49 @@ describe('simulator-palette-search', () => {
|
|||
it('rejects oversized whitespace before trimming simulator palette queries', () => {
|
||||
expect(searchSimulatorTabs([], ' '.repeat(SIMULATOR_PALETTE_QUERY_MAX_BYTES + 1))).toEqual([])
|
||||
})
|
||||
|
||||
it('falls back to the branch label when a cleared display name left it undefined', () => {
|
||||
// Why: Cmd+J runs this search over the same worktree objects as searchWorktrees,
|
||||
// so the store-level display-name corruption reaches here too.
|
||||
const entries = [
|
||||
{
|
||||
tab: makeTab(),
|
||||
worktree: makeWorktree({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: 'refs/heads/feature/mobile-emulator'
|
||||
}),
|
||||
repoName: 'orca',
|
||||
worktreeSortIndex: 0,
|
||||
isCurrentTab: false,
|
||||
isCurrentWorktree: false
|
||||
}
|
||||
]
|
||||
|
||||
expect(searchSimulatorTabs(entries, 'mobile-emulator')[0]).toMatchObject({
|
||||
worktreeName: 'feature/mobile-emulator',
|
||||
worktreeRange: { start: 'feature/'.length, end: 'feature/mobile-emulator'.length }
|
||||
})
|
||||
})
|
||||
|
||||
it('lists a branch-less row on the empty query without throwing', () => {
|
||||
const entries = [
|
||||
{
|
||||
tab: makeTab(),
|
||||
worktree: makeWorktree({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: undefined as unknown as string,
|
||||
path: '/repos/design-review'
|
||||
}),
|
||||
repoName: 'orca',
|
||||
worktreeSortIndex: 0,
|
||||
isCurrentTab: false,
|
||||
isCurrentWorktree: false
|
||||
}
|
||||
]
|
||||
|
||||
expect(searchSimulatorTabs(entries, '')[0]).toMatchObject({
|
||||
worktreeName: 'design-review',
|
||||
worktreeRange: null
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import type { Tab, TabGroup, Worktree } from '../../../shared/types'
|
||||
import { isClipboardTextByteLengthOverLimit } from '../../../shared/clipboard-text'
|
||||
import { resolveWorktreeDisplayName } from './worktree-default-display-name'
|
||||
import type { MatchRange } from './worktree-palette-search'
|
||||
|
||||
export type SearchableSimulatorTab = {
|
||||
|
|
@ -180,6 +181,8 @@ export function searchSimulatorTabs(
|
|||
for (const entry of entries) {
|
||||
const title = entry.tab.label || 'Mobile Emulator'
|
||||
const secondaryText = 'Mobile Emulator tab'
|
||||
// Why: a cleared display name leaves this undefined at runtime; findRange would throw.
|
||||
const worktreeName = resolveWorktreeDisplayName(entry.worktree)
|
||||
const baseResult = {
|
||||
tabId: entry.tab.id,
|
||||
worktreeId: entry.worktree.id,
|
||||
|
|
@ -187,7 +190,7 @@ export function searchSimulatorTabs(
|
|||
title,
|
||||
secondaryText,
|
||||
repoName: entry.repoName,
|
||||
worktreeName: entry.worktree.displayName,
|
||||
worktreeName,
|
||||
isCurrentTab: entry.isCurrentTab,
|
||||
isCurrentWorktree: entry.isCurrentWorktree
|
||||
}
|
||||
|
|
@ -253,7 +256,7 @@ export function searchSimulatorTabs(
|
|||
continue
|
||||
}
|
||||
|
||||
const worktreeRange = findRange(entry.worktree.displayName, trimmedQuery)
|
||||
const worktreeRange = findRange(worktreeName, trimmedQuery)
|
||||
if (worktreeRange) {
|
||||
results.push({
|
||||
...baseResult,
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import { resolveWorktreeDisplayName } from './worktree-default-display-name'
|
||||
import type { MatchRange } from './worktree-palette-search'
|
||||
import type {
|
||||
SearchableWorkspaceTab,
|
||||
|
|
@ -114,6 +115,8 @@ export function searchWorkspaceTabs(
|
|||
const results: WorkspaceTabPaletteSearchResult[] = []
|
||||
|
||||
for (const entry of entries) {
|
||||
// Why: a cleared display name leaves this undefined at runtime; findRange would throw.
|
||||
const worktreeName = resolveWorktreeDisplayName(entry.worktree)
|
||||
const baseResult = {
|
||||
tabId: entry.tab.id,
|
||||
entityId: entry.tab.entityId,
|
||||
|
|
@ -123,7 +126,7 @@ export function searchWorkspaceTabs(
|
|||
title: entry.title,
|
||||
secondaryText: entry.secondaryText,
|
||||
repoName: entry.repoName,
|
||||
worktreeName: entry.worktree.displayName,
|
||||
worktreeName,
|
||||
isCurrentTab: entry.isCurrentTab,
|
||||
isCurrentWorktree: entry.isCurrentWorktree
|
||||
}
|
||||
|
|
@ -200,7 +203,7 @@ export function searchWorkspaceTabs(
|
|||
continue
|
||||
}
|
||||
|
||||
const worktreeRange = findRange(entry.worktree.displayName, trimmedQuery)
|
||||
const worktreeRange = findRange(worktreeName, trimmedQuery)
|
||||
if (worktreeRange) {
|
||||
results.push({
|
||||
...baseResult,
|
||||
|
|
|
|||
|
|
@ -438,4 +438,36 @@ describe('workspace-tab-palette-search', () => {
|
|||
'terminal-other'
|
||||
])
|
||||
})
|
||||
|
||||
it('falls back to the branch label when a cleared display name left it undefined', () => {
|
||||
// Why: Cmd+J runs this search over the same worktree objects as searchWorktrees,
|
||||
// so the store-level display-name corruption reaches here too.
|
||||
const cleared = makeWorktree({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: 'refs/heads/feature/workspace-tab-search'
|
||||
})
|
||||
const entries = buildEntries({ worktrees: [cleared] })
|
||||
|
||||
expect(searchWorkspaceTabs(entries, 'workspace-tab-search')[0]).toMatchObject({
|
||||
worktreeName: 'feature/workspace-tab-search',
|
||||
worktreeRange: {
|
||||
start: 'feature/'.length,
|
||||
end: 'feature/workspace-tab-search'.length
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
it('lists a branch-less row on the empty query without throwing', () => {
|
||||
const cleared = makeWorktree({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: undefined as unknown as string,
|
||||
path: path.join('repos', 'design-review')
|
||||
})
|
||||
const entries = buildEntries({ worktrees: [cleared] })
|
||||
|
||||
expect(searchWorkspaceTabs(entries, '')[0]).toMatchObject({
|
||||
worktreeName: 'design-review',
|
||||
worktreeRange: null
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -0,0 +1,95 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
resolveWorktreeBranchLabel,
|
||||
resolveWorktreeDisplayName
|
||||
} from './worktree-default-display-name'
|
||||
|
||||
describe('resolveWorktreeBranchLabel', () => {
|
||||
it('strips refs/heads/ like the raw branchName call it replaces', () => {
|
||||
expect(resolveWorktreeBranchLabel({ branch: 'refs/heads/feature/jump' })).toBe('feature/jump')
|
||||
})
|
||||
|
||||
it('returns empty for a folder workspace, which carries no branch', () => {
|
||||
expect(resolveWorktreeBranchLabel({ branch: '' })).toBe('')
|
||||
})
|
||||
|
||||
it('returns empty instead of throwing when branch is absent at runtime', () => {
|
||||
// The palette renders every row on an empty query, before any branch search runs,
|
||||
// so an unguarded branchName() here crashed the whole palette.
|
||||
expect(resolveWorktreeBranchLabel({ branch: undefined as unknown as string })).toBe('')
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveWorktreeDisplayName', () => {
|
||||
it('prefers the custom name', () => {
|
||||
expect(
|
||||
resolveWorktreeDisplayName({
|
||||
displayName: 'Design review',
|
||||
branch: 'refs/heads/feature/jump',
|
||||
path: '/repos/orca'
|
||||
})
|
||||
).toBe('Design review')
|
||||
})
|
||||
|
||||
it('falls back to the branch when the name was blanked to an empty string', () => {
|
||||
expect(
|
||||
resolveWorktreeDisplayName({
|
||||
displayName: '',
|
||||
branch: 'refs/heads/feature/jump',
|
||||
path: '/repos/orca'
|
||||
})
|
||||
).toBe('feature/jump')
|
||||
})
|
||||
|
||||
it('treats a whitespace-only name as blank', () => {
|
||||
expect(
|
||||
resolveWorktreeDisplayName({
|
||||
displayName: ' ',
|
||||
branch: 'refs/heads/main',
|
||||
path: '/repos/orca'
|
||||
})
|
||||
).toBe('main')
|
||||
})
|
||||
|
||||
it('falls back to the branch when a cleared name left the field undefined', () => {
|
||||
expect(
|
||||
resolveWorktreeDisplayName({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: 'refs/heads/main',
|
||||
path: '/repos/orca'
|
||||
})
|
||||
).toBe('main')
|
||||
})
|
||||
|
||||
it('falls back to the folder name for a branch-less folder workspace', () => {
|
||||
expect(
|
||||
resolveWorktreeDisplayName({ displayName: '', branch: '', path: '/repos/design-review' })
|
||||
).toBe('design-review')
|
||||
})
|
||||
|
||||
it('resolves the folder name from a Windows path', () => {
|
||||
expect(
|
||||
resolveWorktreeDisplayName({
|
||||
displayName: '',
|
||||
branch: '',
|
||||
path: 'C:\\Users\\alice\\repos\\design-review'
|
||||
})
|
||||
).toBe('design-review')
|
||||
})
|
||||
|
||||
it('keeps emoji and non-ASCII names intact', () => {
|
||||
expect(
|
||||
resolveWorktreeDisplayName({ displayName: '🚀 Läufer', branch: '', path: '/repos/x' })
|
||||
).toBe('🚀 Läufer')
|
||||
})
|
||||
|
||||
it('returns empty rather than throwing when every source is missing', () => {
|
||||
expect(
|
||||
resolveWorktreeDisplayName({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: undefined as unknown as string,
|
||||
path: undefined as unknown as string
|
||||
})
|
||||
).toBe('')
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,34 @@
|
|||
import { branchName } from '@/lib/git-utils'
|
||||
import { basename } from '@/lib/path'
|
||||
import type { Worktree } from '../../../shared/types'
|
||||
|
||||
type WorktreeDisplayNameSource = Pick<Worktree, 'displayName' | 'branch' | 'path'>
|
||||
|
||||
/**
|
||||
* `branch` is typed non-optional but is absent on folder workspaces and
|
||||
* partially hydrated rows, and `branchName` throws on undefined. Every render
|
||||
* and search read of the branch label must go through here.
|
||||
*/
|
||||
export function resolveWorktreeBranchLabel(worktree: Pick<Worktree, 'branch'>): string {
|
||||
return typeof worktree.branch === 'string' ? branchName(worktree.branch) : ''
|
||||
}
|
||||
|
||||
/**
|
||||
* Renderer mirror of main-side `mergeWorktree`: a missing or blank custom name
|
||||
* falls back to the branch, then the folder. `displayName` is typed non-optional
|
||||
* but arrives undefined at runtime once a custom name is cleared (crash
|
||||
* a1f81ea1), so every read must go through here instead of dereferencing it.
|
||||
*/
|
||||
export function resolveWorktreeDisplayName(worktree: WorktreeDisplayNameSource): string {
|
||||
const custom = typeof worktree.displayName === 'string' ? worktree.displayName.trim() : ''
|
||||
if (custom) {
|
||||
return custom
|
||||
}
|
||||
|
||||
const branch = resolveWorktreeBranchLabel(worktree).trim()
|
||||
if (branch) {
|
||||
return branch
|
||||
}
|
||||
|
||||
return typeof worktree.path === 'string' ? basename(worktree.path).trim() : ''
|
||||
}
|
||||
|
|
@ -141,6 +141,63 @@ describe('worktree-palette-search', () => {
|
|||
expect(searchWorktrees([makeWorktree()], query, repoMap, null, null)).toEqual([])
|
||||
})
|
||||
|
||||
it('falls back to branch text when a cleared display name left it undefined', () => {
|
||||
const cleared = makeWorktree({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: 'refs/heads/feature/worktree-jump'
|
||||
})
|
||||
|
||||
expect(() => searchWorktrees([cleared], 'jump', repoMap, null, null)).not.toThrow()
|
||||
// Highlight range indexes the branch-derived label the palette actually renders.
|
||||
expect(searchWorktrees([cleared], 'jump', repoMap, null, null)[0]).toMatchObject({
|
||||
worktreeId: 'wt-1',
|
||||
matchedField: 'displayName',
|
||||
displayNameRange: { start: 'feature/worktree-'.length, end: 'feature/worktree-jump'.length }
|
||||
})
|
||||
})
|
||||
|
||||
it('falls back to the folder name when both display name and branch are missing', () => {
|
||||
const folderWorkspace = makeWorktree({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: '',
|
||||
path: '/tmp/design-review'
|
||||
})
|
||||
|
||||
expect(searchWorktrees([folderWorkspace], 'design', repoMap, null, null)[0]).toMatchObject({
|
||||
matchedField: 'displayName',
|
||||
displayNameRange: { start: 0, end: 6 }
|
||||
})
|
||||
})
|
||||
|
||||
it('survives a cleared display name on composite repo/branch queries', () => {
|
||||
const cleared = makeWorktree({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: undefined as unknown as string
|
||||
})
|
||||
|
||||
expect(() => searchWorktrees([cleared], 'orca/jump', repoMap, null, null)).not.toThrow()
|
||||
})
|
||||
|
||||
it('still lists a branch-less row on the empty query, which renders every row', () => {
|
||||
// Why: the empty query short-circuits before any branch read, so the row reaches the
|
||||
// render loop untouched — the label resolution there has to be guarded too.
|
||||
const cleared = makeWorktree({
|
||||
displayName: undefined as unknown as string,
|
||||
branch: undefined as unknown as string
|
||||
})
|
||||
|
||||
expect(searchWorktrees([cleared], '', repoMap, null, null)).toEqual([
|
||||
{
|
||||
worktreeId: 'wt-1',
|
||||
matchedField: null,
|
||||
displayNameRange: null,
|
||||
branchRange: null,
|
||||
repoRange: null,
|
||||
supportingText: null
|
||||
}
|
||||
])
|
||||
})
|
||||
|
||||
it('returns a truncated comment snippet with the highlighted match range', () => {
|
||||
const results = searchWorktrees(
|
||||
[
|
||||
|
|
|
|||
|
|
@ -1,5 +1,8 @@
|
|||
import { branchName } from '@/lib/git-utils'
|
||||
import { issueCacheKey as getIssueCacheKey } from '@/store/slices/github'
|
||||
import {
|
||||
resolveWorktreeBranchLabel,
|
||||
resolveWorktreeDisplayName
|
||||
} from './worktree-default-display-name'
|
||||
import type { HostedReviewInfo } from '../../../shared/hosted-review'
|
||||
import type { Repo, Worktree } from '../../../shared/types'
|
||||
import { extractWorktreePaletteCommentSnippet } from './worktree-palette-comment-snippet'
|
||||
|
|
@ -101,7 +104,7 @@ export function searchWorktrees(
|
|||
for (const worktree of worktrees) {
|
||||
if (composite) {
|
||||
const repoName = repoMap.get(worktree.repoId)?.displayName ?? ''
|
||||
const branch = branchName(worktree.branch)
|
||||
const branch = resolveWorktreeBranchLabel(worktree)
|
||||
const repoIdx = repoName.toLowerCase().indexOf(composite.repoPart)
|
||||
const branchIdx = branch.toLowerCase().indexOf(composite.branchPart)
|
||||
if (repoIdx !== -1 && branchIdx !== -1) {
|
||||
|
|
@ -117,7 +120,7 @@ export function searchWorktrees(
|
|||
// that happens to contain a slash (e.g. "feature/foo") still get hits.
|
||||
}
|
||||
|
||||
const nameIndex = worktree.displayName.toLowerCase().indexOf(q)
|
||||
const nameIndex = resolveWorktreeDisplayName(worktree).toLowerCase().indexOf(q)
|
||||
if (nameIndex !== -1) {
|
||||
results.push(
|
||||
makeResult(worktree.id, 'displayName', {
|
||||
|
|
@ -127,7 +130,7 @@ export function searchWorktrees(
|
|||
continue
|
||||
}
|
||||
|
||||
const branch = branchName(worktree.branch)
|
||||
const branch = resolveWorktreeBranchLabel(worktree)
|
||||
const branchIndex = branch.toLowerCase().indexOf(q)
|
||||
if (branchIndex !== -1) {
|
||||
results.push(
|
||||
|
|
|
|||
|
|
@ -51,4 +51,21 @@ describe('applyWorktreeUpdates', () => {
|
|||
expect(result['repo-b']?.[0]).toBe(samePathDifferentProject)
|
||||
expect(result['repo-b']?.[0]?.displayName).toBe('Project B')
|
||||
})
|
||||
|
||||
it('never lets a present-but-undefined value clobber existing worktree fields', () => {
|
||||
const target = makeWorktree({
|
||||
id: 'repo-a::/Users/alice/project',
|
||||
repoId: 'repo-a',
|
||||
displayName: 'Project A',
|
||||
comment: 'notes'
|
||||
})
|
||||
|
||||
const result = applyWorktreeUpdates({ 'repo-a': [target] }, target.id, {
|
||||
displayName: undefined,
|
||||
comment: 'edited'
|
||||
})
|
||||
|
||||
expect(result['repo-a']?.[0]?.displayName).toBe('Project A')
|
||||
expect(result['repo-a']?.[0]?.comment).toBe('edited')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -326,11 +326,51 @@ export function findWorktreeById(
|
|||
return undefined
|
||||
}
|
||||
|
||||
type RequiredKey<T> = { [K in keyof T]-?: undefined extends T[K] ? never : K }[keyof T]
|
||||
|
||||
// Why: a present-but-undefined key in a spread ERASES the field. That is the
|
||||
// intended wire signal for clearing optional metadata (pushTarget), but on a
|
||||
// field Worktree declares required it produced a live `displayName: undefined`
|
||||
// that crashed the worktree palette (crash a1f81ea1). Typed off Worktree so a
|
||||
// newly-required field is protected automatically.
|
||||
const ERASURE_PROTECTED_KEYS: Record<Extract<RequiredKey<Worktree>, keyof WorktreeMeta>, true> = {
|
||||
displayName: true,
|
||||
comment: true,
|
||||
linkedIssue: true,
|
||||
linkedPR: true,
|
||||
linkedLinearIssue: true,
|
||||
isArchived: true,
|
||||
isUnread: true,
|
||||
isPinned: true,
|
||||
sortOrder: true,
|
||||
lastActivityAt: true
|
||||
}
|
||||
|
||||
export function withoutErasedRequiredWorktreeFields(
|
||||
updates: Partial<WorktreeMeta>
|
||||
): Partial<WorktreeMeta> {
|
||||
const erased = Object.keys(ERASURE_PROTECTED_KEYS).filter(
|
||||
(key) =>
|
||||
updates[key as keyof WorktreeMeta] === undefined &&
|
||||
Object.prototype.hasOwnProperty.call(updates, key)
|
||||
)
|
||||
if (erased.length === 0) {
|
||||
return updates
|
||||
}
|
||||
|
||||
const next = { ...updates }
|
||||
for (const key of erased) {
|
||||
delete next[key as keyof WorktreeMeta]
|
||||
}
|
||||
return next
|
||||
}
|
||||
|
||||
export function applyWorktreeUpdates(
|
||||
worktreesByRepo: Record<string, Worktree[]>,
|
||||
worktreeId: string,
|
||||
updates: Partial<WorktreeMeta>
|
||||
rawUpdates: Partial<WorktreeMeta>
|
||||
): Record<string, Worktree[]> {
|
||||
const updates = withoutErasedRequiredWorktreeFields(rawUpdates)
|
||||
const repoId = getRepoIdFromWorktreeId(worktreeId)
|
||||
const worktrees = worktreesByRepo[repoId]
|
||||
if (!worktrees) {
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import type { RuntimeWorktreeListResult } from '../../../../shared/runtime-types
|
|||
import {
|
||||
findWorktreeById,
|
||||
applyWorktreeUpdates,
|
||||
withoutErasedRequiredWorktreeFields,
|
||||
getRepoIdFromWorktreeId,
|
||||
type DirectSshWorktreeFetchOptions,
|
||||
type WorktreeFetchOptions,
|
||||
|
|
@ -706,8 +707,10 @@ function notifyRuntimeScopeForbiddenIfNeeded(error: unknown): boolean {
|
|||
function applyDetectedWorktreeUpdates(
|
||||
detectedWorktreesByRepo: AppState['detectedWorktreesByRepo'],
|
||||
worktreeId: string,
|
||||
updates: Partial<WorktreeMeta>
|
||||
rawUpdates: Partial<WorktreeMeta>
|
||||
): AppState['detectedWorktreesByRepo'] {
|
||||
// Why: mirrors applyWorktreeUpdates — detected rows feed the same palette.
|
||||
const updates = withoutErasedRequiredWorktreeFields(rawUpdates)
|
||||
let changed = false
|
||||
const nextByRepo: AppState['detectedWorktreesByRepo'] = {}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue