diff --git a/docs/plans/2026-03-30-fix-sidebar-session-staleness.md b/docs/plans/2026-03-30-fix-sidebar-session-staleness.md new file mode 100644 index 00000000..391df54c --- /dev/null +++ b/docs/plans/2026-03-30-fix-sidebar-session-staleness.md @@ -0,0 +1,38 @@ +# Fix Sidebar Session Staleness + +## Goal + +The sidebar session list goes stale and never recovers. Fresh data arrives from the server every few seconds (confirmed: 200 OK, 50 items), gets committed to Redux (`resultVersion` incrementing), but the sidebar renders old data. Fix the root causes so the sidebar always reflects current server state. + +## Root Causes (confirmed via live profiling on localhost:3347) + +### 1. Window/top-level state divergence (primary cause) + +`sidebarSelectors.ts:38` reads `sessions.windows.sidebar.projects`, falling back to `sessions.projects`. WebSocket patches update `sessions.projects` (top-level) and sync only to the **active** surface via `syncActiveWindowFromTopLevel()`. When sidebar isn't active, its window state goes stale — and the selector always picks the stale window data over the fresh top-level data because it exists. + +Key locations: +- `src/store/selectors/sidebarSelectors.ts:38` — selector reads stale window state +- `src/store/sessionsSlice.ts:152` — `syncActiveWindowFromTopLevel()` only syncs active surface +- `src/store/sessionsSlice.ts:243-245, 263-265` — commit guards skip sync when not active +- `src/store/sessionsThunks.ts:609-614` — `queueActiveSessionWindowRefresh()` only refreshes active surface + +### 2. Silent error swallowing in refresh path + +`refreshVisibleSessionWindowSilently()` (sessionsThunks.ts:420-427) catches all errors and just sets `loading: false`. No retry, no error state surfaced, no logging. If a refresh fails, sidebar stays stale forever. + +### 3. Zero observability + +`sessionsThunks.ts` has zero `console.log`, `console.warn`, or `console.error` calls. The entire refresh coordination system (generation checks, identity matching, commit/discard decisions) is completely silent. Makes debugging impossible. + +## Approach + +Fix the selector to always use fresh data, add recovery mechanisms for failed refreshes, and add logging so these issues are visible in the future. Verify with a running server using Chrome browser automation. + +## Verification Criteria + +1. **Sidebar stays fresh**: With a running dev server, create new Claude sessions and verify they appear in the sidebar within seconds — not just in Redux state, but in the rendered DOM. +2. **Surface switching doesn't cause staleness**: Switch between sidebar and history surfaces, verify sidebar data stays current after switching back. +3. **Error recovery**: Simulate a failed fetch (e.g., kill server briefly), verify sidebar recovers on next successful fetch rather than staying stale forever. +4. **Logging exists**: Key decision points in the refresh flow (commit, discard, error, retry) emit debug-level log messages. +5. **All existing tests pass**: `npm run check` green. +6. **No regressions in history view**: History surface still works correctly since it shares the same window system. diff --git a/src/components/settings/WorkspaceSettings.tsx b/src/components/settings/WorkspaceSettings.tsx index 87b5da79..2a1d457a 100644 --- a/src/components/settings/WorkspaceSettings.tsx +++ b/src/components/settings/WorkspaceSettings.tsx @@ -54,7 +54,7 @@ export default function WorkspaceSettings({ className="h-10 w-full px-3 text-sm bg-muted border-0 rounded-md focus:outline-none focus:ring-1 focus:ring-border md:h-8 md:w-auto" > - + diff --git a/src/store/sessionsSlice.ts b/src/store/sessionsSlice.ts index a846ae2d..1284bf41 100644 --- a/src/store/sessionsSlice.ts +++ b/src/store/sessionsSlice.ts @@ -149,19 +149,38 @@ function commitWindowPayload( window.partialReason = payload.partialReason } -function syncActiveWindowFromTopLevel(state: SessionsState) { - if (!state.activeSurface) return - const window = ensureWindow(state, state.activeSurface) +function syncWindowProjectsFromTopLevel(window: SessionWindowState, state: SessionsState) { window.projects = state.projects window.lastLoadedAt = state.lastLoadedAt window.totalSessions = state.totalSessions window.oldestLoadedTimestamp = state.oldestLoadedTimestamp window.oldestLoadedSessionId = state.oldestLoadedSessionId window.hasMore = state.hasMore +} + +function syncActiveWindowFromTopLevel(state: SessionsState) { + if (!state.activeSurface) return + const window = ensureWindow(state, state.activeSurface) + syncWindowProjectsFromTopLevel(window, state) window.loading = state.loadingMore window.loadingKind = state.loadingKind } +function syncAllWindowsFromTopLevel(state: SessionsState) { + if (!state.windows) return + for (const [surface, window] of Object.entries(state.windows)) { + if (!window) continue + // Skip windows with active search queries — their results are query-specific + if (window.appliedQuery) continue + syncWindowProjectsFromTopLevel(window, state) + // Only sync loading state to the active surface + if (surface === state.activeSurface) { + window.loading = state.loadingMore + window.loadingKind = state.loadingKind + } + } +} + export const sessionsSlice = createSlice({ name: 'sessions', initialState, @@ -279,7 +298,7 @@ export const sessionsSlice = createSlice({ state.lastLoadedAt = Date.now() const valid = new Set(state.projects.map((p) => p.projectPath)) state.expandedProjects = new Set(Array.from(state.expandedProjects).filter((k) => valid.has(k))) - syncActiveWindowFromTopLevel(state) + syncAllWindowsFromTopLevel(state) }, clearProjects: (state) => { state.projects = [] @@ -309,7 +328,7 @@ export const sessionsSlice = createSlice({ state.lastLoadedAt = Date.now() const valid = new Set(state.projects.map((p) => p.projectPath)) state.expandedProjects = new Set(Array.from(state.expandedProjects).filter((k) => valid.has(k))) - syncActiveWindowFromTopLevel(state) + syncAllWindowsFromTopLevel(state) }, applySessionsPatch: ( state, @@ -329,7 +348,7 @@ export const sessionsSlice = createSlice({ const valid = new Set(state.projects.map((p) => p.projectPath)) state.expandedProjects = new Set(Array.from(state.expandedProjects).filter((k) => valid.has(k))) - syncActiveWindowFromTopLevel(state) + syncAllWindowsFromTopLevel(state) }, clearPaginationMeta: (state) => { state.totalSessions = undefined diff --git a/src/store/sessionsThunks.ts b/src/store/sessionsThunks.ts index 5953f0bf..cff38741 100644 --- a/src/store/sessionsThunks.ts +++ b/src/store/sessionsThunks.ts @@ -4,8 +4,11 @@ import { type SearchOptions, type SearchResult, } from '@/lib/api' +import { createLogger } from '@/lib/client-logger' import type { AppDispatch, RootState } from './store' import type { ProjectGroup } from './types' + +const log = createLogger('SessionsThunks') import { commitSessionWindowReplacement, commitSessionWindowVisibleRefresh, @@ -340,7 +343,10 @@ async function refreshVisibleSessionWindowSilently(args: { query?: string searchTier?: SearchOptions['tier'] }) => { - if (!canCommit()) return false + if (!canCommit()) { + log.debug('Discarded refresh result for', surface, '— identity mismatch or generation changed') + return false + } dispatch(commitSessionWindowVisibleRefresh({ ...payload, preserveLoading: preserveLoadingState, @@ -417,12 +423,19 @@ async function refreshVisibleSessionWindowSilently(args: { query: identity.query, searchTier: identity.searchTier, }) - } catch { - if (!preserveLoadingState && canCommit()) { - dispatch(setSessionWindowLoading({ - surface, - loading: false, - })) + } catch (error) { + log.warn('Background refresh failed for', surface, error instanceof Error ? error.message : error) + if (canCommit()) { + if (!preserveLoadingState) { + dispatch(setSessionWindowError({ + surface, + error: error instanceof Error ? error.message : 'Background refresh failed', + })) + dispatch(setSessionWindowLoading({ + surface, + loading: false, + })) + } } } } diff --git a/test/unit/client/components/SettingsView.behavior.test.tsx b/test/unit/client/components/SettingsView.behavior.test.tsx index aa8e34e2..72ace317 100644 --- a/test/unit/client/components/SettingsView.behavior.test.tsx +++ b/test/unit/client/components/SettingsView.behavior.test.tsx @@ -110,7 +110,7 @@ describe('SettingsView behavior sections', () => { return select.querySelector('option[value="recency-pinned"]') !== null }) - expect(sortModeSelect.querySelector('option[value="recency-pinned"]')?.textContent).toBe('Recency (pinned)') + expect(sortModeSelect.querySelector('option[value="recency-pinned"]')?.textContent).toBe('Recency (tabs first)') fireEvent.change(sortModeSelect, { target: { value: 'recency-pinned' } }) expect(store.getState().settings.settings.sidebar.sortMode).toBe('recency-pinned') diff --git a/test/unit/client/store/sidebar-staleness.test.ts b/test/unit/client/store/sidebar-staleness.test.ts new file mode 100644 index 00000000..0bdbfa78 --- /dev/null +++ b/test/unit/client/store/sidebar-staleness.test.ts @@ -0,0 +1,239 @@ +// Tests for sidebar session staleness bug: WebSocket patches must reach sidebar +// window state even when sidebar is not the active surface. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { configureStore } from '@reduxjs/toolkit' +import { enableMapSet } from 'immer' +import sessionsReducer, { + applySessionsPatch, + commitSessionWindowReplacement, + commitSessionWindowVisibleRefresh, + markWsSnapshotReceived, + setActiveSessionSurface, + setSessionWindowLoading, +} from '@/store/sessionsSlice' +import * as sessionsThunks from '@/store/sessionsThunks' + +const { + fetchSessionWindow, + refreshActiveSessionWindow, +} = sessionsThunks +const queueActiveSessionWindowRefresh = ((sessionsThunks as any).queueActiveSessionWindowRefresh ?? refreshActiveSessionWindow) as typeof refreshActiveSessionWindow +const _resetSessionWindowThunkState = ((sessionsThunks as any)._resetSessionWindowThunkState ?? (() => {})) as () => void + +const fetchSidebarSessionsSnapshot = vi.fn() +const searchSessions = vi.fn() + +vi.mock('@/lib/api', async () => { + const actual = await vi.importActual('@/lib/api') + return { + ...actual, + fetchSidebarSessionsSnapshot: (...args: any[]) => fetchSidebarSessionsSnapshot(...args), + searchSessions: (...args: any[]) => searchSessions(...args), + } +}) + +enableMapSet() + +function makeProject(path: string, sessionId: string, lastActivityAt: number) { + return { + projectPath: path, + sessions: [{ + provider: 'claude', + sessionId, + projectPath: path, + lastActivityAt, + title: `Session in ${path}`, + }], + } +} + +function createStore() { + return configureStore({ + reducer: { sessions: sessionsReducer }, + middleware: (m) => m({ serializableCheck: false }), + }) +} + +function createStoreWithSessions(preloaded: Record) { + return configureStore({ + reducer: { sessions: sessionsReducer }, + preloadedState: { + sessions: { + ...sessionsReducer(undefined, { type: '@@INIT' }), + ...preloaded, + }, + }, + middleware: (m) => m({ serializableCheck: false }), + }) +} + +describe('sidebar staleness', () => { + beforeEach(() => { + vi.clearAllMocks() + _resetSessionWindowThunkState() + }) + afterEach(() => { + _resetSessionWindowThunkState() + }) + + describe('applySessionsPatch syncs to all windows', () => { + it('updates sidebar window when history is the active surface', () => { + const sidebarProjects = [makeProject('/proj-a', 'old-session', 1_000)] + + const store = createStoreWithSessions({ + activeSurface: 'history', + wsSnapshotReceived: true, + projects: sidebarProjects, + windows: { + sidebar: { + projects: sidebarProjects, + lastLoadedAt: 1_000, + }, + history: { + projects: sidebarProjects, + lastLoadedAt: 1_000, + }, + }, + }) + + const freshProject = makeProject('/proj-b', 'new-session', 5_000) + store.dispatch(applySessionsPatch({ + upsertProjects: [freshProject], + removeProjectPaths: [], + })) + + // Top-level must have the new project + const topLevel = store.getState().sessions.projects + expect(topLevel.some(p => p.projectPath === '/proj-b')).toBe(true) + + // Sidebar window must ALSO have the new project + const sidebarWindow = store.getState().sessions.windows.sidebar + expect(sidebarWindow.projects.some((p: any) => p.projectPath === '/proj-b')).toBe(true) + }) + + it('updates sidebar window when no active surface is set', () => { + const store = createStoreWithSessions({ + wsSnapshotReceived: true, + projects: [makeProject('/proj-a', 's1', 1_000)], + windows: { + sidebar: { + projects: [makeProject('/proj-a', 's1', 1_000)], + lastLoadedAt: 1_000, + }, + }, + }) + + store.dispatch(applySessionsPatch({ + upsertProjects: [makeProject('/proj-c', 's3', 9_000)], + removeProjectPaths: [], + })) + + const sidebarWindow = store.getState().sessions.windows.sidebar + expect(sidebarWindow.projects.some((p: any) => p.projectPath === '/proj-c')).toBe(true) + }) + }) + + describe('refresh path updates sidebar even when not active', () => { + it('queueActiveSessionWindowRefresh updates sidebar data when sidebar is active', async () => { + const freshProjects = [makeProject('/proj-fresh', 'fresh-1', 9_000)] + fetchSidebarSessionsSnapshot.mockResolvedValue({ + projects: freshProjects, + totalSessions: 1, + oldestIncludedTimestamp: 9_000, + oldestIncludedSessionId: 'claude:fresh-1', + hasMore: false, + }) + + const staleProjects = [makeProject('/proj-stale', 'stale-1', 1_000)] + const store = createStoreWithSessions({ + activeSurface: 'sidebar', + wsSnapshotReceived: true, + projects: staleProjects, + lastLoadedAt: 1_000, + windows: { + sidebar: { + projects: staleProjects, + lastLoadedAt: 1_000, + resultVersion: 1, + }, + }, + }) + + await store.dispatch(queueActiveSessionWindowRefresh() as any) + + const sidebar = store.getState().sessions.windows.sidebar + expect(sidebar.projects.some((p: any) => p.projectPath === '/proj-fresh')).toBe(true) + expect(sidebar.projects.some((p: any) => p.projectPath === '/proj-stale')).toBe(false) + }) + }) + + describe('error recovery in silent refresh', () => { + it('surfaces error state when silent refresh fails', async () => { + fetchSidebarSessionsSnapshot.mockRejectedValueOnce(new Error('Network error')) + + const existingProjects = [makeProject('/proj-a', 's1', 1_000)] + const store = createStoreWithSessions({ + activeSurface: 'sidebar', + wsSnapshotReceived: true, + projects: existingProjects, + lastLoadedAt: 1_000, + windows: { + sidebar: { + projects: existingProjects, + lastLoadedAt: 1_000, + resultVersion: 1, + }, + }, + }) + + await store.dispatch(queueActiveSessionWindowRefresh() as any) + + const sidebar = store.getState().sessions.windows.sidebar + // After a failed refresh, loading should be false + expect(sidebar.loading).toBeFalsy() + // The error should be surfaced, not swallowed + expect(sidebar.error).toBeDefined() + }) + }) + + describe('activity sort uses ratchetedActivity when present', () => { + it('sorts items with ratchetedActivity above those without', async () => { + const { sortSessionItems } = await import('@/store/selectors/sidebarSelectors') + + const recentlyActive = { + id: 'active', + sessionId: 'active', + provider: 'claude', + sessionType: 'claude', + title: 'Session with recent client activity', + hasTitle: true, + timestamp: 1_000, + hasTab: false, + isRunning: false, + ratchetedActivity: 5_000, + } + + const recentServer = { + id: 'server-recent', + sessionId: 'server-recent', + provider: 'claude', + sessionType: 'claude', + title: 'Session with recent server timestamp only', + hasTitle: true, + timestamp: 9_000, + hasTab: false, + isRunning: false, + } + + const sorted = sortSessionItems( + [recentServer, recentlyActive], + 'activity', + ) + + // Item with ratchetedActivity sorts above item without, regardless of server timestamp + expect(sorted[0].id).toBe('active') + expect(sorted[1].id).toBe('server-recent') + }) + }) +})