From 14d183324828599db40b744cd6caae6803ad0f54 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 24 Aug 2026 09:12:25 -0700 Subject: [PATCH] fix(cmd-j): preserve duplicate recent tab occurrences --- .../WorktreeJumpPalette.recent-tabs.test.tsx | 20 +++++ .../src/components/WorktreeJumpPalette.tsx | 84 ++++++++++++++++--- .../worktree-jump-palette-test-fixtures.ts | 30 +++++++ .../src/lib/recent-workspace-tab-rows.test.ts | 15 ++++ .../src/lib/recent-workspace-tab-rows.ts | 21 ++++- 5 files changed, 153 insertions(+), 17 deletions(-) diff --git a/src/renderer/src/components/WorktreeJumpPalette.recent-tabs.test.tsx b/src/renderer/src/components/WorktreeJumpPalette.recent-tabs.test.tsx index 990d9219627..9acf590819f 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.recent-tabs.test.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.recent-tabs.test.tsx @@ -12,6 +12,7 @@ import { makePaneKey } from '../../../shared/stable-pane-id' import { LEAF_ID, makeAgentEntry, + makeDuplicateRecentTabState, makeGroup, makeManyTabState, makeRecentTabState, @@ -229,6 +230,25 @@ describe('WorktreeJumpPalette recent chats & terminals', () => { expect(testContainer.textContent).toContain('Recent Worktrees') }) + it('keeps duplicate persisted tab ids as separate recent rows and digit targets', async () => { + await renderPalette(makeDuplicateRecentTabState()) + + expect( + getRenderedRowIds().filter( + (id) => id === 'workspace-tab:tab-duplicate' || id.includes(':workspace-tab:tab-duplicate') + ) + ).toEqual(['workspace-tab:tab-duplicate', 'palette-dup:1:workspace-tab:tab-duplicate']) + + await act(async () => { + emitCmdJRowIndexJump(1) + }) + await flushEffects() + + expect(activateWorkspaceTabPaletteResult).toHaveBeenCalledWith( + expect.objectContaining({ tabId: 'tab-duplicate', worktreeId: 'wt-beta' }) + ) + }) + it('caps the recent section so the worktree header stays above the fold', async () => { await renderPalette(makeManyTabState(12)) diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index 9a54ed398c7..d8558962fbd 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -350,9 +350,60 @@ function isCurrentOpenTabItem(item: OpenTabPaletteItem): boolean { return item.type === 'browser-page' ? item.result.isCurrentPage : item.result.isCurrentTab } +/** + * Builds a stable key for one open-tab result before adding an occurrence ordinal. The command id + * is intentionally not enough: two hosts can publish the same tab id, and a duplicate snapshot can + * briefly contain the same row twice. + */ +function getRecentTabOccurrenceBase(item: OpenTabPaletteItem): string { + if (item.type === 'browser-page') { + const result = item.result + return JSON.stringify([ + item.type, + item.id, + result.executionHostId ?? '', + result.worktreeId, + result.workspaceId, + result.pageId + ]) + } + if (item.type === 'simulator-tab') { + const result = item.result + return JSON.stringify([ + item.type, + item.id, + result.executionHostId ?? '', + result.worktreeId, + result.groupId, + result.tabId + ]) + } + const result = item.result + return JSON.stringify([ + item.type, + item.id, + result.executionHostId ?? '', + result.worktreeId, + result.groupId, + result.tabId, + result.entityId + ]) +} + +function buildRecentTabOccurrenceIds(items: readonly OpenTabPaletteItem[]): string[] { + const nextOrdinalByBase = new Map() + return items.map((item) => { + const base = getRecentTabOccurrenceBase(item) + const ordinal = nextOrdinalByBase.get(base) ?? 0 + nextOrdinalByBase.set(base, ordinal + 1) + return `recent-tab:${base}:${ordinal}` + }) +} + /** An open tab's recent-section row plus the inputs inclusion needs. */ type OpenTabRecentRow = { item: OpenTabPaletteItem + occurrenceId: string worktree: Worktree row: RecentWorkspaceTabRow } @@ -1419,17 +1470,22 @@ function WorktreeJumpPaletteContent({ // once inclusion would drop it (a current tab that quiets down), or the pip blanks mid-open. const openTabRecentRows = useMemo(() => { const entries: OpenTabRecentRow[] = [] - for (const item of openTabItems) { + const occurrenceIds = buildRecentTabOccurrenceIds(openTabItems) + for (const [index, item] of openTabItems.entries()) { const worktree = resolveWorktree(item.result.worktreeId, item.result.executionHostId) if (!worktree) { continue } + const occurrenceId = occurrenceIds[index] entries.push({ item, + occurrenceId, worktree, row: { id: item.id, + occurrenceId, worktreeId: worktree.id, + worktreeHostId: worktree.hostId, unifiedTabId: item.type === 'browser-page' ? null : item.result.tabId, terminalTab: item.type === 'workspace-tab' && item.result.contentType === 'terminal' @@ -1442,8 +1498,8 @@ function WorktreeJumpPaletteContent({ return entries }, [openTabItems, resolveWorktree, terminalTabsById]) - const recentTabRowById = useMemo( - () => new Map(openTabRecentRows.map(({ row }) => [row.id, row])), + const recentTabRowByItem = useMemo( + () => new Map(openTabRecentRows.map(({ item, row }) => [item, row])), [openTabRecentRows] ) @@ -1558,15 +1614,17 @@ function WorktreeJumpPaletteContent({ visible ]) - // Why: walk the frozen order rather than re-sorting the tab list — the frozen ids are the ranking, + // Why: walk the frozen order rather than re-sorting the tab list — the frozen occurrence ids are the ranking, // so agent churn never reshuffles rows under the cursor. Cap after resolving, not before: a chip // applied mid-open narrows `openTabItems`, and capping first would leave the section empty. const recentTabItems = useMemo(() => { - const itemById = new Map(openTabItems.map((item) => [item.id, item])) + const itemByOccurrenceId = new Map( + openTabRecentRows.map(({ occurrenceId, item }) => [occurrenceId, item]) + ) return recentTabOrder - .flatMap((id) => itemById.get(id) ?? []) + .flatMap((occurrenceId) => itemByOccurrenceId.get(occurrenceId) ?? []) .slice(0, EMPTY_QUERY_RECENT_TAB_CAP) - }, [openTabItems, recentTabOrder]) + }, [openTabRecentRows, recentTabOrder]) const settingsResults = useMemo( () => buildCmdJSettingsResults(settingsSections), @@ -1873,10 +1931,10 @@ function WorktreeJumpPaletteContent({ ]) // Why: badges number the snapshotted recent rows only — ⌘N is meaningless on a typed query. - const recentTabShortcutIndexById = useMemo( + const recentTabShortcutIndexByItem = useMemo( () => new Map( - hasQuery ? [] : paletteSections.visibleOpenTabItems.map((item, index) => [item.id, index]) + hasQuery ? [] : paletteSections.visibleOpenTabItems.map((item, index) => [item, index]) ), [hasQuery, paletteSections] ) @@ -3450,7 +3508,7 @@ function WorktreeJumpPaletteContent({ ) // Why regardless of query: a searched-for tab is exactly when you need to know it's // still working — the map covers every open tab, not just the recent section. - const recentRow = recentTabRowById.get(entry.id) ?? null + const recentRow = recentTabRowByItem.get(entry) ?? null return ( )} @@ -3594,7 +3652,7 @@ function WorktreeJumpPaletteContent({ )} @@ -3672,7 +3730,7 @@ function WorktreeJumpPaletteContent({ )} diff --git a/src/renderer/src/components/worktree-jump-palette-test-fixtures.ts b/src/renderer/src/components/worktree-jump-palette-test-fixtures.ts index 5dfbcdaf6a8..19c97ada254 100644 --- a/src/renderer/src/components/worktree-jump-palette-test-fixtures.ts +++ b/src/renderer/src/components/worktree-jump-palette-test-fixtures.ts @@ -136,6 +136,36 @@ export function makeRecentTabState(overrides: Partial = {}): Partial { + const alpha = makeWorktree('wt-alpha', 'Alpha workspace', { hostId: 'ssh:alpha' }) + const beta = makeWorktree('wt-beta', 'Beta workspace', { hostId: 'ssh:beta' }) + return { + worktreesByRepo: { 'repo-1': [alpha, beta] }, + showSleepingWorkspaces: true, + ptyIdsByTabId: { + 'term-alpha': ['pty-term-alpha'], + 'term-beta': ['pty-term-beta'] + }, + tabsByWorktree: { + 'wt-alpha': [makeTerminalTab('term-alpha', 'wt-alpha', 'Alpha duplicate')], + 'wt-beta': [makeTerminalTab('term-beta', 'wt-beta', 'Beta duplicate')] + }, + unifiedTabsByWorktree: { + 'wt-alpha': [makeUnifiedTab('tab-duplicate', 'wt-alpha', 'term-alpha', 'Alpha duplicate')], + 'wt-beta': [makeUnifiedTab('tab-duplicate', 'wt-beta', 'term-beta', 'Beta duplicate')] + }, + groupsByWorktree: { + 'wt-alpha': [makeGroup('wt-alpha', ['tab-duplicate'])], + 'wt-beta': [makeGroup('wt-beta', ['tab-duplicate'])] + }, + activeGroupIdByWorktree: { + 'wt-alpha': 'group-wt-alpha', + 'wt-beta': 'group-wt-beta' + } + } +} + /** One tab-heavy worktree plus `count` bare ones, so both sections overflow their caps. */ export function makeManyTabState(count: number): Partial { const ids = Array.from({ length: count }, (_, index) => `${index}`) diff --git a/src/renderer/src/lib/recent-workspace-tab-rows.test.ts b/src/renderer/src/lib/recent-workspace-tab-rows.test.ts index 5f344214e92..90a7b88fdc6 100644 --- a/src/renderer/src/lib/recent-workspace-tab-rows.test.ts +++ b/src/renderer/src/lib/recent-workspace-tab-rows.test.ts @@ -206,6 +206,21 @@ describe('orderRecentWorkspaceTabs', () => { ]) }) + it('returns each occurrence identity when palette ids collide', () => { + const rows = [ + row('workspace-tab:duplicate', { + occurrenceId: 'recent-tab:alpha', + worktreeId: 'wt-alpha' + }), + row('workspace-tab:duplicate', { + occurrenceId: 'recent-tab:beta', + worktreeId: 'wt-beta' + }) + ] + + expect(order(rows, sources([]))).toEqual(['recent-tab:alpha', 'recent-tab:beta']) + }) + it('treats rows without a terminal tab as idle', () => { const rows = [row('browser', { terminalTab: null, unifiedTabId: null }), row('blocked')] diff --git a/src/renderer/src/lib/recent-workspace-tab-rows.ts b/src/renderer/src/lib/recent-workspace-tab-rows.ts index e0d0d7751f5..f20da28d752 100644 --- a/src/renderer/src/lib/recent-workspace-tab-rows.ts +++ b/src/renderer/src/lib/recent-workspace-tab-rows.ts @@ -10,6 +10,8 @@ import { tabHasLivePty } from './tab-has-live-pty' import type { WorktreeStatus } from './worktree-status' import type { TabGroup } from '../../../shared/tab-types' import type { TerminalTab } from '../../../shared/terminal-tab-types' +import type { ExecutionHostId } from '../../../shared/execution-host' +import { getWorktreeVisitTimestamp } from './worktree-visit-recency' /** * Row model for Cmd+J's empty-query "Recent chats & terminals" section. @@ -20,7 +22,15 @@ import type { TerminalTab } from '../../../shared/terminal-tab-types' export type RecentWorkspaceTabRow = { /** Palette item id. */ id: string + /** + * Per-occurrence identity used while the palette is open. Persisted tab ids can collide across + * hosts (or in a transient duplicate snapshot), so ordering must not use `id` as its key. + * Callers that know their ids are unique may omit this and fall back to `id`. + */ + occurrenceId?: string worktreeId: string + /** Host owner used to distinguish same-id rows published by two hosts. */ + worktreeHostId?: ExecutionHostId /** Unified tab id — the key `TabGroup.recentTabIds` uses. Null for rows outside a tab group. */ unifiedTabId: string | null /** Terminal tab whose panes carry agent state. Null for editor, browser and simulator rows. */ @@ -38,7 +48,7 @@ export type RecentWorkspaceTabOrderInputs = { } type RankedRow = { - id: string + occurrenceId: string needsAttention: boolean attentionClass: SmartClass attentionTimestamp: number @@ -152,11 +162,14 @@ export function orderRecentWorkspaceTabs(inputs: RecentWorkspaceTabOrderInputs): .map((row): RankedRow => { const attention = resolveRecentWorkspaceTabAttention(row, paneSources, now) return { - id: row.id, + occurrenceId: row.occurrenceId ?? row.id, needsAttention: attention.cls <= NEEDS_ATTENTION_MAX_CLASS, attentionClass: attention.cls, attentionTimestamp: attention.attentionTimestamp, - visitedAt: lastVisitedAtByWorktreeId[row.worktreeId], + visitedAt: getWorktreeVisitTimestamp(lastVisitedAtByWorktreeId, { + id: row.worktreeId, + hostId: row.worktreeHostId + }), worktreeId: row.worktreeId, worktreeOrder: worktreeOrder.get(row.worktreeId) ?? Number.MAX_SAFE_INTEGER, focusOrdinal: @@ -166,5 +179,5 @@ export function orderRecentWorkspaceTabs(inputs: RecentWorkspaceTabOrderInputs): } }) .sort(compareRankedRows) - .map((row) => row.id) + .map((row) => row.occurrenceId) }