diff --git a/src/main/runtime/rpc/methods/worktree-schemas.test.ts b/src/main/runtime/rpc/methods/worktree-schemas.test.ts index 975693bb7..219791cb9 100644 --- a/src/main/runtime/rpc/methods/worktree-schemas.test.ts +++ b/src/main/runtime/rpc/methods/worktree-schemas.test.ts @@ -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() + }) }) diff --git a/src/main/runtime/rpc/methods/worktree-schemas.ts b/src/main/runtime/rpc/methods/worktree-schemas.ts index b79fb320f..905a58f95 100644 --- a/src/main/runtime/rpc/methods/worktree-schemas.ts +++ b/src/main/runtime/rpc/methods/worktree-schemas.ts @@ -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, diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index 61f3a4eb5..efe91e4e7 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -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 { {entry.match.displayNameRange ? ( ) : ( - worktree.displayName + worktreeLabel )} {isCurrentWorktree && ( diff --git a/src/renderer/src/components/cmd-j/worktree-checks-review-index.test.ts b/src/renderer/src/components/cmd-j/worktree-checks-review-index.test.ts index 78889b1d2..e401aa066 100644 --- a/src/renderer/src/components/cmd-j/worktree-checks-review-index.test.ts +++ b/src/renderer/src/components/cmd-j/worktree-checks-review-index.test.ts @@ -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) + }) }) diff --git a/src/renderer/src/components/cmd-j/worktree-checks-review-index.ts b/src/renderer/src/components/cmd-j/worktree-checks-review-index.ts index 507eee554..992fab427 100644 --- a/src/renderer/src/components/cmd-j/worktree-checks-review-index.ts +++ b/src/renderer/src/components/cmd-j/worktree-checks-review-index.ts @@ -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, diff --git a/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts b/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts index a53eeae41..e7e81727b 100644 --- a/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts +++ b/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts @@ -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({ diff --git a/src/renderer/src/components/sidebar/worktree-meta-updates.ts b/src/renderer/src/components/sidebar/worktree-meta-updates.ts index 26769c650..538b8de6b 100644 --- a/src/renderer/src/components/sidebar/worktree-meta-updates.ts +++ b/src/renderer/src/components/sidebar/worktree-meta-updates.ts @@ -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 = { comment: args.commentInput.trim(), - ...(trimmedDisplayName !== args.currentDisplayName && { - displayName: trimmedDisplayName || undefined - }) + ...(trimmedDisplayName !== args.currentDisplayName && { displayName: trimmedDisplayName }) } if (finalLinkedIssue !== undefined) { updates.linkedIssue = finalLinkedIssue diff --git a/src/renderer/src/lib/browser-palette-search.test.ts b/src/renderer/src/lib/browser-palette-search.test.ts index 6e76c9ab5..23cf01969 100644 --- a/src/renderer/src/lib/browser-palette-search.test.ts +++ b/src/renderer/src/lib/browser-palette-search.test.ts @@ -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 + }) + }) }) diff --git a/src/renderer/src/lib/browser-palette-search.ts b/src/renderer/src/lib/browser-palette-search.ts index 759335a4a..942331640 100644 --- a/src/renderer/src/lib/browser-palette-search.ts +++ b/src/renderer/src/lib/browser-palette-search.ts @@ -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, diff --git a/src/renderer/src/lib/simulator-palette-search.test.ts b/src/renderer/src/lib/simulator-palette-search.test.ts index 48b7ea98d..cfc8aa4a7 100644 --- a/src/renderer/src/lib/simulator-palette-search.test.ts +++ b/src/renderer/src/lib/simulator-palette-search.test.ts @@ -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 + }) + }) }) diff --git a/src/renderer/src/lib/simulator-palette-search.ts b/src/renderer/src/lib/simulator-palette-search.ts index 1260700ca..aded93466 100644 --- a/src/renderer/src/lib/simulator-palette-search.ts +++ b/src/renderer/src/lib/simulator-palette-search.ts @@ -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, diff --git a/src/renderer/src/lib/workspace-tab-palette-results.ts b/src/renderer/src/lib/workspace-tab-palette-results.ts index 4f1a5798e..9d829355a 100644 --- a/src/renderer/src/lib/workspace-tab-palette-results.ts +++ b/src/renderer/src/lib/workspace-tab-palette-results.ts @@ -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, diff --git a/src/renderer/src/lib/workspace-tab-palette-search.test.ts b/src/renderer/src/lib/workspace-tab-palette-search.test.ts index fb02f8c74..03b0f55fa 100644 --- a/src/renderer/src/lib/workspace-tab-palette-search.test.ts +++ b/src/renderer/src/lib/workspace-tab-palette-search.test.ts @@ -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 + }) + }) }) diff --git a/src/renderer/src/lib/worktree-default-display-name.test.ts b/src/renderer/src/lib/worktree-default-display-name.test.ts new file mode 100644 index 000000000..89184c421 --- /dev/null +++ b/src/renderer/src/lib/worktree-default-display-name.test.ts @@ -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('') + }) +}) diff --git a/src/renderer/src/lib/worktree-default-display-name.ts b/src/renderer/src/lib/worktree-default-display-name.ts new file mode 100644 index 000000000..765b6d084 --- /dev/null +++ b/src/renderer/src/lib/worktree-default-display-name.ts @@ -0,0 +1,34 @@ +import { branchName } from '@/lib/git-utils' +import { basename } from '@/lib/path' +import type { Worktree } from '../../../shared/types' + +type WorktreeDisplayNameSource = Pick + +/** + * `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): 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() : '' +} diff --git a/src/renderer/src/lib/worktree-palette-search.test.ts b/src/renderer/src/lib/worktree-palette-search.test.ts index 4cb69b75e..81d549a34 100644 --- a/src/renderer/src/lib/worktree-palette-search.test.ts +++ b/src/renderer/src/lib/worktree-palette-search.test.ts @@ -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( [ diff --git a/src/renderer/src/lib/worktree-palette-search.ts b/src/renderer/src/lib/worktree-palette-search.ts index f8dce9958..074587e8a 100644 --- a/src/renderer/src/lib/worktree-palette-search.ts +++ b/src/renderer/src/lib/worktree-palette-search.ts @@ -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( diff --git a/src/renderer/src/store/slices/worktree-helpers.test.ts b/src/renderer/src/store/slices/worktree-helpers.test.ts index fbd887504..2750be444 100644 --- a/src/renderer/src/store/slices/worktree-helpers.test.ts +++ b/src/renderer/src/store/slices/worktree-helpers.test.ts @@ -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') + }) }) diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index fee8e4797..bea4a374b 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -326,11 +326,51 @@ export function findWorktreeById( return undefined } +type RequiredKey = { [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, 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 +): Partial { + 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, worktreeId: string, - updates: Partial + rawUpdates: Partial ): Record { + const updates = withoutErasedRequiredWorktreeFields(rawUpdates) const repoId = getRepoIdFromWorktreeId(worktreeId) const worktrees = worktreesByRepo[repoId] if (!worktrees) { diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 1d7ae9c5c..8b9f4e34e 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -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 + rawUpdates: Partial ): AppState['detectedWorktreesByRepo'] { + // Why: mirrors applyWorktreeUpdates — detected rows feed the same palette. + const updates = withoutErasedRequiredWorktreeFields(rawUpdates) let changed = false const nextByRepo: AppState['detectedWorktreesByRepo'] = {}