mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 16:03:49 +00:00
fix(cmd-j): preserve duplicate recent tab occurrences
This commit is contained in:
@@ -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))
|
||||
|
||||
|
||||
@@ -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<string, number>()
|
||||
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<OpenTabRecentRow[]>(() => {
|
||||
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<PaletteItem[]>(() => {
|
||||
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 (
|
||||
<CommandItem
|
||||
@@ -3512,7 +3570,7 @@ function WorktreeJumpPaletteContent({
|
||||
</span>
|
||||
)}
|
||||
<PaletteRowShortcutBadge
|
||||
index={recentTabShortcutIndexById.get(entry.id)}
|
||||
index={recentTabShortcutIndexByItem.get(entry)}
|
||||
modifierKeys={digitShortcutModifiers}
|
||||
/>
|
||||
</div>
|
||||
@@ -3594,7 +3652,7 @@ function WorktreeJumpPaletteContent({
|
||||
</span>
|
||||
)}
|
||||
<PaletteRowShortcutBadge
|
||||
index={recentTabShortcutIndexById.get(entry.id)}
|
||||
index={recentTabShortcutIndexByItem.get(entry)}
|
||||
modifierKeys={digitShortcutModifiers}
|
||||
/>
|
||||
</div>
|
||||
@@ -3672,7 +3730,7 @@ function WorktreeJumpPaletteContent({
|
||||
</span>
|
||||
)}
|
||||
<PaletteRowShortcutBadge
|
||||
index={recentTabShortcutIndexById.get(entry.id)}
|
||||
index={recentTabShortcutIndexByItem.get(entry)}
|
||||
modifierKeys={digitShortcutModifiers}
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -136,6 +136,36 @@ export function makeRecentTabState(overrides: Partial<AppState> = {}): Partial<A
|
||||
}
|
||||
}
|
||||
|
||||
/** Two host-qualified worktrees intentionally publish the same unified tab id. */
|
||||
export function makeDuplicateRecentTabState(): Partial<AppState> {
|
||||
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<AppState> {
|
||||
const ids = Array.from({ length: count }, (_, index) => `${index}`)
|
||||
|
||||
@@ -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')]
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user