diff --git a/apps/desktop/src/main/__tests__/import-tasks-settings-page.test.ts b/apps/desktop/src/main/__tests__/import-tasks-settings-page.test.ts index 6b4cb62aeb..b69c9bf3bb 100644 --- a/apps/desktop/src/main/__tests__/import-tasks-settings-page.test.ts +++ b/apps/desktop/src/main/__tests__/import-tasks-settings-page.test.ts @@ -610,6 +610,577 @@ describe('ImportTasksSettingsPage durable import state', () => { }); }); +describe('ImportTasksSettingsPage source switching', () => { + const LOADING = /Reading external conversations/; + + it('shows the reading spinner the first time a source is opened', async () => { + let settle: ((r: CatalogResult) => void) | undefined; + const pending = new Promise((resolve) => { + settle = resolve; + }); + const harness = await renderPage({ + adapterIds: ['codex', 'claude-code'], + bySource: { + codex: [catalog(externalSession({ id: 's-codex', name: 'Codex conv' }))], + 'claude-code': [pending], + }, + }); + + assert.match(harness.container.textContent, /Codex conv/, 'codex loads on mount'); + assert.doesNotMatch(harness.container.textContent, LOADING, 'no spinner once codex is loaded'); + + const cc = segment(harness.container, 'claude-code'); + assert.ok(cc, 'claude-code segment renders'); + await act(async () => { + cc.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + + // First visit to claude-code: nothing cached, so the blank + spinner shows. + assert.match(harness.container.textContent, LOADING, 'first-time load shows the spinner'); + assert.doesNotMatch(harness.container.textContent, /Codex conv/, 'codex rows are cleared'); + + await act(async () => { + settle?.(catalog(externalSession({ id: 's-cc', name: 'CC conv' }))); + await Promise.resolve(); + await Promise.resolve(); + }); + + assert.match(harness.container.textContent, /CC conv/, 'claude-code rows arrive'); + assert.doesNotMatch(harness.container.textContent, LOADING, 'spinner clears when loaded'); + + await act(async () => harness.root.unmount()); + }); + + it('shows a previously-loaded source instantly with no spinner, then refreshes in place', async () => { + let settleRevisit: ((r: CatalogResult) => void) | undefined; + const revisitRefresh = new Promise((resolve) => { + settleRevisit = resolve; + }); + const harness = await renderPage({ + adapterIds: ['codex', 'claude-code'], + bySource: { + // [initial load, background refresh on revisit] + codex: [catalog(externalSession({ id: 's-codex', name: 'Codex conv' })), revisitRefresh], + 'claude-code': [catalog(externalSession({ id: 's-cc', name: 'CC conv' }))], + }, + }); + + assert.match(harness.container.textContent, /Codex conv/); + + await act(async () => { + segment(harness.container, 'claude-code')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /CC conv/, 'claude-code loaded'); + + // Revisit codex: cached rows appear immediately with no blanking spinner + // (the background refresh is still pending here). + await act(async () => { + segment(harness.container, 'codex')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /Codex conv/, 'cached codex rows shown instantly'); + assert.doesNotMatch(harness.container.textContent, LOADING, 'no spinner on revisit'); + + await act(async () => { + settleRevisit?.(catalog(externalSession({ id: 's-codex', name: 'Codex conv refreshed' }))); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /Codex conv refreshed/, 'background refresh lands'); + assert.doesNotMatch(harness.container.textContent, LOADING, 'still no spinner after refresh'); + + await act(async () => harness.root.unmount()); + }); + + it('does not let a stale background refresh overwrite a newer source selection', async () => { + let settleStaleCodexRefresh: ((r: CatalogResult) => void) | undefined; + const staleCodexRefresh = new Promise((resolve) => { + settleStaleCodexRefresh = resolve; + }); + const harness = await renderPage({ + adapterIds: ['codex', 'claude-code'], + bySource: { + codex: [catalog(externalSession({ id: 's-codex', name: 'Codex conv' })), staleCodexRefresh], + 'claude-code': [ + catalog(externalSession({ id: 's-cc', name: 'CC conv' })), + catalog(externalSession({ id: 's-cc', name: 'CC conv' })), + ], + }, + }); + + await act(async () => { + segment(harness.container, 'claude-code')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /CC conv/); + + // Revisit codex (cache hit → background refresh left pending)... + await act(async () => { + segment(harness.container, 'codex')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + // ...then switch straight back to claude-code before that refresh resolves. + await act(async () => { + segment(harness.container, 'claude-code')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /CC conv/, 'claude-code is the current source'); + + await act(async () => { + settleStaleCodexRefresh?.( + catalog(externalSession({ id: 's-codex', name: 'Stale codex conv' })), + ); + await Promise.resolve(); + await Promise.resolve(); + }); + + assert.match(harness.container.textContent, /CC conv/, 'claude-code rows remain'); + assert.doesNotMatch( + harness.container.textContent, + /Stale codex conv/, + 'the superseded codex refresh never lands under claude-code', + ); + + await act(async () => harness.root.unmount()); + }); + + it('drops an in-flight import poll after switching source, with no stuck spinner', async (context) => { + context.mock.timers.enable({ apis: ['setTimeout'] }); + let settleStalePoll: ((r: CatalogResult) => void) | undefined; + const stalePoll = new Promise((resolve) => { + settleStalePoll = resolve; + }); + const importing = externalSession({ + id: 's-codex', + name: 'Codex conv', + importState: { importedCount: 0, importedSessionIds: [], isImporting: true }, + }); + const harness = await renderPage({ + adapterIds: ['codex', 'claude-code'], + bySource: { + // [initial load with an import in flight, background poll read left pending] + codex: [{ sessions: [importing], nextCursor: null }, stalePoll], + 'claude-code': [catalog(externalSession({ id: 's-cc', name: 'CC conv' }))], + }, + }); + assert.match(harness.container.textContent, /Codex conv/); + + // The importing row schedules a poll; fire it so refreshLoadedCatalog is in + // flight against the pending read. + await act(async () => { + context.mock.timers.runAll(); + await Promise.resolve(); + await Promise.resolve(); + }); + + // Switch to claude-code while the codex poll is still in flight. + await act(async () => { + segment(harness.container, 'claude-code')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /CC conv/, 'claude-code loaded'); + assert.doesNotMatch(harness.container.textContent, LOADING, 'no stuck reading spinner'); + + // The stale codex poll resolves last — it must not overwrite claude-code. + await act(async () => { + settleStalePoll?.({ + sessions: [externalSession({ id: 's-codex', name: 'Codex conv refreshed' })], + nextCursor: null, + }); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /CC conv/, 'still showing claude-code'); + assert.doesNotMatch( + harness.container.textContent, + /Codex conv refreshed/, + 'stale poll result is dropped', + ); + assert.doesNotMatch(harness.container.textContent, LOADING, 'still no spinner'); + + await act(async () => harness.root.unmount()); + }); + + it('clears the reading spinner when a pending search returns to a cached term', async (context) => { + context.mock.timers.enable({ apis: ['setTimeout'] }); + // The uncached search never resolves, so its spinner generation stays in + // flight; the return-to-'' refresh never resolves either, so only the + // cache-hit path — not a completed refresh — can retire the spinner. + const pendingSearch = new Promise(() => {}); + const refreshPending = new Promise(() => {}); + const harness = await renderPage({ + // [initial '' load, uncached 'zzz' search, background refresh on return to ''] + catalogs: [ + catalog(externalSession({ id: 's-codex', name: 'Codex conv' })), + pendingSearch, + refreshPending, + ], + }); + assert.match(harness.container.textContent, /Codex conv/, 'initial load shows rows'); + + // Type an uncached term (the source and archived controls disable during a + // load, but the search box does not, so this is the reachable way to leave a + // request pending). It blanks to the spinner and never resolves. + await act(async () => { + setSearchInput(harness.container, 'zzz'); + await Promise.resolve(); + }); + // Fire the 250ms debounce only after the effect above has registered it. + await act(async () => { + context.mock.timers.runAll(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, LOADING, 'uncached search shows the spinner'); + + // Return the search to the already-loaded empty term. The cached rows must + // come back with no spinner even though the older 'zzz' load is still + // pending and this hit's own refresh has not landed — the cache hit has to + // clear the stranded loading state itself. + await act(async () => { + setSearchInput(harness.container, ''); + await Promise.resolve(); + }); + await act(async () => { + context.mock.timers.runAll(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /Codex conv/, 'cached rows shown instantly'); + assert.doesNotMatch( + harness.container.textContent, + LOADING, + 'the stranded search spinner is cleared on the cache hit', + ); + + await act(async () => harness.root.unmount()); + }); + + it('clears a pending Load More lock when switching back to a cached source', async () => { + // Both revisit refreshes and the Load More append are left pending, so the + // only thing that can release the Load More lock is the cache-hit reset. + const codexRefreshPending = new Promise(() => {}); + const codexLoadMorePending = new Promise(() => {}); + const claudeCodeRefreshPending = new Promise(() => {}); + const harness = await renderPage({ + adapterIds: ['codex', 'claude-code'], + bySource: { + // [initial load (paged), revisit refresh, Load More append] + codex: [ + { sessions: [externalSession({ id: 's-codex', name: 'Codex conv' })], nextCursor: 'c1' }, + codexRefreshPending, + codexLoadMorePending, + ], + // [initial load (paged), revisit refresh] + 'claude-code': [ + { sessions: [externalSession({ id: 's-cc', name: 'CC conv' })], nextCursor: 'cc1' }, + claudeCodeRefreshPending, + ], + }, + }); + + // Load claude-code so it is cached with its own paged Load More. + await act(async () => { + segment(harness.container, 'claude-code')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /CC conv/, 'claude-code loaded'); + + // Revisit codex (cache hit; background refresh left pending), then start a + // Load More whose append never resolves so `loadingMore` stays set. + await act(async () => { + segment(harness.container, 'codex')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + const codexLoadMore = buttonWithText(harness.container, 'Load more'); + assert.ok(codexLoadMore, 'codex Load More renders'); + await act(async () => { + codexLoadMore.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + const busyLoadMore = Array.from( + harness.container.querySelectorAll('button'), + ).find((button) => button.textContent?.includes('Loading…')); + assert.ok(busyLoadMore, 'Load More shows the pending label while the append is in flight'); + + // Switch back to the cached claude-code before that append resolves. Its + // Load More must not inherit the stranded lock from codex's pending append. + await act(async () => { + segment(harness.container, 'claude-code')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /CC conv/, 'cached claude-code rows shown'); + const cachedLoadMore = buttonWithText(harness.container, 'Load more'); + assert.ok(cachedLoadMore, "claude-code's Load More is released, not stuck on 'Loading…'"); + assert.equal(cachedLoadMore.hasAttribute('disabled'), false, 'Load More is enabled again'); + + await act(async () => harness.root.unmount()); + }); + + it('keeps every loaded page when a revisited multi-page source refreshes', async () => { + const harness = await renderPage({ + adapterIds: ['codex', 'claude-code'], + bySource: { + // [page 1, page 2 via Load More, refresh page 1, refresh page 2] + codex: [ + { + sessions: [externalSession({ id: 's-codex-1', name: 'Codex page one' })], + nextCursor: 'codex-cursor-1', + }, + { sessions: [externalSession({ id: 's-codex-2', name: 'Codex page two' })], nextCursor: null }, + { + sessions: [externalSession({ id: 's-codex-1', name: 'Codex page one' })], + nextCursor: 'codex-cursor-1', + }, + { sessions: [externalSession({ id: 's-codex-2', name: 'Codex page two' })], nextCursor: null }, + ], + 'claude-code': [catalog(externalSession({ id: 's-cc', name: 'CC conv' }))], + }, + }); + + // Page in the second page of codex via Load More. + const loadMore = buttonWithText(harness.container, 'Load more'); + assert.ok(loadMore, 'codex has a second page to load'); + await act(async () => { + loadMore.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /Codex page one/); + assert.match(harness.container.textContent, /Codex page two/, 'both pages are loaded'); + + // Switch away to claude-code, then back to codex. + await act(async () => { + segment(harness.container, 'claude-code')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /CC conv/); + + await act(async () => { + segment(harness.container, 'codex')!.click(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + // The cache hit shows both pages instantly; the background refresh must + // re-read the *whole* loaded window rather than shrink the list back to the + // first page. + assert.match(harness.container.textContent, /Codex page one/, 'first page kept'); + assert.match( + harness.container.textContent, + /Codex page two/, + 'the second page survives the background refresh', + ); + // codex page 1 + Load More + refresh page 1 + refresh page 2, plus the one + // claude-code load = 5. A first-page-only refresh would stop at 4. + assert.equal(harness.listCalls(), 5, 'the revisit refresh re-read every loaded page'); + + await act(async () => harness.root.unmount()); + }); + + it('does not start a second catalog read when revisiting a still-importing source', async (context) => { + context.mock.timers.enable({ apis: ['setTimeout'] }); + const importing = externalSession({ + id: 's-codex', + name: 'Codex conv', + importState: { importedCount: 0, importedSessionIds: [], isImporting: true }, + }); + const harness = await renderPage({ + adapterIds: ['codex', 'claude-code'], + bySource: { + codex: [{ sessions: [importing], nextCursor: null }], + 'claude-code': [catalog(externalSession({ id: 's-cc', name: 'CC conv' }))], + }, + }); + assert.match(harness.container.textContent, /Codex conv/); + assert.equal(harness.listCalls(), 1, 'codex loaded once on mount'); + + // Load claude-code (now cached), then return to the still-importing codex. + await act(async () => { + segment(harness.container, 'claude-code')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.equal(harness.listCalls(), 2, 'claude-code loaded'); + + await act(async () => { + segment(harness.container, 'codex')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + // The cache hit shows the importing rows again, but must NOT kick off its own + // background readCatalogWindow: the 1s import poll is the single refresher for + // an importing selection, and a second concurrent read shares the same request + // generation and can land a stale pre-import page on top of a newer poll + // result (the timer is deliberately left un-fired here). + assert.match(harness.container.textContent, /Codex conv/, 'cached codex rows shown'); + assert.equal( + harness.listCalls(), + 2, + 'revisiting an importing source starts no second catalog read', + ); + + await act(async () => harness.root.unmount()); + }); + + it('updates the cache for a recovered import even after switching away', async () => { + let settleImport: ((r: { ok: false; reason: 'commit_outcome_unknown' }) => void) | undefined; + const importResult = new Promise<{ ok: false; reason: 'commit_outcome_unknown' }>((resolve) => { + settleImport = resolve; + }); + const codexRecovered = externalSession({ + id: 's-codex', + name: 'Codex conv', + importState: { importedCount: 1, importedSessionIds: ['codex-task'], isImporting: false }, + }); + // The revisit's background refresh never resolves, so the returned view is the + // cache alone — proving the cache itself holds the recovered state. + const codexRevisitPending = new Promise(() => {}); + const harness = await renderPage({ + adapterIds: ['codex', 'claude-code'], + bySource: { + // [mount, recovery readCatalogWindow, revisit background refresh] + codex: [ + { sessions: [externalSession({ id: 's-codex', name: 'Codex conv' })], nextCursor: null }, + { sessions: [codexRecovered], nextCursor: null }, + codexRevisitPending, + ], + 'claude-code': [catalog(externalSession({ id: 's-cc', name: 'CC conv' }))], + }, + importResult, + }); + + // Start an import on codex, then switch to claude-code before the (unknown) + // outcome resolves. + const importButton = harness.container.querySelector( + 'button[aria-label="Import Codex conv"]', + ); + assert.ok(importButton); + await act(async () => importButton.click()); + + await act(async () => { + segment(harness.container, 'claude-code')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /CC conv/); + + // The import comes back unknown; recovery confirms it landed while codex is not + // the current selection. + await act(async () => { + settleImport?.({ ok: false, reason: 'commit_outcome_unknown' }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /The imported task is available now/); + assert.match(harness.container.textContent, /CC conv/, 'the current view is untouched by recovery'); + + // Returning to codex must show the recovered "imported" state straight from + // the cache — not the stale pre-import row that would invite a duplicate + // import — even though the background refresh has not landed. + await act(async () => { + segment(harness.container, 'codex')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /Codex conv/); + assert.match( + harness.container.textContent, + /Imported once/, + 'the cache reflects the recovered import on return', + ); + + await act(async () => harness.root.unmount()); + }); + + it('publishes a recovered import to the current view after leaving and returning to its source', async () => { + let settleImport: ((r: { ok: false; reason: 'commit_outcome_unknown' }) => void) | undefined; + const importResult = new Promise<{ ok: false; reason: 'commit_outcome_unknown' }>((resolve) => { + settleImport = resolve; + }); + const codexRecovered = externalSession({ + id: 's-codex', + name: 'Codex conv', + importState: { importedCount: 1, importedSessionIds: ['codex-task'], isImporting: false }, + }); + // The revisit's own background refresh never resolves, so recovery is the only + // thing that can update the screen — proving recovery publishes rather than + // leaving the view to wait on a slow refresh. + const codexRevisitPending = new Promise(() => {}); + const harness = await renderPage({ + adapterIds: ['codex', 'claude-code'], + bySource: { + // [mount, revisit background refresh (pending), recovery readCatalogWindow] + codex: [ + { sessions: [externalSession({ id: 's-codex', name: 'Codex conv' })], nextCursor: null }, + codexRevisitPending, + { sessions: [codexRecovered], nextCursor: null }, + ], + 'claude-code': [catalog(externalSession({ id: 's-cc', name: 'CC conv' }))], + }, + importResult, + }); + + const importButton = harness.container.querySelector( + 'button[aria-label="Import Codex conv"]', + ); + assert.ok(importButton); + await act(async () => importButton.click()); + + // Switch to claude-code, then back to codex — all before the import resolves. + await act(async () => { + segment(harness.container, 'claude-code')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + await act(async () => { + segment(harness.container, 'codex')!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /Codex conv/, 'back on codex, pre-import rows shown'); + assert.doesNotMatch(harness.container.textContent, /Imported once/, 'not recovered yet'); + + // Recovery lands. codex is the current selection again, but at a *newer* + // generation than when the import started, so a generation check would refuse + // to publish. Matching the selection tuple, recovery must still reach the + // screen — not just the cache — even though the revisit refresh is pending. + await act(async () => { + settleImport?.({ ok: false, reason: 'commit_outcome_unknown' }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + assert.match(harness.container.textContent, /The imported task is available now/); + assert.match( + harness.container.textContent, + /Imported once/, + 'recovery publishes to the returned-to view, not only the cache', + ); + + await act(async () => harness.root.unmount()); + }); +}); + function externalSession( overrides: Partial = {}, ): DesktopExternalSessionCatalogItem { @@ -626,6 +1197,10 @@ function externalSession( async function renderPage(options: { catalog?: CatalogResult; catalogs?: Array>; + // Multi-source tests: `listSources` reports these, and `list` draws per-source + // queues from `bySource` (keyed by adapterId) instead of the flat `catalogs`. + adapterIds?: string[]; + bySource?: Record>>; importResult?: | { ok: false; reason: 'commit_outcome_unknown' } | Promise<{ ok: false; reason: 'commit_outcome_unknown' }>; @@ -671,16 +1246,29 @@ async function renderPage(options: { host?: DesktopRuntimeHostRef; }> = []; const catalogs = options.catalogs ?? [options.catalog ?? { sessions: [], nextCursor: null }]; + const sourceCounts: Record = {}; (window as unknown as { maka: unknown }).maka = { externalSessions: { listSources: async (host?: DesktopRuntimeHostRef) => { hostCalls.push({ operation: 'listSources', host }); - return { adapterIds: ['codex'] }; + return { adapterIds: options.adapterIds ?? ['codex'] }; }, - list: async (input: { includeArchived?: boolean }, host?: DesktopRuntimeHostRef) => { + list: async ( + input: { includeArchived?: boolean; adapterId: string }, + host?: DesktopRuntimeHostRef, + ) => { hostCalls.push({ operation: 'list', host }); listInputs.push({ includeArchived: input.includeArchived === true }); - const result = catalogs[Math.min(listCalls++, catalogs.length - 1)]; + listCalls++; + if (options.bySource) { + const queue = options.bySource[input.adapterId] ?? [{ sessions: [], nextCursor: null }]; + const index = Math.min(sourceCounts[input.adapterId] ?? 0, queue.length - 1); + sourceCounts[input.adapterId] = (sourceCounts[input.adapterId] ?? 0) + 1; + const perSource = queue[index]; + if (perSource instanceof Error) throw perSource; + return perSource; + } + const result = catalogs[Math.min(listCalls - 1, catalogs.length - 1)]; if (result instanceof Error) throw result; return result; }, @@ -729,3 +1317,27 @@ function buttonWithText(container: HTMLElement, text: string): HTMLButtonElement (button) => button.textContent === text, ); } + +// Drives the search TextInput the way goal-dialog.test does: set the value and +// invoke the React onChange the renderer wired to it, so `searchDraft` updates +// without a real input event. The caller fires the debounce timer afterward. +function setSearchInput(container: HTMLElement, value: string): void { + const input = Array.from(container.querySelectorAll('input')).find( + (element) => element.type !== 'checkbox' && element.type !== 'radio', + ); + assert.ok(input, 'search input renders'); + input.value = value; + const propsKey = Object.keys(input).find((key) => key.startsWith('__reactProps$')); + assert.ok(propsKey, 'missing React props on the search input'); + const props = (input as unknown as Record)[propsKey] as { + onChange?: (event: { target: HTMLInputElement; defaultPrevented: boolean }) => void; + }; + assert.ok(props.onChange, 'missing search change handler'); + props.onChange({ target: input, defaultPrevented: false }); +} + +function segment(container: HTMLElement, value: string): HTMLButtonElement | undefined { + return Array.from( + container.querySelectorAll('button[role="radio"]'), + ).find((button) => button.getAttribute('data-value') === value); +} diff --git a/apps/desktop/src/renderer/settings/import-tasks-settings-page.tsx b/apps/desktop/src/renderer/settings/import-tasks-settings-page.tsx index c154954ef2..bd7740f55d 100644 --- a/apps/desktop/src/renderer/settings/import-tasks-settings-page.tsx +++ b/apps/desktop/src/renderer/settings/import-tasks-settings-page.tsx @@ -48,6 +48,36 @@ type CatalogState = { const EMPTY_CATALOG: CatalogState = { sessions: [], nextCursor: null }; const EXTERNAL_SESSION_IMPORT_POLL_MS = 1_000; +/** + * A loaded catalog is cached per (source, archived filter, search) so switching + * back to a source or filter already viewed shows its rows instantly instead of + * blanking to the "reading external conversations" spinner on every switch. The + * key is the same tuple `catalogSelectionRef` tracks, so a cache hit and a + * selection match always agree on what "this catalog" is. + */ +function catalogSelectionKey(adapterId: string, includeArchived: boolean, search: string): string { + return `${adapterId} ${includeArchived ? '1' : '0'} ${search}`; +} + +// A few dozen selections is plenty to make back-and-forth switching instant +// without letting a long search session grow the cache without bound. Re-insert +// on write so the oldest untouched selection is the one evicted. +const CATALOG_CACHE_LIMIT = 24; + +function writeCatalogCache( + cache: Map, + key: string, + value: CatalogState, +): void { + cache.delete(key); + cache.set(key, value); + while (cache.size > CATALOG_CACHE_LIMIT) { + const oldest = cache.keys().next().value; + if (oldest === undefined) break; + cache.delete(oldest); + } +} + type CatalogWindow = CatalogState & { targetSource: DesktopExternalSessionCatalogItem | undefined; }; @@ -114,7 +144,6 @@ type ImportAttempt = { importedCountBefore: number; latestImportedSessionIdBefore: string | undefined; loadedCatalogItemCountBefore: number; - catalogSelectionGeneration: number; }; type ImportRecovery = @@ -199,6 +228,16 @@ export function ImportTasksSettingsPage(props: { // source's rows under the new source's label. const requestGeneration = useRef(0); const recoveryGeneration = useRef(0); + // Last loaded catalog per selection key, so revisiting a source or filter is + // instant. Read/written only inside the async loaders; never rendered + // directly (the `catalog` state is what renders). + const catalogCacheRef = useRef(new Map()); + // Mirrors the committed `catalog` so a page append can extend the visible + // window without threading it through a stale render closure. + const catalogStateRef = useRef(catalog); + useEffect(() => { + catalogStateRef.current = catalog; + }, [catalog]); const catalogSelectionRef = useRef({ adapterId, includeArchived, search, generation: 0 }); if ( catalogSelectionRef.current.adapterId !== adapterId || @@ -231,6 +270,10 @@ export function ImportTasksSettingsPage(props: { setImportError(null); setAdapterIds([]); setAdapterId(null); + // A different host (or locale-driven remount) is a different catalog space; + // the cache keys carry neither, so drop everything rather than serve a + // previous host's rows. + catalogCacheRef.current.clear(); setCatalog(EMPTY_CATALOG); try { const result = await window.maka.externalSessions.listSources(host); @@ -252,8 +295,24 @@ export function ImportTasksSettingsPage(props: { async (sourceId: string, cursor?: string) => { const generation = ++requestGeneration.current; const append = cursor !== undefined; - if (append) setLoadingMore(true); - else { + const key = catalogSelectionKey(sourceId, includeArchived, search); + const cached = append ? undefined : catalogCacheRef.current.get(key); + if (append) { + setLoadingMore(true); + } else if (cached !== undefined) { + // Already loaded this selection once. Show it immediately and refresh in + // the background instead of blanking to the spinner on every switch. + setCatalog(cached); + // Clear any spinner/Load More lock left by a superseded request. A + // still-pending search or pagination load from the previous selection + // will never reach its own `finally` reset (its generation is now + // stale), so restoring cached rows without this would strand the + // full-page spinner or a disabled Load More over an otherwise complete + // view until this hit's background refresh happens to land. + setCatalogLoading(false); + setLoadingMore(false); + setImportRecovery(null); + } else { setCatalogLoading(true); setCatalog(EMPTY_CATALOG); setImportRecovery(null); @@ -261,17 +320,52 @@ export function ImportTasksSettingsPage(props: { setCatalogError(null); setImportError(null); try { - const result = await window.maka.externalSessions.list({ - adapterId: sourceId, - includeArchived, - ...(search ? { text: search } : {}), - ...(cursor === undefined ? {} : { cursor }), - }, host); - if (generation !== requestGeneration.current) return; - setCatalog((current) => ({ - sessions: append ? [...current.sessions, ...result.sessions] : result.sessions, - nextCursor: result.nextCursor, - })); + if (cached !== undefined) { + // While a revisited selection still shows an import in flight, the 1s + // import poll owns refreshing it. Starting our own readCatalogWindow + // here would run a second read under the *same* request generation as + // that poll, and a pre-import page read started here can land after a + // newer poll result and snap "Imported once" back to "Importing…". + // Leave the poll as the single refresher and keep the cached rows up. + if (cached.sessions.some((session) => session.importState.isImporting)) { + return; + } + // Refresh a revisited selection by re-reading the *whole* loaded + // window, not just page one: the cache can be several pages deep from + // Load More, and a bare first-page read here would overwrite it and + // silently drop every page the user already paged in. + const refreshed = await readCatalogWindow({ + adapterId: sourceId, + includeArchived, + text: search, + minimumItemCount: cached.sessions.length, + host, + isCurrent: () => mountedRef.current && generation === requestGeneration.current, + }); + if (refreshed === undefined) return; + const next: CatalogState = { + sessions: refreshed.sessions, + nextCursor: refreshed.nextCursor, + }; + writeCatalogCache(catalogCacheRef.current, key, next); + setCatalog(next); + } else { + const result = await window.maka.externalSessions.list({ + adapterId: sourceId, + includeArchived, + ...(search ? { text: search } : {}), + ...(cursor === undefined ? {} : { cursor }), + }, host); + if (generation !== requestGeneration.current) return; + const next: CatalogState = { + sessions: append + ? [...catalogStateRef.current.sessions, ...result.sessions] + : result.sessions, + nextCursor: result.nextCursor, + }; + writeCatalogCache(catalogCacheRef.current, key, next); + setCatalog(next); + } } catch (error) { if (generation !== requestGeneration.current) return; setCatalogError(localizedShellErrorMessage(error, copy.loadFailedFallback, locale)); @@ -282,12 +376,15 @@ export function ImportTasksSettingsPage(props: { } } }, - [copy.loadFailedFallback, host, includeArchived, locale, search], + [copy.loadFailedFallback, host, includeArchived, locale, mountedRef, search], ); const refreshLoadedCatalog = useCallback( async (sourceId: string, loadedItemCount: number) => { - const generation = ++requestGeneration.current; + // Observe, don't preempt: the poll captures the current generation rather + // than claiming a new one, so it defers to any in-flight authoritative + // load/recovery instead of retiring that load's own catalogLoading reset. + const generation = requestGeneration.current; try { const result = await readCatalogWindow({ adapterId: sourceId, @@ -297,7 +394,18 @@ export function ImportTasksSettingsPage(props: { host, isCurrent: () => mountedRef.current && generation === requestGeneration.current, }); - if (result !== undefined) setCatalog(result); + if (result !== undefined) { + const refreshed: CatalogState = { + sessions: result.sessions, + nextCursor: result.nextCursor, + }; + writeCatalogCache( + catalogCacheRef.current, + catalogSelectionKey(sourceId, includeArchived, search), + refreshed, + ); + setCatalog(refreshed); + } } catch { // Catalog polling is best-effort. Keep the last authoritative page // visible and retry rather than replacing it with a transient error. @@ -375,19 +483,40 @@ export function ImportTasksSettingsPage(props: { throw new Error('External Session source disappeared during import recovery'); } + const recoveredState: CatalogState = { + sessions: result.sessions, + nextCursor: result.nextCursor, + }; + // Always refresh the cache for the selection this attempt came from, even + // if the user has since switched source or filter. Recovery has confirmed + // the import landed; leaving the pre-import rows in the cache would greet + // the user with an "Import" button (not "Import again") when they return, + // inviting a duplicate import until a later background refresh corrects it. + writeCatalogCache( + catalogCacheRef.current, + catalogSelectionKey(attempt.adapterId, attempt.includeArchived, attempt.text), + recoveredState, + ); + + // Publish to screen whenever the user is currently viewing the same + // selection tuple this attempt came from. Match the tuple, not the + // generation captured at import time: navigating A→B→A lands back on the + // same selection with a *newer* generation, and a generation check would + // refuse to publish there — leaving a clickable "Import" on a row that + // already imported until a slow background refresh happened to catch up. const currentSelection = catalogSelectionRef.current; if ( currentSelection.adapterId === attempt.adapterId && currentSelection.includeArchived === attempt.includeArchived && - currentSelection.generation === attempt.catalogSelectionGeneration + currentSelection.search === attempt.text ) { // Recovery is the newest authoritative read for this exact catalog - // selection. Retire an older poll/load-more response before publishing - // it so that response cannot put pre-import state back on screen. + // selection. Retire an older poll/load-more/revisit response before + // publishing it so that response cannot put pre-import state back up. requestGeneration.current += 1; setCatalogLoading(false); setLoadingMore(false); - setCatalog(result); + setCatalog(recoveredState); } setUncertainImports((current) => current.filter( @@ -441,7 +570,6 @@ export function ImportTasksSettingsPage(props: { importedCountBefore: session.importState.importedCount, latestImportedSessionIdBefore: session.importState.importedSessionIds[0], loadedCatalogItemCountBefore: catalog.sessions.length, - catalogSelectionGeneration: catalogSelectionRef.current.generation, }; setActiveImport(attempt); setImportError(null);